Uh oh!
There was an error while loading. Please reload this page.
Share terminal retry counting model + port SDK oracle test (4/n) - #9267
Merged
Amaury Levé (Evangelink) merged 7 commits intoJun 20, 2026
Conversation
First slice of unifying the shared MTP terminal reporter with the dotnet/sdk orchestrator fork, validated by porting the SDK's own reporter test as the expected-UI oracle. Adds the orchestrator handshake surface to the shared reporter (additive; the in-process host never reports handshake failures, so output stays byte-identical): - HandshakeFailure(assemblyPath, tfm, exitCode, output, error, reportEvenWhenHelp) + HasHandshakeFailure + end-of-run AppendHandshakeFailureRecap. - Rich AssemblyRunCompleted(executionId, exitCode, outputData, errorData) overload that surfaces an unknown execution id as a handshake failure (no throw) and prints the executable summary (exit code + stdout/stderr) on non-zero exit. - TerminalTestReporter(IConsole, options) ctor overload for orchestrator callers that drive cancellation out-of-band via StartCancelling. - TestProgressState.Success; string-based AppendAssemblyLinkTargetFrameworkAndArchitecture overload; AppendExecutableSummary; new TerminalResources HandshakeFailuresHeader/ExitCode (resx + xlf regenerated via UpdateXlf, hand-maintained !IS_CORE_MTP accessors updated). Ports the SDK's AssemblyRunCompleted_WhenExecutionIdUnknown test (dotnet/sdk#51608) to MSTest, asserting both HasHandshakeFailure and the rendered failure context. Verified: platform clean net8.0/net9.0/netstandard2.0 (0 warnings incl. IDE/SA); full Microsoft.Testing.Platform.UnitTests suite green (1199, 0 failed); standalone TerminalReporterContract consumer still compiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Second slice of unifying the shared MTP terminal reporter with the dotnet/sdk orchestrator fork, validated by porting the SDK's per-assembly-counts test. Adds the orchestrator per-assembly summary rendering (additive; gated on the new ShowAssembly options which default off, so the in-process host is unaffected and its output stays byte-identical): - ShowAssembly + ShowAssemblyStartAndComplete options. - Rich AssemblyRunCompleted now prints the per-assembly summary line (link + result + compact counts + duration) when ShowAssembly && ShowAssemblyStartAndComplete. - AppendAssemblySummary/AppendAssemblyResult/AppendAssemblyTestCounts/AppendGlyphCount porting the compact '[+P/xF/?S]' (ASCII) / '[checkP/xF/downS]' (ANSI) counts block, with the '/rN' retry segment wired to TestProgressState.RetriedFailedTests. - Orchestrator TestCompleted overload (assembly, tfm, arch, executionId, instanceId, ...) that updates per-assembly counts (delegates to the shared per-execution-id path; instanceId-based retry attribution arrives in the retry slice). - New TerminalResources.FailedWithErrors (resx + xlf + !IS_CORE_MTP accessor). Ports the SDK's AssemblyRunCompleted_WithShowAssemblyStartAndComplete_PrintsPerAssemblyCounts test (dotnet/sdk#52128) to MSTest, asserting the rendered '[+3/x0/?1]' counts block. Stacked on #9263. Verified: platform clean net8.0/net9.0/netstandard2.0 (0 warnings); full unit suite green (1200, 0 failed); standalone contract consumer compiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…3/n) Third slice of unifying the shared MTP terminal reporter with the dotnet/sdk orchestrator fork. Adds the per-assembly listing to the FINAL run summary: when ShowAssembly is set and more than one assembly ran, each assembly is listed under the run-level verdict with its own result + compact counts. Additive — the in-process host leaves ShowAssembly off, so its summary stays byte-identical. Ports the SDK's TestExecutionCompleted_WithMultipleAssemblies_PrintsPerAssemblyCountsInSummary test (dotnet/sdk#52128) to MSTest, asserting the rendered '[+2/x1/?0]' and '[+5/x0/?2]' per-assembly count blocks. Stacked on #9265. Verified: platform clean net8.0/net9.0/netstandard2.0 (0 warnings); full unit suite green (1201, 0 failed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fourth slice of unifying the shared MTP terminal reporter with the dotnet/sdk
orchestrator fork: adopt the instanceId-based retry counting model so retried
assemblies render the '/r{N}' segment and report the latest attempt's results.
- TestProgressState now derives PassedTests/FailedTests/SkippedTests from per-test
reported results keyed by (testNodeUid, attempt). TotalTests is computed. A new
instance id (registered via NotifyHandshake on AssemblyRunStarted) is a retry
attempt that supersedes the previous attempt's tally and bumps RetriedFailedTests.
Adds ReportPassingTest/ReportSkippedTest/ReportFailedTest, NotifyHandshake, TryCount.
- TestCompleted now threads the instance id (the in-process host uses its single
fixed execution id as the instance id; the orchestrator overload passes the real
one) and routes through the Report* methods.
- AssemblyRunStarted registers the attempt via NotifyHandshake.
- Updated the SilenceDrivenHeartbeatRenderer test helper to seed counts via Report*.
Ports the SDK's AssemblyRunCompleted_WhenTestsWereRetried_ShowsRetriedCount test,
asserting the rendered '[+1/x0/?0/r1]' block.
Stacked on #9266. Verified: platform clean net8.0/net9.0/netstandard2.0 (0 warnings);
full unit suite green (1202, 0 failed) — the ~89 byte-exact in-process tests confirm
the single-attempt counts are unchanged; contract consumer compiles.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Levé (Evangelink)
left a comment
MemberAuthor
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
PR #9267 — Share terminal retry counting model + port SDK oracle test (4/n)
| # | Dimension | Verdict |
|---|---|---|
| 2 | Threading & Concurrency | 🟡 1 MODERATE |
| 13 | Test Completeness | 🟡 1 MODERATE |
| 17 | Documentation Accuracy | 🔵 1 NIT |
✅ 19/22 dimensions clean.
- Threading —
_testUidToResults(Dictionary) and_orderedInstanceIds(List) inTestProgressStateare non-thread-safe but undocumented as single-threaded-only. The current architecture (single pipeline consumer per assembly) prevents actual races; the ask is a doc comment to make the invariant auditable and prevent future regressions. - Test Completeness — the retry oracle test covers 2 attempts; the 3-attempt (fail→fail→pass) accumulation of
RetriedFailedTestsis untested.TryCountis also never directly asserted. - NIT —
instanceId: executionIdin the in-processTestCompletedoverload relies on the caller passing the same value toAssemblyRunStarted; that pairing is undocumented.
Algorithmic correctness (✅)
The retry-deduplication state machine in ReportGenericTestResult is correct. Traced all four transitions:
| Previous attempt | Current attempt | RetriedFailedTests change | Counter change |
|---|---|---|---|
| (none) | any | +0 | +1 in new category |
| Fail | Pass | +prev.Failed (+1) | −1 fail, +1 pass |
| Fail | Fail | +prev.Failed (+1) | net 0 (old subtracted, new added) |
| Pass | Pass | +0 | net 0 |
TotalTests stays invariant across retries (correct: same test, new attempt). RetriedFailedTests accumulates only value.Failed so a pass→pass "retry" (degenerate case) correctly contributes 0.
TotalTests refactor (✅)
Removing asm.TotalTests++ from TestDiscovered in TerminalTestReporter.Summary.cs is safe: in discovery mode TotalTests is now DiscoveredTests (incremented via DiscoveredTests++); in execution mode it is PassedTests + SkippedTests + FailedTests. Both are correct.
Performance (✅)
_orderedInstanceIds.IndexOf is O(N) where N = retry count (almost always ≤ 3). _testUidToResults is O(1) lookup. The static lambdas in ReportPassingTest/ReportSkippedTest/ReportFailedTest are compiler-cached (no per-call delegate allocation). No concerns.
Public API surface (✅)
All new and modified types are internal sealed; no PublicAPI.Unshipped.txt changes needed.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Document the single-consumer threading invariant on TestProgressState (its members are unsynchronized because the message pipeline delivers one executionId's events serially). - Document the in-process host instanceId == executionId contract at the AssemblyRunStarted call site, mirroring the note on the in-process TestCompleted overload. - Add a TestProgressState unit test asserting TryCount: a new instance id is a new attempt, re-seeing the current attempt is a no-op, and three distinct attempts accumulate to 3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds AssemblyRunCompleted_WhenTestsWereRetriedTwice_AccumulatesRetriedCount covering the fail->fail->pass path so RetriedFailedTests must accumulate (rendered as /r2), guarding against a regression that resets rather than accumulates the retry count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Base automatically changed from
copilot/share-terminal-summary-per-assembly to
mainJune 20, 2026 08:53
…l-retry-model # Conflicts: # src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Handshake.cs # src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs # src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs # src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporterOptions.cs # src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs # test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs
Contributor
There was a problem hiding this comment.
Pull request overview
This PR (slice 4/n of the terminal reporter unification work) ports the dotnet/sdk orchestrator’s instanceId-based retry counting model into the shared Microsoft.Testing.Platform terminal reporter, so retried tests are de-duplicated and the per-assembly counts render the /r{N} retry segment.
Changes:
- Reworks
TestProgressStateto derive pass/fail/skip totals from per-test results keyed by(testNodeUid, attempt)and to track retry attempts viainstanceIdhandshakes (includingTryCountandRetriedFailedTests). - Threads
instanceIdthroughTerminalTestReporter.TestCompletedand registers attempts viaAssemblyRunStarted(..., instanceId)so retries supersede earlier attempt tallies instead of double-counting. - Ports the SDK oracle test asserting retry rendering (
[+1/x0/?0/r1]) and updates heartbeat-renderer tests to seed counts via the new reporting APIs.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs | Adds the ported SDK oracle test for retry rendering and extends ExpectedCounts to include /r{N}. |
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/SilenceDrivenHeartbeatRendererTests.cs | Updates the test helper to seed progress counts via NotifyHandshake + Report*Test methods (since counters are no longer directly settable). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.SessionLifecycle.cs | Ensures in-process host registers an instanceId consistent with the in-process execution id to satisfy attempt lookup. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs | Introduces per-test/per-attempt result tracking, retry attempt registration, computed TotalTests, and retry attribution (RetriedFailedTests, TryCount). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.TestCompletion.cs | Routes counting through TestProgressState.Report*Test and forwards the instanceId (including mapping in-process instanceId == executionId). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs | Adjusts discovery counting to rely on computed TotalTests (derived from DiscoveredTests in discovery mode). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs | Registers retry attempts by calling NotifyHandshake(instanceId) on every (re-)start for a given execution id. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 1
Comment on lines
+149
to
+159
| private int GetAttemptNumberFromInstanceId(string instanceId) | ||
| { | ||
| int index = _orderedInstanceIds.IndexOf(instanceId); | ||
| if (index < 0) | ||
| { | ||
| throw ApplicationStateGuard.Unreachable(); | ||
| } | ||
| // Attempt numbers are 1-based. | ||
| return index + 1; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink) added a commit
that referenced
this pull request
Jun 20, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 freeto 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.
What
Slice 4/n (the high-risk one) of the test-first terminal-reporter superset port: adopt the SDK's instanceId-based retry counting model so retried assemblies render the
/r{N}segment and the summary reflects the latest attempt's results.Stacked on #9266. This completes the four reporter behaviors the
dotnet/sdkorchestrator relies on (handshake, per-assembly counts, multi-assembly summary, retry).How
TestProgressStatenow derivesPassedTests/FailedTests/SkippedTestsfrom per-test reported results keyed by(testNodeUid, attempt), andTotalTestsis computed. A new instance id (registered viaNotifyHandshakeonAssemblyRunStarted) is a retry attempt that supersedes the previous attempt's tally and bumpsRetriedFailedTests. AddsReportPassingTest/ReportSkippedTest/ReportFailedTest,NotifyHandshake,TryCount.TestCompletedthreads the instance id — the in-process host uses its single fixed execution id as the instance id (one attempt), the orchestrator overload passes the real one — and routes counting through theReport*methods.AssemblyRunStartedregisters the attempt viaNotifyHandshake.SilenceDrivenHeartbeatRenderertest helper to seed counts viaReport*(the counters are no longer directly settable).Oracle test ported
AssemblyRunCompleted_WhenTestsWereRetried_ShowsRetriedCount— ported from the SDK'sTerminalTestReporterTests: attempt 1 failsflaky-1, attempt 2 (new instance id, same execution id) passes it, so the final tally is[+1/x0/?0/r1].Why it's safe for the in-process host
The in-process path reports each test once under a single fixed instance id, so the retry model collapses to the same single-attempt counts. The ~89 byte-exact in-process tests are the proof.
Verification
Microsoft.Testing.Platform.UnitTestssuite green: 1202 total, 0 failed (incl. all 4 ported SDK oracle tests + the byte-exact in-process tests).TerminalReporterContractconsumer compiles.