add --max-fail=N to stop after N failures#666
Merged
MatthewMckee4 merged 3 commits intomainfrom Apr 11, 2026
Merged
Conversation
Generalizes the existing --fail-fast / --no-fail-fast pair into a nextest-aligned --max-fail=N flag. `--max-fail=3` stops scheduling new tests after three failures, and `--max-fail=all` runs every test regardless. The legacy flags continue to work as aliases: `--fail-fast` maps to `--max-fail=1` and `--no-fail-fast` maps to `--max-fail=all`. When both forms are present, `--max-fail` wins. Closes #551
cefab3e to
efc149f
Compare
Merging this PR will degrade performance by 10.95%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| 👁 | WallTime | karva_benchmark |
22.3 s | 25 s | -10.95% |
Comparing feat/max-fail (beaa5c5) with main (e536e42)
Convert the new --max-fail integration tests to inline snapshots to match the rest of the suite, replace the hand-written Default impl on MaxFail with #[derive(Default)] + #[default], and rename PackageRunner::budget_exhausted to max_fail_reached for a clearer signal at the call sites.
--max-fail previously accepted the literal string "all" on both the CLI and in karva.toml to mean "unlimited", mirroring nextest. That value is redundant: --no-fail-fast already covers the CLI use case and omitting max-fail from the config already means unlimited. Drop the sentinel and replace the bespoke enum with a newtype wrapper around `Option<NonZeroU32>`, which lets serde and clap handle parsing via their standard impls. The newtype preserves the existing method surface (`is_exceeded_by`, `has_limit`, `from_count`, `from_fail_fast`, `is_fail_fast`) so every call site keeps working without a rewrite. Also introduce a real --no-fail-fast clap flag that until now was only referenced in the help text. It overrides --fail-fast, clears any configured limit, and yields to an explicit --max-fail=N when both are passed. The worker-side forwarding no longer emits --max-fail=all; callers pass --max-fail=<n> only when a limit is active. The config form `max-fail = "all"` is removed: use `test_fail_fast_false` (already present) to exercise "config lets every test run" and the renamed `test_no_fail_fast_runs_every_test` for the CLI equivalent.
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 free
to 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.
Summary
This change generalizes karva's
--fail-fast/--no-fail-fastpair into a new--max-fail=Nflag, bringing karva closer to nextest's failing-fast semantics. The motivation is simple: stopping after the first failure is often too aggressive in CI, and running every test to completion is often too lenient. A bounded budget in between — "stop once N tests have failed" — is exactly what nextest offers, and it is what issue #551 asked for.The new flag accepts a positive integer. Passing
--max-fail=3tells karva to keep scheduling tests until three have failed and then shut down. The same value can be set inkarva.tomlunder[test]asmax-fail = 3. Omitting the field leaves the limit unset, which means every test runs — karva's historical default. Everything is wired through the normalTestOptions->TestSettingspath, and the worker processes receive the resolved limit via--max-fail=<n>so the main binary and workers stay in lockstep.The pre-existing
--fail-fastand--no-fail-fastflags keep working.--fail-fastis equivalent to--max-fail=1, and--no-fail-fastclears any limit that was set inkarva.toml, restoring the "run everything" behavior. If both forms are present on the CLI,--max-failwins — this matches nextest's "takes precedence" rule and means users who migrate can drop the old flags at their own pace without any behavior surprises. Every integration and snapshot test that used--fail-fastcontinues to pass unchanged.Example
Note that
test_third_failandtest_fourth_skippedare never scheduled: once two failures accumulate, the runner stops walking the discovered tree and the summary reflects only what actually ran.Defaults
Nextest's default is
--max-fail=1. Karva's default has historically been "run everything", and this PR intentionally keeps that default to avoid breaking the dozens of existing integration tests that rely on seeing every failure. Users who want nextest-style behavior can opt in with--max-fail=1or set it globally inkarva.toml. Users who set a limit in config and want to override it for a single run can pass--no-fail-faston the command line. This is called out both indocs/cli.mdand indocs/configuration.md, which are regenerated bycargo run -p karva_dev generate-allalongside this PR.Implementation Notes
The core enforcement lives in
PackageRunner, which holds aCell<u32>of failure counts and compares it against the configuredMaxFailafter every test variant. The previous booleanfail_fastchecks inexecute_moduleandexecute_packagehave been replaced with a singlemax_fail_reached()helper, so there is only one place that decides "stop scheduling." The worker writes the existing fail-fast signal file once its local budget is exhausted, which causes sibling workers to wind down through the already-existingWorkerManager::wait_for_completionpolling loop.MaxFailitself is a thin newtype wrapper aroundOption<NonZeroU32>inkarva_metadata, withNonemeaning "no limit" andSome(n)meaning "stop afternfailures." Serde's built-in transparent handling covers the TOML form (max-fail = 5becomesSome(5), omitting the key isNone), and clap parses--max-faildirectly asNonZeroU32, so there is no customFromStror visitor code to maintain.Closes #551
Test Plan
just test— full suite passes (824 tests)uvx prek run -a— cleanbasic::test_max_fail_stops_after_n_failures—--max-fail=2against four failing tests, asserting exactly two runbasic::test_no_fail_fast_runs_every_test—--no-fail-fastruns every test even when some failbasic::test_max_fail_one_is_equivalent_to_fail_fast—--max-fail=1matches the historical fail-fast behaviorconfiguration::test_max_fail_from_configexercises thekarva.tomlformtest_fail_fast,test_fail_fast_across_modules,test_fail_fast_true,test_fail_fast_false, andtest_cli_fail_fast_overrides_configcontinue to pass, confirming the legacy flags behave identically