Harden MTP artifact post-processing and add an opt-out - #55480
Merged
Evangelink merged 1 commit intoJul 28, 2026
Merged
Conversation
Artifact post-processing runs after a test run has already completed and reported its results, so it must never be able to change that outcome. Two ways it could are fixed: - Any exception outside a small hand-picked set escaped ExecuteAsync, was not caught by the caller, and reached the top-level CLI handler, turning a finished run into a crash with a different exit code. Every failure is now logged to trace and downgraded to the existing warning. - Cancelling mid-job kills the post-processing process, and the resulting non-zero exit code was then reported as a post-processing failure. Both warning sites now route through ReportFailureUnlessCancelled. Also adds '--no-artifact-post-processing', which skips post-processing so each test application keeps its own artifacts instead of test applications being relaunched to merge them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e002463b-c925-4c2f-90c3-b56d480faa7d
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens Microsoft Testing Platform (MTP) artifact post-processing in dotnet test so it remains strictly best-effort after a run completes, adds cancellation-aware warning suppression, and introduces an explicit opt-out switch for users who want to keep per-test-application artifacts unmerged.
Changes:
- Add
--no-artifact-post-processingto skip relaunch/merge post-processing for MTP test runs. - Make post-processing failures non-fatal by catching unexpected exceptions and downgrading them to warnings; suppress warnings when the run was cancelled.
- Add/update unit + E2E coverage and refresh the MTP help snapshot and localization resources for the new option.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Test/snapshots/MTPHelpSnapshotTests.VerifyMTPHelpOutput.verified.txt | Updates MTP help snapshot to include --no-artifact-post-processing. |
| test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsArtifactPostProcessingMTP.cs | Adds E2E coverage asserting the opt-out keeps one TRX per test app and produces no merged TRX. |
| test/dotnet.Tests/CommandTests/Test/ArtifactPostProcessingManagerTests.cs | Adds unit tests for unexpected failure hardening and cancellation-based warning suppression. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Test/TestCommandDefinition.MicrosoftTestingPlatform.cs | Defines the new --no-artifact-post-processing option for the MTP dotnet test command. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/CommandDefinitionStrings.resx | Adds the localized string resource for the new option’s description. |
| src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs | Wires the new option to skip post-processing when specified. |
| src/Cli/dotnet/Commands/Test/MTP/ArtifactPostProcessingManager.cs | Implements catch-all hardening + cancellation-aware warning reporting helper. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.cs.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.de.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.es.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.fr.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.it.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ja.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ko.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.pl.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.pt-BR.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ru.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.tr.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.zh-Hans.xlf | Adds new localization unit for the option description. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.zh-Hant.xlf | Adds new localization unit for the option description. |
Copilot's findings
- Files reviewed: 20/20 changed files
- Comments generated: 1
Evangelink
enabled auto-merge
July 27, 2026 17:30
YuliiaKovalova
approved these changes
Jul 28, 2026
Evangelink
deleted the
dev/amauryleve/harden-mtp-artifact-post-processing
branch
July 28, 2026 10:44
Member
Author
|
/backport to release/11.0.1xx-preview7 |
Contributor
|
Started backporting to |
pull Bot
pushed a commit
to AraHaan/sdk
that referenced
this pull request
Aug 3, 2026
Follow-up to dotnet#55419 and dotnet#55480. The remaining gaps against microsoft/testfx RFC 018 that are owned by this repo: - Skip post-processing when the run was cut short by --maximum-failed-tests or --timeout. Only Ctrl+C skipped it, so a truncated run's artifacts were merged into one authoritative-looking report that hid the truncation. The decision moves into ShouldPostProcessArtifacts so it is directly testable. - Forward --config-file to the relaunched merge host. A merge host resolves its extensions like a test host does, so a configuration file that governs which extensions load has to reach it, or the merge runs with a different extension set than the run that produced the artifacts. - Prefer a directory the elected application already writes to when --results-directory is absent, instead of whichever input sorted first. - Make the 15-minute merge-host bound overridable through DOTNET_CLI_TEST_ARTIFACT_POST_PROCESSING_TIMEOUT_SECONDS, with 0 removing it. Values the framework would reject collapse to 'no bound' rather than throwing out of the wait and failing every merge. - Print a progress line before merging. Post-processing runs after the last module exits and before the summary, so a slow merge was indistinguishable from a hung CLI. - Emit test/artifact-post-processing telemetry (RFC 7.11). Kinds and extensions outside a fixed well-known list are bucketed as 'other' so a private post-processor's identifier is never uploaded. - Guard the whole of ExecuteAsync, not just each job, so planning, the progress line and telemetry also cannot turn a finished run into a crash. A merge killed by Ctrl+C no longer counts as a failed job. Adds documentation/general/dotnet-test-artifact-post-processing.md and the telemetry event entry; the feature had no documentation at all. Also fixes MultiProjectRun_MergesTrxArtifacts, which failed on main: this suite runs under Microsoft.Testing.Platform, so its execution id leaked into the inner 'dotnet test' and made both invocations look like one execution, collapsing their merged reports onto the same name. Relates to dotnet#47613. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a41f8680-43fb-4285-b81e-ed5a2788fbd2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #55419, which added MTP artifact post-processing to
dotnet test.Post-processing runs after a test run has completed and reported its results, and it works by relaunching test applications with a hidden merge tool. That makes it a best-effort convenience layered on top of a finished run — it must never be able to change that run's outcome. Two ways it currently can are fixed here, and an opt-out is added.
Post-processing could crash a completed run
ArtifactPostProcessingManager.ExecuteAsynccaught only a hand-picked set of exceptions (IOException,UnauthorizedAccessException,InvalidOperationException,Win32Exception,NotSupportedException,TimeoutException). Anything outside that set escapedExecuteAsync; the caller inMicrosoftTestingPlatformTestCommand.Rundoes not catch it, so it reached the top-level handler inProgram.csand turned an already-finished test run into a CLI crash with a different exit code and a raw stack trace.It is now a catch-all that logs the full exception to trace and downgrades to the existing
ArtifactPostProcessingFailedwarning. The original artifacts are still on disk and still reported, which is exactly what that warning already promises.Ctrl+C was reported as a post-processing failure
Cancelling the run kills the post-processing child process along with everything else. Its non-zero exit code was then surfaced as an
ArtifactPostProcessingProcessFailedwarning, blaming post-processing for something the user asked for. Both warning sites now route throughReportFailureUnlessCancelled, which suppresses the warning when the cancellation token is signalled. Loop termination is unchanged — the existing top-of-loop guard still stops the remaining jobs.New
--no-artifact-post-processingEvery MTP run currently relaunches test applications to merge artifacts, with no way to turn it off. The new flag skips post-processing entirely, so each test application keeps its own artifacts:
Notes
While preparing this I also tried excluding superseded retry attempts from the merge inputs, on the theory that a merged TRX could double-count retried tests. That turned out to be wrong and is deliberately not included:
Microsoft.Testing.Extensions.Retryre-runs only the previously failed tests, so a later attempt's report contains just that subset — the SDK's ownTestProgressState.ReportGenericTestResultmodels this by subtracting per-test-uid only when a uid reappears. Dropping earlier attempts would have silently lost every test that passed first time and was never retried, which is worse than the duplicate it was meant to prevent. Correctly de-duplicating requires per-test knowledge that lives inside the artifacts, i.e. inside the merge tool, so it belongs upstream in TestFX rather than here.Testing
ExecuteAsync_WhenJobFailsUnexpectedly_ReportsWarningWithoutThrowing— mutation-tested: narrowing the catch back tocatch (IOException ex)makes theArgumentExceptionescape and the test fail.ExecuteAsync_WhenCancelledBeforeStarting_RunsNoJobs— asserts the results directory is never created, which is the first observable side effect of running a job. Mutation-tested: deleting the top-of-loop cancellation guard makes it fail. (Asserting on the absence of a warning would have been vacuous here, since a job that runs and fails under cancellation is silent by design.)ReportFailureUnlessCancelled_When{Cancelled,NotCancelled}— cover the new suppression directly.MultiProjectRun_WithNoArtifactPostProcessing_KeepsOneReportPerTestApplication— end-to-end, asserts exactly two unmerged TRX reports and nomerged-file.Full
build.cmd, then against the built redist SDK: theArtifactPostProcessing*,TestApplicationHandlerTests,TerminalTestReporterTestsandTestProgressStateTestsunit sets (67 tests), plusMTPHelpSnapshotTests, both artifact post-processing E2E tests andRunTestProjectWithWithRetryFeature. The help snapshot is updated for the new option; all 15.xlffiles were regenerated by the build rather than hand-edited.Relates to #47613.