fix(engine): report every test once on fail-fast runs

docs/docs/reference/programmatic-configuration.md

@@ -75,7 +75,7 @@ On Linux and macOS with .NET 8 or later, SIGTERM (sent by `docker stop`, Kuberne
 
 | Property | Type | Default | Description |
 |---|---|---|---|
-| `FailFast` | `bool` | `false` | Cancels the remaining test run after the first test failure. |
+| `FailFast` | `bool` | `false` | Cancels the remaining test run after the first test failure. Tests that had not started are reported as skipped. |
 | `ThreadPoolTuningEnabled` | `bool` | `true` | Raises the thread pool's minimum worker thread count when blocking tests starve the pool. See [Controlling Parallelism](../execution/parallelism.md). |
 
 The `TUNIT_DISABLE_THREADPOOL_TUNING` environment variable takes precedence over `ThreadPoolTuningEnabled`.

src/TUnit.Core/AbstractExecutableTest.cs

@@ -35,6 +35,12 @@ public abstract class AbstractExecutableTest
     // dependency recursion, so TestRunner must keep using its execution dedup ledger.
     internal bool RequiresExecutionDedup { get; set; }
 
+    // Set by TestRunner once the test has every lock and slot it needs and is handed to
+    // TestCoordinator, which always publishes its result. A cancelled run uses this to find
+    // tests that failed before execution (data source or registration errors) and were never
+    // published, because their State is already Failed (#7119).
+    internal bool HasStartedExecution { get; set; }
+
     public required TestContext Context
     {
         get;

src/TUnit.Engine/Scheduling/TestRunner.cs

@@ -1,5 +1,6 @@
 using System.Collections.Concurrent;
 using TUnit.Core;
+using TUnit.Core.Logging;
 using TUnit.Core.Settings;
 using TUnit.Engine.Interfaces;
 using TUnit.Engine.Logging;
@@ -159,6 +160,7 @@ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, Ca
                     acquiredLimiter = limiter;
                 }
 
+                test.HasStartedExecution = true;
                 test.State = TestState.Running;
                 test.StartTime = DateTimeOffset.UtcNow;
 
@@ -181,6 +183,16 @@ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, Ca
                 acquiredLimiter?.Release();
             }
         }
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+        {
+            // The run was stopped (fail-fast, timeout or abort) while the test waited for a lock or
+            // slot, so it never started. That is not a test failure: the scheduler reports every
+            // test left unstarted once the run winds down (#7119).
+            if (_logger.IsDebugEnabled)
+            {
+                await _logger.LogDebugAsync($"Test {test.TestId} was not started because the test run was cancelled.").ConfigureAwait(false);
+            }
+        }
         catch (Exception ex)
         {
             // TestCoordinator already handles marking as failed and sending Failed message
@@ -205,4 +217,9 @@ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, Ca
     /// Gets the first exception that triggered fail-fast cancellation.
     /// </summary>
     public Exception? GetFirstFailFastException() => _firstFailFastException;
+
+    /// <summary>
+    /// Whether a failure has triggered fail-fast cancellation in this session.
+    /// </summary>
+    internal bool IsFailFastTriggered => _failFastCancellationSource.IsCancellationRequested;
 }

src/TUnit.Engine/Scheduling/TestScheduler.cs

@@ -29,6 +29,10 @@ internal sealed class TestScheduler : ITestScheduler
     private readonly NotInParallelLock _notInParallelLock;
     private readonly MaxParallelismLimiter _maxParallelismLimiter;
 
+    // Dynamic tests the queue processor has scheduled, so a cancelled run can report the ones
+    // that never started. Null until the first dynamic test arrives.
+    private List<AbstractExecutableTest>? _dynamicTests;
+
     public TestScheduler(
         TUnitFrameworkLogger logger,
         ITestGroupingService groupingService,
@@ -123,6 +127,13 @@ public async Task<bool> ScheduleAndExecuteAsync(
         }
         finally
         {
+            // Before the session hooks, so every selected test has a result by the time the
+            // session ends.
+            if (cancellationToken.IsCancellationRequested)
+            {
+                await ReportTestsNotStartedAsync(executableTests).ConfigureAwait(false);
+            }
+
             // Also on the fail-fast path, where cancelled tests make the phases throw: the
             // cancellation callback only starts the session hooks, so await them (and the
             // static property disposal that follows them) here, before the caller disposes
@@ -133,6 +144,93 @@ public async Task<bool> ScheduleAndExecuteAsync(
         return sessionHooksSucceeded;
     }
 
+    // A cancelled run (fail-fast, timeout or abort) stops tests before they start: the scheduler
+    // stops handing out slots and the global [NotInParallel] phase never runs. Without a result
+    // those tests would vanish from the run's totals, so give each one a result now (#7119). Tests
+    // that started report their own result, so only those still waiting are reported here.
+    private async ValueTask ReportTestsNotStartedAsync(List<AbstractExecutableTest> executableTests)
+    {
+        // Fail-fast is the user's request to stop after a failure, so its unstarted tests are
+        // skipped; any other cancellation aborted the run, so those tests are cancelled.
+        var skipReason = _testRunner.IsFailFastTriggered
+            ? "Test was not run because --fail-fast stopped the test run after a failure"
+            : null;
+
+        try
+        {
+            foreach (var test in executableTests)
+            {
+                await ReportIfNotStartedAsync(test, skipReason).ConfigureAwait(false);
+            }
+
+            if (_dynamicTests is { } dynamicTests)
+            {
+                foreach (var test in dynamicTests)
+                {
+                    await ReportIfNotStartedAsync(test, skipReason).ConfigureAwait(false);
+                }
+            }
+
+            // Dynamic tests still queued when the processor stopped on the cancelled token.
+            while (_dynamicTestQueue.TryDequeue(out var test))
+            {
+                if (test != null)
+                {
+                    await ReportIfNotStartedAsync(test, skipReason).ConfigureAwait(false);
+                }
+            }
+        }
+        catch (Exception ex)
+        {
+            // Best effort: the platform can stop accepting messages once its own token is
+            // cancelled, and that must not replace the run's outcome.
+            await _logger.LogDebugAsync($"Could not report tests that were not started: {ex}").ConfigureAwait(false);
+        }
+    }
+
+    // Reads State and then writes the result without an atomic transition. That is safe because
+    // this runs only after every phase task and the dynamic queue processor have completed, on
+    // the success and the exception paths alike, so no test can still be moving between states.
+    private async ValueTask ReportIfNotStartedAsync(AbstractExecutableTest test, string? skipReason)
+    {
+        if (test.State == TestState.Failed && !test.HasStartedExecution)
+        {
+            // Failed before execution (data source, argument registration or dependency
+            // resolution error). TestCoordinator normally publishes that failure, but the test
+            // never reached it, so publish it here: it is a real failure, not a skip.
+            var exception = test.Result?.Exception
+                ?? new InvalidOperationException("Test failed before it could be executed");
+            var failedAt = DateTimeOffset.UtcNow;
+            test.StartTime ??= failedAt;
+            test.EndTime ??= failedAt;
+            TestSessionContext.Current?.MarkFailure();
+            await _messageBus.Failed(test.Context, exception, test.StartTime.GetValueOrDefault()).ConfigureAwait(false);
+            return;
+        }
+
+        if (test.State is not (TestState.NotStarted or TestState.WaitingForDependencies or TestState.Queued))
+        {
+            return;
+        }
+
+        // Skipped tests do not fail the session, so this branch leaves the session outcome alone.
+        // The cancelled branch below marks a failure, matching how TestCoordinator reports a
+        // cancelled test.
+        if (skipReason is not null)
+        {
+            _testStateManager.MarkSkipped(test, skipReason);
+            await _messageBus.Skipped(test.Context, skipReason).ConfigureAwait(false);
+            return;
+        }
+
+        var now = DateTimeOffset.UtcNow;
+        test.StartTime ??= now;
+        test.EndTime = now;
+        test.SetResult(TestState.Cancelled);
+        TestSessionContext.Current?.MarkFailure();
+        await _messageBus.Cancelled(test.Context, test.StartTime.GetValueOrDefault()).ConfigureAwait(false);
+    }
+
     private async Task<bool> RunAfterTestSessionAsync(CancellationToken cancellationToken)
     {
         var sessionHookExceptions = await _afterHookPairTracker.GetOrCreateAfterTestSessionTask(
@@ -282,7 +380,7 @@ private async Task ExecuteAllPhasesAsync(GroupedTests groupedTests, Cancellation
             keyedPhase = _constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(groupedTests.KeyedNotInParallel, cancellationToken).AsTask();
         }
 
-        await RunPhasesConcurrentlyAsync(parallelPhase, keyedPhase, cancellationToken).ConfigureAwait(false);
+        await RunPhasesConcurrentlyAsync(parallelPhase, keyedPhase).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.
@@ -317,7 +415,7 @@ private async Task ExecuteAllPhasesAsync(GroupedTests groupedTests, Cancellation
             var groupKeyedPhase = constrainedTests.KeyedTests.Length > 0
                 ? _constraintKeyScheduler.ExecuteTestsWithConstraintsAsync(constrainedTests.KeyedTests, cancellationToken).AsTask()
                 : null;
-            await RunPhasesConcurrentlyAsync(unconstrainedPhase, groupKeyedPhase, cancellationToken).ConfigureAwait(false);
+            await RunPhasesConcurrentlyAsync(unconstrainedPhase, groupKeyedPhase).ConfigureAwait(false);
         }
 
         if (groupedTests.NotInParallel.Length > 0)
@@ -350,6 +448,8 @@ private async Task ProcessDynamicTestQueueAsync(CancellationToken cancellationTo
             // Execute the batch of dynamic tests if any were found
             if (dynamicTests.Count > 0)
             {
+                TrackDynamicTests(dynamicTests);
+
                 if (_logger.IsTraceEnabled)
                     await _logger.LogTraceAsync($"Executing {dynamicTests.Count} dynamic test(s)").ConfigureAwait(false);
 
@@ -376,13 +476,22 @@ private async Task ProcessDynamicTestQueueAsync(CancellationToken cancellationTo
 
         if (dynamicTests.Count > 0)
         {
+            TrackDynamicTests(dynamicTests);
+
             if (_logger.IsTraceEnabled)
                 await _logger.LogTraceAsync($"Executing {dynamicTests.Count} remaining dynamic test(s)").ConfigureAwait(false);
 
             await ExecuteDynamicBatchAsync(dynamicTests, cancellationToken).ConfigureAwait(false);
         }
     }
 
+    // Only the queue processor writes this, and the cancelled-run report reads it after the
+    // processor has finished, so it needs no locking.
+    private void TrackDynamicTests(List<AbstractExecutableTest> dynamicTests)
+    {
+        (_dynamicTests ??= []).AddRange(dynamicTests);
+    }
+
     // Ends the wait loop instead of faulting when the token is cancelled. The phases can finish
     // without throwing after fail-fast fires (the failing test was the last to run), and
     // ExecuteGroupedTestsAsync then awaits this processor on its normal path, so a cancelled
@@ -500,6 +609,9 @@ private async Task ExecuteSequentiallyAsync(
     {
         foreach (var test in tests)
         {
+            // Stop on a cancelled run instead of walking every remaining test into a cancelled
+            // lock wait. The tests left behind are reported once the run winds down.
+            cancellationToken.ThrowIfCancellationRequested();
             await ExecuteScheduledTestAsync(test, cancellationToken).ConfigureAwait(false);
         }
     }
@@ -576,37 +688,15 @@ private async ValueTask ExecuteScheduledTestAsync(AbstractExecutableTest test, C
         await test.ExecutionTask.ConfigureAwait(false);
     }
 
-    // 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)
+    // The phases are already started and carry the token themselves. When fail-fast stops them,
+    // the cancellation propagates as-is: the failing test already has its own result, and
+    // re-throwing its exception here made the framework report it a second time as an
+    // "Unhandled exception" node (#7119).
+    private static Task RunPhasesConcurrentlyAsync(Task? a, Task? b)
     {
         if (a is null) return b ?? Task.CompletedTask;
         if (b is null) return a;
-        return WaitForTasksWithFailFastHandling([a, b], cancellationToken);
-    }
-
-    private async Task WaitForTasksWithFailFastHandling(IEnumerable<Task> tasks, CancellationToken cancellationToken)
-    {
-        try
-        {
-            await Task.WhenAll(tasks).ConfigureAwait(false);
-        }
-        catch (Exception)
-        {
-            if (cancellationToken.IsCancellationRequested)
-            {
-                var firstFailure = _testRunner.GetFirstFailFastException();
-
-                if (firstFailure != null)
-                {
-                    throw firstFailure;
-                }
-            }
-
-            throw;
-        }
+        return Task.WhenAll(a, b);
     }
 
     internal static int GetMaxParallelism(ILogger logger, ICommandLineOptions commandLineOptions)

tests/TUnit.Engine.Tests/Scheduling/FailFastReportingTests.cs

@@ -0,0 +1,52 @@
+using Shouldly;
+using TUnit.Engine.Tests.Enums;
+
+namespace TUnit.Engine.Tests.Scheduling;
+
+/// <summary>
+/// #7119: on a --fail-fast run every selected test must get exactly one result node. Tests that
+/// fail-fast stops before they start are reported as skipped, and the failure that triggered
+/// fail-fast is not reported a second time as an "Unhandled exception" node when the
+/// unconstrained and keyed phases were running at the same time. A test that failed before it
+/// ran (data source error) and that fail-fast stopped is still reported as failed.
+/// </summary>
+public class FailFastReportingTests(TestMode testMode) : InvokableTestBase(testMode)
+{
+    private static readonly string[] TestNames =
+        ["Fails", "Parallel1", "Parallel2", "Keyed1", "Keyed2", "Global1", "Global2", "DataSourceFails"];
+
+    [Test]
+    public async Task FailFast_ReportsEveryTestOnce_AndCancelledTestsAsSkipped()
+    {
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60));
+        var process = new ProcessResultCapture();
+
+        await RunTestsWithFilter("/*/*/FailFastReporting7119/*",
+        [
+            _ => process.AssertEndedWithoutCrash(),
+            result => result.ResultSummary.Outcome.ShouldBe("Failed"),
+            result => result.Results.ShouldNotContain(
+                x => x.TestName!.StartsWith("Unhandled exception", StringComparison.Ordinal),
+                "The fail-fast failure was reported a second time as an 'Unhandled exception' node"),
+            result => result.Results.Select(x => x.TestName).OrderBy(x => x).ShouldBe(TestNames.OrderBy(x => x)),
+            result => result.Results.Single(x => x.TestName == "Fails").Outcome.ShouldBe("Failed"),
+            // Its data source throws, so it is failed before it runs. Fail-fast stops it before it
+            // reaches the coordinator, so only the cancelled-run sweep can report it.
+            result => result.Results.Single(x => x.TestName == "DataSourceFails").Outcome.ShouldBe("Failed"),
+            result => result.ResultSummary.Counters.Failed.ShouldBe(2),
+            // Fails runs no later than second, so fail-fast stops at least the two global
+            // [NotInParallel] tests and the tests still waiting for the single slot.
+            result => result.Results.Where(x => x.TestName is not ("Fails" or "DataSourceFails"))
+                .ShouldAllBe(x => x.Outcome == "Passed" || x.Outcome == "NotExecuted"),
+            result => result.Results.Count(x => x.Outcome == "NotExecuted").ShouldBeGreaterThanOrEqualTo(4),
+            result => result.Results.Where(x => x.TestName!.StartsWith("Global", StringComparison.Ordinal))
+                .ShouldAllBe(x => x.Outcome == "NotExecuted"),
+        ],
+        new RunOptions()
+            .WithArgument("--fail-fast")
+            .WithArgument("--maximum-parallel-tests")
+            .WithArgument("1")
+            .WithForcefulCancellationToken(cts.Token)
+            .Capture(process));
+    }
+}

tests/TUnit.TestProject/Bugs/_7119/FailFastReporting7119.cs

@@ -0,0 +1,38 @@
+using TUnit.Core.Enums;
+using TUnit.TestProject.Attributes;
+
+namespace TUnit.TestProject.Bugs._7119;
+
+/// <summary>
+/// Run with --fail-fast and --maximum-parallel-tests 1. The unconstrained and keyed phases run
+/// at the same time and share the single slot, so Fails (Critical priority, first in the
+/// unconstrained phase) runs no later than second. Fail-fast then cancels the tests still
+/// waiting for the slot, and the global [NotInParallel] phase never starts. Each test must be
+/// reported exactly once: Fails as failed, every test fail-fast stopped as skipped, and no
+/// extra "Unhandled exception" node for the failure. DataSourceFails fails while its data is
+/// generated, so it is already failed before it runs; fail-fast stops it before it reaches the
+/// coordinator, and it must still be reported as failed.
+/// </summary>
+[EngineTest(ExpectedResult.Failure)]
+public class FailFastReporting7119
+{
+    [Test, ExecutionPriority(Priority.Critical)]
+    public void Fails() => throw new InvalidOperationException("Intentional failure to trigger fail-fast (#7119)");
+
+    [Test] public Task Parallel1() => Task.Delay(100);
+    [Test] public Task Parallel2() => Task.Delay(100);
+
+    [Test, NotInParallel("7119.keyed")] public Task Keyed1() => Task.Delay(100);
+    [Test, NotInParallel("7119.keyed")] public Task Keyed2() => Task.Delay(100);
+
+    [Test, NotInParallel] public Task Global1() => Task.Delay(100);
+    [Test, NotInParallel] public Task Global2() => Task.Delay(100);
+
+    public static IEnumerable<int> ThrowingData() =>
+        throw new InvalidOperationException("Intentional data source failure (#7119)");
+
+    [Test, MethodDataSource(nameof(ThrowingData))]
+    public void DataSourceFails(int value)
+    {
+    }
+}