Inconsistency among NUnit2045 "Call independent Assert statements..." vs. NUnit documentation that does not require "independent"

[maettu-this]

Consider this piece of test code where rate = new Rate(interval, window):

Assert.That(rate.Update(10), Is.True);
Assert.That(rate.Value, Is.EqualTo(13));

Assert.That(rate.Update(10), Is.True);
Assert.That(rate.Value, Is.EqualTo(27));

The analyzer (version 3.10 with NUnit 3.14.0) issues "warning NUnit2045: Call independent Assert statements from inside an Assert.Multiple". This statement is not fully correct,...

• ...because calling a method on an object may effect its state, i.e. statements are not "independent".
• ...and NUnit Multiple Asserts does not require "indepentent", whereas the message as well as https://github.com/nunit/nunit.analyzers/blob/master/documentation/NUnit2045.md state so.

Notes:

• The Update() method calculates Value and returns true if Value has changed.
• Rewriting the test using Assert.Multiple() passes:

Assert.Multiple(() =>
{
	Assert.That(rate.Update(10), Is.True);
	Assert.That(rate.Value, Is.EqualTo(13));
});
Assert.Multiple(() =>
{
	Assert.That(rate.Update(10), Is.True);
	Assert.That(rate.Value, Is.EqualTo(27));
});

Potential solutions:

• Remove the "independent" from message and documentation.
• Clarify the intention and constraints of Assert.Multiple() with the NUnit team.

[manfred-brands]

The indepedent is meant for situations like:

Assert.That(instance, Is.Not.Null);
Assert.That(instance.Property1, Is.EqualTo(expectedProperty1));
Assert.That(instance.Property2, Is.EqualTo(expectedProperty2));

The analyzer suggests:

Assert.That(instance, Is.Not.Null);
Assert.Multiple(() =>
{
    Assert.That(instance.Property1, Is.EqualTo(expectedProperty1));
    Assert.That(instance.Property2, Is.EqualTo(expectedProperty2));
});

The Is.Not.Null must be outside the Assert.Multiple because it affects the other two, i.e. we cannot test the other two if the first one fails.
The other two can be done inside an Assert.Multiple because testing the value of Property2 should still be possible if the value of Property1 doesn't meet the expectation.

In your case, if the Update method fails, it might mean that it doesn't make sense to test the Value. If the first one fails, the second one will as well. So it doesn't make sense then to group them as you always get two errors.
If the second is independent then it makes sense to wrap them in an Assert.Multiple so you get as many failures as possible and fix them in one go instead of fixing the first only to have it fail on the next line.

I think the NUnit documentation could be updated to highlight when to group statements.

The analyzer doesn't know what your class does, but it could be updated to assume that a method call might modify state and not suggest a grouping, unless the method is marked with a [Pure] attribute indicating it doesn't modify state.

[maettu-this]

The analyzer ... could be updated to assume that a method call might modify state and not suggest a grouping, unless the method is marked with a [Pure] attribute indicating it doesn't modify state.

This sounds very reasonable. The majority of functions modifies some sort of state, and for all others [Pure] is the way to go.

[OsirisTerje]

Attributing code with [Pure] is not a common practice unfortunately. It is part of the Contracts, and it is not being used by any other Roslyn analyzer I know of. It might even be obsoleted. Requiring that means we're trying to be preaching, not sure if that will have any effect on the community at large.
I would prefer the grouping suggestion even if the method is non-pure. The cases where it should not be used (which I don't find is too many)) can be suppressed.