Surface exit-code verdict and per-assembly clarity in GitHubActionsReport#9974
Conversation
Addresses feedback on issue #9003 for the GitHub Actions report extension: - Map MTP exit codes (zero tests, --minimum-expected-tests, aborted, --maximum-failed-tests, adapter/host failures) to friendly reasons. - Step summary now flips to failure icon and renders an exit-code callout when the run failed without any failed test. - Annotation reporter emits a run-level ::error for non-test-result failures so they surface in the Annotations tab. - Each assembly's step-summary section carries its own verdict for multi-assembly (dotnet test) clarity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
There was a problem hiding this comment.
Pull request overview
Adds exit-code-aware verdicts to GitHub Actions summaries and annotations, including clearer per-assembly reporting.
Changes:
- Maps MTP exit codes to localized failure reasons.
- Adds run-level summary callouts and annotations.
- Updates documentation, API tracking, and tests.
Show a summary per file
| File | Description |
|---|---|
GitHubActionsSummaryReporter.cs |
Adds exit-code verdicts and warnings. |
GitHubActionsAnnotationReporter.cs |
Emits run-level failure annotations. |
GitHubActionsExitCode.cs |
Maps exit codes to names and reasons. |
GitHubActionsExtensions.cs |
Injects exit-code service and registers lifecycle handling. |
InternalAPI/InternalAPI.Unshipped.txt |
Tracks changed internal APIs. |
PACKAGE.md |
Documents exit-code and multi-assembly behavior. |
Resources/GitHubActionsResources.resx |
Adds exit-code messages. |
Resources/xlf/GitHubActionsResources.cs.xlf |
Adds Czech localization entries. |
Resources/xlf/GitHubActionsResources.de.xlf |
Adds German localization entries. |
Resources/xlf/GitHubActionsResources.es.xlf |
Adds Spanish localization entries. |
Resources/xlf/GitHubActionsResources.fr.xlf |
Adds French localization entries. |
Resources/xlf/GitHubActionsResources.it.xlf |
Adds Italian localization entries. |
Resources/xlf/GitHubActionsResources.ja.xlf |
Adds Japanese localization entries. |
Resources/xlf/GitHubActionsResources.ko.xlf |
Adds Korean localization entries. |
Resources/xlf/GitHubActionsResources.pl.xlf |
Adds Polish localization entries. |
Resources/xlf/GitHubActionsResources.pt-BR.xlf |
Adds Portuguese localization entries. |
Resources/xlf/GitHubActionsResources.ru.xlf |
Adds Russian localization entries. |
Resources/xlf/GitHubActionsResources.tr.xlf |
Adds Turkish localization entries. |
Resources/xlf/GitHubActionsResources.zh-Hans.xlf |
Adds Simplified Chinese entries. |
Resources/xlf/GitHubActionsResources.zh-Hant.xlf |
Adds Traditional Chinese entries. |
GitHubActionsExitCodeTests.cs |
Tests exit-code classification and formatting. |
GitHubActionsSummaryReporterTests.cs |
Tests summary verdicts and callouts. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 3
- Review effort level: Medium
This comment has been minimized.
This comment has been minimized.
…verage - Qualify PACKAGE.md and code comments: the exit-code callout/annotation covers session-derived outcomes (ZeroTests, MinimumExpectedTests, TestAdapterTestSessionFailure, MaxFailedTests). A hard abort/cancellation short-circuits end-of-session reporting and early/late-host codes are out of reach, so those are no longer over-claimed. - Add GitHubActionsReportTests acceptance tests driving the extension through a real MTP session (GITHUB_ACTIONS=true + temp GITHUB_STEP_SUMMARY) for pass / fail / zero-tests / minimum-expected-tests, asserting emission and suppression of the callout and run-level annotation end to end. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
This comment has been minimized.
This comment has been minimized.
…nown-reason test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
This comment has been minimized.
This comment has been minimized.
…aims, fix markdownlint - Add UTF-8 BOM to GitHubActionsExitCodeTests.cs and GitHubActionsReportTests.cs per .editorconfig utf-8-bom requirement for *.cs. - Remove the aborted-session / host-failure examples from GitHubActionsExitCode class doc and the summary callout comment, matching the documented reachable scope (session-derived outcomes only). - Fix PACKAGE.md markdownlint (MD032 list spacing, MD060 table pipe spacing). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
This comment has been minimized.
This comment has been minimized.
Addresses review r3588595379: the shared GITHUB_STEP_SUMMARY file was opened with FileMode.Append + FileShare.ReadWrite. Append only seeks to EOF at open (no atomic OS append), so concurrent test-host processes in a multi-assembly dotnet test run could interleave or clobber each other's section. AppendStepSummaryWithRetryAsync now opens with FileShare.Read (denies other writers) and retries on the sharing-violation IOException with bounded backoff, serializing the per-assembly append so every assembly's section is preserved. Added unit tests for first-attempt write, retry-then-succeed, and give-up-and- rethrow (best-effort warning). Acceptance suite (12 tests) still green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
This comment has been minimized.
This comment has been minimized.
Addresses review r3593645686: the retry previously wrapped open+write+flush, so a mid-write failure (e.g. disk full) after a partial append would be retried and re-append the full section, corrupting the summary. Contention only occurs while acquiring the exclusive append handle, so retries are now scoped to the open; any failure once writing has started propagates to the best-effort warning path. Added a unit test proving a post-acquire write failure throws after a single attempt (no retry). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
This comment has been minimized.
This comment has been minimized.
Addresses code-scanning nit r3594649673: expand the test helper's Position setter to explicitly consume the assigned value (discard) so the 'value ignored' analysis no longer fires. Test-only helper; behavior unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1e3944b-d2cf-4ab7-ac53-c8ace4ddc260
🧪 Test quality grade — PR #9974
20 of 21 tests earn A — the suite is exceptionally well-written. The single C-grade test ( Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
What
Follows up on feedback in #9003 for the
Microsoft.Testing.Extensions.GitHubActionsReportextension:--minimum-expected-testsviolation, a run that discovered zero tests, an aborted session, a--maximum-failed-testsstop, adapter/host failures — showed a misleading green ✅.dotnet test) clarity: make each assembly's contribution to the job summary carry its own verdict.Changes
GitHubActionsExitCodehelper maps every documented MTP exit code to a friendly, localized reason and classifies test-result vs. non-test-result outcomes.--report-gh-step-summary): flips the header to ❌ when the run failed even with 0 failed tests, and renders a> [!WARNING]callout naming the exit code + reason.--report-gh-annotations): the annotation reporter is now anITestSessionLifetimeHandlerand emits a single run-level::errorfor non-test-result failures, so e.g. a--minimum-expected-testsviolation surfaces in the Annotations tab even when no individual test failed. Plain per-test failures are unchanged (no redundant error).GITHUB_STEP_SUMMARYsection; those now each carry their own verdict/exit-code callout, so you can see which assembly failed and why. (A true cross-process grand total would require heavy cross-process coordination and was intentionally left out — noted inPACKAGE.md.)ITestApplicationProcessExitCodewired into both reporters, new localized resources (+ regenerated.xlf), updatedInternalAPI.Unshipped.txtandPACKAGE.md.Tests
GitHubActionsSummaryReporterTests(new signature + callout coverage) and addedGitHubActionsExitCodeTests.Microsoft.Testing.Extensions.UnitTestsbuild clean; full suite 676 tests, 672 passed, 4 skipped, 0 failed.Relates to #9003.