Test result is `Failed` even though the assertion exception gets handled

[maettu-this]

Describe the bug The attached repro sample implements a test where an assertion fails but its exception is handled, i.e. the test is expected to be indicated Passed. But the TestCentric GUI reports the test as Failed.

To Reproduce Steps to reproduce the behavior:

1. Download WrongResultIssue.zip.
2. Build the project in VS2022 (Debug or Release, doesn't matter).
3. Open the resulting "WrongResultIssue.dll" in the TestCentric GUI.
4. [Run All Tests] or selectively run SomeTest.

Expected behavior The test's result is indicated Passed.

Screenshots

Environment (please complete the following information):

• TestCentric Version 2.0.0-beta4 on NUnit 3.14.0
• OS: Win10
• Target Runtime for your test assembly: .NET Framework 4.8

[CharliePoole]

This isn't a TestCentric issue - the test fails and TestCentric reports it accordingly.

If you report it to the NUnit team, I'm sure they'll tell you it's not an issue there either but by design.

In NUnit V2, it was possible for a test to catch it's own failing assertion and change the result so that the failing test passed. That is no longer possible with NUnit 3. An exception is still used in order to cause the failing test to to exit but the failure is reported and recorded before that ever happens. The general philosophy was that a test should not be able to "cancel" its own failure.

There are different ways to handle this depending on the actual use case but catching the exception is no longer a useful option.

[maettu-this]

@CharliePoole, thanks for the background. Knowning this, I have found https://github.com/nunit/nunit/issues/2758 dealing with this topic, including the hint on using IsolatedContext().

I guess there were good reasons for changing this behavior. I disagree with @rprouse's comment don't see that as a breaking change, for two reasons:

• Runners state that an AssertionException is thrown. And Assert leading to an AssertionException seems obvious logic.
• Is really every feature of NUnit documented? Is it always clear what is considered a feature?

It's obviously hopeless to try convincing the NUnit team to bring back this behavior. I will have to adapt my test implementations where I catch AssertionException now. Uff, another x hours this upgrade is taking...

PS: I catch AssertionException for dealing with instabilities in the test setup, instabilities in communication with physical devices. In case such test fails, it evaluates the reason and error tolerance and then either cancels or rethrows. For handling this, neither Retry nor IsolatedContext sounds like the right thing to do. And Assert.Throws even less, as the test fails only in e.g. 1 out of a thousand times. But I'm sure I'll find a way to handle this.

[CharliePoole]

I'd say you're right about trying to bring it back. Your arguments (made by other people) were considered back at the time the change was made.

Regarding "breaking" I now believe that every project needs a written definition of what they mean by breaking. Otherwise, users push for the loosest possible meaning. :-)

[maettu-this]

Giving an update on this topic, the discussion in https://github.com/nunit/nunit/issues/4701 has actually led to another approach to deal with this issue:

NUnit.Framework.Internal.TestExecutionContext.CurrentContext.CurrentResult.AssertionResults.Clear();

NUnit might even provide a non-internal wrapper method for this approach in the future.

[CharliePoole]

Thanks, I added a comment on that issue. Frankly, as noted, it still seems like a bad idea to me.