AssertException in TearDown hides Exception in Test

[Sputnik24]

I know from other comments in issues that it is not recommended to do Assertions during TearDown. Nevertheless, I want to report the following to check if it is a known issue/by design or a bug.

Simple code example to demonstrate the issue:

using NUnit.Framework;

namespace DemoTests; 

public class TearDownTests {
    [Test]
    public void Assertion() {
        Assert.IsTrue(false);
    }

    [Test]
    public void Exception() {
        throw new Exception("Test exception");
    }

    [Test]
    public void Pass() {
        
    }

    [TearDown]
    public void TearDown() {
        Assert.Warn("Warning");
    }
}

Result:

• Assertion() tests shows both failures and warnings
• Pass() shows Warning from TearDown
• Exception() only shows Warning and hides/forgets/overrides the Exception

This happens with any AssertException in TearDown. Of course, when I catch the exception and "rethrough" it using something like Assert.Fail(ex.Message), I see both failures as in Assertion() tests, but for me it is one of the big advantages of NUnit that I don't need to catch all my exceptions to let a test fail but that it fails automatically.

[manfred-brands]

Thanks @Sputnik24 for reporting this.

In NUnit there is code that combines assertion results from a TearDown with existing ones, this however overwrites exception results.

There is code to combine results if an exception occurs after assertion failures, but not when assertion occur after an exception.
The later overwrites the exception.

I guess the underlying assumptions were that execution would stop after an exception.

If however the TearDown method throws an exception, you see both as that is handled as a special case.

NUnit could be changed to treat an exception as an assertion failure and always add it to the assertion list.

[OsirisTerje]

NUnit could be changed to treat an exception as an assertion failure and always add it to the assertion list.

Sounds like a good idea.

But should it then always terminate the test, or should it also work as a normal assertion in a Assert.Multiple (haven't checked how it works now btw...)

[Sputnik24]

To my opinion it should not break the existing behavior and terminate the test.

[OsirisTerje]

The current behavior is that it does terminate the test.
The test is reported as failed, the exception message and stacktrace is written to the output though, but there is nothing in the assertion list currently.

PS: It also terminates an Assert.Multiple.

[Sputnik24]

By the way: I just encountered that it also happens with Assert.Pass(). Assert.Pass() in Test has the described behavior (I like to use the message in Pass() to transmit a message in the result for documentation). For me it is better to put this in the TearDown, but Assert.Pass() in TearDown leads to a Failed test: TearDown : NUnit.Framework.SuccessException : Test passes OK.
--TearDown

[manfred-brands]

Failed test: TearDown : NUnit.Framework.SuccessException

A bit of a contradiction ..

[manfred-brands]

@OsirisTerje I picked this back up and have a question about the RepeatAttribute. Should the following test pass or fail?

        [Test, Repeat(3, false)]
        public void SucceedsOnSecondTry()
        {
            Count++;

            if (Count < 2)
                Assert.Fail();
        }

The test is currently asserted as failing in RepeatWithoutStoppingWorksAsExpectedOnFixturesWithSetupAndTeardown.

In the current main, this test is only executed once as the exception from Assert.Fail is not caught in the RepeatTestCommand.
If I replace Assert.Fail with Assert.That(Count, Is.GreaterThanOrEqualTo(2)). The test is executed 3 times, but the end result is still failed because there is one AssertionResult recorded when Count == 1.

Note that the RetryTestCommand does catch exceptions and resets results between runs.
This seems to be more appropriate to me.

[OsirisTerje]

@manfred-brands This is the Repeat attribute, so all repeats should pass. We also have the Retry attribute for the case where previous runs can fail and last one succeeds for a pass.

I believe the test should pass when we receive a Fail in the case here. So we need an Assert.Throws here to catch that.

Actually, I dont understand why we have the stopOnFailure argument there for a Repeat command. What what should be the expected result? And isn't that more or less the same as the Retry if it doesn't stop on failue.

Just found #4861 which is the issue for implementing stopOnFailure. Btw, this is not documented https://docs.nunit.org/articles/nunit/writing-tests/attributes/repeat.html?q=repeat .
I can't see in that issue what is the expected result when it fails. It says it is for performance testing, but I am struggling to see the semantics of it. @cfuerbachersparks - you raised the issue, can you comment on the semantics of this ? @stevenaw Comments?

[cfuerbachersparks]

In our case if one of the iterations fails, the test itself should display failed, but continue until the given iteration count has been reached. We are using the framework to test hardware and the user is generating the code from another source. We got a ticket that the user wants a way to always execute a test 100 times to check if the system works under load, based on that i created #4861. We haven't come across the issue that Assert.Fail is not being caught, but i would expect it to be caught like Assert.That.