Skip to content

Add artifact merge engines and Kind metadata flow (RFC 018 phase 1)#9805

Open
Evangelink wants to merge 5 commits into
mainfrom
dev/amauryleve/miniature-broccoli
Open

Add artifact merge engines and Kind metadata flow (RFC 018 phase 1)#9805
Evangelink wants to merge 5 commits into
mainfrom
dev/amauryleve/miniature-broccoli

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Implements the first testfx-side pieces of RFC 018 — Artifact post-processing for dotnet test (MTP). Two independent, self-contained foundations that every later phase (dispatcher tool, SDK election) reuses. No SDK dependency; the SDK orchestration lands separately once this merges.

1. Report merge engines (pure, invocation-agnostic — no I/O in the core)

Validated the RFC's "collect same-kind inputs → re-aggregate → emit one" shape across both data models:

Engine Format Merge unit Re-aggregation
TrxReportEngine.Merge / MergeToFileAsync XML Results/TestDefinitions/TestEntries counters summed, Times earliest/latest, TestLists deduped by id, outcome Failed if any input failed
JUnitReportMerger XML <testsuite> counters derived by summing per-suite (correct even when root aggregates are missing); supports bare <testsuite> roots
CtrfReportMerger JSON (JsonNode) results.tests[] summary derived from the merged tests[] so summary.tests always matches the array; start/stop min/max

HTML is intentionally not file-merged (it's a rendered template with an embedded JSON blob) — this validates the design's opt-in / return-null semantics rather than breaking it.

2. Kind metadata flow (producer → IPC wire)

So the dotnet test orchestrator can group artifacts by format without inspecting file contents:

  • SessionFileArtifact gains an optional kind (experimental-gated, [TPEXP]); the shipped 4-arg constructor is preserved for binary compatibility.
  • IPC: FileArtifactMessage.Kind + FileArtifactMessageFieldsId.Kind = 7 + serializer read/write/field-count (forward/backward compatible — old readers skip the unknown field).
  • DotnetTestDataConsumer propagates Kind onto the wire message.
  • Report producers tag their kind: TRX microsoft.testing.trx, JUnit microsoft.testing.junit, CTRF microsoft.testing.ctrf, HTML microsoft.testing.html.

Tests

  • Unit tests for all three merge engines (null/empty inputs, counter summing, outcome, dedup, timestamps, bare-root/missing-aggregate robustness, summary-missing).
  • IPC protocol round-trip tests assert Kind survives the pipe.
  • Full repo builds clean (only the unrelated WasiPlayground sample fails on a missing workload); Platform IPC/protocol, contract-package, and Extensions unit tests all green.

Not in this PR (future phases, per the RFC)

  • IArtifactPostProcessor typed contract wrapping the engines.
  • Handshake advertisement of post-processor kinds.
  • Dispatcher ITool + tool-host-over-pipe composition.
  • User tools (merge-trx, …) and the dotnet/sdk orchestration.

cc @Evangelink

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

Implements the first testfx-side pieces of RFC 018 (artifact post-processing
for dotnet test / MTP):

- Report merge engines (pure, invocation-agnostic, no I/O in the core):
  - TrxReportEngine.Merge / MergeToFileAsync (XML-level TRX merge).
  - JUnitReportMerger (unions <testsuite>, counters derived per-suite).
  - CtrfReportMerger (JSON merge; summary derived from merged tests[]).
- Kind metadata flow so the dotnet/sdk orchestrator can group artifacts by
  format: SessionFileArtifact gains an optional kind (experimental), the
  FileArtifactMessage IPC record + serializer carry Kind (field id 7), the
  dotnet-test consumer propagates it, and report producers tag their kind
  (trx/junit/ctrf/html).

Unit tests cover all three merge engines and the IPC round-trip.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds report-merging foundations and propagates artifact format metadata through MTP’s artifact and IPC layers.

Changes:

  • Adds TRX, JUnit, and CTRF merge engines with unit tests.
  • Adds experimental SessionFileArtifact.Kind metadata and IPC serialization.
  • Tags TRX, JUnit, CTRF, and HTML report artifacts by format.
Show a summary per file
File Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs Covers IPC kind serialization.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolEdgeCaseTests.cs Tests nullable kind round trips.
test/UnitTests/Microsoft.Testing.Platform.DotnetTestProtocolContract.UnitTests/DotnetTestProtocolSerializerTests.cs Verifies protocol-contract serialization.
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxReportEngineMergeTests.cs Tests TRX merging.
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportMergerTests.cs Tests JUnit merging.
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs Tests CTRF merging.
src/Platform/SharedExtensionHelpers/ReportGeneratorBase.cs Publishes report artifact kinds.
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/FileArtifactMessagesSerializer.cs Serializes the new kind field.
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/ObjectFieldIds.cs Assigns IPC field ID 7.
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/FileArtifactMessages.cs Adds kind to the wire model.
src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs Propagates session artifact kinds.
src/Platform/Microsoft.Testing.Platform/PublicAPI/PublicAPI.Unshipped.txt Tracks the experimental public API.
src/Platform/Microsoft.Testing.Platform/Messages/FileArtifacts.cs Adds the kind-aware constructor and property.
src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt Tracks IPC internal API changes.
src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Implements TRX merging.
src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxDataConsumer.cs Tags in-process TRX artifacts.
src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt Tracks TRX internal APIs.
src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt Tracks linked IPC field API.
src/Platform/Microsoft.Testing.Extensions.MSBuild/InternalAPI.Unshipped.txt Tracks linked IPC field API.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportMerger.cs Implements JUnit merging.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportGenerator.cs Tags JUnit artifacts.
src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt Tracks JUnit internal APIs.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/InternalAPI/InternalAPI.Unshipped.txt Tracks shared generator API.
src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportGenerator.cs Tags HTML artifacts.
src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt Tracks linked IPC field API.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt Tracks CTRF internal APIs.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportMerger.cs Implements CTRF merging.
src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportGenerator.cs Tags CTRF artifacts.

Review details

  • Files reviewed: 28/28 changed files
  • Comments generated: 4
  • Review effort level: Medium

Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxDataConsumer.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
@Evangelink Evangelink marked this pull request as ready for review July 13, 2026 14:43
Evangelink and others added 2 commits July 13, 2026 16:50
…ure-broccoli

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.HtmlReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt
- Carry microsoft.testing.trx through the out-of-process TRX path by adding an
  (experimental) Kind to FileArtifact and propagating it in DotnetTestDataConsumer
  and TrxProcessLifetimeHandler; centralize the kind in TrxReportEngine.TrxArtifactKind.
- Deduplicate <UnitTest> definitions by id when merging TRX (ids are deterministic
  from the test UID and the schema forbids duplicates).
- Preserve <RunInfos> (crash/exit diagnostics) and <CollectorDataEntries> from every
  input's <ResultSummary> instead of discarding them; omit them when empty.
- Correct the inaccurate 'attachment paths are absolute' remark and relocate each
  input's attachment deployment tree into the merged deployment root in
  MergeToFileAsync (best-effort, never fails the merge).
- Add unit tests for dedup and diagnostics preservation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 13, 2026 15:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 30/30 changed files
  • Comments generated: 11
  • Review effort level: Medium

Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.JUnitReport/JUnitReportMerger.cs Outdated
Comment thread test/UnitTests/Microsoft.Testing.Extensions.UnitTests/JUnitReportMergerTests.cs Outdated
Comment thread test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxProcessLifetimeHandler.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxDataConsumer.cs Outdated
@github-actions

This comment has been minimized.

@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 13, 2026
…ure-broccoli

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.HtmlReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.MSBuild/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt
#	src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt
Copilot AI review requested due to automatic review settings July 13, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 30/30 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.Merge.cs Outdated
- Skip relocation when the merged 'In' root overlaps the source 'In' root (output
  written beside the input with a matching runName), preventing the copy from
  recursing into its own destination; the original hrefs already resolve there.
- Skip file reparse points (symlinks) before File.Copy so a link inside the confined
  tree can't pull in content from outside it.
- Overwrite copied attachment files so a reused output directory can't leave stale
  bytes behind while the merged XML references the fresh per-input destination.
- Add a unit test for the overlapping-root skip case.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 13, 2026 20:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 30/30 changed files
  • Comments generated: 1
  • Review effort level: Medium

// Overwrite so a reused output directory can't leave stale bytes behind while the merged
// XML is rewritten to reference the (per-input isolated) destination, mirroring the
// File.Create overwrite used for the merged TRX itself.
File.Copy(file, Path.Combine(destinationDirectory, Path.GetFileName(file)), overwrite: true);
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9805

42 tests graded across 6 files (39 new, 3 modified). 37 A's and 5 B's. The B-grade tests each verify only a single assertion where the test setup is rich enough to warrant checking a second property; the one modified test (FileArtifactMessagesSerializeDeserialize) omits the new Kind field from its round-trip loop. All new merger tests are well-structured with clear AAA and meaningful multi-field assertions.

GradeTestNotes
B (80–89) mod DotnetTestProtocolSerializerTests.
FileArtifactMessagesSerializeDeserialize
The for loop checks six fields but omits the newly added Kind — consider adding Assert.AreEqual(expected.Kind, actualArtifact.Kind).
B (80–89) new JUnitReportMergerTests.
Merge_
TimestampIsEarliestAcrossInputs
Single AreEqual assertion; consider also asserting the later input timestamp was NOT chosen to make the comparison explicit.
B (80–89) new TrxReportEngineMergeTests.
Merge_
WhenAnyReportFailed_
OutcomeIsFailed
Single assertion on outcome; the rich two-input setup could also verify that counters from both inputs were preserved.
B (80–89) new TrxReportEngineMergeTests.
Merge_
WhenFailedCounterIsPositive_
OutcomeIsFailed
Single AreEqual on outcome; given the nuanced setup (both "Completed" but positive failed counter), consider also asserting the merged counter value.
B (80–89) new TrxReportEngineMergeTests.
Merge_
WithAllCompletedReports_
OutcomeIsCompleted
Single AreEqual on outcome; consider also asserting Counters to confirm passed/total are reflected in the happy path.
A (90–100) new CtrfReportMergerTests.
Merge_
ConcatenatesTests
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
DerivesSummaryCountersFromTests
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
DerivesSummaryFromTests_
WhenInputSummaryMissing
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
GeneratesFreshReportId
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
KeepsReportFormatAndToolFromFirstReport
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
SummaryStartIsEarliestAndStopIsLatest
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
WithNoReports_
ThrowsArgumentException
No issues found.
A (90–100) new CtrfReportMergerTests.
Merge_
WithNullReports_
ThrowsArgumentNullException
No issues found.
A (90–100) new CtrfReportMergerTests.
MergeToFileAsync_
WritesMergedFileToDisk
No issues found.
A (90–100) mod DotnetTestProtocolSerializerTests.
FileArtifactMessages_
RoundTrips
Clear AAA; seven field-level assertions covering both the non-null and null-kind paths. No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
DerivesCountersFromSuitesWhenRootAggregatesMissing
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
SetsProvidedReportName
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
SumsRootCounters
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
SupportsBareTestSuiteRootedInput
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
UnionsSuitesAndReassignsSequentialIds
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
WithNoReports_
ThrowsArgumentException
No issues found.
A (90–100) new JUnitReportMergerTests.
Merge_
WithNullReports_
ThrowsArgumentNullException
No issues found.
A (90–100) new JUnitReportMergerTests.
MergeToFileAsync_
WritesMergedFileToDisk
No issues found.
A (90–100) mod ProtocolEdgeCaseTests.
FileArtifactMessages_
WhenMultipleArtifactsWithMixedNulls_
RoundTrips
Eight field-level assertions across all three artifacts, covering both set and null Kind. No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
DeduplicatesTestDefinitionsById
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
DeduplicatesTestListsById
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
PreservesCollectorDataEntriesFromInputs
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
PreservesResultFileAttachmentPaths
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
PreservesRunInfosFromInputs
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
SetsProvidedRunIdAndName
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
SumsCounters
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
TimesUseEarliestStartAndLatestFinish
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
UnionsResults
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
UnionsTestDefinitionsAndEntries
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
WhenNoDiagnostics_
OmitsEmptyRunInfosAndCollectorDataEntries
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
WithNoReports_
ThrowsArgumentException
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
WithNullReports_
ThrowsArgumentNullException
No issues found.
A (90–100) new TrxReportEngineMergeTests.
Merge_
WithSingleReport_
KeepsAllResults
No issues found.
A (90–100) new TrxReportEngineMergeTests.
MergeToFileAsync_
IsolatesCollidingAttachmentsPerInputAndRewritesHrefs
Integration-style test with six meaningful assertions covering file content and href rewriting. No issues found.
A (90–100) new TrxReportEngineMergeTests.
MergeToFileAsync_
WhenMergedRootOverlapsInputTree_
SkipsRelocationWithoutRecursing
No issues found.
A (90–100) new TrxReportEngineMergeTests.
MergeToFileAsync_
WhenRunNameEscapesOutputDirectory_
SkipsRelocation
No issues found.
A (90–100) new TrxReportEngineMergeTests.
MergeToFileAsync_
WritesMergedFileToDisk
No issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · 142.1 AIC · ⌖ 10.2 AIC · ⊞ 8.9K · [◷]( · )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants