-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Failure to load dependency assembly causes hang #460
Comments
This reproduces the failure to find nunit.framework.dll: <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net462</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="NUnit" Version="3.9.0" />
</ItemGroup>
</Project> using NUnit.Framework;
using System.Threading.Tasks;
namespace NUnitNotFoundRepro
{
public class Class1
{
[Test]
public static async Task Foo()
{
}
}
} |
Next up: fixing the load failure. Turns out it's a lot harder than it looks to set up a test for this scenario. |
@jnm2 You reopened this , and says PR #462 fixes it. But from your comment it seems this is only a partial fix? Have I got that right? Can you explain how you see these breakers working? I am not quite getting what you're trying to achieve here (my understanding is lacking :-) ) The cause of the underlying issue is what is uncertain here, right? Does this in any way match the issues we had earlier with the DIA stuff? |
The underlying cause is known. Before #462: If an async test method has an attribute and the attribute class is defined in an assembly that is not in the same directory as the test adapter, 1) navigation data goes missing AND 2) test case conversion runs so slowly (produces and catches so many FileNotFoundExceptions) it appears to hang. I fixed 2) in #462 and I want to fix 1) tonight. An example of when this happens: you're running the VSIX which doesn't have nunit.framework.dll in the same folder as the test adapter assembly, and you have an async method with In order to actually test this properly, we must have an attribute defined in an assembly which is not in the same folder as our test project's bin output because that's where the adapter assembly is running from. Having tried both using local projects and CodeDOM, my current try is to target net46 on the test project and use Roslyn to set up two such assemblies in a temp folder. The issue with Roslyn is that we have to run in-appdomain with VSTest for DiaSession to work, at which point VSTest has already loaded an ancient version of System.Collections.Immutable into that AppDomain. Every version of Roslyn that I tried caused MissingMethodException because it was compiled against a newer version of S.C.Immutable. |
Hooray! Setting up the AppDomain to get Roslyn to work was easy. My brain last night was not working. |
Test is failing beautifully. =) |
The tests are always the hard part. Fixed in five minutes. |
General note: I overstated the problem. It's not all navigation data that goes missing; it's only the navigation we have to help DiaSession find: async methods and base class methods. |
Using the 3.10 dev VSIX (2f4079d, https://ci.appveyor.com/project/CharliePoole/nunit3-vs-adapter/build/3.10.0-dev-00750/artifacts).
There's one project where in the process of locating navigation info, we call
GetCustomAttributes(testMethod)
which causes our navigation provider's reflection-only appdomain to try to load nunit.framework.dll and fail to locate it.I had added some fail-safety and it will continue on and succeed, but it's so slow. In this one project, it calls
GetCustomAttributes
on so many test methods (each of which causes the nunit.framework.dll load failure) that it appears to hang for an unacceptably long time.I don't yet know why it fails to find nunit.framework.dll, but before we fix that, I want to add another circuit breaker. I wonder what the best approach is—disable calls to
GetStateMachineType
for the current test assembly after the first exception, and same forGetDeclaringType
?Stack trace
I'll prioritize this above everything else.
/cc @OsirisTerje for implementation questions above and for 3.10 planning.
The text was updated successfully, but these errors were encountered: