Repository navigation
[rel/4.5] Preserve GitHub Actions report commands under dotnet test - #12055
Amaury Levé (Evangelink) merged 1 commit into
Conversation
Backport #12046 so marked GitHub Actions workflow commands use the existing informational display-message transport instead of being suppressed as ordinary host output. Preserve protocol gates, summaries and exit codes. Adapt the release test fixtures without bringing main-only coverage mode or unrelated annotation execution tests. (cherry picked from commit 160aafb) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 784fab32-6b58-4d5b-907a-6ec7bd276612
There was a problem hiding this comment.
🧪 Test quality review — PR #12055
🧵 Parallel safety
STATUS: PARTIAL — the pre-step inputs for the parallel-safety specialist were not available in this run, so parallel-safety coverage was not completed. This is not a clean result.
Per-test grades
Only a light review of the changed unit tests was done (annotation reporter, slow-test reporter, passthrough output device). The full grading rubric and per-test scorecard were not run. No inline suggestions were posted.
- The new assertions are exact (full annotation strings, output data type, cancellation token), and the cases are data-driven.
GitHubActionsSlowTestReporterTests: the multi-lineAreSequenceEqualchecks are deterministic.DotnetTestPassthroughOutputDeviceTestsuses a strict mock to prove non-marker output is swallowed.
No correctness or reliability defects were observed in this pass.
This advisory review is heuristic and non-blocking. Re-run the combined review with /review-tests, or only the parallel-safety specialist with /parallel-audit.
🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · 22.4 AIC · ⌖ 0.706 AIC · ⊞ 18K · ◷
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Summary
Medium-low risk. The new internal GitHubActionsCommandOutputDeviceData : TextOutputDeviceData marks GitHub Actions commands, and DotnetTestPassthroughOutputDevice forwards them as informational DisplayMessage. This mirrors the existing Azure DevOps marker pattern. Forwarding is gated on protocol 1.3.0 inside ForwardDisplayMessageAsync, so older SDKs never receive an unknown message id.
Findings
-
Scope: unrelated
.mcp.jsondeletion (non-blocking). The PR deletes the repo-root.mcp.json(the markitdown, azure-devops, github and microsoftdocs MCP server configs). Nothing in the PR title, the description of the fix, or the other files in this diff relates to it. That conflicts with the single-concern principle, and a servicingrel/4.5branch is an odd place for it. If the removal is intentional, split it out or explain it in the PR description. If not, restore the file. -
Behavior check, non-pipe path (no action needed). In the terminal and simplified console devices,
TextOutputDeviceDatafalls through toWriteMessage(text)instead of theFormattedTextOutputDeviceDatacase. I checkedTerminalOutputDevice.DataConsumption.csandSimplifiedConsoleOutputDeviceBase.TestEvents.cs. The only difference is that the default color and padding are no longer passed, so rendering should be equivalent. Tests were added in the PR. -
Minor: the annotation reporter prepends
"\n"to each line, so the forwarded display message carries that leading newline too. The protocol doc says the text arrives "unchanged", so it would help to state this in the doc or in a test. I did not verify how the SDK renders the leading blank line.
Dimension coverage
- Public API: the new type is internal and is listed in
InternalAPI.Unshipped.txt. No public surface change. - Backward compatibility and IPC: no wire format change. Version gating is preserved.
- Localization: no
.resxor.xlfchanges. - Tests: unit and acceptance tests cover the passthrough, reporter and extension paths.
- Docs: the protocol doc and
PACKAGE.mdare updated consistently. - Threading and security: no new shared state, and the forwarded text is generated by the extension.
I did not build or run the tests.
Summary
Backports #12046, original merge
160aafb007cd57e54fd4d9687123f005ca042d6d, torel/4.5. Related issue: #11996.GitHub Actions groups, failure/skip annotations, run-level annotations and slow-test notices were ordinary formatted output, which the existing
dotnet testforwarding device deliberately suppresses. Mark those commands explicitly and forward their unchanged text through the existing informationalDisplayMessagechannel. Preserve ordinary informational suppression, startup diagnostic fallback, standalone rendering, JSON discovery, feature gates, summaries and actual exit codes.Servicing rationale and release adaptations
This is an existing-feature fix:
rel/4.5already ships the GitHub reporting package, passthrough output device and protocol 1.3.0 display-message transport. No public API, dependency, resource, SDK/TFM upgrade, wire type, protocol bump or main-only JSON-RPC work is included. The original PR records the design alternatives; this backport was validated independently with release binaries rather than borrowing its main results.Forwarding requires a receiver negotiating protocol 1.3.0+. SDK 10.0.402 remains unsupported for workflow-command forwarding and receives no unknown frames; its independent job summary still works. The package README documents this limit.
Two release-test adaptations were necessary: the acceptance fixture does not contain main's unrelated
coveragemode, so only the required pass/discovery/slow behavior is retained; the annotation unit suite lacks main's execution fixture, so only the four marker-relevant execution tests and their helper were brought over, preserving all existing release tests. Production implementation matches the source fix; internal API tracking and related documentation are retained.Validation
125 distinct cases passed, no skips: 14 platform units, 52 GitHub producer units and 59 related packed acceptance cases. The 20 new regressions are included in the 59 and were also run separately against both package variants.
Validated source tree
4dfd1608e7b0cae0e25640c9c10ff8d0b71ffdd0was committed unchanged as624d39477f63715e0ae102e0b46e3a247b5ea4d5. Baseline packages were built from pristine release commita223d46d5784d315e7bae2a04d85185b191dd2b3. Immediately before publication, liverel/4.5was refreshed to851ffb1578b10fb7c79e6e96245a662481a43e56; its two intervening commits affect only the separate server-client unit-test project. The backport retains a 16-file PR diff and merges cleanly with that tip.Windows x64, release-pinned SDK 12.0.100-alpha.1.26480.103, Release configuration. Both shipping packages are 2.5.2-dev, built for netstandard2.0/net8.0/net9.0; packed consumers exercise net8.0/net10.0/net462. The unchanged outer acceptance project targets net11.0 and uses its repository-configured SDK runtime. Builds used two MSBuild workers, a worktree-owned package cache and a short process-only PATH. Each acceptance run used the existing fresh fixture cache; no sibling packages were consumed. Existing method-level scheduling was preserved, with fresh mutable assets for each SDK test.
Independent discovery exactly matched executed TRX UIDs. JUnit XML and CTRF JSON counts/outcomes matched. Each of the baseline, fixed-regression and fixed-related runs verified 14 generated modules / 28 paired platform+reporter DLLs against its selected NuGet payload SHA-256s. Matching version strings alone were not used as provenance.
Commands below use
$repofor this worktree,$root = "$repo\artifacts\gh12046",$dotnet = "$repo\.dotnet\dotnet.exe"and$evidencefor the task-owned evidence folder. The two external solution filters select the baseline products and the focused hosts plus packages required by the acceptance fixture/pack validator; they are retained with the logs, not added as release source files.The real SDK acceptance tests exercise project, solution and bare invocation, passing and failing/skipped/zero-result modules, slow notices and mixed-module aggregation. An additional direct-module probe ran the same verified net10.0 consumer through the pinned SDK and SDK 10.0.402, using a separate SDK-selection directory for the latter:
The pinned SDK emitted these complete stdout lines; SDK 10 emitted neither:
Test-value evidence
Counterfactual: pristine release shipping packages, with the identical new test/fixture bytes retained. All 13 red outcomes were verified as empty-command-sequence/notice assertions, not compilation, launch, discovery, zero-execution or skip failures. The unchanged suppression/legacy/JSON cases passed in both states. This evidence applies to the exact backport commit above.
DotnetTest_PassingModule_ForwardsUnprefixedGitHubGroupsDotnetTest_FailingAndZeroTestModules_PreserveAnnotationsAndExitCodesDotnetTest_SlowTest_ForwardsUnprefixedNoticeDotnetTest_MixedModules_PreservesEachModulesCommandsAndSummaryDotnetTestPipe_GitHubCommands_AreForwardedWithCorrelationLive CI and GitHub runner UI rendering were not run locally; the full repository suite and non-Windows execution were not run. Local binlogs, reports, exact package hashes, consumer provenance manifests and replay layouts are retained as task-owned evidence, not presented as uploaded CI artifacts. No unresolved build or composed-product validation blocker remains.