Skip to content

Fix #6198: surface global test init/cleanup timeout/cancellation failures #8288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Evangelink wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix #6198: surface global test init/cleanup timeout/cancellation failures #8288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6fab19a
Surface global test init/cleanup timeout/cancellation failures (#6198)
Copilot May 16, 2026
5898925
Merge branch 'main' into dev/evangelink/issue-6198-global-init-cleanu…
Evangelink May 16, 2026
5166ae7
Merge branch 'main' into dev/evangelink/issue-6198-global-init-cleanu…
Evangelink May 17, 2026
3bd965a
Preserve timeout outcomes and cleanup attribution for global fixtures
Copilot May 17, 2026
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
15 changes: 13 additions & 2 deletions src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ private async Task<TestResult> ExecuteInternalAsync(object?[]? arguments, Cancel
// After that, we invoke local cleanups (including Dispose) and finally global cleanups at last.
foreach ((MethodInfo method, TimeoutInfo? timeoutInfo) in Parent.Parent.GlobalTestInitializations)
{
await InvokeGlobalInitializeMethodAsync(method, timeoutInfo, timeoutTokenSource).ConfigureAwait(false);
TestFailedException? globalTestInitException = await InvokeGlobalInitializeMethodAsync(method, timeoutInfo, timeoutTokenSource).ConfigureAwait(false);
if (globalTestInitException is not null)
{
// The returned TestFailedException already carries the per-method timeout/cancellation
// message produced by FixtureMethodRunner; throwing it lets the outer catch record it
Comment on lines +81 to +85
// as the test failure (HandleMethodException returns TestFailedException instances as-is).
throw globalTestInitException;
Copy link
Copy Markdown
Member Author

@Evangelink Evangelink May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When globalTestInitException has Outcome = Timeout (the common case for this bug fix), throwing it into the inner catch causes result.Outcome to be set to Failed instead of Timeout. The catch block's logic is:

// else branch (not OperationCanceledException):
result.TestFailureException ??= HandleMethodException(ex, realException, ...); // returns TestFailedException as-is

if (result.Outcome != UnitTestOutcome.Passed)  // true (default is Failed)
{
    result.Outcome = ex is AssertInconclusiveException ... ? Inconclusive : Failed;  // always Failed
}

This is inconsistent with local TestInitialize timeouts, where RunTestInitializeMethodAsync has a special path that preserves tfe.Outcome:

if (testInitializeException is TestFailedException tfe)
{
    result.Outcome = tfe.Outcome;  // correctly Timeout
    result.TestFailureException = testInitializeException;
    return false;
}

Consider adopting the same pattern here instead of throwing. For example, after the foreach loop exit you could check if globalTestInitException is not null and return early via a setTestContextSucessful = false-style guard, or handle it directly analogously to RunTestInitializeMethodAsync.

The current tests only verify message text (AssertOutputContains("...timed out after 1000ms")), not the Outcome value, so this regression isn't caught by the test suite.

Copy link
Copy Markdown
Contributor

Copilot AI May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3bd965a. I updated ExecuteInternalAsync to preserve TestFailedException.Outcome (including Timeout) instead of normalizing to Failed, and added focused unit coverage (TestMethodInfoInvokeShouldPreserveTimeoutOutcomeForGlobalTestInitialize).

}
}

// TODO remove dry violation with TestMethodRunner
Expand Down Expand Up @@ -195,7 +202,11 @@ private async Task<TestResult> ExecuteInternalAsync(object?[]? arguments, Cancel
result.TestFailureException ??= HandleMethodException(ex, realException, TestClassName, TestMethodName);
}

if (result.Outcome != UnitTestOutcome.Passed)
if (result.TestFailureException is TestFailedException testFailedException)
{
result.Outcome = testFailedException.Outcome;
}
else if (result.Outcome != UnitTestOutcome.Passed)
{
result.Outcome = ex is AssertInconclusiveException || ex.InnerException is AssertInconclusiveException
? UnitTestOutcome.Inconclusive
Expand Down
38 changes: 36 additions & 2 deletions src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Lifecycle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ private async SynchronizationContextPreservingTask RunTestCleanupMethodAsync(Tes
_isTestCleanupInvoked = true;
MethodInfo? testCleanupMethod = Parent.TestCleanupMethod;
Exception? testCleanupException;
TestFailedException? globalTestCleanupException = null;
MethodInfo? globalTestCleanupFailingMethod = null;
MethodInfo? currentGlobalTestCleanupMethod = null;
try
{
try
Expand Down Expand Up @@ -79,13 +82,40 @@ private async SynchronizationContextPreservingTask RunTestCleanupMethodAsync(Tes

foreach ((MethodInfo method, TimeoutInfo? timeoutInfo) in Parent.Parent.GlobalTestCleanups)
{
await InvokeGlobalCleanupMethodAsync(method, timeoutInfo, timeoutTokenSource).ConfigureAwait(false);
currentGlobalTestCleanupMethod = method;
globalTestCleanupException = await InvokeGlobalCleanupMethodAsync(method, timeoutInfo, timeoutTokenSource).ConfigureAwait(false);

if (globalTestCleanupException is not null)
{
// Stop on first global cleanup failure, mirroring the local test cleanup loop above.
globalTestCleanupFailingMethod = method;
break;
Comment on lines +86 to +92
}

currentGlobalTestCleanupMethod = null;
}
}
}
catch (Exception ex)
{
testCleanupException = ex;
if (globalTestCleanupFailingMethod is null && currentGlobalTestCleanupMethod is not null)
{
globalTestCleanupFailingMethod = currentGlobalTestCleanupMethod;
}

if (globalTestCleanupFailingMethod is not null)
{
testCleanupMethod = globalTestCleanupFailingMethod;
}
}

// If no earlier cleanup failure was recorded but a global test cleanup failed, surface it
// so the wrapping logic below produces a TestFailedException with the per-method message.
if (testCleanupException is null && globalTestCleanupException is not null)
{
testCleanupException = globalTestCleanupException;
Copy link
Copy Markdown
Member Author

@Evangelink Evangelink May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When globalTestCleanupException is a TestFailedException with Outcome = Timeout, the re-wrapping code:

Exception realException = testCleanupException.GetRealException(); // returns itself (not TargetInvocationException)
UnitTestOutcome outcomeFromRealException = realException is AssertInconclusiveException
    ? UnitTestOutcome.Inconclusive
    : UnitTestOutcome.Failed;  // always Failed, never Timeout

...causes the final test outcome to be Failed rather than Timeout. Separately, GetFormattedExceptionMessage() on the TestFailedException produces a message that already contains the method name and "timed out after Xms", so the final UTA_CleanupMethodThrows format wraps it as:

TestCleanup method 'GlobalTestCleanupMethod' threw exception: Test cleanup method 'GlobalTestCleanupMethod' timed out after 1000ms

This is a minor redundancy in the message (method name appears twice, "timed out" framing is inside "threw exception").

This same issue affects local TestCleanup timeouts too (pre-existing), so this PR doesn't make it worse — but it's worth tracking since adding a is TestFailedException tfe → result.Outcome = tfe.Outcome fast-path to RunTestCleanupMethodAsync (similar to RunTestInitializeMethodAsync) would fix both.

Copy link
Copy Markdown
Contributor

Copilot AI May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3bd965a. RunTestCleanupMethodAsync now preserves TestFailedException.Outcome for cleanup paths and tracks the currently executing global cleanup method so thrown global cleanup exceptions are attributed to the correct method. I also added focused unit tests for global cleanup timeout outcome and thrown-method attribution.

testCleanupMethod = globalTestCleanupFailingMethod;
}

// If testCleanup was successful, then don't do anything
Expand All @@ -95,7 +125,11 @@ private async SynchronizationContextPreservingTask RunTestCleanupMethodAsync(Tes
}

Exception realException = testCleanupException.GetRealException();
UnitTestOutcome outcomeFromRealException = realException is AssertInconclusiveException ? UnitTestOutcome.Inconclusive : UnitTestOutcome.Failed;
UnitTestOutcome outcomeFromRealException = testCleanupException is TestFailedException testFailedException
? testFailedException.Outcome
: realException is AssertInconclusiveException
? UnitTestOutcome.Inconclusive
: UnitTestOutcome.Failed;
result.Outcome = result.Outcome.GetMoreImportantOutcome(outcomeFromRealException);

realException = testCleanupMethod != null
Expand Down
4 changes: 0 additions & 4 deletions ...Tests/MSTest.Acceptance.IntegrationTests/TimeoutCooperativeGlobalTestCancellationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace MSTest.Acceptance.IntegrationTests;
public sealed class TimeoutCooperativeGlobalTestCancellationTests : AcceptanceTestBase<TimeoutCooperativeGlobalTestCancellationTests.TestAssetFixture>
{
[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so timeouts on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task CooperativeCancellation_WhenGlobalTestInitTimeoutExpires_StepThrows(string tfm)
{
Expand All @@ -27,7 +26,6 @@ public async Task CooperativeCancellation_WhenGlobalTestInitTimeoutExpires_StepT
}

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so timeouts on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task CooperativeCancellation_WhenGlobalTestInitTimeoutExpiresAndUserChecksToken_StepThrows(string tfm)
{
Expand All @@ -44,7 +42,6 @@ public async Task CooperativeCancellation_WhenGlobalTestInitTimeoutExpiresAndUse
}

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalCleanupMethodAsync is currently discarded in TestMethodInfo.Lifecycle.cs, so timeouts on global test cleanup methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task CooperativeCancellation_WhenGlobalTestCleanupTimeoutExpires_StepThrows(string tfm)
{
Expand All @@ -61,7 +58,6 @@ public async Task CooperativeCancellation_WhenGlobalTestCleanupTimeoutExpires_St
}

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalCleanupMethodAsync is currently discarded in TestMethodInfo.Lifecycle.cs, so timeouts on global test cleanup methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task CooperativeCancellation_WhenGlobalTestCleanupTimeoutExpiresAndUserChecksToken_StepThrows(string tfm)
{
Expand Down
4 changes: 1 addition & 3 deletions test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TimeoutWhenCanceledTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.Acceptance.IntegrationTests;
Expand Down Expand Up @@ -34,13 +34,11 @@ public async Task ClassInitBase_WhenTestContextCanceled_ClassInitializeTaskIsCan
=> await RunAndAssertTestWasCanceledAsync(tfm, "TESTCONTEXT_CANCEL_", "baseClassInit");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so cancellations on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestInitialize_WhenTestContextCanceled_GlobalTestInitializeTaskIsCanceled(string tfm)
=> await RunAndAssertTestWasCanceledAsync(tfm, "TESTCONTEXT_CANCEL_", "globalTestInit");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalCleanupMethodAsync is currently discarded in TestMethodInfo.Lifecycle.cs, so cancellations on global test cleanup methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestCleanup_WhenTestContextCanceled_GlobalTestCleanupTaskIsCanceled(string tfm)
=> await RunAndAssertTestWasCanceledAsync(tfm, "TESTCONTEXT_CANCEL_", "globalTestCleanup");
Expand Down
7 changes: 1 addition & 6 deletions test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TimeoutWhenExpiresTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.Acceptance.IntegrationTests;
Expand Down Expand Up @@ -119,31 +119,26 @@ public async Task TestCleanup_WhenTimeoutExpires_TestCleanupIsCanceled_Attribute
=> await RunAndAssertAttributeTakesPrecedenceAsync(tfm, "testCleanup");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so timeouts on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestInitialize_WhenTimeoutExpires_GlobalTestInitializeTaskIsCanceled(string tfm)
=> await RunAndAssertTestTimedOutAsync(tfm, "LONG_WAIT_", "globalTestInit");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so timeouts on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestInitialize_WhenTimeoutExpiresAndTestContextTokenIsUsed_GlobalTestInitializeExits(string tfm)
=> await RunAndAssertTestTimedOutAsync(tfm, "TIMEOUT_", "globalTestInit");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalInitializeMethodAsync is currently discarded in TestMethodInfo.Execution.cs, so timeouts on global test initialize methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestInitialize_WhenTimeoutExpires_GlobalTestInitializeIsCanceled_AttributeTakesPrecedence(string tfm)
=> await RunAndAssertAttributeTakesPrecedenceAsync(tfm, "globalTestInit");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalCleanupMethodAsync is currently discarded in TestMethodInfo.Lifecycle.cs, so timeouts on global test cleanup methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestCleanup_WhenTimeoutExpires_GlobalTestCleanupTaskIsCanceled(string tfm)
=> await RunAndAssertTestTimedOutAsync(tfm, "LONG_WAIT_", "globalTestCleanup");

[TestMethod]
[Ignore("Tracked by https://github.com/microsoft/testfx/issues/6198. The TestFailedException returned by InvokeGlobalCleanupMethodAsync is currently discarded in TestMethodInfo.Lifecycle.cs, so timeouts on global test cleanup methods do not fail the test.")]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task GlobalTestCleanup_WhenTimeoutExpires_GlobalTestCleanupIsCanceled_AttributeTakesPrecedence(string tfm)
=> await RunAndAssertAttributeTakesPrecedenceAsync(tfm, "globalTestCleanup");
Expand Down
57 changes: 57 additions & 0 deletions test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ public TestMethodInfoTests()
DummyTestClass.TestInitializeMethodBody = value => { };
DummyTestClass.TestMethodBody = instance => { };
DummyTestClass.TestCleanupMethodBody = value => { };
DummyTestClass.GlobalTestInitializeMethodBodyAsync = _ => Task.CompletedTask;
DummyTestClass.GlobalTestCleanupMethodBodyAsync = _ => Task.CompletedTask;
}

protected override void Dispose(bool disposing)
Expand Down Expand Up @@ -921,6 +923,21 @@ public async Task TestMethodInfoInvokeWhenTestThrowsAssertInconclusiveReturnsExp
"Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.Execution.TestMethodInfoTests.DummyTestClass.DummyTestInitializeMethod");
}

public async Task TestMethodInfoInvokeShouldPreserveTimeoutOutcomeForGlobalTestInitialize()
{
DummyTestClass.GlobalTestInitializeMethodBodyAsync = _ => Task.Delay(TimeSpan.FromSeconds(10));
_testAssemblyInfo.GlobalTestInitializations.Add((typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyGlobalTestInitializeMethod))!, TimeoutInfo.FromTimeout(1)));

TestResult result = await _testMethodInfo.InvokeAsync(null);

result.Outcome.Should().Be(UnitTestOutcome.Timeout);
var exception = result.TestFailureException as TestFailedException;
exception.Should().NotBeNull();
exception!.Outcome.Should().Be(UnitTestOutcome.Timeout);
exception.Message.Should().Contain("DummyGlobalTestInitializeMethod");
exception.Message.Should().Contain("timed out after 1ms");
}

public async Task TestMethodInfoInvokeWhenConstructorThrowsAssertInconclusiveReturnsExpectedResult()
{
// Arrange.
Expand Down Expand Up @@ -1218,6 +1235,36 @@ public async Task TestMethodInfoInvokeShouldSetMoreImportantOutcomeIfTestCleanup
result.Outcome.Should().Be(UnitTestOutcome.Failed);
}

public async Task TestMethodInfoInvokeShouldPreserveTimeoutOutcomeForGlobalTestCleanup()
{
DummyTestClass.GlobalTestCleanupMethodBodyAsync = _ => Task.Delay(TimeSpan.FromSeconds(10));
_testAssemblyInfo.GlobalTestCleanups.Add((typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyGlobalTestCleanupMethod))!, TimeoutInfo.FromTimeout(1)));

TestResult result = await _testMethodInfo.InvokeAsync(null);

result.Outcome.Should().Be(UnitTestOutcome.Timeout);
var exception = result.TestFailureException as TestFailedException;
exception.Should().NotBeNull();
exception!.Outcome.Should().Be(UnitTestOutcome.Timeout);
exception.Message.Should().Contain("DummyGlobalTestCleanupMethod");
exception.Message.Should().Contain("timed out after 1ms");
}

public async Task TestMethodInfoInvokeShouldAttributeGlobalTestCleanupMethodWhenItThrows()
{
DummyTestClass.TestCleanupMethodBody = _ => { };
DummyTestClass.GlobalTestCleanupMethodBodyAsync = _ => throw new InvalidOperationException("global cleanup failed");
_testClassInfo.TestCleanupMethod = typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyTestCleanupMethod))!;
_testAssemblyInfo.GlobalTestCleanups.Add((typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyGlobalTestCleanupMethod))!, timeoutInfo: null));

TestResult result = await _testMethodInfo.InvokeAsync(null);

result.Outcome.Should().Be(UnitTestOutcome.Failed);
var exception = result.TestFailureException as TestFailedException;
exception.Should().NotBeNull();
exception!.Message.Should().Contain($"Test cleanup method '{typeof(DummyTestClass).Name}.{nameof(DummyTestClass.DummyGlobalTestCleanupMethod)}' threw exception");
}

public async Task TestMethodInfoInvokeShouldCallDisposeForDisposableTestClass()
{
bool disposeCalled = false;
Expand Down Expand Up @@ -1848,6 +1895,10 @@ public class DummyTestClass : DummyTestClassBase

public static Func<Task> DummyAsyncTestMethodBody { get; set; } = null!;

public static Func<TestContext, Task> GlobalTestInitializeMethodBodyAsync { get; set; } = null!;

public static Func<TestContext, Task> GlobalTestCleanupMethodBodyAsync { get; set; } = null!;

public static TestContext GetTestContext() => s_tc;

public TestContext TestContext
Expand Down Expand Up @@ -1889,6 +1940,12 @@ public Task DummyAsyncTestMethod() =>

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
public void DummyParamsArgumentMethod(int i, params string[] args) => TestMethodBody(this);

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
public static async Task DummyGlobalTestInitializeMethod(TestContext testContext) => await GlobalTestInitializeMethodBodyAsync(testContext);

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
public static async Task DummyGlobalTestCleanupMethod(TestContext testContext) => await GlobalTestCleanupMethodBodyAsync(testContext);
}

public class DummyTestClassWithRetryAttributeMethods
Expand Down
Loading