Uh oh!
There was an error while loading. Please reload this page.
Share terminal handshake-failure surface + port SDK oracle test (1/n) - #9263
Merged
Amaury Levé (Evangelink) merged 2 commits intoJun 20, 2026
Merged
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the shared Microsoft.Testing.Platform terminal reporter with the first set of dotnet test orchestrator-only surface area for handshake failures and executable exit summaries, and ports an SDK “oracle” test to lock the UI behavior in testfx.
Changes:
- Add handshake-failure tracking/rendering (incl. end-of-run recap) and incorporate handshake failures into the overall run-failed verdict.
- Add an orchestrator-focused
AssemblyRunCompleted(executionId, exitCode, outputData, errorData)overload that avoids throwing on unknown execution IDs and prints an executable summary on non-zero exits. - Port an SDK regression test to validate the “unknown execution id” handshake-failure UI output.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs | Adds SDK-ported regression test asserting “unknown execution id” doesn’t throw and reports a handshake failure. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressState.cs | Adds Success flag for orchestrator-set completion success state. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Summary.cs | Marks run failed if any handshake failure occurred; appends a handshake-failure recap at end of summary. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Lifecycle.cs | Adds orchestrator overload of AssemblyRunCompleted with exit code/stdout/stderr; clears handshake-failure state on session end. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Handshake.cs | Introduces handshake failure recording, recap rendering, and executable-summary rendering helpers. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.Formatting.cs | Adds a string-based AppendAssemblyLinkTargetFrameworkAndArchitecture overload used by handshake-failure paths. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs | Adds constructor overload for orchestrator callers that drive cancellation out-of-band via StartCancelling. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalResources.resx | Adds new localized string keys for HandshakeFailuresHeader and ExitCode. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalResources.cs | Adds strongly-typed resource accessors for the new terminal resource keys. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.cs.xlf | Regenerates XLF with new resource keys for Czech. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.de.xlf | Regenerates XLF with new resource keys for German. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.es.xlf | Regenerates XLF with new resource keys for Spanish. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.fr.xlf | Regenerates XLF with new resource keys for French. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.it.xlf | Regenerates XLF with new resource keys for Italian. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.ja.xlf | Regenerates XLF with new resource keys for Japanese. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.ko.xlf | Regenerates XLF with new resource keys for Korean. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.pl.xlf | Regenerates XLF with new resource keys for Polish. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.pt-BR.xlf | Regenerates XLF with new resource keys for Portuguese (Brazil). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.ru.xlf | Regenerates XLF with new resource keys for Russian. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.tr.xlf | Regenerates XLF with new resource keys for Turkish. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.zh-Hans.xlf | Regenerates XLF with new resource keys for Chinese (Simplified). |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/xlf/TerminalResources.zh-Hant.xlf | Regenerates XLF with new resource keys for Chinese (Traditional). |
Copilot's findings
- Files reviewed: 22/22 changed files
- Comments generated: 5
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
commented
Jun 19, 2026
MemberAuthor
🧪 Test quality grade — PR #9263
This advisory comment was generated automatically. Grades are heuristic
|
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.
| # | Dimension | Verdict |
|---|---|---|
| 2 | Threading & Concurrency | 🔴 1 BLOCKING |
| 13 | Test Completeness & Coverage | 🟡 1 MODERATE |
✅ 20/22 dimensions clean. (Dimensions 14, 18, 19, 20, 22 are N/A for this change set.)
- Threading —
_handshakeFailuresCount = 0plain write inTestExecutionCompletedis unsynchronized with theInterlocked.IncrementinHandshakeFailure. A process-exit thread that racesTestExecutionCompletedat the session boundary can leave_handshakeFailurespopulated while the counter reads zero, makingHasHandshakeFailurereturnfalseand suppressing the orchestrator's non-zero exit. Fix:Interlocked.Exchange(ref _handshakeFailuresCount, 0)inside (or alongside) the_handshakeFailuresLockblock. - Test coverage — the ported oracle test validates the immediate inline failure output and
HasHandshakeFailure, but neitherAppendHandshakeFailureRecap(end-of-run re-print) nor therunFailed = ... || HasHandshakeFailuresummary-verdict path is exercised. Low risk given the explicit 1/n scope, but the recap is the primary UX surface — worth a follow-up test.
Overall assessment: The design is clean and the additive scope is well-controlled. The BLOCKING item is a one-line fix (Interlocked.Exchange); everything else — the overloaded constructor, the HandshakeFailure/HasHandshakeFailure pair, the XLF regeneration, the resource additions, and the ported oracle test — is correct.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jun 19, 2026
…cycle test - Eliminate the separate _handshakeFailuresCount (which was Interlocked-incremented but reset with a plain write outside the lock, so HasHandshakeFailure could disagree with the list under concurrency). HasHandshakeFailure now reads _handshakeFailures.Count under _handshakeFailuresLock, so the flag and the list are always observed consistently. [BLOCKING fix] - Skip the assembly link (and its trailing space) in HandshakeFailure when the assembly path is empty (the defensive unknown-execution-id path), so we don't render a blank, unactionable prefix. - Fix 'hand-shake'/'hand-shakes' doc typos -> 'handshake(s)'. - Add AssemblyRunCompleted_WhenExecutionIdUnknown_SummaryReprintsRecapAndReportsFailure covering the full lifecycle: the end-of-run recap re-print and the runFailed |= HasHandshakeFailure verdict path, plus the post-completion reset. Verified: platform clean net8.0/net9.0/netstandard2.0 (0 warnings); full unit suite green (1200, 0 failed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
deleted the
copilot/share-terminal-handshake-surface
branch
June 20, 2026 07:59
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
First slice of unifying the shared Microsoft.Testing.Platform terminal reporter with the
dotnet/sdkdotnet testorchestrator fork — driven and validated by porting the SDK's own reporter test as the expected-UI oracle, per the idea of bringing the SDK's UI-state assertions into testfx so the superset can be built test-first.Follows the terminal-sharing effort (#9246 → #9256). The SDK calls orchestrator-only methods the shared reporter lacked; this adds the first, cleanly-additive group.
Added (all additive — the in-process host never reports handshake failures, so its output stays byte-identical)
HandshakeFailure(assemblyPath, tfm, exitCode, output, error, reportEvenWhenHelp)+HasHandshakeFailure+ an end-of-runAppendHandshakeFailureRecap(no-op when there are none).AssemblyRunCompleted(executionId, exitCode, outputData, errorData)overload that surfaces an unknown execution id as a handshake failure (instead of throwingKeyNotFoundException) and prints the executable summary (exit code + stdout/stderr) on a non-zero exit.TerminalTestReporter(IConsole, options)ctor overload for orchestrator callers that drive cancellation out-of-band viaStartCancelling.TestProgressState.Success; a string-basedAppendAssemblyLinkTargetFrameworkAndArchitectureoverload;AppendExecutableSummary.TerminalResourceskeysHandshakeFailuresHeader/ExitCode(resx + 13 xlf regenerated viaUpdateXlf; hand-maintained!IS_CORE_MTPaccessors updated).Oracle test ported
AssemblyRunCompleted_WhenExecutionIdUnknown_DoesNotThrowAndReportsHandshakeFailure— ported from the SDK'sTerminalTestReporterTests(regression for dotnet/sdk#51608), translated to MSTest + the testfxStringBuilderConsoleharness. It asserts bothHasHandshakeFailureand the rendered failure UI (Zero tests ran,Exit code: 1, the capturedstdout/stderr).Scope
This is 1/n. The remaining SDK orchestrator surface — per-assembly counts in the summary (
ShowAssembly), the instanceId-based retry counting model, and the unifiedTestCompleted/TestInProgress/TestDiscoveredsignatures — lands in follow-ups, each validated by porting the matching SDK test (AssemblyRunCompleted_WithShowAssemblyStartAndComplete_PrintsPerAssemblyCounts,TestExecutionCompleted_WithMultipleAssemblies_...,AssemblyRunCompleted_WhenTestsWereRetried_ShowsRetriedCount).Verification
Microsoft.Testing.Platform.UnitTestssuite green: 1199 total, 0 failed (the ~89 byte-exact in-process tests confirm unchanged in-process UI).TerminalReporterContractconsumer still compiles in isolation.