NUnit Planning

[rprouse]

#3301 brings up the fact that Partial Trust is no longer supported in .NET Core and conflicts with HandleProcessCorruptedStateExceptions in the full framework. This and other changes make me think that we might want to start thinking about the next version of NUnit. .NET development has changed dramatically since @CharliePoole started working on NUnit 3 and even since the first full release of NUnit 3. It might be time to start thinking about what NUnit 4 will be.

My initial thoughts are to evolve in the direction that .NET is going with a .NET Core / .NET 5 only version and continue to maintain and release bug fixes only for NUnit 3 for the next few years for people that are still using the earlier versions of .NET Core or the full framework. This is very much off the top of my head and plans will change, but I'd like to get the discussion going.

Some of my thoughts,

• drop partial trust, app domains and running in-process except in NUnitLite ✅
• drop remoting in favour of another communication protocol
• add something like nunit.core with concrete types for all the messages that are passed via XML currently
• maybe break the asserts and constraints out to their own project that can be used separately?
• drop or make the legacy asserts a separate project? ✅
• drop support for everything older than .NET 4.5? ✅

I'm sure there is a lot more out there, but this should get the discussion started 😈

[jnm2]

I like everything you listed! I have some more thoughts about NUnit.Core but that deserves its own thread probably.

What do you think about supporting either net472 or net48 long-term? If we have a netstandard2.0 build, that would be runnable on .NET Framework too. Having net48 would probably not be as much of a burden as netstandard2.0.

Would you envision not supporting .NET Standard or .NET Framework? .NET Standard's future is uncertain now that all .NET runtimes are unifying under .NET 5.
For example, if we want to be able to make use of C# 8 default interface methods (ability to evolve interfaces without breaking changes), we can't support netstandard2.0 or any version of .NET Framework because the CLR doesn't understand it and would crash. In contrast, we could still light up for something like C# 8's IAsyncEnumerable on .NET Core without crashing on .NET Framework.

[rprouse]

Would you envision not supporting .NET Standard or .NET Framework? .NET Standard's future is uncertain now that all .NET runtimes are unifying under .NET 5.

At this point, I'm not committed to anything. I like the idea of committing to 4.7.2 or 4.8 longer term, but I think we need feedback from the community on what they are using and what they are starting new projects with. I like the idea of having just one target like .NET Standard that just works with 90% of what people are using these days, but like you said, the future of .NET Standard is up in the air. I don't think we should rush into anything and wait to see how everything pans out with .NET 5. That said, I do think there are technologies that we know we want to drop, so we can start planning for that with a bit stronger conviction.

[CharliePoole]

I think that two thngs should be clarified at the point this is announced...

1. Exactly what is meant by supporting or not supporting. We all know there are a lot of different meanings, like "You can't write tests that target older frameworks" or "You can't test applications that target older frameworks" or "Your tests that target older frameworks will be run in a newer framework." There are probably other variations and somebody should spell out exactly what is supported.

2. The scope of this decision. Since it's being discussed in the framework project, it ostensibly applies only to that project but it seems there might be implications for several of the other projects as well. We should spell that out for the users in a way that minimizes confusion.

[CharliePoole]

Looking back at Rob's list, I see the first two items relate to the engine rather than the framework, so maybe this is a more global discussion than just the framework.

BTW, I skipped the list of items to make my earlier comment... I'm pretty much in agreement with all of them.

[rprouse]

Good points @CharliePoole, I agree that both are important. For your second point, I am pretty sure that it will encompass the framework, engine, console and adapter, so it is something the entire @nunit/core-team will need to be involved in.

[ChrisMaddock]

Here are my thoughts:

• I'd be keen to make a few breaking changes to the NUnit console/engine, including...
  -> Removal of App Domains
  -> Breaking changes to some command line options, where we've since come up with better ideas. Includes labels and params.
  -> Removal of settings file/service
  -> Changes to when assemblies referencing NUnit are considered errors or not
  -> Slimming of framework options, with possible review of Mono support (based on level of usage)
  -> Ability for passing command line params through to extensions, although this may not be a breaking change!

I also think:

• We should keep inprocess running, but that's a more details discussion than for this thread!
• I'm not sure I've yet seen any reason to couple the 4.0 releases of the framework/adapter/engine? It would be nice to instead add an NUnit 4.0 driver to the engine for the new framework, and release a new version of either project, when their each ready!

[CharliePoole]

@ChrisMaddock Some thoughts on your thoughts... 😄

• I'd remove the ability of the user to specify how AppDomains are used but still reserve the possibility to make use of them internally where it is appropriate.
• Can you explain about the desire to remove Settings file and service?
• I don't see any advantage of keeping InProcess. Of course, we sometimes use it as a bit of a crutch to help in debugging, but not having it might motivate creating better ways to debug.
• I agree whole-heartedly with decoupling the 4.0 releases of the framework and engine.

The main thing I'd add, which may only be a breaking change for third-party runners, is splitting up the engine into two layers as once discussed.

[ChrisMaddock]

Just come across a another point: review the Engine API, and make some tactical breaking changes, e.g.

• Single way to construct test packages
• Refinement of RuntimeFramework offering (Returning a single string to reflect the potentially multiple frameworks required by a single package is potentially misleading. Also ambiguity in target frameworks and runtime frameworks)

[ChrisMaddock]

The main thing I'd add, which may only be a breaking change for third-party runners, is splitting up the engine into two layers as once discussed.

Good idea. I was hoping this would be internal, but I guess we consider any repackaging changes

I'd remove the ability of the user to specify how AppDomains are used but still reserve the possibility to make use of them internally where it is appropriate.

I guess this and 'inprocess' come hand in hand, really. Maybe inprocess really should go...

Can you explain about the desire to remove Settings file and service?

I don't believe the engine should hold any machine-specific-configuration, e.g. stored in locally saved files. If runners wish to use this, they should package their own solution.

It's currently only used in the RecentFilesService, which is a sensible use...although in my opinion is a more of a runner-specific service than something that the engine should provide.

I expect we might disagree on this one! Shall we move it to a separate thread, if we do? 🙂

[CharliePoole]

Regarding settings, I understood you to mean that you would get rid of the entire file. I agree that the engine itself should not use settings. We did it that way in V2 but have been moving away from it. The current standard, as I understand it, is that runners retrieve any default or saved settings and then pass the equivalent package settings in a test package. The engine is supposed to only pay attention to what is in the package.

I do think that providing a standard way to save and retrieve Settings for clients is a reasonable thing for the engine to do. That's what SettingsService does. Of course, it could be removed but then each client would need to define it's own way of storing settings. This isn't the only runner-oriented service the engine provides either. Recent Files and Filter parsing are two other services that the engine itself has no need of but provides to clients.

[CharliePoole]

Regarding RuntimeFramework determination... I noticed that problem in a recent issue as well. IIRC, we originally only selected the highest runtime for all assemblies run in the same process. It's possible that we broke that in making some other change.

I think the logic of this was that the user, in telling us to run multiple assemblies in the same process, should expect them to all run under the same framework. We also do it for RunAsX86.

However, if we are doing breaking changes, it may make sense for RuntimeFrameworkService to only deal with a single assembly and let the runner decide what to do about combining settings for multiple assemblies.

[rprouse]

One other change that we should probably make to the engine is to remove all of the code that searches for newer versions of the engine. It was a good idea to allow users to install a newer version of the engine and override what was shipped with a runner, but we never got buy-in from the people that wrote runners, it was buggy and hard to diagnose problems when you couldn't tell which engine people were using AND we ended up turning it off by default around v3.4.

In the framework, I'd also really like to trim down or maybe remove the PlatformAttribute. At the very least, I think the values should be Enum flags, not strings so they are discoverable. .NET Core opens up so many operating systems that we will never keep up. Similarly, we are way out of date on our Runtime support. Interestingly, nobody is asking for support for the newer OS's or Runtimes, so I expect it isn't used.

The only thing that may still be useful are the Architecture flags, but those could be done with an Assume.That and maybe some helper functions?

[NightOwl888]

I would like to see some of the JUnit 4 features make it over to .NET. There is a Rule framework that makes setting up test sequences and global context easier.

[stevenaw]

Regarding Partial Trust, I've noticed another conflict it can cause on .NET Framework when dealing with spans. APIs such as MemoryExtensions.AsSpan<T>(T[]) will wrap an array in a span, but will fail under Partial Trust scenarios unless we specify [SecurityCritical] on the methods.

[z002Holpp]

Hi,
is there anywhere a roadmap or planning document where I could see a release timeline for NUnit 4?
Is a release of NUnit 4 at all planned in the near future?

[rprouse]

@z002Holpp NUnit 4 is still a ways out. It is more an idea at this point than a product.

[stevenaw]

@rprouse If we're still considering ideas, a few things I'd like to suggest:

• Better top-to-bottom support for generic APIs in assertions. There is some support already, but most things get boxed to objects. Generics could simplify some things by moving some checks into generic constraints (ex: anything collection -based would require IEnumerable or IEnumerable<T>, with possible specialty overloads for IList<T>. The JIT can also do some magic here as some type checks would likely become JIT intrinsic on some runtimes. It could also make for a better dev experience for users

• Improved async support. It would be great not to do async-over-sync in assertions, or possibly at TestMethodCommand level either

• Some way for runners to tell the framework of it needs to buffer everything in memory or not (Output text writer in TestCaseResult consumes a lot of memory #3336). Perhaps something that returns IAsyncEnumetable<TestResult>, though I think that would require a netstandard2.1 build target

[manfred-brands]

Better top-to-bottom support for generic APIs in assertions

I second that. When moving from NUnit2 to NUnit3 we noticed that test got slower. Less of an issue when there are few asserts, but some of our projects assert values in a loop. Those test got significantly slower. We had to create a LightWeightAssert wrapper with explicit double parameters to get around this.

• Performance in general.

Every time there is an Assert.That(value, Is.Equal... we create a new NUnitEqualityComparer which in turn create a list with 16 new comparers where most of them will not be needed and are basically stateless methods. Instead of passing an instance of nunitequalitycomparer to the constructor, we could pass it to the IChainComparer.Equal method instead as the only place that is called is from NUnitEqualityComparer.AreEqual. That saves 17 allocations and pressure on the garbage collector for every Is.Equal call.

NUnit3 reads great with the new fluent like api, but it came as a cost.

[CharliePoole]

I agree with @stevenaw about Generics... I've tried twiice to do an extensive rewrite based on generics, but I couldn't do it without introducing breaking changes. I'm fairly sure it can only be done in a rewrite that breaks things... i.e. a major release. If @rprouse 's intent is to continue with infrequent major releases, then it may be best to focus mostly on the breaking changes and make other improvements in 4.1, 4.2, etc.

I suspect the change from async over sync to native async requires the same sort of approach, although I have never tried it.

Buffering or not buffering doesn't seem like a breaking change provided the API is able to support it... e.g. through a newly introduced package setting. So that seems less urgent to get defined for a 4.0 release.

Minor correction for @manfred-brands... the fluent api wasn't new in 3.0. It was introduced in 2.4 in 2007. So any performance loss was probably already present but unnoticed. It's possible that fixing it could be a simple implementation detail since it doesn't change (in my opinion, anyway) a public/published API.

[rprouse]

I also tried a generics rewrite before we released 3.0 but it ended up going so deep and touching so much that I gave up. Even though it is a major release, I'd still like to keep breaking changes to a minimum. Even small changes like removing ExpectedException attribute made it very difficult for our users to upgrade large test suites. I'd like to prevent that again. I'm okay removing a few things that people don't use anymore, changing our communication with the engine or updating how people extend NUnit, but I'm less comfortable with changes that break most users.

That said, I think that many of our asserts could have generic versions in a non-breaking manner and if someone wants to take another try, power to you 😄

[stevenaw]

I think I recall reading a comment a while back from still another team member (@jnm2 ?) about an attempt to make the constraints generic. Seems like a popular ideal to work towards :D

I like the idea of minimizing friction between upgrades too. Perhaps it's something which could be tackled piece-by-piece in a non-breaking way, as you suggested. In any case, I'd also like to avoid changes which require wholesale updating of written + working tests by people. Ideally, a generics-based version should "just work"

Buffering or not buffering doesn't seem like a breaking change provided the API is able to support it... e.g. through a newly introduced package setting. So that seems less urgent to get defined for a 4.0 release.

Hm, thanks @CharliePoole . For some reason I had thought FrameworkController defined an interface, updates to which could be considered breaking. I see I was wrong. I like the idea of thematically trying to improve memory consumption in the next release, but I suppose any single non-breaking change wouldn't strictly have to ship in a semver-major release.

the fluent api wasn't new in 3.0. It was introduced in 2.4 in 2007. So any performance loss was probably already present but unnoticed. It's possible that fixing it could be a simple implementation detail since it doesn't change (in my opinion, anyway) a public/published API.

I agree with both @CharliePoole and @manfred-brands here. I like the idea of trying to improve memory consumption in the framework but I think the fluent API concern here is just an implementation detail. There's likely some internal things we can do to cache or pool comparers. For example, I would guess that the large majority of assertions are made against primitives like int or string. There may be an easy way to do a type-based lookup internally for these happy-paths using a Dictionary<Type, IChainComparer>.

[manfred-brands]

Although I have seen it, I'm not a fan of mixing classic and new asserts in a single file, it should be one or the other. Having said that, Classic.Assert looks ok to me.

@CharliePoole I think I had replaced most usages of classic asserts in our test project, except where testing those. To be sure we should also move those tests to a Classic.Test project so that our main library and test doesn't reference the classic project.

[OsirisTerje]

I am not really looking at mixing them the way I showed above. More to point out that the Test attribute is in the NUNit.Framework namespace, and that will collide when the Classic asserts are added. We could create new perhaps derived attribute classes, but I am not sure if that is worth the effort.
There are also some classic asserts some people would prefer to use together with the constraints, like Assert.True. I have noticed some people do that due to a shorter syntax.

@CharliePoole @manfred-brands Please also see PR #4417 where I have started moving the classic asserts over to a new project.
I agree that we need a Classic.Tests project, but what I notice when working with the test project in particular is that there are a mix of namespaces that are not quite standard usage, and there are a a mix of different types of tests, some are pure unit tests, but others look more like system tests. There are also tests that checks both classic and constraints in one tests.
The good thing about moving them into its own project, is that it might trigger stuff. Like right now, the Visual Studio testing I fixed back in March wont start up, something crashes the microsoft Testhost. Will try to figure out what, some missing stuff I guess, tomorrow.

@charlie Interesting thoughts. First I think it would be nice to clean the project, separate these two, and also get rid of the params ones. Clean up namespaces, getting the tests organized. Right now I feel I am wading in an ocean of code... Guess that is what we get after 20 or so years :-)

[OsirisTerje]

so that our own tests only use them if they are testing those asserts but nowhere else.

@CharliePoole In the PR I have tried to do exactly that. It seems to have been done efforts for this earlier (@manfred-brands ?) but I did find a few places where that was done. In addition, as mentioned above, a few places which used both.

[OsirisTerje]

@manfred-brands @stevenaw @jnm2 Are you able to test the Issue4416 branch inside Visual Studio? I get testhost issues, but am on a vacation laptop so might be missing something.

It did work back in March after the fixes I did then, but don't know if it is something that has been changed or something on my computer.

[stevenaw]

Thanks for putting the PR together @OsirisTerje !
I'm also on vacation at the moment and without a laptop at all. I had been setting up a new computer recently and also seen that issue. I hadn't gotten to the bottom of it. My suspicion was that it could be 17.6 related as I think my previous machine still used 17.5 and had worked - though I hadn't yet tried downgrading to confirm

[manfred-brands]

@OsirisTerje @stevenaw I got back from holiday myself just last night.
I do see the message you see, but only after the tests have run:

Starting test discovery for requested test run
========== Starting test discovery ==========
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit Adapter 4.5.0.0: Test discovery complete
========== Test discovery finished: 5708 Tests found in 1.5 sec ==========
Executing all tests in project: nunit.framework.tests
========== Starting test run ==========
NUnit Adapter 4.5.0.0: Test execution started
Running all tests in D:\NUnit\nunit4\src\NUnitFramework\tests\bin\Debug\net462\nunit.framework.tests.dll
   NUnit3TestExecutor discovered 6020 of 6032 NUnit test cases using Current Discovery mode, Non-Explicit run
...
NUnit Adapter 4.5.0.0: Test execution complete
========== Test run finished: 6030 Tests (5826 Passed, 0 Failed, 18 Skipped) run in 33.4 sec ==========
========== Starting test discovery ==========
Microsoft.VisualStudio.TestPlatform.ObjectModel.TestPlatformException: Could not find testhost
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting.DotnetTestHostManager.GetTestHostProcessStartInfo(IEnumerable`1 sources, IDictionary`2 environmentVariables, TestRunnerConnectionInfo connectionInfo)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManager.SetupChannel(IEnumerable`1 sources, String runSettings)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyDiscoveryManager.InitializeDiscovery(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEventsHandler2 eventHandler, Boolean skipDefaultAdapters)

Visual Studio tries to find test in any assembly loaded in the solution, even if running them in one project only.
If I unload our indirect test projects (mock-assembly, nunit.testdata, nunit.testdata.fsharp, slow-nunit-test and the nunit-lite projects, the error goes away but I still see proof of VS trying to discover test in all projects:

NUnit Adapter 4.5.0.0: Test execution complete
========== Test run finished: 6030 Tests (5826 Passed, 0 Failed, 18 Skipped) run in 33.7 sec ==========
========== Starting test discovery ==========
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit Adapter 4.5.0.0: Test discovery starting
NUnit failed to load D:\NUnit\nunit4\src\NUnitFramework\nunit.framework.classic\bin\Debug\net462\nunit.framework.classic.dll
NUnit failed to load D:\NUnit\nunit4\src\NUnitFramework\nunitlite\bin\Debug\net462\nunitlite.dll
NUnit Adapter 4.5.0.0: Test discovery complete
NUnit Adapter 4.5.0.0: Test discovery complete
No test is available in D:\NUnit\nunit4\src\NUnitFramework\nunit.framework.classic\bin\Debug\net462\nunit.framework.classic.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.
No test is available in D:\NUnit\nunit4\src\NUnitFramework\nunitlite\bin\Debug\net462\nunitlite.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.
NUnit Adapter 4.5.0.0: Test discovery complete
NUnit Adapter 4.5.0.0: Test discovery complete
NUnit Adapter 4.5.0.0: Test discovery complete
========== Test discovery finished: 16959 Tests found in 2.2 sec ==========

There is an issue in VSTest which sort of blames it on us:
This project takes depedency on nUnit that adds the TestContainer capability importing NUnit.props, but this is not a test project

We explicitly add TestContainer to two of our indirect test projects. When I remove these, the errors go away.

The actual VSTest issue referenced above says it occurs on project including NUnit, but not the Microsoft.NET.Test.Sdk
Do we need it as it (the TestContainer) is supplied by the latter?
Should we remove the TestContainer from our NUnit.props? I cannot find any official documentation what it does.

I have seen errors in the past where our NUnitExtension projects get errors as they do not include Microsoft.NET.Test.Sdk

[manfred-brands]

I found some "documentation" about ProjectCapabilities. It says: The project may contain unit tests. . It is explained more in TestContainer.

[OsirisTerje]

Thanks @manfred-brands !
I agree we should not include those at all, since it is in the test.sdk, as you say.
Removing them brings back the tests, and they start to run, but the indirect test projects are still loaded for net462.
And under net462 it tries to run the test-data project, and never finish.....

We need to figure out how to mark a net462 project as not-testable.
I think we also need to clean up the indirect test projects. The small slow-test project doesnt need to be included in the framework-test, it can run by itself. The only thing I see is exported there is the delay. Might be some indirect use, but so far I haven't found that.

[manfred-brands]

The vstest issue linked above mention properties that might need setting: IsTestProject

[OsirisTerje]

Yes, just did that, and after a reload of VS they disappeared :-) Adding IsTestProject=false to the indirect test projects,

[mikkelbu]

@OsirisTerje I think we should close this one (and also remove it as pinned at the top of the issues)

[OsirisTerje]

@mikkelbu @nunit/framework-team This is really a very long discussion of things that should be considered for future versions, some of them have been handled in 4.0, but there are still more. So, perhaps better to transfer this to Discussions and change the name to NUnit Future Planning ?

[SeanKilleen]

To jump in with a suggestion: milestones in GitHub may be useful here. Each item can be turned into an idea and labeled with a "future planning" milestone. Then when planning a release, a milestone can be created for the release and items can be associated with it. Thus we see the progress towards the release and which items have been defined for a "future" bucket that we can pick from. Could also be a handy way to triage feature requests and write up how we'd approach them.

I'm not really a contributor to this repo (though I hope to change that at some point!), but wanted to offer the suggestion as I've seen it work nicely before.

[OsirisTerje]

Good point! Make sense to me.

[CharliePoole]

Suggestion: If there are desirable breaking changes that were missed in V4, create a milestone to hold them.

Hinit: The framework API for .NET framework is different from .NET Core and IMO should be obsoleted. :-)

[manfred-brands]

Hinit: The framework API for .NET framework is different from .NET Core and IMO should be obsoleted. :-)

You mean anything that has to do with Tread Abort: e.g. Timeout ?
Other details like NUnitCallContext seem internal although it has its own tests.
But yes we have to check any unit tests that are specific for NETFRAMEWORK or NETCORE/NET5++ to see why there is a difference.

[CharliePoole]

@manfred-brands I mean the code in NUnit.Framework.Api namespace, primarily FrameworkController.

I originally implemented it, using System.Web.UI.ICallbackEventHandler to send back events to the caller. When we added .NET Core support, @rprouse created a new API - a better one, I think - but made it conditional on NETCOREAPP. I think that newer API could be made to work for .NET Framework as well. You'd have to deprecate the old API and stop using it in the engine first, then remove it in a major release.

[mikkelbu]

I also like @SeanKilleen suggestion. Then we just need to go through the 78 comments and extract the information :). I'll make a TODO for it.

[stevenaw]

@mikkelbu FYI I've just made a "Future Planning" milestone: https://github.com/nunit/nunit/milestone/47
In order to move #4384 out of 4.1 as I don't think the issue is called out in any of the above discussion