feat: cross-process test report aggregation — merged HTML report + single GitHub summary

[thomhurst]

Closes #4522. Addresses discussion #6408.

Problem

Running many test projects (solution dotnet test, per-microservice dotnet run) spawns one process per suite. Each process emits its own HTML report and its own GitHub step summary block — N projects means N blocks and N artifacts, and failures get buried. Processes can't see each other, so nothing merges.

Key idea: nobody needs to know who finishes last

GITHUB_STEP_SUMMARY is a per-step file that stays freely rewritable until the step ends (the runner reads it once, at step completion). So every finishing process persists its results, takes a cross-process file lock, and re-renders one merged block from all results present so far — the true last finisher naturally leaves the complete aggregate. Last-writer-wins over monotonically-growing input needs no coordination protocol, no daemon, no "am I last?" answer.

What's included

1. JSON sidecar (foundation, on by default)
Every HTML report now gets a machine-readable sibling: {asm}-{os}-{tfm}.tunit-report.json — a faithful, versioned serialization of ReportData (hand-written Utf8JsonWriter/JsonDocument, AOT-safe, forward-compatible reader). Opt out: TUNIT_DISABLE_JSON_REPORT. This is the substrate everything else consumes.

2. Cooperative in-engine aggregation (ON by default where a shared dir is resolvable — GitHub Actions, or explicit TUNIT_AGGREGATE_DIR; escape hatch TUNIT_AGGREGATE_REPORTS=off)
For the single-step/many-processes layout (the #4522 scenario). Each process drops its sidecar into a shared dir (auto-derived on GHA from RUNNER_TEMP + run/attempt/job; TUNIT_AGGREGATE_DIR elsewhere), then under a lock regenerates:

• one merged HTML report (native TUnit styling — the ask in Consolidated HTML Test Report for Solutions with Multiple Test Projects #6408), and
• one summary block, delimited by invisible HTML-comment markers. Only the region between TUnit's own markers is ever rewritten; foreign summary content (other tools, echo >> lines) is never touched. Conservative marker pairing + temp-file-and-File.Replace writes mean even a killed process can't corrupt the file.

3. tunit-report dotnet tool (new package TUnit.Reporting.Tool)
For layouts cooperative merging can't cover — one step per project, or matrix jobs across machines (steps/jobs each get their own summary file, so a later step can't rewrite an earlier block):

TUNIT_AGGREGATE_REPORTS=defer   # suites persist sidecars, write no summary blocks
tunit-report merge -d <dir> [-o merged.html] [--github-summary] [--fail-on-failures]

The tool compiles the engine's own serializer/merger/renderer sources via <Compile Include> links (no MTP dependency, tiny payload), so the tool always reads exactly what the engine writes.

4. Docs — new guide docs/guides/report-aggregation + cross-links from the HTML-report and CI/CD-reporting pages.

Verified

• Unit tests: JSON round-trip, merger (summary sums, class/id collision disambiguation, wall-clock overlap math), splice safety (foreign-content preservation, torn-block cases, idempotency), suite labels, sidecar path mapping. 241/241 green net10.0 + net8.0.
• End-to-end: ran TUnit.UnitTests under net8.0 + net10.0 with simulated GHA env sharing one summary file — got exactly one marked block ("2 suites", 482 tests, per-runtime labels), pre-existing foreign summary content preserved, merged HTML rendered both suites.
• tunit-report merge on crafted sidecars: merged HTML + summary with failure grouping, flaky section, source links, artifact links.
• Docs site builds with no broken links; tool packs (PackAsTool); added to GetPackageProjectsModule and TUnit.slnx.

Design notes

• On by default (per discussion) so it dogfoods in this repo's own pipelines. Known trade-off: TUnit can't distinguish "first suite of a single-step run" from "step 2 of a multi-step pipeline" — multi-step layouts get one cumulative block per step under the default and should set defer (documented). Plain local runs (no resolvable shared dir) silently no-op. RunEngineTestsModule sets off for its child processes — hundreds of short-lived, deliberately-failing suites would otherwise re-merge the job report on every exit.
• Independent code review found two real bugs, both fixed here: the lock-acquisition timeout path threw instead of gracefully skipping the merge, and SourceLinks templates weren't persisted in the sidecar (merged reports would have lost clickable source links). Sidecar round-trip now covers them.
• Aggregation rides on the HTML reporter's data pipeline; TUNIT_DISABLE_HTML_REPORTER disables sidecars and therefore aggregation.
• Rejected alternatives: sidecar daemon/named pipes (orphan + lifetime fragility in CI), MTP solution-level extension point (doesn't exist), orchestrator project (restructure cost + cross-assembly hook issues [BUG] Hooks are ignored if the class in which they are defined lives in a different assembly than the tests themselves. #556/Discovery of hooks from shared assemblies #1496).
• Observed while testing (pre-existing, not addressed here): ReportData.TotalDurationMs came out as ~4ms for a 2.6s run when the session span is present — the merged outputs sidestep it by deriving wall-clock from test timestamps, but the per-suite HTML header may be affected on main too.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 medium · 52 minor

Alerts:
⚠ 53 issues (≤ 0 issues of at least minor severity)

Results:
53 new issues

Category	Results
BestPractice	1 medium
CodeStyle	52 minor

View in Codacy

🟢 Metrics 364 complexity

Metric	Results
Complexity	364

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b71373e417

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thomhurst]

@claude review

[claude]

Claude finished @thomhurst's task in 1m 58s —— View job

---

Claude finished @thomhurst's task

Todo list

• Gather context (PR description, diff stat, existing automated review comments)
• Read changed files in TUnit.Engine/Reporters/Aggregation/**, GitHubReporter.cs, HtmlReporter.cs, TUnit.Reporting.Tool/Program.cs
• Check test coverage in ReportAggregationTests.cs
• Verify the two Codex-flagged issues (malformed schemaVersion, unreadable sidecar in tool)
• Post final review

Review

Overall this is a well-scoped design: no daemon/coordination protocol, atomic writes, and a genuinely simple last-writer-wins model over a monotonically growing input. The lock/splice/merge separation is clean and the "torn marker → append, never splice" rule in GitHubSummaryRegion.Splice is the right conservative default.

Confirmed — both Codex findings are real bugs:

1. ReportDataJson.TryDeserialize can throw on a malformed schemaVersion (ReportDataJson.cs:270-275). JsonElement.GetInt32() throws FormatException (not a JsonException) when the number doesn't fit in int — e.g. a foreign/corrupt sidecar with "schemaVersion": 999999999999999. That's uncaught here and propagates up through ReadAllSidecars (only catches IOException/UnauthorizedAccessException, ReportAggregator.cs:171) and through the tool's LoadSidecars, killing the whole merge over one bad file — contradicting the documented "never throws… must not take down every sibling's merge" contract. Fix: use version.TryGetInt32(out var v) instead of GetInt32(), treating a non-representable value as invalid (return null).
   Tests cover missing/newer schemaVersion (ReportAggregationTests.cs:76-77) but not an out-of-range numeric one, so this gap wasn't caught. Fix this →

2. TUnit.Reporting.Tool's LoadSidecars doesn't guard the per-file read (Program.cs:154). File.ReadAllText(file, ...) isn't wrapped in try/catch, so a locked/deleted-mid-scan/permission-denied sidecar throws out to Main's top-level catch, exiting the whole tool with code 2 — even though line 124's "warning: skipped {skipped} unreadable..." message implies per-file failures are tolerated. In a recursively-scanned CI artifact-download directory this is a realistic failure mode. Fix: wrap the ReadAllText call in a try/catch (IOException or UnauthorizedAccessException) that increments skipped and continues, mirroring ReportAggregator.ReadAllSidecars. Fix this →

Other observations (no action required, noted for awareness):

• ReportDataMerger.Merge (ReportDataMerger.cs), GitHubSummaryRegion.Splice, and AggregatedSummaryWriter.Render all read cleanly — deterministic ordering, correct wall-clock derivation from test timestamps (sidestepping the pre-existing TotalDurationMs session-span issue you flagged), and conservative marker splicing.
• ReportAggregator.AcquireLockAsync's timeout path correctly returns null rather than throwing, and GitHubReporter.TryWriteAggregatedSummaryAsync treats that as "a sibling will produce a fresher merge" rather than falling through to double-write — matches the design note about the previously-fixed lock-timeout bug.
• RunEngineTestsModule setting TUNIT_AGGREGATE_REPORTS=off for its child processes is the right call given hundreds of deliberately-failing short-lived suites.

I did not run the build/test suite (no tool access to dotnet in this review context) — the PR description states 241/241 tests green on net8.0/net10.0, which I have no reason to doubt given the code read cleanly. Both findings above are narrow, single-line fixes; happy to apply them if you'd like.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b1043510d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Code review (re-review after refactor: apply code-review cleanups)

Re-reviewed at 5b1043510, the current head — this is the commit that landed after the previous @claude review pass, described as applying code-review cleanups.

Both previously-flagged bugs are still present

Neither issue from the earlier review was fixed by this commit; the code around them was refactored but the underlying defects carried through unchanged, and ReportAggregationTests.cs wasn't touched, so there's still no regression test for either.

1. schemaVersion overflow still throws, contradicting the "never throws" contract. ReportDataJson.cs#L304-L307
   version.GetInt32() still throws FormatException (not JsonException) for a schemaVersion too large for Int32, uncaught by the surrounding catch (JsonException). This isn't just a TUnit.Reporting.Tool concern — ReportAggregator.ReadAllSidecars (ReportAggregator.cs#L150-L162) only catches IOException/UnauthorizedAccessException, so one corrupt sidecar in the shared directory still kills the in-process engine merge, not only the standalone tool. Fix is unchanged from last review: version.TryGetInt32(out var v), treat failure as invalid.

2. TUnit.Reporting.Tool's LoadSidecars still doesn't guard the per-file read. Program.cs#L153-L155
   The read was changed from File.ReadAllText to File.ReadAllBytes (to support hash-based dedup instead of string dedup — a nice improvement on its own), but it's still unwrapped, so a locked/deleted-mid-scan/permission-denied sidecar still throws out to Main's top-level catch and exits the whole tool with code 2, despite the "warning: skipped N unreadable..." message implying per-file tolerance.

Architectural note on the "single lock cycle" change (not a bug, but worth naming)

HtmlReporter.TryWriteSidecarAndAggregateAsync now drives both the merged-HTML write and the GitHub-summary write inside one lock/scan cycle, calling back into GitHubReporter.WriteAggregatedSummary (HtmlReporter.cs#L199-L226), replacing the old design where GitHubReporter independently re-acquired the lock and rebuilt the summary. That's a legitimate win (one lock acquisition + one sidecar scan per process instead of two), but it also merges what used to be two independent failure domains: if aggregator.WriteMergedHtml(suites) throws, the summary write is never attempted at all, whereas previously a merged-HTML failure and a summary-write failure were unrelated attempts. This happens to match the pre-existing suppression semantics (the old TryWriteAggregatedSummaryAsync catch also suppressed the per-suite fallback on any exception), so it's not a regression — but it does mean summary correctness now transitively depends on unrelated HTML-generation succeeding. Worth a one-line comment at the call site if that coupling is intentional, so a future reader doesn't try to "fix" it by re-splitting the try/catch.

Everything else in this commit looks solid

AtomicFile.cs extraction, ReportDataJson's collapse of eight per-type array readers into a generic ReadArray<T>, ReportSummary.Add/TotalUnsuccessful, and ReportDataMerger.ComputeWallClockBounds (deduping timestamp parsing between per-suite and whole-run duration calculations) are clean, well-scoped refactors — read correctly, no behavior drift found, and the TUnitActivitySource/ClassTimelineAttribute constants-only partials are a genuine improvement over the old hand-mirrored Shims.cs (only exist under #if NET, consistent with the original file already being NET-only).

---

🤖 Generated with Claude Code

[claude]

Code review (re-review after fix: harden sidecar reading against corrupt and unreadable files)

Re-reviewed at f8e61a0, the current head — the commit pushed right after my last review.

Both previously-flagged bugs are now fixed

1. schemaVersion overflow. ReportDataJson.cs#L301-L312 now uses version.TryGetInt32(out var schemaVersion) instead of GetInt32(), so a corrupt/foreign sidecar with a huge or non-integer schemaVersion (e.g. 999999999999 or 1.5) is rejected as an invalid sidecar instead of throwing FormatException past the catch (JsonException) guard. GetInt/GetNullableInt got the same treatment, closing the same hole for total/passed/lineNumber/etc. New test cases at ReportAggregationTests.cs#L78-L79 cover exactly this.
2. Unguarded read in TUnit.Reporting.Tool. Program.cs#L153-L165 now wraps File.ReadAllBytes in try/catch (IOException or UnauthorizedAccessException), incrementing skipped and continuing — matching ReportAggregator.ReadAllSidecars's tolerance and the tool's own "warning: skipped N unreadable..." messaging instead of exiting the whole tool with code 2 on one bad file.

Both fixes are narrow, correctly scoped (the ValueKind == JsonValueKind.Number check still precedes TryGetInt32, so short-circuiting is preserved), and backed by new regression tests. I don't see any new issues introduced by this commit.

Still-open minor note (non-blocking, carried from last review)

The "single lock cycle" coupling in HtmlReporter.TryWriteSidecarAndAggregateAsync — summary write now transitively depends on WriteMergedHtml not throwing — got an explanatory comment about why both writes share one lock cycle, but not one calling out that this also merges their failure domains. Not a regression (matches pre-existing suppression semantics) and not something I'd block on; still worth a one-liner if you want to head off a future "helpful" re-split of the try/catch.

Overall

This addresses the last remaining review findings cleanly. I did not run the build/test suite (no matching .NET SDK — repo pins 10.0.302 via global.json, only 10.0.301/10.0.204 etc. available here); the PR description's 241/241 green claim is consistent with what I'd expect from a change this targeted.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8e61a0d71

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00bd7bb18b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Code review (re-review of 00bd7bb, "fix: address review findings on merge fidelity and sidecar robustness")

Reviewed the commit pushed after my last two reviews (5b10435 → f8e61a0), which addressed the previously-flagged schemaVersion overflow and unguarded-read issues. This new commit is scoped to three things: malformed-shape sidecar rejection, per-class-span retagging on merge, and source-control-metadata agreement checks.

Verified correct

• IsMalformedSidecar (ReportDataJson.cs#L299-L306) correctly widens the catch to InvalidOperationException (wrong-shaped nested JSON, e.g. "groups":[1]) alongside JsonException. New test cases at ReportAggregationTests.cs (groups: [1], deeper nested wrong-shape) cover this.
• GetDouble now checks TryGetDouble + IsNaN/IsInfinity, so 1e9999 degrades to 0 instead of poisoning downstream duration math — matches the TryGetInt32 pattern already used for schemaVersion.
• SingleDistinctOrNull / SingleDistinctSourceLinksOrNull (ReportDataMerger.cs#L292-L338) correctly replace FirstNonEmpty: commit/branch/PR/repo/source-link metadata now only survives a merge when every suite that has a value agrees, instead of silently taking the first suite's value. SourceLinkTemplates being a record makes the != comparison in SingleDistinctSourceLinksOrNull a real value comparison, not reference equality — correct. Well covered by Merge_Keeps_SourceControl_Metadata_Only_When_Suites_Agree.
• SpanData → record and the RetagClassSpan with-expression usage is clean and correctly #if NET-gated to match where Spans is populated at all.

Worth flagging: the retag likely doesn't fire for namespaced classes

RetagClassSpan matches renamedClasses (keyed by ReportTestGroup.ClassName) against the span's tunit.test.class tag value. But these two are populated from different sources and use different formats:

• ReportTestGroup.ClassName comes from TestMethodIdentifierProperty.TypeName, which is built by TestContextExtensions.GetClassTypeName() (TestContextExtensions.cs#L9-L20) — this deliberately excludes the namespace (type.Name, optionally with a +-joined nested-type prefix).
• The tunit.test.class span tag is set from testDetails.ClassType.FullName (e.g. ObjectLifecycleService.cs:335), which includes the namespace.

So for any test class in a namespace (i.e. virtually all real projects), renamedClasses.TryGetValue(tags[i].Value, ...) in ReportDataMerger.cs#L128-L142 will miss, because it's looking up "MyNamespace.MyTests" in a dictionary keyed by "MyTests". This isn't a regression from this commit — HtmlReportGenerator.WriteTimelines's suiteByClass/classModes join (used for single-process reports too) has the same ClassName-vs-FullName mismatch, so [ClassTimeline] likely doesn't render for any namespaced class today, merged or not. But it does mean the new retag logic this commit adds is fixing a merge-only symptom of a join that's already broken upstream — worth a quick manual check with a real namespaced [ClassTimeline] class before calling this "survives multi-TFM merges," since the existing unit/integration tests only use span tags fed directly to ReportDataMerger.Merge (bypassing the real ClassType.FullName population path) and so wouldn't catch this.

Suggest normalizing the join key: either populate the tunit.test.class tag with the same simple-name format GetClassTypeName() produces, or have HtmlReportGenerator/ReportDataMerger key off FullName consistently on both sides.

Everything else

No other issues found in this commit. CLAUDE.md-relevant concerns (AOT safety, allocation discipline) look fine — renamedClasses is allocated lazily only when a rename is actually needed, and the generic ReadArray<T> / record patterns don't introduce reflection.

🤖 Generated with Claude Code