Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions TUnit.Engine/Services/TestExecution/TestCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, Cancellatio
}
else
{
// Run Before(TestSession/Assembly/Class) hooks (incl. BeforeEvery) before constructing
// the instance so the documented lifecycle contract holds — hooks precede the
// constructor (#6192). Hook tasks are cached, so ExecuteAsync's later awaits are no-ops.
await _testExecutor.EnsureClassAndAssemblyHooksExecutedAsync(test, cancellationToken).ConfigureAwait(false);

// Flow AsyncLocals captured by Before(Assembly)/Before(Class) hooks into the
// constructor. Must run here (not inside the gate above) so the restored ambient
// ExecutionContext is still in effect when CreateInstanceAsync runs the constructor.
test.Context.ClassContext.RestoreExecutionContext();

test.Context.Metadata.TestDetails.ClassInstance = await test.CreateInstanceAsync().ConfigureAwait(false);

// Drop the cached eligible-objects list so any later consumer rebuilds it with the new ClassInstance included — the initial list was built before the instance existed.
Expand Down Expand Up @@ -350,6 +360,15 @@ private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, C
return;
}

// Run Before(TestSession/Assembly/Class) hooks (incl. BeforeEvery) before constructing the
// instance so hooks precede the constructor (#6192). Cached, so ExecuteAsync re-awaits are no-ops.
await _testExecutor.EnsureClassAndAssemblyHooksExecutedAsync(test, cancellationToken).ConfigureAwait(false);

// Flow AsyncLocals captured by Before(Assembly)/Before(Class) hooks into the constructor.
// Must run here (not inside the gate above) so the restored ambient ExecutionContext is still
// in effect when CreateInstanceAsync runs the constructor.
test.Context.ClassContext.RestoreExecutionContext();

test.Context.Metadata.TestDetails.ClassInstance = await test.CreateInstanceAsync().ConfigureAwait(false);

// Drop the cached eligible-objects list so any later consumer rebuilds it with the new ClassInstance included — the initial list was built before the instance existed.
Expand Down
51 changes: 48 additions & 3 deletions TUnit.Engine/TestExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ internal class TestExecutor
private readonly IContextProvider _contextProvider;
private readonly EventReceiverOrchestrator _eventReceiverOrchestrator;

// Cached hook-factory delegates so the per-test before-hook awaits don't allocate a fresh closure
// each time (these run on the hot path and the factory is only invoked on the first cache miss).
private readonly Func<CancellationToken, ValueTask> _beforeTestSessionHookFactory;
private readonly Func<Assembly, CancellationToken, ValueTask> _beforeAssemblyHookFactory;

public TestExecutor(
HookExecutor hookExecutor,
TestLifecycleCoordinator lifecycleCoordinator,
Expand All @@ -41,6 +46,9 @@ public TestExecutor(
_afterHookPairTracker = afterHookPairTracker;
_contextProvider = contextProvider;
_eventReceiverOrchestrator = eventReceiverOrchestrator;

_beforeTestSessionHookFactory = ct => _hookExecutor.ExecuteBeforeTestSessionHooksAsync(ct);
_beforeAssemblyHookFactory = (assembly, ct) => _hookExecutor.ExecuteBeforeAssemblyHooksAsync(assembly, ct);
}


Expand All @@ -49,11 +57,11 @@ public TestExecutor(
/// This is called before creating test instances to ensure resources are available.
/// Registers the corresponding After(TestSession) hook to run on cancellation.
/// </summary>
public async Task EnsureTestSessionHooksExecutedAsync(CancellationToken cancellationToken)
public async ValueTask EnsureTestSessionHooksExecutedAsync(CancellationToken cancellationToken)
{
// Get or create and cache Before hooks - these run only once
await _beforeHookTaskCache.GetOrCreateBeforeTestSessionTask(
ct => _hookExecutor.ExecuteBeforeTestSessionHooksAsync(ct),
_beforeTestSessionHookFactory,
cancellationToken).ConfigureAwait(false);

// Register After Session hook to run on cancellation (guarantees cleanup)
Expand All @@ -62,6 +70,43 @@ await _beforeHookTaskCache.GetOrCreateBeforeTestSessionTask(
() => _hookExecutor.ExecuteAfterTestSessionHooksAsync(CancellationToken.None));
}

/// <summary>
/// Ensures Before(TestSession), Before(Assembly) and Before(Class) hooks (including their
/// BeforeEvery counterparts) have executed before the test class instance is constructed, so the
/// documented lifecycle contract (hooks run before instantiation) holds — see issue #6192.
/// The hook tasks are cached in <see cref="BeforeHookTaskCache"/>, so the matching awaits later in
/// <see cref="ExecuteAsync"/> are no-ops. Only the pure cached getters are invoked here; the
/// After-hook pair registration stays single-sourced in ExecuteAsync to avoid double-registration.
/// </summary>
public async ValueTask EnsureClassAndAssemblyHooksExecutedAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
{
var testClass = test.Metadata.TestClassType;

await _beforeHookTaskCache.GetOrCreateBeforeTestSessionTask(
_beforeTestSessionHookFactory,
cancellationToken).ConfigureAwait(false);

// Flow AsyncLocals captured by BeforeTestSession into the BeforeAssembly hook, and likewise
// BeforeAssembly into BeforeClass. This mirrors the RestoreExecutionContext chain in
// ExecuteAsync (the assembly/class hook tasks are cached, so when ExecuteAsync re-awaits them
// it re-applies the same captured contexts).
test.Context.ClassContext.AssemblyContext.TestSessionContext.RestoreExecutionContext();

await _beforeHookTaskCache.GetOrCreateBeforeAssemblyTask(
testClass.Assembly,
_beforeAssemblyHookFactory,
cancellationToken).ConfigureAwait(false);

test.Context.ClassContext.AssemblyContext.RestoreExecutionContext();

await _beforeHookTaskCache.GetOrCreateBeforeClassTask(testClass, _hookExecutor, cancellationToken).ConfigureAwait(false);

// Note: the caller (TestCoordinator) restores ClassContext.RestoreExecutionContext() right
// before constructing the instance so AsyncLocals captured by BeforeAssembly/BeforeClass flow
// into the constructor. Restoring here wouldn't persist — the ambient ExecutionContext is
// reset when this async method returns to the caller.
}

/// <summary>
/// Creates a test executor delegate that wraps the provided executor with hook orchestration.
/// Uses focused services that follow SRP to manage lifecycle and execution.
Expand All @@ -88,7 +133,7 @@ await _eventReceiverOrchestrator.InvokeFirstTestInSessionEventReceiversAsync(

await _beforeHookTaskCache.GetOrCreateBeforeAssemblyTask(
testAssembly,
(assembly, ct) => _hookExecutor.ExecuteBeforeAssemblyHooksAsync(assembly, ct),
_beforeAssemblyHookFactory,
cancellationToken).ConfigureAwait(false);

// Register After Assembly hook to run on cancellation (guarantees cleanup)
Expand Down
133 changes: 133 additions & 0 deletions TUnit.TestProject/Bugs/6192/Bug6192Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
using System.Threading;
using TUnit.TestProject.Attributes;

namespace TUnit.TestProject.Bugs._6192;

// Regression test for #6192: lifecycle hooks must run BEFORE the test class is constructed.
// Previously the constructor ran before Before(Assembly)/Before(Class) (and their BeforeEvery
// counterparts), so the assertions in the constructors below would throw.

public class Bug6192HookOrderProbe
{
// volatile to make the happens-before intent explicit (the framework's task-caching already
// provides the edge between hook completion and the constructor that reads these).
public static volatile bool BeforeEveryClassRan;
public static volatile bool BeforeEveryAssemblyRan;
public static volatile bool BeforeClassRan;
public static volatile bool BeforeAssemblyRan;

// AsyncLocal set inside a [Before(Class)] hook (which opts in via context.AddAsyncLocalValues()).
// Its value must flow into the constructor, which proves ClassContext.RestoreExecutionContext()
// runs before construction in the pre-construction hook gate (#6192 / review feedback).
public static readonly AsyncLocal<string?> ClassHookAsyncLocal = new();

[BeforeEvery(Class)]
public static void BeforeEveryClass(ClassHookContext context)
{
if (context.ClassType == typeof(Bug6192Tests) || context.ClassType == typeof(Bug6192RetryTests))
{
BeforeEveryClassRan = true;
}
}

[BeforeEvery(Assembly)]
public static void BeforeEveryAssembly(AssemblyHookContext context)
{
if (context.Assembly == typeof(Bug6192Tests).Assembly)
{
BeforeEveryAssemblyRan = true;
}
}
}

[EngineTest(ExpectedResult.Pass)]
public class Bug6192Tests
{
// Non-BeforeEvery hooks go through the same cache path; assert they precede the constructor too.
[Before(Assembly)]
public static void BeforeAssembly() => Bug6192HookOrderProbe.BeforeAssemblyRan = true;

[Before(Class)]
public static void BeforeClass(ClassHookContext context)
{
Bug6192HookOrderProbe.BeforeClassRan = true;
Bug6192HookOrderProbe.ClassHookAsyncLocal.Value = "set-by-before-class";
// Opt in to flowing this AsyncLocal forward (otherwise TUnit doesn't capture it).
context.AddAsyncLocalValues();
}

public Bug6192Tests()
{
// The constructor must run AFTER the lifecycle hooks (issue #6192).
if (!Bug6192HookOrderProbe.BeforeEveryAssemblyRan)
{
throw new InvalidOperationException("Constructor ran before [BeforeEvery(Assembly)] hook");
}

if (!Bug6192HookOrderProbe.BeforeEveryClassRan)
{
throw new InvalidOperationException("Constructor ran before [BeforeEvery(Class)] hook");
}

if (!Bug6192HookOrderProbe.BeforeAssemblyRan)
{
throw new InvalidOperationException("Constructor ran before [Before(Assembly)] hook");
}

if (!Bug6192HookOrderProbe.BeforeClassRan)
{
throw new InvalidOperationException("Constructor ran before [Before(Class)] hook");
}

// AsyncLocals captured by a class-level hook must flow into the constructor, otherwise the
// hook ran but ClassContext.RestoreExecutionContext() did not run before construction.
if (Bug6192HookOrderProbe.ClassHookAsyncLocal.Value != "set-by-before-class")
{
throw new InvalidOperationException("Before(Class) AsyncLocal did not flow into the constructor");
}
}

[Test]
public void TestA()
{
}

[Test]
public void TestB()
{
}
}

// Same contract must hold on the retry code path (ExecuteTestLifecycleAsync), which has its own
// pre-construction hook gate separate from the no-retry fast path.
[EngineTest(ExpectedResult.Pass)]
public class Bug6192RetryTests
{
public static readonly AsyncLocal<string?> ClassHookAsyncLocal = new();

[Before(Class)]
public static void BeforeClass(ClassHookContext context)
{
ClassHookAsyncLocal.Value = "set-by-before-class-retry";
context.AddAsyncLocalValues();
}

public Bug6192RetryTests()
{
if (!Bug6192HookOrderProbe.BeforeEveryClassRan)
{
throw new InvalidOperationException("Constructor ran before [BeforeEvery(Class)] hook (retry path)");
}

if (ClassHookAsyncLocal.Value != "set-by-before-class-retry")
{
throw new InvalidOperationException("Before(Class) AsyncLocal did not flow into the constructor (retry path)");
}
}

[Retry(1)]
[Test]
public void RetriedTest()
{
}
}
Loading