From 9f7bba723df5ff6cba073ed43366361f53516487 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 25 Apr 2026 10:45:50 +0100 Subject: [PATCH 1/6] fix(engine): run unconstrained Parallel and KeyedNotInParallel concurrently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discussion #5700: with `[Test] T1` and `[Test, NotInParallel("k")] T2`, T1 was expected to run alongside T2 (per docs/execution/parallelism.md: "Tests with no common key may still run concurrently"), but in practice T1 always finished before T2 started. Cause: TestScheduler.ExecuteGroupedTestsAsync awaited each grouping bucket in sequence. The Parallel bucket (no constraints) drained fully before the KeyedNotInParallel bucket began, so unconstrained tests never overlapped keyed ones. Run those two buckets concurrently — they share no synchronization state. ParallelGroups still serialize (cross-group exclusion) and the global NotInParallel bucket still drains last (must run alone). Adds a regression test (Bugs/5700) that asserts Test1 observes Test2 active during its execution, plus an engine-level wrapper test. --- .../KeyedNotInParallelCrossPhaseTests.cs | 28 +++++++ TUnit.Engine/Scheduling/TestScheduler.cs | 29 +++++-- TUnit.TestProject/Bugs/5700/Repro.cs | 78 +++++++++++++++++++ 3 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs create mode 100644 TUnit.TestProject/Bugs/5700/Repro.cs diff --git a/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs b/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs new file mode 100644 index 0000000000..99d9c00139 --- /dev/null +++ b/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs @@ -0,0 +1,28 @@ +using Shouldly; +using TUnit.Engine.Tests.Enums; + +namespace TUnit.Engine.Tests.Scheduling; + +/// +/// Regression for https://github.com/thomhurst/TUnit/discussions/5700. +/// An unconstrained [Test] must run concurrently with [Test, NotInParallel("key")] tests +/// in the same class — keyed NotInParallel only blocks tests sharing a key, it must not +/// serialize against the rest of the suite. +/// +public class KeyedNotInParallelCrossPhaseTests(TestMode testMode) : InvokableTestBase(testMode) +{ + [Test] + public async Task UnconstrainedTest_RunsAlongsideKeyedNotInParallelTest() + { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + + await RunTestsWithFilter("/*/*/Repro5700/*", + [ + result => result.ResultSummary.Outcome.ShouldBe("Completed"), + result => result.ResultSummary.Counters.Total.ShouldBe(3), + result => result.ResultSummary.Counters.Passed.ShouldBe(3), + result => result.ResultSummary.Counters.Failed.ShouldBe(0), + ], + new RunOptions().WithForcefulCancellationToken(cts.Token)); + } +} diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs index ea64e58369..dad1c73018 100644 --- a/TUnit.Engine/Scheduling/TestScheduler.cs +++ b/TUnit.Engine/Scheduling/TestScheduler.cs @@ -171,11 +171,31 @@ private async Task ExecuteGroupedTestsAsync( // Start dynamic test queue processing in background var dynamicTestProcessingTask = ProcessDynamicTestQueueAsync(cancellationToken); + // Unconstrained Parallel and KeyedNotInParallel buckets share no state — keyed tests + // only block other tests sharing a key (per docs/parallelism.md). Running them + // sequentially makes a `[Test] T1` always run before `[Test, NotInParallel("k")] T2` + // which is observable as "Test1 never overlaps Test2" (discussion #5700). Run them + // concurrently instead. ParallelGroups still serialize (cross-group exclusion) and + // global NotInParallel still drains last (must run alone). + var concurrentPhases = new List(2); + if (groupedTests.Parallel.Length > 0) { if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Starting {groupedTests.Parallel.Length} parallel tests").ConfigureAwait(false); - await ExecuteTestsAsync(groupedTests.Parallel, cancellationToken).ConfigureAwait(false); + concurrentPhases.Add(ExecuteTestsAsync(groupedTests.Parallel, cancellationToken)); + } + + if (groupedTests.KeyedNotInParallel.Length > 0) + { + if (_logger.IsTraceEnabled) + await _logger.LogTraceAsync($"Starting {groupedTests.KeyedNotInParallel.Length} keyed NotInParallel tests").ConfigureAwait(false); + concurrentPhases.Add(_constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(groupedTests.KeyedNotInParallel, cancellationToken).AsTask()); + } + + if (concurrentPhases.Count > 0) + { + await WaitForTasksWithFailFastHandling(concurrentPhases.ToArray(), cancellationToken).ConfigureAwait(false); } foreach (var group in groupedTests.ParallelGroups) @@ -218,13 +238,6 @@ private async Task ExecuteGroupedTestsAsync( } } - if (groupedTests.KeyedNotInParallel.Length > 0) - { - if (_logger.IsTraceEnabled) - await _logger.LogTraceAsync($"Starting {groupedTests.KeyedNotInParallel.Length} keyed NotInParallel tests").ConfigureAwait(false); - await _constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(groupedTests.KeyedNotInParallel, cancellationToken).ConfigureAwait(false); - } - if (groupedTests.NotInParallel.Length > 0) { if (_logger.IsTraceEnabled) diff --git a/TUnit.TestProject/Bugs/5700/Repro.cs b/TUnit.TestProject/Bugs/5700/Repro.cs new file mode 100644 index 0000000000..b82e3d658e --- /dev/null +++ b/TUnit.TestProject/Bugs/5700/Repro.cs @@ -0,0 +1,78 @@ +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._5700; + +/// +/// Regression test for https://github.com/thomhurst/TUnit/discussions/5700. +/// +/// Test1 has no parallel constraint. Test2 has [NotInParallel("Tests.Test2")] — a +/// keyed constraint that should only block other tests sharing the key (per +/// docs/execution/parallelism.md). The buggy scheduler ran all unconstrained +/// tests to completion before starting keyed-NotInParallel tests, so Test1 +/// never overlapped Test2. +/// +/// Assertions: +/// - Test1 must observe Test2 running concurrently (proves cross-phase parallelism). +/// - Test2 invocations must never run concurrently with each other (proves keyed +/// serialization still works). +/// +[EngineTest(ExpectedResult.Pass)] +public class Repro5700 +{ + private static int _test1Active; + private static int _test2Active; + private static int _test1ObservedTest2; + private static int _test2ConcurrentViolations; + + [Test] + public async Task Test1_RunsAlongsideKeyedTest2() + { + Interlocked.Increment(ref _test1Active); + try + { + // Sample for 3s; keyed Test2 cases serialize to ~2s so Test1 must + // overlap at least one Test2 invocation if cross-phase parallelism works. + for (var i = 0; i < 60; i++) + { + if (Volatile.Read(ref _test2Active) > 0) + { + Interlocked.Increment(ref _test1ObservedTest2); + } + await Task.Delay(50); + } + } + finally + { + Interlocked.Decrement(ref _test1Active); + } + } + + [Test] + [Arguments(1)] + [Arguments(2)] + [NotInParallel("Tests.Test2")] + public async Task Test2_KeyedSerializes(int param) + { + var concurrent = Interlocked.Increment(ref _test2Active); + try + { + if (concurrent > 1) + { + Interlocked.Increment(ref _test2ConcurrentViolations); + } + await Task.Delay(1000); + _ = param; + } + finally + { + Interlocked.Decrement(ref _test2Active); + } + } + + [After(Class)] + public static async Task AssertParallelismShape() + { + await Assert.That(_test2ConcurrentViolations).IsZero(); + await Assert.That(_test1ObservedTest2).IsGreaterThan(0); + } +} From 108055159b1621f1d168085573da339ee2fe2213 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 25 Apr 2026 10:55:12 +0100 Subject: [PATCH 2/6] =?UTF-8?q?chore(5700):=20cleanups=20from=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20unused=20state,=20drop=20ToArray,=20name=20k?= =?UTF-8?q?ey?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TUnit.Engine/Scheduling/TestScheduler.cs | 2 +- TUnit.TestProject/Bugs/5700/Repro.cs | 28 +++++++++--------------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs index dad1c73018..f9b7175b0e 100644 --- a/TUnit.Engine/Scheduling/TestScheduler.cs +++ b/TUnit.Engine/Scheduling/TestScheduler.cs @@ -195,7 +195,7 @@ private async Task ExecuteGroupedTestsAsync( if (concurrentPhases.Count > 0) { - await WaitForTasksWithFailFastHandling(concurrentPhases.ToArray(), cancellationToken).ConfigureAwait(false); + await WaitForTasksWithFailFastHandling(concurrentPhases, cancellationToken).ConfigureAwait(false); } foreach (var group in groupedTests.ParallelGroups) diff --git a/TUnit.TestProject/Bugs/5700/Repro.cs b/TUnit.TestProject/Bugs/5700/Repro.cs index b82e3d658e..b654720c5b 100644 --- a/TUnit.TestProject/Bugs/5700/Repro.cs +++ b/TUnit.TestProject/Bugs/5700/Repro.cs @@ -5,7 +5,7 @@ namespace TUnit.TestProject.Bugs._5700; /// /// Regression test for https://github.com/thomhurst/TUnit/discussions/5700. /// -/// Test1 has no parallel constraint. Test2 has [NotInParallel("Tests.Test2")] — a +/// Test1 has no parallel constraint. Test2 has [NotInParallel(Test2Key)] — a /// keyed constraint that should only block other tests sharing the key (per /// docs/execution/parallelism.md). The buggy scheduler ran all unconstrained /// tests to completion before starting keyed-NotInParallel tests, so Test1 @@ -19,7 +19,8 @@ namespace TUnit.TestProject.Bugs._5700; [EngineTest(ExpectedResult.Pass)] public class Repro5700 { - private static int _test1Active; + private const string Test2Key = "Tests.Test2"; + private static int _test2Active; private static int _test1ObservedTest2; private static int _test2ConcurrentViolations; @@ -27,30 +28,22 @@ public class Repro5700 [Test] public async Task Test1_RunsAlongsideKeyedTest2() { - Interlocked.Increment(ref _test1Active); - try + // Test1 = 3s sample window; Test2 cases serialize to ~2s, so Test1 + // must overlap at least one Test2 invocation when the fix is in place. + for (var i = 0; i < 60; i++) { - // Sample for 3s; keyed Test2 cases serialize to ~2s so Test1 must - // overlap at least one Test2 invocation if cross-phase parallelism works. - for (var i = 0; i < 60; i++) + if (Volatile.Read(ref _test2Active) > 0) { - if (Volatile.Read(ref _test2Active) > 0) - { - Interlocked.Increment(ref _test1ObservedTest2); - } - await Task.Delay(50); + Interlocked.Increment(ref _test1ObservedTest2); } - } - finally - { - Interlocked.Decrement(ref _test1Active); + await Task.Delay(50); } } [Test] [Arguments(1)] [Arguments(2)] - [NotInParallel("Tests.Test2")] + [NotInParallel(Test2Key)] public async Task Test2_KeyedSerializes(int param) { var concurrent = Interlocked.Increment(ref _test2Active); @@ -61,7 +54,6 @@ public async Task Test2_KeyedSerializes(int param) Interlocked.Increment(ref _test2ConcurrentViolations); } await Task.Delay(1000); - _ = param; } finally { From abea5801435897adbd56cfaf04c46e0accb31043 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 25 Apr 2026 11:04:50 +0100 Subject: [PATCH 3/6] =?UTF-8?q?fix(5700):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20TCS=20rendezvous,=20phase=20helper,=20cross-key=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Repro5700 now uses TaskCompletionSource rendezvous instead of a 3s wall-clock sample window so the assertion is deterministic on slow CI runners. Test1 signals once it is live and waits for any Test2 invocation; each Test2 invocation does the inverse. Either side timing out flags the bug. - New CrossKeyOverlap5700 fixture covers the symmetric scenario (two [NotInParallel] tests with different keys must overlap). - Extract `RunPhasesConcurrentlyAsync(Task?, Task?, CancellationToken)` helper in TestScheduler. Unifies the new Parallel + KeyedNotInParallel call site with the existing ConstrainedParallelGroups loop and removes the per-phase `List` + `ToArray()` dance (zero allocation in the common single-phase case). - Bump CloudShop OrderWorkflow Step3 WaitsFor timeout from 25s → 60s. The worker eventual-consistency window was already borderline; under the new cross-phase concurrency the RabbitMQ + worker pipeline shares CPU with unconstrained tests and 25s no longer fits a busy CI runner. --- .../KeyedNotInParallelCrossPhaseTests.cs | 15 +++++++ TUnit.Engine/Scheduling/TestScheduler.cs | 40 ++++++++--------- .../Bugs/5700/CrossKeyOverlap.cs | 37 ++++++++++++++++ TUnit.TestProject/Bugs/5700/Repro.cs | 43 +++++++++++-------- .../Tests/Orders/OrderWorkflowTests.cs | 4 +- 5 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs diff --git a/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs b/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs index 99d9c00139..4b8bc1c4e0 100644 --- a/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs +++ b/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelCrossPhaseTests.cs @@ -25,4 +25,19 @@ await RunTestsWithFilter("/*/*/Repro5700/*", ], new RunOptions().WithForcefulCancellationToken(cts.Token)); } + + [Test] + public async Task KeyedNotInParallel_DifferentKeys_RunInParallel() + { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + + await RunTestsWithFilter("/*/*/CrossKeyOverlap5700/*", + [ + result => result.ResultSummary.Outcome.ShouldBe("Completed"), + result => result.ResultSummary.Counters.Total.ShouldBe(2), + result => result.ResultSummary.Counters.Passed.ShouldBe(2), + result => result.ResultSummary.Counters.Failed.ShouldBe(0), + ], + new RunOptions().WithForcefulCancellationToken(cts.Token)); + } } diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs index f9b7175b0e..3065d04fe9 100644 --- a/TUnit.Engine/Scheduling/TestScheduler.cs +++ b/TUnit.Engine/Scheduling/TestScheduler.cs @@ -177,26 +177,23 @@ private async Task ExecuteGroupedTestsAsync( // which is observable as "Test1 never overlaps Test2" (discussion #5700). Run them // concurrently instead. ParallelGroups still serialize (cross-group exclusion) and // global NotInParallel still drains last (must run alone). - var concurrentPhases = new List(2); - + Task? parallelPhase = null; if (groupedTests.Parallel.Length > 0) { if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Starting {groupedTests.Parallel.Length} parallel tests").ConfigureAwait(false); - concurrentPhases.Add(ExecuteTestsAsync(groupedTests.Parallel, cancellationToken)); + parallelPhase = ExecuteTestsAsync(groupedTests.Parallel, cancellationToken); } + Task? keyedPhase = null; if (groupedTests.KeyedNotInParallel.Length > 0) { if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Starting {groupedTests.KeyedNotInParallel.Length} keyed NotInParallel tests").ConfigureAwait(false); - concurrentPhases.Add(_constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(groupedTests.KeyedNotInParallel, cancellationToken).AsTask()); + keyedPhase = _constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(groupedTests.KeyedNotInParallel, cancellationToken).AsTask(); } - if (concurrentPhases.Count > 0) - { - await WaitForTasksWithFailFastHandling(concurrentPhases, cancellationToken).ConfigureAwait(false); - } + await RunPhasesConcurrentlyAsync(parallelPhase, keyedPhase, cancellationToken).ConfigureAwait(false); foreach (var group in groupedTests.ParallelGroups) { @@ -223,19 +220,13 @@ private async Task ExecuteGroupedTestsAsync( if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Starting constrained parallel group '{kvp.Key}' with {constrainedTests.UnconstrainedTests.Length} unconstrained and {constrainedTests.KeyedTests.Length} keyed tests").ConfigureAwait(false); - var tasks = new List(); - if (constrainedTests.UnconstrainedTests.Length > 0) - { - tasks.Add(ExecuteTestsAsync(constrainedTests.UnconstrainedTests, cancellationToken)); - } - if (constrainedTests.KeyedTests.Length > 0) - { - tasks.Add(_constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(constrainedTests.KeyedTests, cancellationToken).AsTask()); - } - if (tasks.Count > 0) - { - await WaitForTasksWithFailFastHandling(tasks.ToArray(), cancellationToken).ConfigureAwait(false); - } + var unconstrainedPhase = constrainedTests.UnconstrainedTests.Length > 0 + ? ExecuteTestsAsync(constrainedTests.UnconstrainedTests, cancellationToken) + : null; + var groupKeyedPhase = constrainedTests.KeyedTests.Length > 0 + ? _constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(constrainedTests.KeyedTests, cancellationToken).AsTask() + : null; + await RunPhasesConcurrentlyAsync(unconstrainedPhase, groupKeyedPhase, cancellationToken).ConfigureAwait(false); } if (groupedTests.NotInParallel.Length > 0) @@ -402,6 +393,13 @@ private async Task ExecuteWithGlobalLimitAsync( } #endif + private Task RunPhasesConcurrentlyAsync(Task? a, Task? b, CancellationToken cancellationToken) + { + if (a is null) return b ?? Task.CompletedTask; + if (b is null) return a; + return WaitForTasksWithFailFastHandling([a, b], cancellationToken); + } + private async Task WaitForTasksWithFailFastHandling(IEnumerable tasks, CancellationToken cancellationToken) { try diff --git a/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs b/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs new file mode 100644 index 0000000000..a1bc36d02d --- /dev/null +++ b/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs @@ -0,0 +1,37 @@ +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._5700; + +/// +/// Companion to Repro5700. Verifies the symmetric scenario for #5700: +/// two [NotInParallel] tests with different keys live in the same keyed +/// bucket and must overlap each other (their keys do not intersect). +/// Uses a TaskCompletionSource rendezvous so it does not depend on timing. +/// +[EngineTest(ExpectedResult.Pass)] +public class CrossKeyOverlap5700 +{ + private const string KeyA = "Tests.5700.A"; + private const string KeyB = "Tests.5700.B"; + + private static readonly TaskCompletionSource KeyALive = new(TaskCreationOptions.RunContinuationsAsynchronously); + private static readonly TaskCompletionSource KeyBLive = new(TaskCreationOptions.RunContinuationsAsynchronously); + + [Test] + [NotInParallel(KeyA)] + public async Task KeyedA_RunsAlongsideKeyedB() + { + KeyALive.TrySetResult(); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + await KeyBLive.Task.WaitAsync(cts.Token); + } + + [Test] + [NotInParallel(KeyB)] + public async Task KeyedB_RunsAlongsideKeyedA() + { + KeyBLive.TrySetResult(); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + await KeyALive.Task.WaitAsync(cts.Token); + } +} diff --git a/TUnit.TestProject/Bugs/5700/Repro.cs b/TUnit.TestProject/Bugs/5700/Repro.cs index b654720c5b..9166aabd8f 100644 --- a/TUnit.TestProject/Bugs/5700/Repro.cs +++ b/TUnit.TestProject/Bugs/5700/Repro.cs @@ -11,33 +11,33 @@ namespace TUnit.TestProject.Bugs._5700; /// tests to completion before starting keyed-NotInParallel tests, so Test1 /// never overlapped Test2. /// -/// Assertions: -/// - Test1 must observe Test2 running concurrently (proves cross-phase parallelism). -/// - Test2 invocations must never run concurrently with each other (proves keyed -/// serialization still works). +/// The test uses a rendezvous (TaskCompletionSource) rather than wall-clock +/// sampling so it stays deterministic on slow CI runners: Test1 waits for +/// any Test2 invocation to start, and each Test2 invocation waits for Test1 +/// to be live. Either side timing out → fix is broken. +/// +/// Test2 also asserts the keyed constraint still serializes its own invocations. /// [EngineTest(ExpectedResult.Pass)] public class Repro5700 { private const string Test2Key = "Tests.Test2"; + private static readonly TaskCompletionSource Test1Live = new(TaskCreationOptions.RunContinuationsAsynchronously); + private static readonly TaskCompletionSource Test2Live = new(TaskCreationOptions.RunContinuationsAsynchronously); private static int _test2Active; - private static int _test1ObservedTest2; private static int _test2ConcurrentViolations; [Test] public async Task Test1_RunsAlongsideKeyedTest2() { - // Test1 = 3s sample window; Test2 cases serialize to ~2s, so Test1 - // must overlap at least one Test2 invocation when the fix is in place. - for (var i = 0; i < 60; i++) - { - if (Volatile.Read(ref _test2Active) > 0) - { - Interlocked.Increment(ref _test1ObservedTest2); - } - await Task.Delay(50); - } + Test1Live.TrySetResult(); + + // If keyed Test2 cannot run alongside Test1, this wait never completes. + // The 15s ceiling is enough headroom for any plausible scheduling delay + // while still failing fast on the bug. + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + await Test2Live.Task.WaitAsync(cts.Token); } [Test] @@ -53,7 +53,15 @@ public async Task Test2_KeyedSerializes(int param) { Interlocked.Increment(ref _test2ConcurrentViolations); } - await Task.Delay(1000); + + Test2Live.TrySetResult(); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + await Test1Live.Task.WaitAsync(cts.Token); + + // Hold the key briefly so the second Test2 invocation queues behind + // us, exposing any concurrency-violation in keyed serialization. + await Task.Delay(200); } finally { @@ -62,9 +70,8 @@ public async Task Test2_KeyedSerializes(int param) } [After(Class)] - public static async Task AssertParallelismShape() + public static async Task AssertKeyedSerialization() { await Assert.That(_test2ConcurrentViolations).IsZero(); - await Assert.That(_test1ObservedTest2).IsGreaterThan(0); } } diff --git a/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs b/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs index b1c6b53c4c..6e52c7f008 100644 --- a/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs +++ b/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs @@ -88,11 +88,13 @@ public async Task Step3_Worker_Fulfills_Order() // The Worker service listens for PaymentProcessed events on RabbitMQ // and updates the order status to Fulfilled. // WaitsFor polls repeatedly until the assertion passes or the timeout expires. + // Generous timeout so the assertion doesn't trip on a busy CI runner where + // RabbitMQ delivery + Worker processing can lag behind under concurrent load. var order = await Assert.That(async () => await Customer.Client.GetFromJsonAsync($"/api/orders/{orderId}")) .WaitsFor( assert => assert.Satisfies(o => o?.Status == OrderStatus.Fulfilled), - timeout: TimeSpan.FromSeconds(25), + timeout: TimeSpan.FromSeconds(60), pollingInterval: TimeSpan.FromMilliseconds(500)); await Assert.That(order).IsNotNull(); From 2389274b4c54752229495e7b4d141fbba9167461 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 25 Apr 2026 11:12:01 +0100 Subject: [PATCH 4/6] fix(engine): route dynamic test queue through full phase pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #5740 review item: ProcessDynamicTestQueueAsync only handled Parallel and global NotInParallel — dynamically-added tests with [NotInParallel(key)], [ParallelGroup(...)], or constrained-parallel-group attributes were silently dropped. Extract ExecuteAllPhasesAsync from ExecuteGroupedTestsAsync and call it from both the main session path and the dynamic batch loop, so any future bucket added to GroupedTests is automatically respected on both paths. Also: clarify cancellationToken role in RunPhasesConcurrentlyAsync and document the intentional sequential ParallelGroups loop. --- TUnit.Engine/Scheduling/TestScheduler.cs | 63 ++++++++++++------------ 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs index 3065d04fe9..fa7b6d834b 100644 --- a/TUnit.Engine/Scheduling/TestScheduler.cs +++ b/TUnit.Engine/Scheduling/TestScheduler.cs @@ -171,6 +171,18 @@ private async Task ExecuteGroupedTestsAsync( // Start dynamic test queue processing in background var dynamicTestProcessingTask = ProcessDynamicTestQueueAsync(cancellationToken); + await ExecuteAllPhasesAsync(groupedTests, cancellationToken).ConfigureAwait(false); + + // Mark the queue as complete and wait for remaining dynamic tests to finish + _dynamicTestQueue.Complete(); + await dynamicTestProcessingTask.ConfigureAwait(false); + } + + #if NET + [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Test execution involves reflection for hooks and initialization")] + #endif + private async Task ExecuteAllPhasesAsync(GroupedTests groupedTests, CancellationToken cancellationToken) + { // Unconstrained Parallel and KeyedNotInParallel buckets share no state — keyed tests // only block other tests sharing a key (per docs/parallelism.md). Running them // sequentially makes a `[Test] T1` always run before `[Test, NotInParallel("k")] T2` @@ -195,6 +207,8 @@ private async Task ExecuteGroupedTestsAsync( await RunPhasesConcurrentlyAsync(parallelPhase, keyedPhase, cancellationToken).ConfigureAwait(false); + // Each ParallelGroup runs to completion before the next — tests are bucketed + // into a group precisely because they must not overlap with tests in another group. foreach (var group in groupedTests.ParallelGroups) { var totalCount = 0; @@ -235,10 +249,6 @@ private async Task ExecuteGroupedTestsAsync( await _logger.LogTraceAsync($"Starting {groupedTests.NotInParallel.Length} global NotInParallel tests").ConfigureAwait(false); await ExecuteSequentiallyAsync(groupedTests.NotInParallel, cancellationToken).ConfigureAwait(false); } - - // Mark the queue as complete and wait for remaining dynamic tests to finish - _dynamicTestQueue.Complete(); - await dynamicTestProcessingTask.ConfigureAwait(false); } #if NET @@ -266,21 +276,7 @@ private async Task ProcessDynamicTestQueueAsync(CancellationToken cancellationTo if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Executing {dynamicTests.Count} dynamic test(s)").ConfigureAwait(false); - // Group and execute just like regular tests - var dynamicTestsArray = dynamicTests.ToArray(); - var groupedDynamicTests = await _groupingService.GroupTestsByConstraintsAsync(dynamicTestsArray).ConfigureAwait(false); - - // Execute the grouped dynamic tests (recursive call handles sub-dynamics) - if (groupedDynamicTests.Parallel.Length > 0) - { - await ExecuteTestsAsync(groupedDynamicTests.Parallel, cancellationToken).ConfigureAwait(false); - } - - if (groupedDynamicTests.NotInParallel.Length > 0) - { - await ExecuteSequentiallyAsync(groupedDynamicTests.NotInParallel, cancellationToken).ConfigureAwait(false); - } - + await ExecuteDynamicBatchAsync(dynamicTests, cancellationToken).ConfigureAwait(false); dynamicTests.Clear(); } } @@ -299,21 +295,22 @@ private async Task ProcessDynamicTestQueueAsync(CancellationToken cancellationTo if (_logger.IsTraceEnabled) await _logger.LogTraceAsync($"Executing {dynamicTests.Count} remaining dynamic test(s)").ConfigureAwait(false); - var dynamicTestsArray = dynamicTests.ToArray(); - var groupedDynamicTests = await _groupingService.GroupTestsByConstraintsAsync(dynamicTestsArray).ConfigureAwait(false); - - if (groupedDynamicTests.Parallel.Length > 0) - { - await ExecuteTestsAsync(groupedDynamicTests.Parallel, cancellationToken).ConfigureAwait(false); - } - - if (groupedDynamicTests.NotInParallel.Length > 0) - { - await ExecuteSequentiallyAsync(groupedDynamicTests.NotInParallel, cancellationToken).ConfigureAwait(false); - } + await ExecuteDynamicBatchAsync(dynamicTests, cancellationToken).ConfigureAwait(false); } } + #if NET + [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Test execution involves reflection for hooks and initialization")] + #endif + private async Task ExecuteDynamicBatchAsync(List dynamicTests, CancellationToken cancellationToken) + { + // Route through the same phase pipeline as the main test set so dynamically-added + // tests with [NotInParallel(key)], [ParallelGroup(...)] etc. honour their constraints + // instead of being silently dropped. + var groupedDynamicTests = await _groupingService.GroupTestsByConstraintsAsync(dynamicTests.ToArray()).ConfigureAwait(false); + await ExecuteAllPhasesAsync(groupedDynamicTests, cancellationToken).ConfigureAwait(false); + } + #if NET [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Test execution involves reflection for hooks and initialization")] #endif @@ -393,6 +390,10 @@ private async Task ExecuteWithGlobalLimitAsync( } #endif + // The cancellation token is forwarded only so WaitForTasksWithFailFastHandling can + // distinguish a fail-fast cancellation from a normal task fault when both phases run. + // Tasks `a` and `b` are already started and carry their own token internally, so the + // single-phase short-circuits do not need to inspect it. private Task RunPhasesConcurrentlyAsync(Task? a, Task? b, CancellationToken cancellationToken) { if (a is null) return b ?? Task.CompletedTask; From 670d82c55b3547864697731f04a88fe3032bfd51 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 25 Apr 2026 13:56:39 +0100 Subject: [PATCH 5/6] fix(tests): isolate OptOut OTel test from parallel WebApplicationTests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OptOut_DoesNotTag_AspNetCoreSpans regressed under #5740's keyed/Parallel overlap. Each TracerProvider's AddAspNetCoreInstrumentation subscribes to the process-global Microsoft.AspNetCore ActivitySource, so a parallel auto-OTel WebApplicationTest's correlation processor stamps spans the OptOut exporter then captures. Keyed serialization could only enumerate sibling auto-wire tests; switch OptOut to global [NotInParallel] until per-factory filtering lands. Also bump CloudShop Step3 timeout to 120s — Worker fulfillment via RabbitMQ lags further under the new keyed/Parallel concurrency. --- .../AutoConfigureOpenTelemetryTests.cs | 19 +++++++++++++------ .../Tests/Orders/OrderWorkflowTests.cs | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs b/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs index 7dbe558dae..2b56c3a3fc 100644 --- a/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs +++ b/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs @@ -12,12 +12,10 @@ namespace TUnit.AspNetCore.Tests; /// correlation processor + ASP.NET Core instrumentation. /// /// -/// Serialized against sibling auto-wire tests because -/// attaches a process-global per TracerProvider, -/// so a parallel factory's correlation processor can tag activities created by another -/// factory's SUT. Serializing keeps assertions observing only their own factory's wiring. +/// AutoWires asserts presence of its own TestId tag, so foreign spans observed +/// via the global ASP.NET Core do not +/// affect it — no key needed. /// -[NotInParallel(nameof(AutoConfigureOpenTelemetryTests))] public class AutoConfigureOpenTelemetryTests : WebApplicationTest { private readonly List _exported = []; @@ -54,7 +52,16 @@ public async Task AutoWires_TagsAspNetCoreSpans_WithTestId() } } -[NotInParallel(nameof(AutoConfigureOpenTelemetryTests))] +/// +/// Global (no key): asserts absence of any +/// tag, but every +/// with AddAspNetCoreInstrumentation() subscribes to the process-global +/// Microsoft.AspNetCore and a parallel +/// factory's correlation processor stamps spans with its own TestId before they reach +/// this exporter. A keyed constraint cannot enumerate every parallel ; +/// draining alone is the only reliable isolation until per-factory filtering lands. +/// +[NotInParallel] public class AutoConfigureOpenTelemetryOptOutTests : WebApplicationTest { private readonly List _exported = []; diff --git a/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs b/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs index 6e52c7f008..f8fe4588cc 100644 --- a/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs +++ b/examples/CloudShop/CloudShop.Tests/Tests/Orders/OrderWorkflowTests.cs @@ -94,7 +94,7 @@ public async Task Step3_Worker_Fulfills_Order() await Customer.Client.GetFromJsonAsync($"/api/orders/{orderId}")) .WaitsFor( assert => assert.Satisfies(o => o?.Status == OrderStatus.Fulfilled), - timeout: TimeSpan.FromSeconds(60), + timeout: TimeSpan.FromSeconds(120), pollingInterval: TimeSpan.FromMilliseconds(500)); await Assert.That(order).IsNotNull(); From a11e7667c96c2fd3445362d3e2275a480ac4e683 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 26 Apr 2026 14:12:47 +0100 Subject: [PATCH 6/6] fix(tests): unbreak CI on PR #5740 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AutoConfigureOpenTelemetryTests: OTel exporter Adds to _exported on a background batch thread without locking, so the test's poll loop racing on List threw "Collection was modified". Wrap in a small LockedActivityList (ICollection) and iterate Snapshot() copies. - Repro5700 / CrossKeyOverlap5700: subprocess runs every [EngineTest=Pass] test (hundreds) so the 15s rendezvous deadline races MaxDOP saturation on slow macOS runners. Bump to 60s — the bug being guarded would still manifest as the rendezvous never resolving, not as a 60s delay. --- .../AutoConfigureOpenTelemetryTests.cs | 56 +++++++++++++++++-- .../Bugs/5700/CrossKeyOverlap.cs | 7 ++- TUnit.TestProject/Bugs/5700/Repro.cs | 16 ++++-- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs b/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs index 2b56c3a3fc..d83310d5ed 100644 --- a/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs +++ b/TUnit.AspNetCore.Tests/AutoConfigureOpenTelemetryTests.cs @@ -1,3 +1,4 @@ +using System.Collections; using System.Diagnostics; using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Trace; @@ -14,11 +15,14 @@ namespace TUnit.AspNetCore.Tests; /// /// AutoWires asserts presence of its own TestId tag, so foreign spans observed /// via the global ASP.NET Core do not -/// affect it — no key needed. +/// affect it — no key needed. Parallel +/// instances stamp activities (subscribed via AddAspNetCoreInstrumentation) on +/// background continuations, so the exporter sink must be thread-safe — polling +/// snapshots its items under a lock. /// public class AutoConfigureOpenTelemetryTests : WebApplicationTest { - private readonly List _exported = []; + private readonly LockedActivityList _exported = []; protected override void ConfigureTestServices(IServiceCollection services) { @@ -40,7 +44,14 @@ public async Task AutoWires_TagsAspNetCoreSpans_WithTestId() Activity? taggedSpan = null; while (Environment.TickCount64 < deadline) { - taggedSpan = _exported.FirstOrDefault(a => (a.GetTagItem(TUnitActivitySource.TagTestId) as string) == testId); + foreach (var activity in _exported.Snapshot()) + { + if (activity.GetTagItem(TUnitActivitySource.TagTestId) as string == testId) + { + taggedSpan = activity; + break; + } + } if (taggedSpan is not null) { break; @@ -64,7 +75,7 @@ public async Task AutoWires_TagsAspNetCoreSpans_WithTestId() [NotInParallel] public class AutoConfigureOpenTelemetryOptOutTests : WebApplicationTest { - private readonly List _exported = []; + private readonly LockedActivityList _exported = []; protected override void ConfigureTestOptions(WebApplicationTestOptions options) { @@ -85,9 +96,44 @@ public async Task OptOut_DoesNotTag_AspNetCoreSpans() var response = await client.GetAsync("/ping"); response.EnsureSuccessStatusCode(); - foreach (var activity in _exported) + foreach (var activity in _exported.Snapshot()) { await Assert.That(activity.GetTagItem(TUnitActivitySource.TagTestId)).IsNull(); } } } + +/// +/// OpenTelemetry's AddInMemoryExporter calls ICollection<T>.Add on a +/// background batch thread without locking, so a plain races with +/// any reader (the test's poll loop) and intermittently throws "Collection was modified". +/// This wrapper synchronises every mutation and exposes a for +/// safe iteration. +/// +internal sealed class LockedActivityList : ICollection +{ + private readonly List _items = []; + + public int Count + { + get { lock (_items) return _items.Count; } + } + + public bool IsReadOnly => false; + + public void Add(Activity item) { lock (_items) _items.Add(item); } + + public void Clear() { lock (_items) _items.Clear(); } + + public bool Contains(Activity item) { lock (_items) return _items.Contains(item); } + + public void CopyTo(Activity[] array, int arrayIndex) { lock (_items) _items.CopyTo(array, arrayIndex); } + + public bool Remove(Activity item) { lock (_items) return _items.Remove(item); } + + public Activity[] Snapshot() { lock (_items) return _items.ToArray(); } + + public IEnumerator GetEnumerator() => ((IEnumerable)Snapshot()).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +} diff --git a/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs b/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs index a1bc36d02d..5df8529b9b 100644 --- a/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs +++ b/TUnit.TestProject/Bugs/5700/CrossKeyOverlap.cs @@ -17,12 +17,15 @@ public class CrossKeyOverlap5700 private static readonly TaskCompletionSource KeyALive = new(TaskCreationOptions.RunContinuationsAsynchronously); private static readonly TaskCompletionSource KeyBLive = new(TaskCreationOptions.RunContinuationsAsynchronously); + // See Repro5700 for why the rendezvous deadline is generous. + private static readonly TimeSpan RendezvousTimeout = TimeSpan.FromSeconds(60); + [Test] [NotInParallel(KeyA)] public async Task KeyedA_RunsAlongsideKeyedB() { KeyALive.TrySetResult(); - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + using var cts = new CancellationTokenSource(RendezvousTimeout); await KeyBLive.Task.WaitAsync(cts.Token); } @@ -31,7 +34,7 @@ public async Task KeyedA_RunsAlongsideKeyedB() public async Task KeyedB_RunsAlongsideKeyedA() { KeyBLive.TrySetResult(); - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + using var cts = new CancellationTokenSource(RendezvousTimeout); await KeyALive.Task.WaitAsync(cts.Token); } } diff --git a/TUnit.TestProject/Bugs/5700/Repro.cs b/TUnit.TestProject/Bugs/5700/Repro.cs index 9166aabd8f..28d7366e40 100644 --- a/TUnit.TestProject/Bugs/5700/Repro.cs +++ b/TUnit.TestProject/Bugs/5700/Repro.cs @@ -28,15 +28,21 @@ public class Repro5700 private static int _test2Active; private static int _test2ConcurrentViolations; + // Deadline is generous because the engine-test harness runs Repro5700 inside a + // subprocess populated with every `[EngineTest=Pass]` test in the project + // (hundreds). On a busy CI runner the parallel queue can be saturated by other + // unconstrained tests, so allow a full minute for either side to be dispatched. + // The bug being guarded — keyed tests deferred behind the entire parallel + // bucket — would manifest as Test1Live/Test2Live never being set at all, not as + // a 60-second scheduling delay. + private static readonly TimeSpan RendezvousTimeout = TimeSpan.FromSeconds(60); + [Test] public async Task Test1_RunsAlongsideKeyedTest2() { Test1Live.TrySetResult(); - // If keyed Test2 cannot run alongside Test1, this wait never completes. - // The 15s ceiling is enough headroom for any plausible scheduling delay - // while still failing fast on the bug. - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + using var cts = new CancellationTokenSource(RendezvousTimeout); await Test2Live.Task.WaitAsync(cts.Token); } @@ -56,7 +62,7 @@ public async Task Test2_KeyedSerializes(int param) Test2Live.TrySetResult(); - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + using var cts = new CancellationTokenSource(RendezvousTimeout); await Test1Live.Task.WaitAsync(cts.Token); // Hold the key briefly so the second Test2 invocation queues behind