diff --git a/TUnit.Core/Models/GlobalContext.cs b/TUnit.Core/Models/GlobalContext.cs index fa88dd432f..60931fb9db 100644 --- a/TUnit.Core/Models/GlobalContext.cs +++ b/TUnit.Core/Models/GlobalContext.cs @@ -6,14 +6,16 @@ namespace TUnit.Core; public class GlobalContext : Context { - private static readonly AsyncLocal Contexts = new(); + // Static, not AsyncLocal: a lazy-creating AsyncLocal getter poisons the first + // reading branch with a fresh empty instance, hiding the framework's later + // Current = ... assignment from that branch. + private static GlobalContext? _current; public static new GlobalContext Current { - get - { - return Contexts.Value ??= new GlobalContext(); - } - internal set => Contexts.Value = value; + // Factory overload — the parameterless one uses Activator.CreateInstance(), + // which AOT trimming may not preserve for GlobalContext's internal ctor. + get => LazyInitializer.EnsureInitialized(ref _current, static () => new GlobalContext())!; + internal set => Volatile.Write(ref _current, value); } internal GlobalContext() : base(null) diff --git a/TUnit.Core/Settings/TimeoutSettings.cs b/TUnit.Core/Settings/TimeoutSettings.cs index 74d006dff6..eac78cfd19 100644 --- a/TUnit.Core/Settings/TimeoutSettings.cs +++ b/TUnit.Core/Settings/TimeoutSettings.cs @@ -9,13 +9,17 @@ public sealed class TimeoutSettings internal TimeoutSettings() { } /// - /// Default timeout for individual tests. Default: 30 minutes. - /// Overridden per-test by . + /// Default timeout for individual tests. Overridden per-test by . /// Precedence: CLI/env var (N/A for test timeout) → TUnitSettings → built-in default. + /// + /// If this property is never explicitly set, tests without a + /// run without any timeout wrapper. The 30-minute fallback only applies when this + /// property is explicitly assigned. + /// /// public TimeSpan DefaultTestTimeout { - get => _defaultTestTimeout; + get => _defaultTestTimeout ?? TimeSpan.FromMinutes(30); set { ValidatePositive(value); @@ -23,6 +27,28 @@ public TimeSpan DefaultTestTimeout } } + // Null means the user never assigned a project-level default, so tests without [Timeout] + // bypass TimeoutHelper entirely instead of paying wrap overhead for a 30-minute backstop. + internal TimeSpan? ExplicitDefaultTestTimeout => _defaultTestTimeout; + + // Coalesces the per-test [Timeout] attribute value with the project-wide opt-in + // DefaultTestTimeout. Null when neither is set — TestCoordinator's null fast path + // then skips the TimeoutHelper wrap entirely. + internal TimeSpan? GetEffectiveTestTimeout(TimeSpan? attributeTimeout) + => attributeTimeout ?? _defaultTestTimeout; + + // Test-only seam: the public setter validates positive-only and can't write null, which + // leaves the harness unable to restore the "unset" state after a snapshot/restore cycle. + internal void SetExplicitDefaultTestTimeout(TimeSpan? value) + { + if (value is { } positive) + { + ValidatePositive(positive); + } + + _defaultTestTimeout = value; + } + /// /// Default timeout for hook methods (Before/After at every level). Default: 5 minutes. /// Overridden per-hook by . @@ -71,7 +97,7 @@ public TimeSpan ProcessExitHookDelay } } - private TimeSpan _defaultTestTimeout = TimeSpan.FromMinutes(30); + private TimeSpan? _defaultTestTimeout; private TimeSpan _defaultHookTimeout = TimeSpan.FromMinutes(5); private TimeSpan _forcefulExitTimeout = TimeSpan.FromSeconds(30); private TimeSpan _processExitHookDelay = TimeSpan.FromMilliseconds(500); diff --git a/TUnit.Engine.Tests/DefaultTimeoutClassificationTests.cs b/TUnit.Engine.Tests/DefaultTimeoutClassificationTests.cs new file mode 100644 index 0000000000..aa53af35be --- /dev/null +++ b/TUnit.Engine.Tests/DefaultTimeoutClassificationTests.cs @@ -0,0 +1,47 @@ +using Shouldly; +using TUnit.Engine.Tests.Enums; + +namespace TUnit.Engine.Tests; + +/// +/// Integration test for PR #5728 Bug 2: a timeout that fires via +/// TUnitSettings.Default.Timeouts.DefaultTestTimeout (instead of a [Timeout] +/// attribute) must surface as a timeout failure end-to-end, not a generic error. +/// +/// Before the fix, TestDetails.Timeout was left null when the timeout was resolved +/// from settings, so TUnitMessageBus.GetFailureStateProperty fell through to +/// ErrorTestNodeStateProperty — which caused JUnit/GitHub/HTML reporters to label +/// real timeouts as errors. +/// +public class DefaultTimeoutClassificationTests(TestMode testMode) : InvokableTestBase(testMode) +{ + [Test] + public async Task Hanging_Test_Fails_With_Timeout_Message_When_Only_DefaultTestTimeout_Is_Set() + { + // The matching [Before(TestDiscovery)] hook in TestProject detects this class name + // in the filter string and programmatically sets DefaultTestTimeout=200ms for the + // subprocess — so a hanging test timing out via DefaultTestTimeout can be observed + // end-to-end. The gate is the filter (not an env var) because Microsoft.Testing.Platform's + // test-host-controller mode under --hangdump does not reliably propagate env vars + // through to the test host process on all CI runners. + await RunTestsWithFilter( + "/*/*/DefaultTimeoutClassificationTests/Hanging_Test_With_DefaultTestTimeout_Should_Timeout", + [ + result => result.ResultSummary.Outcome.ShouldBe("Failed"), + result => result.ResultSummary.Counters.Total.ShouldBe(1), + result => result.ResultSummary.Counters.Failed.ShouldBe(1), + // Test body sleeps 10s but the 200ms default timeout must fire well before that — + // bounds guard against the timeout silently not firing (would sleep the full 10s). + result => TimeSpan.Parse(result.Results[0].Duration).ShouldBeLessThan(TimeSpan.FromSeconds(5)), + result => + { + // Confirm the failure was classified as a timeout rather than a generic error. + // "Timed out" appears in TimeoutTestNodeStateProperty's explanation; a generic + // error path would surface the raw exception message without the "timed out" phrase. + var errorMessage = result.Results.First().Output?.ErrorInfo?.Message; + errorMessage.ShouldNotBeNull("Expected an error message for the timed-out test"); + errorMessage!.ToLowerInvariant().ShouldContain("timed out"); + } + ]); + } +} diff --git a/TUnit.Engine/Building/TestBuilder.cs b/TUnit.Engine/Building/TestBuilder.cs index 207d8a24e6..c366936314 100644 --- a/TUnit.Engine/Building/TestBuilder.cs +++ b/TUnit.Engine/Building/TestBuilder.cs @@ -1085,8 +1085,7 @@ private async ValueTask CreateTestContextAsync(string testId, TestM MethodMetadata = metadata.MethodMetadata, AttributesByType = attributes.ToAttributeDictionary(), MethodGenericArguments = testData.ResolvedMethodGenericArguments, - ClassGenericArguments = testData.ResolvedClassGenericArguments, - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + ClassGenericArguments = testData.ResolvedClassGenericArguments }; var context = _contextProvider.CreateTestContext( @@ -1178,8 +1177,7 @@ private TestDetails CreateFailedTestDetails(TestMetadata metadata, string testId TestLineNumber = metadata.LineNumber, ReturnType = typeof(Task), MethodMetadata = metadata.MethodMetadata, - AttributesByType = AttributeDictionaryHelper.Empty, - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + AttributesByType = AttributeDictionaryHelper.Empty }; } diff --git a/TUnit.Engine/Building/TestBuilderPipeline.cs b/TUnit.Engine/Building/TestBuilderPipeline.cs index 3d1d0d11de..de56c157e6 100644 --- a/TUnit.Engine/Building/TestBuilderPipeline.cs +++ b/TUnit.Engine/Building/TestBuilderPipeline.cs @@ -256,8 +256,7 @@ private async Task GenerateDynamicTests(TestMetadata m TestLineNumber = metadata.LineNumber, ReturnType = typeof(Task), MethodMetadata = metadata.MethodMetadata, - AttributesByType = attributes.ToAttributeDictionary(), - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + AttributesByType = attributes.ToAttributeDictionary() }; var testBuilderContext = CreateTestBuilderContext(metadata); @@ -383,8 +382,7 @@ private async IAsyncEnumerable BuildTestsFromSingleMetad TestLineNumber = resolvedMetadata.LineNumber, ReturnType = typeof(Task), MethodMetadata = resolvedMetadata.MethodMetadata, - AttributesByType = attributes.ToAttributeDictionary(), - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + AttributesByType = attributes.ToAttributeDictionary() }; var context = _contextProvider.CreateTestContext( @@ -462,8 +460,7 @@ private AbstractExecutableTest CreateFailedTestForDataGenerationError(TestMetada TestLineNumber = metadata.LineNumber, ReturnType = typeof(Task), MethodMetadata = metadata.MethodMetadata, - AttributesByType = AttributeDictionaryHelper.Empty, - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + AttributesByType = AttributeDictionaryHelper.Empty }; var context = _contextProvider.CreateTestContext( @@ -515,8 +512,7 @@ private AbstractExecutableTest CreateFailedTestForGenericResolutionError(TestMet TestLineNumber = metadata.LineNumber, ReturnType = typeof(Task), MethodMetadata = metadata.MethodMetadata, - AttributesByType = AttributeDictionaryHelper.Empty, - Timeout = Core.Settings.TUnitSettings.Default.Timeouts.DefaultTestTimeout + AttributesByType = AttributeDictionaryHelper.Empty }; var context = _contextProvider.CreateTestContext( diff --git a/TUnit.Engine/Framework/TUnitServiceProvider.cs b/TUnit.Engine/Framework/TUnitServiceProvider.cs index b2a10470c6..9c57e67609 100644 --- a/TUnit.Engine/Framework/TUnitServiceProvider.cs +++ b/TUnit.Engine/Framework/TUnitServiceProvider.cs @@ -170,7 +170,7 @@ public TUnitServiceProvider(IExtension extension, ParallelLimitLockProvider = Register(new ParallelLimitLockProvider()); - ContextProvider = Register(new ContextProvider(this, TestSessionId, Filter?.ToString())); + ContextProvider = Register(new ContextProvider(this, TestSessionId, FilterParser.StringifyFilter(Filter))); var hookExecutor = Register(new HookExecutor(HookDelegateBuilder, ContextProvider, EventReceiverOrchestrator)); var lifecycleCoordinator = Register(new TestLifecycleCoordinator()); diff --git a/TUnit.Engine/Framework/TUnitTestFramework.cs b/TUnit.Engine/Framework/TUnitTestFramework.cs index 5563cbd807..aa4ec04454 100644 --- a/TUnit.Engine/Framework/TUnitTestFramework.cs +++ b/TUnit.Engine/Framework/TUnitTestFramework.cs @@ -53,16 +53,18 @@ public async Task ExecuteRequestAsync(ExecuteRequestContext context) { var serviceProvider = GetOrCreateServiceProvider(context); - serviceProvider.Initializer.Initialize(context); - - await serviceProvider.HookDelegateBuilder.InitializeAsync(); - + // Install contexts before init runs so anything reading GlobalContext.Current + // sees the populated instance instead of the lazy fallback (null TestFilter). GlobalContext.Current = serviceProvider.ContextProvider.GlobalContext; GlobalContext.Current.GlobalLogger = serviceProvider.Logger; BeforeTestDiscoveryContext.Current = serviceProvider.ContextProvider.BeforeTestDiscoveryContext; TestDiscoveryContext.Current = serviceProvider.ContextProvider.TestDiscoveryContext; TestSessionContext.Current = serviceProvider.ContextProvider.TestSessionContext; + serviceProvider.Initializer.Initialize(context); + + await serviceProvider.HookDelegateBuilder.InitializeAsync(); + serviceProvider.CancellationToken.Initialise(context.CancellationToken); await _requestHandler.HandleRequestAsync((TestExecutionRequest) context.Request, serviceProvider, context, GetFilter(context)); diff --git a/TUnit.Engine/Services/FilterParser.cs b/TUnit.Engine/Services/FilterParser.cs index db5ededc71..025eab80c7 100644 --- a/TUnit.Engine/Services/FilterParser.cs +++ b/TUnit.Engine/Services/FilterParser.cs @@ -21,10 +21,11 @@ internal class FilterParser } #pragma warning disable TPEXP - public static string? StringifyFilter(ITestExecutionFilter filter) + public static string? StringifyFilter(ITestExecutionFilter? filter) { return filter switch { + null => null, NopFilter => null, TestNodeUidListFilter testNodeUidListFilter => string.Join(',', testNodeUidListFilter.TestNodeUids.Select(x => x.Value)), diff --git a/TUnit.Engine/Services/TestExecution/TestCoordinator.cs b/TUnit.Engine/Services/TestExecution/TestCoordinator.cs index 980e733998..8ed35ae8e1 100644 --- a/TUnit.Engine/Services/TestExecution/TestCoordinator.cs +++ b/TUnit.Engine/Services/TestExecution/TestCoordinator.cs @@ -2,6 +2,7 @@ using TUnit.Core; using TUnit.Core.Exceptions; using TUnit.Core.Logging; +using TUnit.Core.Settings; using TUnit.Core.Tracking; using TUnit.Engine.Helpers; using TUnit.Engine.Interfaces; @@ -150,7 +151,7 @@ public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, Cancellatio { _testInitializer.PrepareTest(test); test.Context.RestoreExecutionContext(); - var testTimeout = test.Context.Metadata.TestDetails.Timeout; + var testTimeout = ResolveAndPropagateTestTimeout(test); await _testExecutor.ExecuteAsync(test, _testInitializer, cancellationToken, testTimeout).ConfigureAwait(false); } finally @@ -373,7 +374,7 @@ private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, C { _testInitializer.PrepareTest(test); test.Context.RestoreExecutionContext(); - var testTimeout = test.Context.Metadata.TestDetails.Timeout; + var testTimeout = ResolveAndPropagateTestTimeout(test); await _testExecutor.ExecuteAsync(test, _testInitializer, cancellationToken, testTimeout).ConfigureAwait(false); } finally @@ -382,6 +383,20 @@ private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, C } } + // Propagation is required so TUnitMessageBus.GetFailureStateProperty (which gates on + // TestDetails.Timeout != null) classifies a DefaultTestTimeout-triggered failure as + // TimeoutTestNodeStateProperty rather than a generic error. + private static TimeSpan? ResolveAndPropagateTestTimeout(AbstractExecutableTest test) + { + var details = test.Context.Metadata.TestDetails; + var testTimeout = TUnitSettings.Default.Timeouts.GetEffectiveTestTimeout(details.Timeout); + if (testTimeout is not null && details.Timeout is null) + { + details.Timeout = testTimeout; + } + return testTimeout; + } + /// /// Disposes the test instance and fires OnDispose callbacks, wrapped in an OpenTelemetry /// activity span for trace timeline visibility. diff --git a/TUnit.TestProject/Bugs/5728/DefaultTimeoutClassificationTests.cs b/TUnit.TestProject/Bugs/5728/DefaultTimeoutClassificationTests.cs new file mode 100644 index 0000000000..6b807c89b4 --- /dev/null +++ b/TUnit.TestProject/Bugs/5728/DefaultTimeoutClassificationTests.cs @@ -0,0 +1,54 @@ +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._5728; + +/// +/// Reproduction for PR #5728 Bug 2: timeout failures triggered by +/// TimeoutSettings.DefaultTestTimeout were misclassified as generic errors +/// instead of timeouts because TestDetails.Timeout was left null when the timeout +/// was resolved from settings rather than a [Timeout] attribute. +/// +/// Only the targeted engine-test filter runs this class — its EngineTest=Failure +/// marker excludes it from the default pass-only harness run. +/// +public class DefaultTimeoutClassificationHooks +{ + // Gated on the filter string so this process-wide setting only takes effect when the + // engine-test harness targets this class — otherwise discovery hooks fire for every + // run of TUnit.TestProject and would shrink every test's default timeout to 200ms. + // + // Read the filter from GlobalContext.Current rather than the BeforeTestDiscoveryContext + // parameter — TUnitTestFramework installs GlobalContext.Current with the stringified + // filter before any hooks run, and it survives the controller-mode + AOT codepaths + // where the hook-parameter's required-init TestFilter has come through empty in CI. + // + // Method name must contain "Before" for the source generator to match it against the + // BeforeTestDiscoveryContext parameter (see HookMetadataGenerator.IsValidHookMethod). + [Before(TestDiscovery)] + public static void BeforeDiscovery_ConfigureDefaultTimeoutWhenTargeted(BeforeTestDiscoveryContext context) + { + var filter = GlobalContext.Current.TestFilter; + if (filter is null || + !filter.Contains("DefaultTimeoutClassificationTests", StringComparison.Ordinal)) + { + return; + } + + context.Settings.Timeouts.DefaultTestTimeout = TimeSpan.FromMilliseconds(200); + } +} + +public class DefaultTimeoutClassificationTests +{ + /// + /// No [Timeout] attribute — timeout is inherited from DefaultTestTimeout. + /// Uses a non-cancellable delay so the timeout path (not cooperative cancellation) + /// fires deterministically, matching the scenario that originally misclassified. + /// + [Test] + [EngineTest(ExpectedResult.Failure)] + public async Task Hanging_Test_With_DefaultTestTimeout_Should_Timeout() + { + await Task.Delay(TimeSpan.FromSeconds(10)); + } +} diff --git a/TUnit.UnitTests/TUnitSettingsTests.cs b/TUnit.UnitTests/TUnitSettingsTests.cs index a300d453de..287fdbd63b 100644 --- a/TUnit.UnitTests/TUnitSettingsTests.cs +++ b/TUnit.UnitTests/TUnitSettingsTests.cs @@ -1,4 +1,5 @@ using TUnit.Core.Settings; +using TUnit.Engine.Helpers; namespace TUnit.UnitTests; @@ -7,7 +8,10 @@ namespace TUnit.UnitTests; [NotInParallel] public class TUnitSettingsTests { - private TimeSpan _savedTestTimeout; + // Nullable: the public getter coalesces to 30-minute fallback, which would silently + // turn the "user never set this" state into a concrete assignment when restored. + // Reading the internal property and restoring via the setter method preserves null. + private TimeSpan? _savedTestTimeout; private TimeSpan _savedHookTimeout; private TimeSpan _savedForcefulExitTimeout; private TimeSpan _savedProcessExitHookDelay; @@ -18,7 +22,7 @@ public class TUnitSettingsTests [Before(HookType.Test)] public void SnapshotSettings() { - _savedTestTimeout = TUnitSettings.Default.Timeouts.DefaultTestTimeout; + _savedTestTimeout = TUnitSettings.Default.Timeouts.ExplicitDefaultTestTimeout; _savedHookTimeout = TUnitSettings.Default.Timeouts.DefaultHookTimeout; _savedForcefulExitTimeout = TUnitSettings.Default.Timeouts.ForcefulExitTimeout; _savedProcessExitHookDelay = TUnitSettings.Default.Timeouts.ProcessExitHookDelay; @@ -30,7 +34,7 @@ public void SnapshotSettings() [After(HookType.Test)] public void RestoreSettings() { - TUnitSettings.Default.Timeouts.DefaultTestTimeout = _savedTestTimeout; + TUnitSettings.Default.Timeouts.SetExplicitDefaultTestTimeout(_savedTestTimeout); TUnitSettings.Default.Timeouts.DefaultHookTimeout = _savedHookTimeout; TUnitSettings.Default.Timeouts.ForcefulExitTimeout = _savedForcefulExitTimeout; TUnitSettings.Default.Timeouts.ProcessExitHookDelay = _savedProcessExitHookDelay; @@ -57,4 +61,73 @@ public async Task Settings_Can_Be_Modified() TUnitSettings.Default.Timeouts.DefaultTestTimeout = TimeSpan.FromMinutes(10); await Assert.That(TUnitSettings.Default.Timeouts.DefaultTestTimeout).IsEqualTo(TimeSpan.FromMinutes(10)); } + + // Covers TestCoordinator's `test.Timeout ?? TUnitSettings...ExplicitDefaultTestTimeout` fallback: + // when the user never assigns DefaultTestTimeout, tests without [Timeout] skip the + // TimeoutHelper wrapper entirely (the right-hand side of the coalesce is null). + [Test] + public async Task ExplicitDefaultTestTimeout_Is_Null_When_Unset() + { + // Fresh instance models the pristine "user never assigned DefaultTestTimeout" state. + // A parallel round-trip assertion on TUnitSettings.Default lives below — this one + // stays isolated so it still passes if the shared-state harness ever regresses. + var freshSettings = new TimeoutSettings(); + + await Assert.That(freshSettings.ExplicitDefaultTestTimeout).IsNull(); + await Assert.That(freshSettings.DefaultTestTimeout).IsEqualTo(TimeSpan.FromMinutes(30)); + } + + // Round-trip on the shared Default: before the fix, snapshotting via the public getter + // collapsed the null "unset" state to 30 minutes, so RestoreSettings permanently pinned + // the backing field and every later test ran with an implicit timeout. + [Test] + public async Task SnapshotRestore_Preserves_Unset_DefaultTestTimeout() + { + var snapshot = TUnitSettings.Default.Timeouts.ExplicitDefaultTestTimeout; + + TUnitSettings.Default.Timeouts.DefaultTestTimeout = TimeSpan.FromMilliseconds(500); + TUnitSettings.Default.Timeouts.SetExplicitDefaultTestTimeout(snapshot); + + await Assert.That(TUnitSettings.Default.Timeouts.ExplicitDefaultTestTimeout).IsEqualTo(snapshot); + } + + [Test] + public async Task ExplicitDefaultTestTimeout_Returns_Assigned_Value() + { + TUnitSettings.Default.Timeouts.DefaultTestTimeout = TimeSpan.FromMilliseconds(200); + + await Assert.That(TUnitSettings.Default.Timeouts.ExplicitDefaultTestTimeout) + .IsEqualTo(TimeSpan.FromMilliseconds(200)); + } + + // End-to-end proof of the fallback: feeding ExplicitDefaultTestTimeout into the same + // TimeoutHelper that TestCoordinator uses must cause a hanging test body to fail with + // TimeoutException. Mirrors TestCoordinator's call site verbatim for this branch. + [Test] + public async Task Configured_Default_Timeout_Fires_On_Hanging_Test() + { + TUnitSettings.Default.Timeouts.DefaultTestTimeout = TimeSpan.FromMilliseconds(200); + + var testTimeout = TUnitSettings.Default.Timeouts.ExplicitDefaultTestTimeout; + await Assert.That(testTimeout).IsNotNull(); + + // Ignore the passed token so TimeoutHelper's timeout branch wins the race + // (a cooperative Task.Delay(ct) would throw TaskCanceledException first). + // A private CTS scoped to this test lets us cancel the delay on exit instead + // of leaking a 30s Task to process end. + using var hangCts = new CancellationTokenSource(); + try + { + await Assert.That(async () => + await TimeoutHelper.ExecuteWithTimeoutAsync( + _ => Task.Delay(TimeSpan.FromSeconds(30), hangCts.Token), + testTimeout!.Value, + CancellationToken.None)) + .Throws(); + } + finally + { + hangCts.Cancel(); + } + } }