[Amaury Levé (Evangelink)]

Pull request

Summary

Backport only the cleanup-test stabilization slice of #12296 to rel/4.5 for MSTest 4.5.3 / MTP 2.5.3.

The local cleanup now signals that it has started before the test cancels its actual source, instead of racing a 100 ms fixture timeout against dispatch. The blocking monitor uses a dedicated long-running task, and cleanup joins the original invocation and workers before restoring PlatformServiceProvider.Instance.

Exactly one file changes: test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodInfoTests.cs (33 additions, 10 deletions). The helper matches upstream exactly. Excludes the analyzer and fatal-process probe portions of #12296, whose fixtures are absent from this release. No production code, dependencies, APIs, target frameworks, test deadlines, or assembly scheduling change. The existing five-second whole-test deadline remains intact, and the C# UTF-8 BOM is preserved.

Design and decision record

This is a bounded test-harness backport using the rationale and implementation from #12296, not a feature or architecture change; a separate decision record is not needed.

Provenance	Exact identity
Release base and commit parent	d54bb2c77ae35141b0ea15472782c34b933ceaab
Upstream merge, #12296	55c9b81067a493fe2d9fd12d88dd22d426f457ca
Backport head	50fef0b4c0b41523b49fa6ff9e3142f3a42468dd
Backport tree	8e7878aeeef0df67a9749ca9ec4fc84265a89325
Validated and committed test-file blob	a41161c7d315bfaf0ef5ec67be22767e01ae9e1a

The fixed runs used this exact test-file blob; its identity was checked again after committing, with no source changes after validation.

Isolation: this assembly uses the internal sequential TestContainer engine, not MSTest's parallel scheduler. Scope remains off. The process-local service provider and dummy lifecycle callbacks keep their existing ownership. Started/release rendezvous and bounded joins cover the original worker lifetime before shared-state restoration. No locks, opt-outs, or serialization settings were added or removed.

Demonstration

Test-only change: no product UI or package demonstration is applicable. The before/after result reports contain the same 24 noncooperative cases per target, with no failed or skipped cases.

Validation and regression proof

Executed on Windows x64 with the release-pinned SDK 12.0.100-alpha.1.26480.103, repository build scripts, desktop MSBuild, and Release configuration. Both baseline and fixed builds completed with exit 0, zero warnings and zero errors, retaining all five project TFMs.

The following commands were executed from the repository root in cmd.exe. The PATH/PATHEXT changes are process-local; they repair native lookup of the already-installed Windows PowerShell, without changing repository/global settings or installing a machine-wide Windows SDK.

set "PATH=C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32;%PATH%"
set "PATHEXT=.COM;.EXE;.BAT;.CMD"

rem Baseline d54bb2c77ae35141b0ea15472782c34b933ceaab: passed, exit 0.
powershell -NoProfile -ExecutionPolicy Bypass -File eng\build.ps1 -restore -build -c Release -projects test\UnitTests\MSTestAdapter.PlatformServices.UnitTests\MSTestAdapter.PlatformServices.UnitTests.csproj -msbuildEngine vs /p:EnableCodeCoverage=false /bl:artifacts\log\Release\cleanup-backport\baseline-6205e1d5ba7b40b9bd69bf1f02f1fb28.binlog

rem Fixed test-file blob a41161c7d315bfaf0ef5ec67be22767e01ae9e1a: passed, exit 0.
powershell -NoProfile -ExecutionPolicy Bypass -File eng\build.ps1 -build -c Release -projects test\UnitTests\MSTestAdapter.PlatformServices.UnitTests\MSTestAdapter.PlatformServices.UnitTests.csproj -msbuildEngine vs /p:EnableCodeCoverage=false /bl:artifacts\log\Release\cleanup-backport\fixed-58bba7432984403a9f98e44a88f37303.binlog

Each host below was executed on both states with these arguments, using its own phase/TFM result directory. All ten host commands returned exit 0. The loop is a compact reproduction of those five expanded host invocations; run it with $phase = 'baseline' after the baseline build and $phase = 'fixed' after the fixed build.

$project = 'MSTestAdapter.PlatformServices.UnitTests'
$tfms = 'net8.0', 'net9.0', 'net462', 'net48', 'net8.0-windows10.0.18362.0'
foreach ($tfm in $tfms) {
    $results = Join-Path (Get-Location).Path "artifacts\TestResults\cleanup-backport\$phase\$tfm"
    New-Item -ItemType Directory -Path $results -Force | Out-Null
    $testArgs = @(
        '--treenode-filter', '/*/*/TestMethodInfoTests/NonCooperative*',
        '--results-directory', $results,
        '--report-trx', '--report-trx-filename', 'results.trx',
        '--report-junit', '--report-junit-filename', 'results.junit.xml',
        '--report-ctrf', '--report-ctrf-filename', 'results.ctrf.json',
        '--show-test-results', 'failed'
    )
    if ($tfm -in 'net462', 'net48') {
        & ".\artifacts\bin\$project\Release\$tfm\$project.exe" @testArgs
    } else {
        & .\.dotnet\dotnet.exe ".\artifacts\bin\$project\Release\$tfm\$project.dll" @testArgs
    }
    if ($LASTEXITCODE -ne 0) { throw "Test host failed: $phase $tfm" }
}

Target	Baseline passed / failed / skipped	Fixed passed / failed / skipped
net8.0	24 / 0 / 0	24 / 0 / 0
net9.0	24 / 0 / 0	24 / 0 / 0
net462	24 / 0 / 0	24 / 0 / 0
net48	24 / 0 / 0	24 / 0 / 0
net8.0-windows10.0.18362.0	24 / 0 / 0	24 / 0 / 0

All 30 TRX/JUnit/CTRF report files were parsed. Each format contains the same exact 24 expected method names for every phase/TFM, all passing: 120 baseline + 120 fixed executions, not three times that number. The three direct helper consumers executed exactly once in every run:

• NonCooperativeTimeoutWaitsForGlobalCleanupAfterLocalCancellation
• NonCooperativeTimeoutOnSynchronizationContextWaitsForGlobalCleanupAfterLocalCancellation
• NonCooperativeTimeoutCancelsRenewedGlobalCleanupAtOriginalDeadline

Reports and console logs are retained locally under artifacts\TestResults\cleanup-backport\{baseline,fixed}\<TFM>\; binary logs are at the paths above. These local artifacts have not been uploaded. git diff --check and the staged diff check passed; the committed helper was compared with upstream and the BOM verified.

Test-value evidence

Contract / cases	Comparison	Result and limit
Global cleanup remains pending after local cancellation, on the default scheduler and synchronization context; first two named cases above	Unmodified release harness versus the exact upstream stabilized helper, same product and assertions	Executed preservation: both cases pass on all five targets before and after. New rendezvous assertions also verify local/global startup and that the invocation is still pending while global cleanup is gated.
Renewed global cleanup retains the original whole-test deadline; third named case above	Same before/after comparison, unchanged 5,000 ms deadline	Executed preservation: passes on all five targets before and after.
Neighboring cancellation, timeout, disposal and context cases	Same 24-case NonCooperative* selection	Executed preservation: 120/120 before and 120/120 after; zero skips.

Evidence gap: the original intermittent 100 ms startup race was not reproduced in the local baseline, so these results are green-before/green-after preservation evidence, not a claimed deterministic red/green reproduction of that flake. No production mutation, full-suite, packaging or acceptance run was performed; this backport changes only the unit-test helper.

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Reviewed VerifyNonCooperativeGlobalCleanupAsync in TestMethodInfoTests.cs. I found no blocking issues.

• Flakiness: Cancelling the local cleanup explicitly after localEntered, instead of racing a 100ms fixture timer, removes the timing dependence. The LongRunning start for the non-sync-context path avoids thread-pool starvation from the blocking monitor.
• Coverage: The new assertions are useful. They check that local cancellation doesn't bypass global cleanup and that the invocation still waits for it.
• Cleanup: The finally waits on the invocation and the dispatched or worker tasks together. It restores PlatformServiceProvider.Instance only when those operations completed, so a hung operation can't leak into later tests.
• Nit (non-blocking): localSource is written on the cleanup thread and read on the test thread. It is safe in practice because localEntered.TrySetResult runs after the assignment and RunContinuationsAsynchronously gives ordering. The finally read of localSource?.Cancel() can run before startup, so it correctly tolerates null.

No inline findings.

[github-actions]

Final test-triage resolution — Azure Pipelines build 1672618

Analysis is final and the evidence is complete. Build microsoft.testfx failed on the merge of PR head 50fef0b. There were 43 failed result records, 115,580 passed and 767 skipped. No crash, hang or timeline signals were found, and no slow-test regressions. The history collection reported no gaps, but it returned 0 prior builds, so there is no baseline for comparison.

Signatures:

1. MSTest.Acceptance.IntegrationTests cleanup cancellation tests (confidence: high that this is deterministic and tied to this PR).

   • Linux: 12 tests failed with Assert.IsNotNull(adapterPayload) and actual: null. They are GlobalCleanup_Normal, _UnrelatedLocalFailure, _LocalCancellationWithoutException, _LocalCancellationException, _CancellationWithUnrelatedLocalFailure, _LocalCooperativeTimeout, _PreservesLocalTimeoutWhenGlobalCleanupThrows, the NonCooperativeTest and SourceGeneration variants, and _TerminalCancellation_NonCooperativeTest. The same failures appear in both the CTRF and JUnit reports for that leg.
   • Windows (x64): CleanupCancellationIsolationTests.GlobalCleanupAfterLocalCancellationReleasesOwnedResource failed with the same assertion in 12 TRX records.
   • The PR title is "[rel/4.5] Stabilize MSTest cleanup cancellation tests", so these tests are probably what the PR is changing. The adapterPayload is never found in the test-host output, which points at the PR's change or its test helper rather than an environmental flake.
   • No retry or fail-then-pass recovery was recorded, so this is not flaky.
   • Next step: check how adapterPayload is parsed from the output of the generated asset, and run the GlobalCleanup_* acceptance tests locally after ./build.sh -pack.

2. Microsoft.Testing.Platform.Acceptance.IntegrationTests, PackagedNet6Consumer_WhenSiblingApphostCannotExecute_RetriesThroughDotnet (Linux, confidence: medium).

   • The packaged net6.0 consumer failed to compile with Program.cs(40,82): error CS1503: cannot convert from 'byte[]' to 'string'.
   • This is a deterministic compile error in the injected consumer source. I could not tell from the evidence whether it comes from this PR.
   • Next step: confirm whether it also fails on the base branch, and look at the consumer source and the net6.0 API surface used there.

The other failed records are the intentional retry probes in the Windows app-model diagnostics artifacts. Their messages ("deliberately fails the first retry attempt", "The first AppContainer host fails so Retry must activate a second host") describe planned failures, so they are not defects.

No issue was created: no matching open issue was found, and the evidence points to this pull request rather than a recurring problem on main or scheduled builds. Build-compilation analysis is left to Build Failure Analysis.

🤖 Automated content by GitHub Copilot. Generated by the Pipeline Test Triage workflow. · copilot · auto · ~32 AIC · ⌖ ~1 AIC · ⊞ 10.1K