RFC 018: Artifact post-processing for dotnet test (MTP)#9187
Draft
Evangelink wants to merge 3 commits into
Draft
RFC 018: Artifact post-processing for dotnet test (MTP)#9187Evangelink wants to merge 3 commits into
Evangelink wants to merge 3 commits into
Conversation
Two-phase artifact post-processing to consolidate per-module artifacts (TRX, coverage, custom) at the end of a dotnet test run via a typed IArtifactPostProcessor extension contract, a reserved host mode, handshake-advertised capabilities, and SDK-side election/orchestration. Opened as an RFC for discussion. Notably raises two unresolved orchestration alternatives: using the existing ITool route as the invocation primitive, and a specialized post-processing host with a live IPC channel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
⚠️ Not ready to approve
The RFC includes a few documentation inaccuracies/ambiguities (repo path attribution and an internally inconsistent election tie-break snippet) that could mislead implementers.
Pull request overview
Adds a new design RFC (017) to the docs/RFCs set, describing a two-phase artifact post-processing/merging mechanism for Microsoft.Testing.Platform (MTP) runs orchestrated by dotnet test, including a proposed IArtifactPostProcessor contract, host invocation modes, and SDK election/orchestration.
Changes:
- Introduces RFC 017 documenting artifact consolidation/post-processing for multi-module
dotnet testruns under MTP. - Specifies the proposed extension contract (
IArtifactPostProcessor), manifest/result schemas, handshake capability advertisement, and election algorithm. - Documents alternative orchestration primitives (
--toolvs reserved switch vs specialized host) and open questions.
File summaries
| File | Description |
|---|---|
| docs/RFCs/017-Artifact-Post-Processing.md | New RFC describing the proposed artifact post-processing contract and SDK/host orchestration flow for consolidating artifacts after multi-module dotnet test runs. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 3
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
- Note that src/Cli/dotnet/Commands/Test/MTP/ lives in the dotnet/sdk repo. - Reword the informal 'already-ish present' Warning comment into a concrete statement. - Fix the A.2 election tie-break: compare ProducingTestModule (a file name) against Path.GetFileName(ModulePath) instead of the full path, so it actually matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
017 is already taken on main by 017-TestHost-Launcher.md (#9349), so rename docs/RFCs/017-Artifact-Post-Processing.md -> 018-Artifact-Post-Processing.md. Rework the design into a layered, decisive recommendation: - Phase 1 (ship first): typed, invocation-agnostic IArtifactPostProcessor engine contract (experimental) + a user-facing ITool per kind (merge-trx) sharing one engine. Delivers user value with no SDK/protocol change (one platform-API change: promote ITool to public). - Phase 2: in-memory election (from data the SDK already has via handshake + live FileArtifactMessages), a platform-owned internal dispatcher ITool, and merged artifacts returned over the existing dotnet-test pipe as FileArtifactMessage. - Election is minimal set-cover (fewest relaunches), arch-constrained for binary coverage; live-channel alternative reframed to lead with reuse of the existing pipe; experimental gating; cross-arch notes. Also fixes markdownlint issues and corrects codebase-accuracy points surfaced in review (ITool is internal today; TrxReportEngine has no file-merge path yet; no --list-tools switch; public records must avoid synthesized init accessors). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evangelink
changed the title
Comment on lines
+289
to
+293
| app = argmax(apps, key=lambda a: ( | ||
| covered_group_count(a, groups), | ||
| produced_input_count(a, groups), | ||
| tfm_order(a.tfm), | ||
| lexical(a.path))) # ascending, deterministic (matches Appendix A.3) |
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.
Summary
Opens RFC 018 — Artifact post-processing for
dotnet test(MTP) for discussion (docs/RFCs/018-Artifact-Post-Processing.md).The RFC proposes a mechanism so that, after a multi-module
dotnet testrun, artifacts of the same kind (TRX,.coverage, Cobertura, custom) are consolidated into single files instead of listed one-per-module — the MTP analogue of VSTest's collect/post-process flow.Addresses dotnet/sdk#47613; related to #7345, #7471, #6586.
Status: Under discussion
The design is now a decisive, layered recommendation rather than an open menu of orchestration routes:
IArtifactPostProcessorengine contract (experimental-gated) plus a user-facingIToolper kind (e.g.merge-trx), both delegating to one shared merge engine. This ships user-visible value (Using TrxReport with --test-modules should include test assembly name and target framework in trx file name #7345) with no SDK or protocol change (one platform-API change: promoteIToolto public), is user-runnable from a shell, and is discoverable via--info.dotnet-testpipe — then a platform-owned internal dispatcherIToolrelaunched once per elected app, with merged artifacts flowing back over the existing pipe asFileArtifactMessage(re-entering the normal reporter path).Key design refinements vs. the original draft:
--internal-post-process-artifactsswitch:--toolis the platform's existing non-test execution path (precedent:ms-trxcompare), so the manual and automated routes share one mechanism. The reserved switch is kept as the documented runner-up (§11.1).DotnetTestConnectionalready streams artifacts live, so the SDK already has the full attributed input list; the manifest is transitional and largely redundant (§7.7, §11.2).Feedback wanted
ITool(recommended) vs. reserved switch vs. specialized live-channel host (§11.1/§11.2, Open question Initial commit! 🎉 #1).Kindasstringvs. a typed wrapper, and Kind-versioning policy (Open questions Porting BugFix#260653: DataRowTests not getting appended by arguments #9/Packaging mstest for .net core #10).cc @Evangelink