-
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
NUnit3TestAdapter 3.15.0 fails to run test: "NUnit failed to load" #648
Comments
Seeing the same when trying to launch Selenium GUI tests on
It seems like the test adapter is not locating the test cases.
Example test case from my framework:
|
@seangwright I see you use NUnit 3.8. If I update to NUnit 3.12 it works for me. |
@OsirisTerje that is correct - explicitly version The CMS I am writing tests for, Kentico, takes a hard dependency on that version and if I use another version then the tests do not work. All that said, is it the case that If that is so, should it be in the NuGet package definition of |
@seangwright There is no explicit requirement about that, so I am bit surprised by why this pops up with 3.15. May I ask what exactly fails when you try a higher version than 3.8.1 ? |
Most people in my team have this problem as well. It fails with this error on every second attempt. first time running a test starts properly I am guessing that the run that starts properly changes some persisting state (file?) and sets it in an invalid way, so that the next one can't start, but can reset the state... so that the next one can start and screw it up again. I hope this helps narrow down the possible problem. |
Same issue in my team with NUnit 3.9 and Visual studio 2015 (NUnit3TestAdapter 3.15) Internal trace I got: RunAll test: one Unit Test: Hope this point help! Edit: Switch to NUnit 3.12 fix the issue |
@jnm2 When I enable verbosity, I see that when using NUnit 3.8.1 it fails to load the dll. |
I cloned https://github.com/seangwright/NUnit-Test-Adapter-Bug-Repro and ran
So this is a problem only with a specific version of VSTest perhaps? |
Running using Test Explorer in VS 16.3 Preview 2 reproduces the problem, so there is some difference between VS and the dotnet CLI. |
@jnm2 That will work because you're then running All Tests. That is a seperate method being called. You need to go into the Test Explorer in Visual Studio and select some tests, that is what really breaks it - and also triggers the pre-filter. The pre-filter doesn't work for any command line case. |
Ah, so we can't write an acceptance test currently because the bug can only happen with live Visual Studio UI. |
I noticed one issue where they reported it also didn't work for Azure Pipeline builds, which uses the command line, but I can't see how that can be. Nothing is changed, so I think that is a fluke. Until some more people reports the same. All the rest of the issues are about this particular case with the pre-filter. This case here is weird though.....But it may point to something.... |
Acceptance test: Yeah. We need to create a test harness that executes that other interface, simulating calling it from VS. |
I wonder if there was a bug present in NUnit framework 3.8 that we're triggering, since it is the one failing to load the assembly. |
NUnit Framework 3.11.0 repros, 3.12.0 works. Looking at which commit now. |
Ok, so : That must mean there is something between 3.14 adapter and 3.15 that drags in something wrong ? |
If it's an NUnit framework bug triggered by passing the framework a prefilter, the adapter should do a version check—or just say too bad, update to a non-buggy framework version. If it's an adapter bug, we can work on a fix in this repo. There's also the engine. Let's find out. |
My methodology was bad. I needed an extra rebuild step for some reason. |
Yes, that would work for this issue, but we also have the two other bugs, the one which makes this prefilter ignore anything with SetupFIxture #649 , the other that makes it not work with custom TestCaseSources #650. That doesnt seem to be dependent upon the framework version. About the prefilter implementation we absolutely need to add the version check. How is that done through the engine ? |
I'm not sure that it is possible without an engine change. This is the commit where NUnit stops failing to load the test assembly when a prefilter is specified: nunit/nunit@d09def3, the merge of nunit/nunit#2878. It's tempting to say that Charlie fixed an actual longstanding bug where NUnit misbehaved rather than ignoring the prefilter if a prefilter was given, but we obviously need to look at everything. |
Note the runner is created AFTER the pre-filter is created. So it seems it should really be in the engine. This should also be done per assembly, since they may exist with different frameworks, even if that is kind of "stupid". So, do we know the framework version before we call it ? Got a feeling it will be happening too late. |
We could use a feature flag in the runsettings for this case, that is, where the nunit is a lower version that we now support. It is a special case. |
I *think* this is the bugfix commit within @CharliePoole's PR that would have fixed this: "Added tests of pre-filtering, which didn't have them before; fixed bug where filter would add a fixture twice" If this is the bugfix, the bug has already been fixed in NUnit Framework 3.11. If it was any other bug that was not the fault of the adapter, we would tell folks to update. Is this case special enough to warrant disabling sending the prefilter to any version of NUnit Framework, since we don't have a way to tell them apart? Does it warrant a runsettings flag to opt out of prefiltering if you can't update to 3.11+ for some reason? |
@seangwright and others: NUnit Framework versions lower than 3.11 were broken this way all along. You couldn't see it before because you weren't using a runner that exercised NUnit Framework this way until you updated to Test Adapter 3.15. What kind of difficulties would you face if you upgraded to NUnit Framework 3.11 or 3.12? |
@jnm2 Have you also had a look at the two other issues? They are correct versions, but still failing. The SetupFixture is the one I am really curious about, the other is probably the way the prefilter is set up that makes it not match the FQN. |
Unfortunately when we made test properties read-only, we discussed source-breaking changes (and decided they would be rare and went ahead in spite of them) but we did not think about binary-breaking changes. The .NET runtime and ECMA spec no longer recognizes a method as the same method if the return type changes to a new type that is not assignable to the old type. Some languages overload calls on return type. There aren't tools out there to help remind us when we miss things, so this is just too bad. The complexity of both source- and binary-breaking changes is so high that I'm going to advocate waiting for v4 if anything else comes up so that users and library developers can plan and reason around our releases with fewer pitfalls. Kentico will need to recompile against NUnit 3.9 and set the version range to a minimum of NUnit 3.9. |
@seangwright There is now a beta version of the fix in https://www.myget.org/feed/nunit/package/nuget/NUnit3TestAdapter/3.15.1-dev-01134 . Would appreciate if you checked it. |
@seangwright 3.15.1 hotfix released now. |
Hi. I'm having the same problem with 3.15.1 in this project. Visual Studio Community 16.3.9, Windows 10 x64 1809. NUnit 3.12. The Test Explorer window is populated but neither a specific nor all tests run. I did play with the X86 and X64 config to no avail.
|
@akarnokd I can confirm this behavior, but it is caused by the underlying Reactive.Streams.tck package which has a dependency on NUnit 3.6.1, and has hardwired that pretty much into their package. I made my own version of, where I upgraded to 3.12, and then it all worked fine. (You have an huge amount of tests, and some of them are really slow, btw. ) And, you had a binding to 3.6.1 too, in your test projects app.config. Remove that one. |
Thanks for the info (I have very limited understanding of the .NET ecosystem unfortunately). The TCK specifies NUnit >= 3.6.1, shouldn't that automatically work with 3.12 given the target 4.5.2 supposedly does auto-redirect? Is it possible to workaround this in my project or does the TCK project have to release a newer version? |
The TCK project has to be updated, but I see it seems rarely updated. I can fork it and add my changes there and raise a PR to them. |
Thanks @OsirisTerje, that would be great! (I suppose one only needs to change the NUnit dependency versions all around the subprojects, right?) |
Yes, that is so. But they need to publish it, I just wonder if the maintainers are still around. |
I'm not sure about the maintainers. It is supposed to be the standard library & TCK for .NET so taking bits of it for my project wouldn't be interoperable. I meant to post a PR regarding NetCore3 support where I now had to upgrade the NUnit anyway (reactive-streams/reactive-streams-dotnet#46). The problem is, RS.NET now fails its own verification test with NUnit 3.12 and I have no idea how or why. |
@akarnokd Was just about to raise the PR when I noted you already had done so. Hope they accept that you do two things in one PR. |
I see the same errors I assume in my fork. |
Some failures are due to these: try
{
Assert.Fail(message, exception);
}
catch (Exception)
{
AsyncErrors.Enqueue(exception);
} As I understand it, in NUnit 3.6, Assert.Fail would throw and the exception would get suppressed. In newer NUnit, the framework remembers Fail so even if the test passes otherwise, the end result is still a test failure. |
That's a very good point. And why is the code written that way? It is better just to create a new exception with the given message and use that |
No idea. |
And there seems to be tests that are designed to fail, calling the FlopAndFail. They seem to expect certain exceptions..... And have then relied on the implementation of NUnit asserts. Very messy..... |
It is a test compatibility kit to verify 3rd party implementations honor a specification by providing NUnit test templates prepared with behavior. However, one must test the TCK itself to know it fails when it should fail, hence triggering a failure mode and then checking if the right AssertionExceptions are thrown. |
Ok, but then they should just queue up the AssertionExceptions themselves, and not rely on the Assert methods. What they do now is a very indirect way of achieving this. |
I started out with 55 failed tests. After replacing the Asserts with AssertionExceptions I am down to 15 failed tests. |
I've been working on the tests myself on that PR. Most test now pass except 3-5 that timeout when the entire class is run but not when the individual tests run. I don't understand why would the particular test case timeout after 500, 1000, 2000 milliseconds but work with 3000 milliseconds. As if Task.Run would need more than two seconds to spin up and do a trivial notification. |
I notice there are multiple Thread.Sleep calls in some methods. Can it be related to those? |
No. Those wait for things not to happen. For example, the source didn't signal an item within 1 second. The failures I'm debugging is that the source should have signaled an item within 1 second yet it didn't. |
I see. Anyway good that you are down to 3-5. That means you have a fix for the ones I have left, so I'll leave this project all to you :-) For anyone who wants to look, the repo @akarnokd is fixing is : https://github.com/reactive-streams/reactive-streams-dotnet |
FWIW, I figured out why the tests kept failing due to the timeout. What happens is that when running the code on .NETCore3, The ThreadPool may end up filled with tasks blocked or doing NUnit work and the pool waits 500ms before creating a new worker thread. Some tests use code that execute |
I am experiencing the issue with NUnit 3.12 and NUnit3TestAdapter 3.15.1 DistributedTests: Test run is aborted. Logging details of the run logs. it's possible that my issue is different, because for me it only works with adapter version 3.7, that is that last one that had Tools folder in the package |
@drvic10k First, please raise new issues, don't add to a closed one. You should not need to add any binaries at the target at all, just the nuget package. Please also add a repro solution if you want us to have a more close look at your issue. |
@OsirisTerje sorry, I didn't realize it was closed |
When reporting a bug, please provide the following information to speed up triage:
Matches
App.sln
in repo below:16.2.3
Full reproduction https://github.com/seangwright/NUnit-Test-Adapter-Bug-Repro
App.sln
is usingApp2.sln
is usingTest fails to run for
NUnit3TestAdapter
3.15.0
but runs correctly for3.14.0
net472
(but with the new Common Project System / SDK style project)On-prem in Visual Studio 2019
Output Window - Tests (for
3.14.0
)Output Window - Tests (for
3.15.0
)The text was updated successfully, but these errors were encountered: