From e1e0b2973452a922ade90a9a6e6f956d1b451310 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 21 Sep 2025 13:09:53 +0100 Subject: [PATCH] fix: ensure After(Test) hooks execute before test instance disposal --- TUnit.Engine/TestExecutor.cs | 31 +++-- .../AfterTestDisposalOrderTest.cs | 126 ++++++++++++++++++ 2 files changed, 145 insertions(+), 12 deletions(-) create mode 100644 TUnit.TestProject/AfterTestDisposalOrderTest.cs diff --git a/TUnit.Engine/TestExecutor.cs b/TUnit.Engine/TestExecutor.cs index dfc3642f2c..8da51b3be1 100644 --- a/TUnit.Engine/TestExecutor.cs +++ b/TUnit.Engine/TestExecutor.cs @@ -116,11 +116,17 @@ await TimeoutHelper.ExecuteWithTimeoutAsync( // Run after hooks and event receivers in finally before re-throwing try { - // Dispose test instance before After(Class) hooks run + // Run After(Test) hooks first (before disposal) + await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false); + + // Invoke test end event receivers + await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false); + + // Then dispose test instance await DisposeTestInstance(executableTest).ConfigureAwait(false); - // Always decrement counters and run After hooks if we're the last test - await ExecuteAfterHooksBasedOnLifecycle(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false); + // Finally run After(Class/Assembly/Session) hooks if we're the last test + await ExecuteAfterClassAssemblySessionHooks(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false); } catch { @@ -144,11 +150,17 @@ await TimeoutHelper.ExecuteWithTimeoutAsync( // This finally block now only runs for the success path if (executableTest.State != TestState.Failed) { - // Dispose test instance before After(Class) hooks run + // Run After(Test) hooks first (before disposal) + await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false); + + // Invoke test end event receivers + await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false); + + // Then dispose test instance await DisposeTestInstance(executableTest).ConfigureAwait(false); - // Always decrement counters and run After hooks if we're the last test - await ExecuteAfterHooksBasedOnLifecycle(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false); + // Finally run After(Class/Assembly/Session) hooks if we're the last test + await ExecuteAfterClassAssemblySessionHooks(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false); } } } @@ -176,16 +188,11 @@ await testExecutor.ExecuteTest(executableTest.Context, } } - private async Task ExecuteAfterHooksBasedOnLifecycle(AbstractExecutableTest executableTest, + private async Task ExecuteAfterClassAssemblySessionHooks(AbstractExecutableTest executableTest, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)] Type testClass, Assembly testAssembly, CancellationToken cancellationToken) { - await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false); - - // Invoke test end event receivers - await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false); - var flags = _lifecycleCoordinator.DecrementAndCheckAfterHooks(testClass, testAssembly); if (executableTest.Context.Events.OnDispose != null) diff --git a/TUnit.TestProject/AfterTestDisposalOrderTest.cs b/TUnit.TestProject/AfterTestDisposalOrderTest.cs new file mode 100644 index 0000000000..f073958357 --- /dev/null +++ b/TUnit.TestProject/AfterTestDisposalOrderTest.cs @@ -0,0 +1,126 @@ +using TUnit.Core; +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject; + +/// +/// Regression test for https://github.com/thomhurst/TUnit/issues/3156 +/// Ensures that After(Test) hooks are executed before the test instance is disposed. +/// +[EngineTest(ExpectedResult.Pass)] +public class AfterTestDisposalOrderTest : IAsyncDisposable +{ + private bool _isDisposed; + private readonly string _testResource = "TestResource"; + + public ValueTask DisposeAsync() + { + _isDisposed = true; + return new ValueTask(); + } + + [Test] + public async Task Test_ShouldAccessResourcesInAfterTestHook() + { + // Test should be able to access resources + await Assert.That(_isDisposed).IsFalse(); + await Assert.That(_testResource).IsNotNull(); + await Assert.That(_testResource).IsEqualTo("TestResource"); + } + + [After(Test)] + public async Task AfterTest_ShouldAccessResourcesBeforeDisposal(TestContext context) + { + // After(Test) hook should be able to access instance resources before disposal + await Assert.That(_isDisposed).IsFalse().Because("Test instance should not be disposed before After(Test) hooks"); + await Assert.That(_testResource).IsNotNull().Because("Should be able to access instance fields in After(Test) hook"); + await Assert.That(_testResource).IsEqualTo("TestResource"); + + // Mark that we successfully accessed resources + context.ObjectBag.Add("AfterTestExecuted", true); + context.ObjectBag.Add("ResourceValue", _testResource); + } + + [After(Class)] + public static async Task AfterClass_VerifyDisposalCompleted(ClassHookContext context) + { + // Verify that After(Test) hooks were executed + foreach (var test in context.Tests) + { + await Assert.That(test.ObjectBag.ContainsKey("AfterTestExecuted")).IsTrue().Because("After(Test) hook should have executed"); + await Assert.That(test.ObjectBag["AfterTestExecuted"]).IsEqualTo(true); + await Assert.That(test.ObjectBag["ResourceValue"]).IsEqualTo("TestResource"); + } + + // By the time After(Class) runs, all test instances should be disposed + // We can't directly check _isDisposed here since this is a static method, + // but the fact that After(Test) ran successfully proves the order is correct + } +} + +/// +/// Additional test to verify disposal order with async operations +/// +[EngineTest(ExpectedResult.Pass)] +public class AsyncAfterTestDisposalOrderTest : IAsyncDisposable +{ + private MyAsyncResource? _resource; + private bool _isDisposed; + + public AsyncAfterTestDisposalOrderTest() + { + _resource = new MyAsyncResource(); + } + + public async ValueTask DisposeAsync() + { + if (_resource != null) + { + await _resource.DisposeAsync(); + _resource = null; + } + _isDisposed = true; + } + + [Test] + public async Task Test_WithAsyncResource() + { + await Assert.That(_resource).IsNotNull(); + await Assert.That(_resource!.IsDisposed).IsFalse(); + + var value = await _resource.GetValueAsync(); + await Assert.That(value).IsEqualTo("AsyncValue"); + } + + [After(Test)] + public async Task AfterTest_ShouldAccessAsyncResourceBeforeDisposal() + { + await Assert.That(_isDisposed).IsFalse().Because("Test instance should not be disposed before After(Test) hooks"); + await Assert.That(_resource).IsNotNull().Because("Resource should still be available in After(Test) hook"); + await Assert.That(_resource!.IsDisposed).IsFalse().Because("Resource should not be disposed yet"); + + // Should be able to use the async resource + var value = await _resource.GetValueAsync(); + await Assert.That(value).IsEqualTo("AsyncValue"); + } + + private class MyAsyncResource : IAsyncDisposable + { + public bool IsDisposed { get; private set; } + + public Task GetValueAsync() + { + if (IsDisposed) + { + throw new ObjectDisposedException(nameof(MyAsyncResource)); + } + return Task.FromResult("AsyncValue"); + } + + public ValueTask DisposeAsync() + { + IsDisposed = true; + return new ValueTask(); + } + } +}