Repository navigation
[release/11.0.1xx-rc1] Honor MTP zero-tests and exit-code policies in dotnet test - #56296
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate findings remain in policy reconstruction and cancellation reporting.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 1
Open findings (1)
What changed in this PR
Backports MTP zero-test and ignored-exit-code policy handling to dotnet test, including aggregation, synthesized exit codes, terminal reporting, and test coverage.
Changes:
- Honors policies from arguments, response files, launch profiles, and environment variables.
- Aligns SDK exit codes with terminal summaries.
- Adds focused unit and integration coverage.
| File | Summary |
|---|---|
test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs |
Reviewed policy parsing coverage. |
test/dotnet.Tests/CommandTests/Test/TestApplicationActionQueueTests.cs |
Reviewed aggregation coverage. |
test/dotnet.Tests/CommandTests/Test/TerminalTestReporterTests.cs |
Reviewed terminal summary coverage. |
test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs |
Reviewed end-to-end policy scenarios. |
src/Cli/dotnet/Commands/Test/MTP/TestApplicationActionQueue.cs |
Reviewed module policy and exit-code aggregation. |
src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs |
Moderate finding: cancellation can incorrectly display a green Passed! headline. |
src/Cli/dotnet/Commands/Test/MTP/Options.cs |
Reviewed policy model definitions. |
src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs |
Moderate findings: repeated zero-test policies and case-variant environment variables may be reconstructed incorrectly. |
drognanar
left a comment
There was a problem hiding this comment.
Two policy-reconstruction issues found at 7509da8, detailed inline. Both can make the aggregate verdict disagree with the test application's effective policy. The configuration finding is source-traced against the SDK-selected MTP commit; the Windows environment mismatch was also reproduced with the exact SDK policy helpers and ProcessStartInfo.Environment. The PR-built SDK was not run end to end.
Reconstruct zero-test and ignored-exit policies from explicit and default test configuration files, and match process environment casing and override order on Windows. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
drognanar
left a comment
There was a problem hiding this comment.
Thanks for the fixes in e08c3f4. I reviewed the follow-up source and added coverage; both originally reported issues are addressed: configuration-file policy reconstruction and Windows launch-profile environment casing. The threads can remain resolved. CI is still running, so this acknowledgment is scoped to those two findings rather than overall merge readiness.
## Description ### What broke Deployment tests can be dynamically skipped—for example, when Azure regional capacity is unavailable. MTP represents a run containing only skipped tests with exit code `8`. The workflow already passed `--ignore-exit-code 8`. That option normalizes the **test module's** result so an expected skip does not fail the job. ### Why it started failing The .NET 11 RC1 SDK has a bug in how `dotnet test` combines results from all test modules ([dotnet/sdk#56214](dotnet/sdk#56214), originally reported as [microsoft/testfx#11145](microsoft/testfx#11145)). The test module accepts `--ignore-exit-code 8` and exits `0`. The SDK then recalculates the overall result, treats an all-skipped run as "zero tests," and returns `8`. ```text Test module → exit 8 → --ignore-exit-code 8 → success ↓ .NET 11 RC1 dotnet test aggregate → zero tests → exit 8 → workflow failure ``` The Deployment E2E history confirms the timing: - [September 10](https://github.com/microsoft/aspire/actions/runs/34432356786), using .NET 10.0.401: `AzureConnectorNamespaceDeploymentTests` skipped its only test, reported `Zero tests ran`, and the job succeeded. - #19999 then updated the repository to .NET 11 RC1. - [September 11](https://github.com/microsoft/aspire/actions/runs/34557498536), using .NET 11 RC1: the same test was skipped, but `dotnet test` returned exit code `8` and the job failed. The SDK fix merged in [dotnet/sdk#56219](dotnet/sdk#56219) and was backported to the .NET 11 RC1 branch in [dotnet/sdk#56296](dotnet/sdk#56296). Aspire's pinned `11.0.100-rc.1.26425.128` SDK predates that backport. ### The fix The affected workflows capture the aggregate exit code and pass it through checked-in Bash or PowerShell normalizers. Exit code `8` becomes success; actual test failures and host errors remain failures. - **Deployment E2E and non-NuGet shared tests** use `dotnet test`, so they normalize the aggregate result. - **NuGet-dependent tests** execute the test assembly directly, where the existing module-level option still returns success. The regression tests execute the checked-in scripts directly for exit codes `0`, `8`, `2`, and `7` instead of extracting and rewriting script fragments from YAML. Conditional test selection also routes changes to the shared runner and Deployment E2E consumers. **This is a temporary workaround.** Remove the aggregate `dotnet test` exit-code normalization after Aspire upgrades to a .NET 11 SDK build containing the upstream fix. ### Surprises and call-outs This fix also updates the non-NuGet paths in the shared `run-tests.yml` workflow. Those paths have the same aggregate exit-code behavior and are used by regular, quarantine, and outerloop test lanes. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [ ] Yes - If yes, did you have an API Review for it? - [ ] Yes - [ ] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [ ] No - [x] No - Does the change make any security assumptions or guarantees? - [ ] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [ ] No - [x] No <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #18629 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: radical <1472+radical@users.noreply.github.com> Co-authored-by: Ankit Jain <radical@gmail.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Summary
Backport #56219 to
release/11.0.1xx-rc1.--zero-tests-policywhen computing the SDK aggregate verdict, including forwarded, response-file, launch-profile, and explicit/default configuration-file policies--ignore-exit-code/TESTINGPLATFORM_EXITCODE_IGNOREpolicy to SDK-synthesized exit codes and keep the terminal summary aligned with the returned codeallow-skipped,strict, ignored exit codes, response files, configuration files, environment precedence, and multi-module aggregationTests
dotnet.Testsclasses passed: 206/206.Fixes #56214.
Related to microsoft/testfx#11145.