Feedback on early versions of NUnit 4

[OsirisTerje]

This thread is for anyone to give their feedback on the developer and upcoming alpha versions of version 4.
See Towards NUnit4 for details.

The early version can be found on our Myget package feed: https://www.myget.org/F/nunit/api/v3/index.json
The current version is:
<PackageReference Include="NUnit" Version="4.0.0-dev-07969" />

[koszeggy]

The lowest framework platforms support in Version 4 are .net framework 4.6.2 and .net 6.0.

Nooo... 🥺

Maybe I cannot really justify that in my open-source libraries I still support .NET Core 2.0 but .NET Framework 3.5 is still going to be officially supported at least until 2029. So please, bring back .NET Framework 3.5 and .NET Standard 2.0 support because I really don't want to migrate to some other testing framework.

Furthermore
In our team projects we are a bit confused about the breaking change of the "classic" asserts. We considered Assert.That-like complex assertions as the low-level tools that are good if you cannot achieve something by the high-level and more compact sugars such as Assert.AreEqual/IsTrue/Throws etc., which in the depth just use the lower-level 'lego bricks' provided by Assert.That.

Considering that you can do almost everything by the high-level (aka. classic) asserts now 99% of our tests don't compile on v4. Though a simple Replace All can solve this problem, the naming 'classic'/'legacy' suggests that this way is now deprecated and we should rewrite almost all of our tests anyway so the voices are increasing in a couple of projects that maybe it's time to move to xUnit. Back then I successfully convinced them that calling a parameterless test 'fact' but a parameterized one 'theory' is simply not obvious enough to me but unless you put back the high level asserts to Assert I will be out of reasons. Or please at least call the new class BasicAssert, CoreAssert, CommonAssert or something that tells that these are the high-level sugars for the most common cases and does not suggest that you should not use them anymore. 🙏

[lahma]

While I understand the grief of losing supported platforms, I also think that you should consider option of not upgrading to v4 in that case. NUnit 3 is a mature framework and works splendidly. I think the general consensus is that we can create new major releases sunsetting the older supported frameworks allowing projects to move forward with both API surface and development efficiency.

I know I'm also biased with seeing the NuGet update option to get the latest and shiniest but for maintenance and supporting older runtimes it might just make sense to stick with 3.x, unless there's something really beneficial in v4 lineup.

I don't have anything against xUnit but I usually run integration tests and parallel workflows so NUnit is the obvious choice for me, whether it's 3.x or 4.x. I personally prefer FluentAssertions as the assertion library but NUnit as the test framework running things.

[koszeggy]

In my private projects I can of course stick with 3.x to continue supporting older frameworks.

But regarding our team projects would you mind sharing your thoughts about classic/legacy asserts I mentioned under Furthermore? Can we consider the classic asserts as compact and convenient sugars for the most common cases (as we did so far) or do the NUnit team discourage developers to use them in the future? The naming 'classic'/'legacy' suggests the latter. We are also biased to update to the latest versions in general (and actually we like the more descriptive messages in 4.x) but if you can confirm our impression that the classic asserts are getting slowly obsoleted so we need to refactor all of the tests anyway we would consider other frameworks as well.

Maybe it would worth to create a poll to see the preference of Assert.That usage vs. classic asserts. If it turns out that a considerable amount of people still like the classic ones I kindly suggest not to remove them from Assert or at least do not put them in a class whose name gives the impression that 'this is the old stuff you should avoid'.

[OsirisTerje]

@koszeggy First, thanks for sharing your thoughts! We appreciate the feedback. There has been too little feedback about the changes we're making, so this is very welcome.

About the constraint/classic assertions

First, just repeating some of the links to what we have written about these things earlier, Towards NUnit4, NUnit 4 planning, On assertion models.

The NUnit philosophy is to embrace and not be opinionated, and to make the framework broad. We don't want to tell others how they should do their work. We are trying to figure out what people are using, how they want things to be, and to follow what the community wants. The number of people engaging and contributing is not that high, unfortunately, so it is hard to figure out what people want.

At the same time, we need to move the framework forward. The constraint model was introduced way back in NUnit 2.4. The code for the classic model and constraint model have been mixed, and that has caused several issues and made maintenance more complicated than it could be. The classic model is a limiting factor, there are several things we simply can't do with that model. The constraint model, however, is made to be expandable. (Will add a link to that later, but couldn't find it now). That means we can't evolve the classic model so much more. The constraint model, on the other hand, has much more possibilities.

You're right that the classic model is now an overlay to the constraint model (not a more high-level, IMHO, but you can call it whatever you like :-) ), The classic model just call the constraint model. We chose to do it that way, to make it easier to maintain the underlying assertion code. Also, the classic model can be seen as a subset of the constraint model, so it makes sense that way too.

We have no way of knowing how many are using one thing or another. Looking at the popularity of fluent assertions, and fluent code in general, and also having code more readable, we would expect, but only guessing, that people would like to use the constraint model more, and also, like in my own case, avoid having to write the AreEqual with the expected value first, always struggled with that one :-)

Then, we have other people, like yourself and certainly many more, who prefer the older, more well-known, and more terse syntax of the classic model.

It is the community who will sort of "decide" whether we should keep or drop anything in the framework. We need the feedback of course.

As you can see in the planning, we have discussed moving both the classic and the constraint assertions into their own packages. Currently, they are both in the framework. Moving them into their own packages doesn't mean we're dropping any of them!

The word Classic has been around for many years to talk about the set of...classic asserts. The namespace name of Legacy was the one we came up with when discussing how to do this. If we could have had these in the same namespace, but in two different assemblies, we would have done so, but that simply doesn't compile. We also want them to be able to co-exist, the classic and constraint model, which means we need to change the naming, thus ClassicAssert.

We have discussed the NUnit.Analyzers package could be used to help here, but it would mean we had to tell people to follow a given process, because once you have moved to v4, then your test project won't compile, and the analyzer is not so happy about that. You could do it with v3, but then once you have used the analyzer, it won't compile either, until you upgrade to v4. It all looked messy. So we dropped that idea. It seemed better to simply use renaming.

We can change the naming, if the naming (e.g. legacy), makes it seem like it's being deprecated. Suggestions?

About the class name, since the word Classic has been used for quite some time in issues and discussions, we thought that sounded, classy! We actually chose that here to avoid indicating that it could be misunderstood for deprecated. The word legacy though, it's more debatable. The suggestions on class names you have like BasicAssert, CoreAssert, CommonAssert, Imho - it doesn't quite ring with me.
But, just suggest, ideas are welcome!

So, no, we're not deprecating or "obsoleting" the classic asserts. They will stay as long as developers want them. But, we will probably not expand too much on them. (I don't think XUnit is doing that either, just saying).
And yes, the major development on assertions will be in the constraint model, simply because that is where it is most easy to do so, as it is designed to be so.

That also means, that after this change, the constraint models will be more prone to breaking changes whereas the classic asserts will stay as they are. You can see this with the removal of the params from the constraint asserts, but they are still there for the classic ones.

About the NUnit team encouraging or discouraging the use of either of these; we don't. It is up to the users which to use. The team members may have different opinions of course of their own preferences. One of us doesn't even use any of these, but the Shouldly package instead. That's also fine by us :-)

Frameworks

The reason for dropping older frameworks is the huge development that has happened in the last few years. As new frameworks come along nearly yearly now, the code we maintain becomes more or "infected" with ifdefs, and thus blocks of different code, then the same for the unit tests, and for each framework we add, the package grows. Two frameworks are where we are today, and that is double the size of one. Adding back the two you mention, will double once more. So the package will be more and more heavy. And then code mess, and testing, to make sure it is actually working in all of these.

We have earlier asked other communities for help in testing the different scenarios that can arise. We don't only have the target framework dimension, but also the runtime framework it should run under. Then we have all the different versions of Visual Studio, Rider, dotnet cmd line, and then Windows versus Linux versus Mac. We simply can't cope with too much, so we have to make some cuts somewhere.

The thinking has been that if you don't upgrade your target framework, you're not likely to upgrade other packages either, like NUnit. Staying on older frameworks like .Net Framework 3.5 doesn't sound like a good idea today. Updating anything there must hurt in many other ways than just the testing framework. There would also be code there that would not be regarded as secure today.

About other test frameworks, XUnit.Runner.VisualStudio 2.5.3, the support there is the same, .Net framework 4.6.2 and .NET 6. A bit of difference between their packages, but I can't see anything there that supports 3.5.

We will also release updates to the 3.X series, the plan is to release a 3.14 version soon now too, and that will continue to support net35 (IIRC).

[koszeggy]

@OsirisTerje: Thank you so much for your elaborated answer.

We can change the naming, if the naming (e.g. legacy), makes it seem like it's being deprecated. Suggestions? [...] The suggestions on class names you have like BasicAssert, CoreAssert, CommonAssert, Imho - it doesn't quite ring with me.

Good to hear you are open for suggestions. I don't have better ideas now - those suggestions just based on the idea that they offer a simple set of tools for the most basic/common cases that translate to the more flexible fluent expressions. But it's okay to me if the naming is based on something else as long as it doesn't suggest obsoletion.

As new frameworks come along nearly yearly now, the code we maintain becomes more or "infected" with ifdefs

I know the pain 😂

The thinking has been that if you don't upgrade your target framework, you're not likely to upgrade other packages either, like NUnit. Staying on older frameworks like .Net Framework 3.5 doesn't sound like a good idea today.

I think we need to differentiate two things here:

1. Your statement above is correct if we are talking about an application, which should be updgraded regularly to target the latest possible frameworks not just to be able to use the new things but also to avoid possible old security issues, etc.
2. But if your project is a library that can be used by others you should aim to the lowest possible versions, or at least the ones that are still officially supported. And as of today .NET Framework 3.5 still will be supported for at least six more years.

Testing against old frameworks isn't obvious anyways so I knew this day would come once. For example, testing against .NET Framework 3.5 recently worked correctly only if I executed the tests as a console app whereas newer targets still work in the regular way.

About other test frameworks, XUnit.Runner.VisualStudio 2.5.3, the support there is the same, .Net framework 4.6.2 and .NET 6. A bit of difference between their packages, but I can't see anything there that supports 3.5.

In my private projects I will not abandon NUnit. Considering the change arose in our team projects where we target .NET 6 or greater but there the issue was not the drop of support but the possible need of complete refactoring if the classic asserts are not recommended anymore (but as you said fortunately this isn't the case).

We will also release updates to the 3.X series, the plan is to release a 3.14 version soon now too, and that will continue to support net35

This is also a very comforting news. 👍

[manfred-brands]

we like the more descriptive messages in 4.x

These will be confusing as the descriptive message will be for the constraint method called by NUnit4 classic assert wrapper.
E.g.:

ClassicAssert.AreEqual(3.1415926, Math.PI, 1e-9, "Pi");

Will give the following message, where the descriptive part is not in any way related to your code:

Pi
Assert.That(actual, Is.EqualTo(expected).Within(delta))
Expected: 3.1415926000000001d +/- 1.0000000000000001E-09d
But was:  3.1415926535897931d
Off by:   -5.3589793047592593E-08d

If (auto) converting to the constraint model:

Assert.That(Math.PI, Is.EqualTo(3.1415926).Within(1e-9), "Pi");

The message is directly related to the code:

Pi
Assert.That(Math.PI, Is.EqualTo(3.1415926).Within(1e-9))
Expected: 3.1415926000000001d +/- 1.0000000000000001E-09d
But was:  3.1415926535897931d
Off by:   -5.3589793047592593E-08d

[koszeggy]

we like the more descriptive messages in 4.x

These will be confusing as the descriptive message will be for the constraint method called by NUnit4 classic assert wrapper. E.g.:

ClassicAssert.AreEqual(3.1415926, Math.PI, 1e-9, "Pi");

Will give the following message, where the descriptive part is not in any way related to your code:

Pi
Assert.That(actual, Is.EqualTo(expected).Within(delta))
Expected: 3.1415926000000001d +/- 1.0000000000000001E-09d
But was:  3.1415926535897931d
Off by:   -5.3589793047592593E-08d

We don't think it's confusing, and actually that IS very much related to our code. Much better and more descriptive than the old way, which simply was (if message wasn't specified):

Expected: x
But was: y

So we are totally okay with seeing the low-level expressions in the messages. Especially that the custom messages are displayed first (that we normally specified if the default message wasn't descriptive enough)

[MaceWindu]

Have a question about #4389 feature (Async Test Sources) while working on code migration to v4. Is it possible to use CancellationToken with this feature (from parameter or at least from context)?

[manfred-brands]

Test discovery is done before any running of the tests. What do you intend to use the token for? Do you want to obey the token or somehow cancel it?
If the first, when should unit cancel it?

[MaceWindu]

1. Obey.
2. This rises another question: who could rise cancellation token? Is it only CancelAfter attribute or it could be rised e.g. by nunit test adapter when user stops tests?

If it supported only by CancelAfter, then my initial question is not valid - I have no tests that require such setup currently.

[MaceWindu]

Long story short: many of our tests work with async code and if we can utilize cancellation token from nunit istead of passing default(CancellationToken) to tested code and get some value from it - it will not harm to use it in our test code.

[manfred-brands]

There is a difference between Async test sources and actual tests. For the first we have not yet a way to cancel this. Ideas welcome.
For tests, if using the cancelafter attribute NUnit can pass the cancellation token to your test. If the test takes longer than the specified limit, NUnit will cancel the token. It is up to your code to obey and stop. See documentation

[manfred-brands]

nunit test adapter when user stops tests?

I don't know what happens here and if there is a signal that could cancel the token. I will have to investigate that.

[MaceWindu]

First impressions after migration process (no test runs yet).
In general I like all the changes, but there are several minor issues:

1. it would be nice to have dev. builds fixed or beta-2 released. E.g. we need fix for NUnit 4.0 fails when [Repeat] is present and test passes #4491, so we build nunit locally.

2. I support removal of Assert.That[Async] overloads with formatted string but it has one issue:

Signature of new API has following parameters:

[Formattable/NUnit]String message,
[CallerArgumentExpression(nameof(actual))] string actualExpression = "",
[CallerArgumentExpression(nameof(expression))] string constraintExpression = ""`

Which results in two situations:

• when template parameters are not strings or there are more than 2 parameters we get compilation error and could fix it
• when template parameters are 1 or 2 strings, they silently passed as CallerArgumentExpression parameters

It will help to have analyzer to detect use of CallerArgumentExpression parameters by user to help with migration and prevent users from incorrect API use. I have a lot of such incorrect asserts after migration and need to use search to find and fix them.

[manfred-brands]

@MaceWindu Those analyzers exist, but you need to run them when you still reference nunit 3.x.
See the migration guide

[MaceWindu]

yes, that's possible to rollback to previous version to fix formats, but it doesn't help with CallerArgumentExpression misuse when user explicitly pass something to those parameters

[manfred-brands]

Do you suggest we need an analyzer rule when using a message parameter containing format specifications like '{0}'.
Or even if they specify any value for those default parameters?

[manfred-brands]

@MaceWindu I created an nunit.analyzer issue for this.

[kkohler2]

In my opinion, NUnit 4 is a failure. Deprecating essentially ALL existing Asserts means either:

1. Staying on NUnit 3.x (so what is the point of 4.x)?
2. Moving to NUnit 4, but using the "legacy" nuget package (so what is the point of moving to 4.x)?
3. Re-writing every unit test to conform to the new asserts (huge waste of time that adds no value.)
   If I need to re-write everything to go to a new version, I'll look into switching to another testing framework (xunit? mstest?)

I don't have any issue with the addition of new assert types. I just don't use them.

[stevenaw]

A variant of @OsirisTerje 's suggestion above for those wishing to use only the classic asserts that I think I saw @SimonCropp share on Twitter a while back is to move the global using into the csproj file or possibly a shared Directory.Build.props file to allow reuse between assemblies. Something like:

  <ItemGroup>
    <Using Include="NUnit.Framework.Legacy.ClassicAssert" Alias="Assert" />
  </ItemGroup>

[OsirisTerje]

@stevenaw Awesome! We should get these into the migration guide for NUnit 4. :-)

[manfred-brands]

@stevenaw I hadn't seen that feature and can't seem to find it in the msbuild documentation, but did find the issue. I'll try it out and create a PR to add this to the migration guide.

[Applesauce314]

you don't need to rewrite anything. There are analyzers with fixers to convert asserts to new syntax

Except my internal documentation, my source generators, my code templates, the brain of that one guy you struggled to get to write unit tests at all, and now you need to go explain to him he has to write them slightly differently...
Not to mention that now my git history for all my test files will point to the same commit...

All on top of the fact that I am not convinced that
Assert.That(result, Is.Null); is actually better then Assert.IsNull(result);

yes the constraint based asserts are "more powerful" but were the classic asserts difficult to maintain?
where people using them wrong in a way that was driving work onto this project?

yes I can use the alias suggestions above, but now my code is different then the online documentation, and a comment in a github discussion i don't think is a API contract to never add anything to either class that will conflict name wise and cause my code to no longer compile.

Maybe I'm too late to the party to complain, but i just felt completely blindsided by this when i went to bump up the NUnit version in my groups project.

[OsirisTerje]

@Applesauce314 As said, you don't need to use the constraint syntax. You can use the classic one. Just add the csproj alias above, and you're fine, and the online documentation is STILL the same. See https://docs.nunit.org/articles/nunit/writing-tests/assertions/assertions.html .

The only thing documentation wise is the prefix of Classic, but surely, that can't be a blocker ?

yes the constraint based asserts are "more powerful" but were the classic asserts difficult to maintain?
where people using them wrong in a way that was driving work onto this project?

No, people were not using them wrong (except mixing expected and actual, but we've all been there)
Yes, they were end-of-line maintenance wise. We could not evolve them further.
E.g. Look at the new messages coming out showing the assert statement in clear text.

And you can stay on the 3-series will be maintained further on, bugfixes and so, just fewer further enhancements.

Not to mention that now my git history for all my test files will point to the same commit.

Nope, you dont need to change them, just the csproj with the alias.