Dispose is not called on test fixtures with LifeCycle.InstancePerTestCase without TearDown method

[Crown0815]

Issue

If I run the following test fixture, the test_that_dispose_was_called passes because the Dispose method is called as expected.

[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
public class Test_fixture : IDisposable
{
    private static bool _disposeWasCalled;

    [Test, Order(0)]
    public void test_running_first() => Assert.Pass("Some test running before");

    [Test, Order(1)]
    public void test_that_dispose_was_called() => Assert.That(_disposeWasCalled);

    [TearDown]
    public static void TearDown(){}

    public void Dispose() => _disposeWasCalled = true;
}

However, if I remove the TearDown method, the Dispose method is no longer called and _wasDisposed remains false.

[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
public class Test_fixture : IDisposable
{
    private static bool _disposeWasCalled;

    [Test, Order(0)]
    public void test_running_first() => Assert.Pass("Some test running before");

    [Test, Order(1)]
    public void test_that_dispose_was_called() => Assert.That(_disposeWasCalled);

    public void Dispose() => _disposeWasCalled = true;
}

Expected

I would have expected Dispose to be called after every test if the fixture has the attribute FixtureLifeCycle(LifeCycle.InstancePerTestCase) set on it, independent of the presence of a TearDown method. This is expectation is based on the documentation of the FixtureLifeCycleAttribute, which states that IDisposable fixtures are disposed after each test.

[Crown0815]

It appears to be linked to exceptions being thrown from the test_running_first. If test_running_first throws no exception (Assert.Pass(...) also throws an exception) the Dispose method is called as expected.

[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
public class Test_fixture : IDisposable
{
    private static bool _disposeWasCalled;

    [Test, Order(0)]
    public void test_running_first() {}

    [Test, Order(1)]
    public void test_that_dispose_was_called() => Assert.That(_disposeWasCalled);

    public void Dispose() => _disposeWasCalled = true;
}

[stevenaw]

Thanks for reporting this @Crown0815 !
I think there's been a few bugs fixed in this area in recent releases. Can you please confirm which version of the NUnit framework you are seeing this on?

[Crown0815]

@stevenaw, I tested this behavior with v3.13.2.

[stevenaw]

Thanks for confirming @Crown0815

I've labeled this for follow-up

[mikkelbu]

I can confirm it on the 3.13-dev branch, but I'm unsure what the problem is, as I've not been involved much with FixtureLifeCycle

[rprouse]

Dispose was working, but this area of the code is fragile. We'll need to look into it.

[manfred-brands]

When there is a TearDown (or SetUp for that matter), the BeforeAndAfterTestCommand runs the actual test inside a try/catch.
When no such method exists, only an AfterTestCommand is run. This runs the actual test without a try/catch so that if the actual test routine throws an exception, the AfterTest command is not executed.

This might affect other use cases as there are more classes deriving from AfterTestCommand.

[manfred-brands]

I'll create a PR.