Skip to content

Commit bb0a848

Browse files
thomhurstthomhurst
authored
fix: ensure After(Test) hooks execute before test instance disposal (#3159)
1 parent abdbed1 commit bb0a848

File tree

2 files changed

+145
-12
lines changed

2 files changed

+145
-12
lines changed

‎TUnit.Engine/TestExecutor.cs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,17 @@ await TimeoutHelper.ExecuteWithTimeoutAsync(
116116
// Run after hooks and event receivers in finally before re-throwing
117117
try
118118
{
119-
// Dispose test instance before After(Class) hooks run
119+
// Run After(Test) hooks first (before disposal)
120+
await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false);
121+
122+
// Invoke test end event receivers
123+
await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false);
124+
125+
// Then dispose test instance
120126
await DisposeTestInstance(executableTest).ConfigureAwait(false);
121127

122-
// Always decrement counters and run After hooks if we're the last test
123-
await ExecuteAfterHooksBasedOnLifecycle(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false);
128+
// Finally run After(Class/Assembly/Session) hooks if we're the last test
129+
await ExecuteAfterClassAssemblySessionHooks(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false);
124130
}
125131
catch
126132
{
@@ -144,11 +150,17 @@ await TimeoutHelper.ExecuteWithTimeoutAsync(
144150
// This finally block now only runs for the success path
145151
if (executableTest.State != TestState.Failed)
146152
{
147-
// Dispose test instance before After(Class) hooks run
153+
// Run After(Test) hooks first (before disposal)
154+
await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false);
155+
156+
// Invoke test end event receivers
157+
await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false);
158+
159+
// Then dispose test instance
148160
await DisposeTestInstance(executableTest).ConfigureAwait(false);
149161

150-
// Always decrement counters and run After hooks if we're the last test
151-
await ExecuteAfterHooksBasedOnLifecycle(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false);
162+
// Finally run After(Class/Assembly/Session) hooks if we're the last test
163+
await ExecuteAfterClassAssemblySessionHooks(executableTest, testClass, testAssembly, cancellationToken).ConfigureAwait(false);
152164
}
153165
}
154166
}
@@ -176,16 +188,11 @@ await testExecutor.ExecuteTest(executableTest.Context,
176188
}
177189
}
178190

179-
private async Task ExecuteAfterHooksBasedOnLifecycle(AbstractExecutableTest executableTest,
191+
private async Task ExecuteAfterClassAssemblySessionHooks(AbstractExecutableTest executableTest,
180192
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties
181193
| DynamicallyAccessedMemberTypes.PublicMethods)]
182194
Type testClass, Assembly testAssembly, CancellationToken cancellationToken)
183195
{
184-
await _hookExecutor.ExecuteAfterTestHooksAsync(executableTest, cancellationToken).ConfigureAwait(false);
185-
186-
// Invoke test end event receivers
187-
await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(executableTest.Context, cancellationToken).ConfigureAwait(false);
188-
189196
var flags = _lifecycleCoordinator.DecrementAndCheckAfterHooks(testClass, testAssembly);
190197

191198
if (executableTest.Context.Events.OnDispose != null)

‎TUnit.TestProject/AfterTestDisposalOrderTest.cs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
using TUnit.Core;
2+
using TUnit.TestProject.Attributes;
3+
4+
namespace TUnit.TestProject;
5+
6+
/// <summary>
7+
/// Regression test for https://github.com/thomhurst/TUnit/issues/3156
8+
/// Ensures that After(Test) hooks are executed before the test instance is disposed.
9+
/// </summary>
10+
[EngineTest(ExpectedResult.Pass)]
11+
public class AfterTestDisposalOrderTest : IAsyncDisposable
12+
{
13+
private bool _isDisposed;
14+
private readonly string _testResource = "TestResource";
15+
16+
public ValueTask DisposeAsync()
17+
{
18+
_isDisposed = true;
19+
return new ValueTask();
20+
}
21+
22+
[Test]
23+
public async Task Test_ShouldAccessResourcesInAfterTestHook()
24+
{
25+
// Test should be able to access resources
26+
await Assert.That(_isDisposed).IsFalse();
27+
await Assert.That(_testResource).IsNotNull();
28+
await Assert.That(_testResource).IsEqualTo("TestResource");
29+
}
30+
31+
[After(Test)]
32+
public async Task AfterTest_ShouldAccessResourcesBeforeDisposal(TestContext context)
33+
{
34+
// After(Test) hook should be able to access instance resources before disposal
35+
await Assert.That(_isDisposed).IsFalse().Because("Test instance should not be disposed before After(Test) hooks");
36+
await Assert.That(_testResource).IsNotNull().Because("Should be able to access instance fields in After(Test) hook");
37+
await Assert.That(_testResource).IsEqualTo("TestResource");
38+
39+
// Mark that we successfully accessed resources
40+
context.ObjectBag.Add("AfterTestExecuted", true);
41+
context.ObjectBag.Add("ResourceValue", _testResource);
42+
}
43+
44+
[After(Class)]
45+
public static async Task AfterClass_VerifyDisposalCompleted(ClassHookContext context)
46+
{
47+
// Verify that After(Test) hooks were executed
48+
foreach (var test in context.Tests)
49+
{
50+
await Assert.That(test.ObjectBag.ContainsKey("AfterTestExecuted")).IsTrue().Because("After(Test) hook should have executed");
51+
await Assert.That(test.ObjectBag["AfterTestExecuted"]).IsEqualTo(true);
52+
await Assert.That(test.ObjectBag["ResourceValue"]).IsEqualTo("TestResource");
53+
}
54+
55+
// By the time After(Class) runs, all test instances should be disposed
56+
// We can't directly check _isDisposed here since this is a static method,
57+
// but the fact that After(Test) ran successfully proves the order is correct
58+
}
59+
}
60+
61+
/// <summary>
62+
/// Additional test to verify disposal order with async operations
63+
/// </summary>
64+
[EngineTest(ExpectedResult.Pass)]
65+
public class AsyncAfterTestDisposalOrderTest : IAsyncDisposable
66+
{
67+
private MyAsyncResource? _resource;
68+
private bool _isDisposed;
69+
70+
public AsyncAfterTestDisposalOrderTest()
71+
{
72+
_resource = new MyAsyncResource();
73+
}
74+
75+
public async ValueTask DisposeAsync()
76+
{
77+
if (_resource != null)
78+
{
79+
await _resource.DisposeAsync();
80+
_resource = null;
81+
}
82+
_isDisposed = true;
83+
}
84+
85+
[Test]
86+
public async Task Test_WithAsyncResource()
87+
{
88+
await Assert.That(_resource).IsNotNull();
89+
await Assert.That(_resource!.IsDisposed).IsFalse();
90+
91+
var value = await _resource.GetValueAsync();
92+
await Assert.That(value).IsEqualTo("AsyncValue");
93+
}
94+
95+
[After(Test)]
96+
public async Task AfterTest_ShouldAccessAsyncResourceBeforeDisposal()
97+
{
98+
await Assert.That(_isDisposed).IsFalse().Because("Test instance should not be disposed before After(Test) hooks");
99+
await Assert.That(_resource).IsNotNull().Because("Resource should still be available in After(Test) hook");
100+
await Assert.That(_resource!.IsDisposed).IsFalse().Because("Resource should not be disposed yet");
101+
102+
// Should be able to use the async resource
103+
var value = await _resource.GetValueAsync();
104+
await Assert.That(value).IsEqualTo("AsyncValue");
105+
}
106+
107+
private class MyAsyncResource : IAsyncDisposable
108+
{
109+
public bool IsDisposed { get; private set; }
110+
111+
public Task<string> GetValueAsync()
112+
{
113+
if (IsDisposed)
114+
{
115+
throw new ObjectDisposedException(nameof(MyAsyncResource));
116+
}
117+
return Task.FromResult("AsyncValue");
118+
}
119+
120+
public ValueTask DisposeAsync()
121+
{
122+
IsDisposed = true;
123+
return new ValueTask();
124+
}
125+
}
126+
}

0 commit comments

Comments
 (0)