Allow --minimum-expected-tests 0 (#7457)#9724
Conversation
Validation now rejects only negative (or non-integer) values, so an explicit '--minimum-expected-tests 0' is accepted. Combined with the zero-tests verdict governance, a zero minimum accepts an empty run (exit code 0) -- the same outcome as '--ignore-exit-code 8' -- giving users a per-module 'zero tests is acceptable' opt-in. Stacked on the verdict-governance change (#9709), which is what makes an explicit '--minimum-expected-tests 0' with zero tests resolve to success instead of the ZeroTests (exit code 8) verdict. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
addaa4c
into
dev/amauryleve/curly-couscous
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.
✅ 22/22 dimensions clean — no findings.
Summary: Clean, well-scoped change. The validation relaxation (value <= 0 → value < 0) is correct — when --minimum-expected-tests 0 is provided, the option's presence routes GetProcessExitCode() into the minimum-expected branch where _totalRanTests < 0 is always false, yielding ExitCode.Success and bypassing the zero-tests-policy check entirely. This is the correct design for an explicit "zero tests is acceptable" opt-in.
Tests thoroughly cover boundaries (negative, non-integer, zero, positive), both at the unit and acceptance integration level. Localization is properly done via .resx with XLF targets correctly marked needs-review-translation. Changelog updated. No public API surface change. No threading, security, or performance concerns.
🧪 Test quality grade — PR #97245 test methods graded across 3 files, all earning A. The PR cleanly extends unit and acceptance coverage for the new
This advisory comment was generated automatically. Grades are heuristic
|
Reverts the ##9724 change that allowed the literal value '--minimum-expected-tests 0'. Keeping '0' invalid restores an unambiguous meaning for GetMinimumExpectedTests() == 0 ('option not set'), which avoids the terminal-reporter verdict divergence tracked in ##9744: with '--min 0' the exit code was 0 (success) while the terminal still rendered a failed 'Zero tests ran' verdict, because the shared verdict helpers could not distinguish 'unset' from 'explicit 0' without a cross-repo change to the vendored TerminalTestReporterOptions. Issue ##7457 item #2 ('--min 0 == --ignore-exit-code 8') remains served by the existing '--ignore-exit-code 8'. The load-bearing behavior for the 'dotnet test --test-modules' orchestrator -- an explicit '--minimum-expected-tests N' governs the zero-tests verdict (exit code 9, not 8) -- is unaffected and stays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reverts the #9724 change that allowed the literal value '--minimum-expected-tests 0'. Keeping '0' invalid restores an unambiguous meaning for GetMinimumExpectedTests() == 0 ('option not set'), which avoids the terminal-reporter verdict divergence tracked in #9744: with '--min 0' the exit code was 0 (success) while the terminal still rendered a failed 'Zero tests ran' verdict, because the shared verdict helpers could not distinguish 'unset' from 'explicit 0' without a cross-repo change to the vendored TerminalTestReporterOptions. Issue #7457 item #2 ('--min 0 == --ignore-exit-code 8') remains served by the existing '--ignore-exit-code 8'. The load-bearing behavior for the 'dotnet test --test-modules' orchestrator -- an explicit '--minimum-expected-tests N' governs the zero-tests verdict (exit code 9, not 8) -- is unaffected and stays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Follow-up to #9709, part of #7457 (item #2). Allows the literal value
--minimum-expected-tests 0.Previously validation rejected
0(value <= 0). It now rejects only negative and non-integer values. Combined with the zero-tests verdict governance in #9709, an explicit--minimum-expected-tests 0accepts an empty run and returns exit code0— the same outcome the reporter asked for in #7457 (""--minimum-expected-tests 0should have the same outcome as--ignore-exit-code 8""). It gives users an explicit, opt-in ""zero tests is acceptable"" escape hatch per test host.Why this is separate from #9709
--minimum-expected-tests Nreturn exit code9(not8) even at zero tests, which thedotnet testorchestrator (Apply dotnet test zero-tests verdict at the whole-run level (microsoft/testfx#7457) dotnet/sdk#55167) relies on to normalize an empty module without swallowing a stricter local minimum. It does not depend on allowing0.Note: allowing a user-typed
--minimum-expected-tests 0is deliberately different from having the SDK auto-inject--min 0into every module — the latter was rejected because older Microsoft.Testing.Platform versions reject--min 0and the SDK cannot know a module's MTP version at argument-build time. Whoever types--min 0explicitly owns that assumption.Stacked on #9709
This PR targets the #9709 branch, because
--minimum-expected-tests 0+ zero tests only resolves to success once #9709's verdict-governance logic is in place (otherwise the ZeroTests(8) verdict is computed first). Review/merge #9709 first; the diff here is only the validation change.Change
PlatformCommandLineProvider.IsMinimumExpectedTestsOptionValidAsync: reject onlyvalue < 0.PlatformResources.*.xlfvia/t:UpdateXlf).Tests
PlatformCommandLineProviderTests:0/1/10now valid; negatives / non-integers still invalid.TestApplicationResultTests:--minimum-expected-tests 0+ zero tests → success.ExecutionTests(acceptance):--filter-uidmatching nothing +--minimum-expected-tests 0→ exit code 0; negative-value validation-message expectation updated.Unit tests pass locally (110/110 across
TestApplicationResultTests+PlatformCommandLineProviderTests). Acceptance tests require-packand run in CI.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com