fix: forward filters to nextest list, key new-test detection off filter-match.status - #41
fix: forward filters to nextest list, key new-test detection off filter-match.status#41cargo-affected-bot wants to merge 6 commits into
Conversation
…er-match.status `cargo affected run -- some_test_name` dropped the positional filter from the `cargo nextest list` step used for new-test detection: the listing enumerated every test while the run only ran matching ones. A new test absent from the DB but not matching the positional was still flagged `(new)`, landed in the generated default-filter handed to `nextest run`, and tripped exit 4 once it was the only selected test. Replace `cargo_build_args` (build-flag allowlist) with `args_for_listing` (run-only denylist) so positionals, -E/--filterset expressions, and the libtest-compatible filter block all carry through to `nextest list`. Replace `Listing.ignored` with `Listing.excluded`, keyed off `filter-match.status == "mismatch"` — a strict superset of the old ignored predicate that unifies #[ignore]d tests, positional substring filters, -E expressions, and the project's own default-filter into one exclusion check across `new`/`stranded`/`affected`. Phantoms (in DB but absent from the listing entirely) remain in `affected` so `collect --diff`'s live-vs-phantom split in `handle_no_profraw_dirs` keeps working. Closes #39 Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # src/collect.rs # src/run.rs
The fix renamed the `listing.ignored` field to `listing.excluded` everywhere in code but left one doc comment in new_test.rs referencing the now-removed field. Update it to match. Co-Authored-By: Claude <noreply@anthropic.com>
cargo-affected-bot
left a comment
There was a problem hiding this comment.
The switch to a run-only denylist with filter-match.status is sound, and I traced the key paths: the --target-dir case the old allowlist guarded is a non-issue here (collect already forwards its own --target-dir plus nextest_args verbatim at the run step, so a user-supplied one was already broken there; run passes build_dir=None), and the green functional suite across all three platforms confirms nextest tags #[ignore]d tests as filter-match: mismatch and that the phantom contract still holds.
One concrete gap in RUN_ONLY_BARE: it lists the canonical run-only flags but misses their nextest aliases. Most notably --nocapture — nextest keeps it as an alias of --no-capture precisely for libtest muscle memory (run.rs:89-90), so cargo affected run -- --nocapture would forward --nocapture to cargo nextest list, which rejects it and fails the listing step. --ff / --nff are visible_aliases of --fail-fast / --no-fail-fast with the same problem. Suggestion below adds all three.
… args The run-only denylist enumerated the canonical flags but missed the libtest-compatible aliases nextest keeps for them: --nocapture (alias of --no-capture), --ff (--fail-fast), and --nff (--no-fail-fast). Forwarding any of these to `cargo nextest list` fails the listing step, since list defines none of them. Add the aliases to RUN_ONLY_BARE and cover them with args_for_listing_drops_run_only_aliases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # src/run.rs # src/selection.rs
The `new_tests` field doc referenced [`affected`] as a reference-style link but never defined the target, so `cargo doc` under `-D warnings` failed with an unresolved-link error. The orphaned [`new_tests`] definition left on the `affected` field (its text no longer references new_tests) is removed, and the [`affected`]: Self::affected definition is added to the new_tests block — matching the reference-style convention used by config_tests in the same struct. Co-Authored-By: Claude <noreply@anthropic.com>
Problem
cargo affected run -- some_test_nameforwarded the positional filter tocargo nextest runbut dropped it from thecargo nextest listinvocation used for new-test detection. The build args were extracted viacargo_build_args— a build-flag allowlist that silently dropped positional substring filters,-E/--filtersetexpressions, and the libtest-compatible--exact/--skip/--run-ignoredblock. So:cargo nextest listenumerated all tests (positional filter dropped).cargo nextest runreceived the positional filter and only ran matching tests.A new test (absent from the DB) that didn't match the positional filter was still flagged
(new), landed in the generated--config-filedefault-filter, and was handed tonextest run— which then skipped it because it didn't match the positional. If the selection collapsed to only such tests,nextest runexited 4 (no tests to run).src/run.rs::run_testsdoesn't pass--no-tests=warn, so that exit propagated.Same failure class as the bugs fixed in #38, via a different trigger.
Solution
Two coordinated changes from the issue proposal:
cargo_build_args→args_for_listing(run-only denylist). Forward everything tonextest listexcept knownnextest run-only flags (--retries,--no-fail-fast,--no-tests,--max-fail,-j/--test-threads/--jobs,--status-level,--message-format,--show-progress, etc.). Positionals and-E/--filtersetnow carry through so the listing reflects the run's full filter config.Key detection off
filter-match.statusinstead of theignoredboolean.cargo nextest list --message-format jsontags every testcase withfilter-match: { status: "matches" | "mismatch", reason: ... }. The newListing.excludedfield stores tests withstatus == "mismatch"— a strict superset of the oldignoredset that unifies all four filter sources nextest knows about:#[ignore]d tests, positional substring filters,-E/--filtersetexpressions, and the project's owndefault-filter. Selection now checkslisting.excludedin the three exclusion sites that fix: correct new-test detection for ignored tests and feature builds #38 left as separatelisting.ignoredfilters (new,stranded,affected).Phantoms (tests in DB but absent from the listing entirely — renamed/deleted) are NOT in
listing.excludedand still flow throughaffectedforcollect --diff's live-vs-phantom discrimination inhandle_no_profraw_dirs. Thediff_collect_all_phantom_selection_prunes_cleanlytest anchors that contract and still passes.Tradeoff
The denylist must stay complete against nextest's CLI surface. A future
nextest run-only flag not in the denylist would be forwarded tocargo nextest list, which rejects unknown args and exits non-zero —cargo affected run -- --that-new-flagwould break loudly until the denylist is updated. The previous build-flag allowlist instead failed silently on a future cargo build flag (listing mismatch, no error). The denylist's loud failure is aligned with this repo's "fail loudly over silently degrading" principle, but it's a maintenance commitment worth noting.Testing
Added
run_with_positional_filter_excludes_unmatching_new_testsintests/functional/new_test.rsthat reproduces the original failure:tests/integration_new.rswithtest_brand_new.cargo affected run -v -- add— the positional filter matchestest_addbut nottest_brand_new.Before the fix, this exited 4 with
error: no tests to runbecausetest_brand_newwas the sole selected test and the positional filter excluded it. After the fix,test_brand_newis taggedmismatchin the listing, the selection collapses to empty (test_add is unchanged, test_brand_new is excluded), and the run exits 0 cleanly.All other functional tests pass (37/37) including the pre-existing
ignored_test_not_perpetually_new,newly_ignored_test_excluded_from_affected,new_test_detection_uses_run_features, anddiff_collect_all_phantom_selection_prunes_cleanly— these test the same exclusion logic from different angles and all pass with the unifiedexcluded/filter-match.statuspredicate.Updated unit tests
args_for_listing_drops_run_only_flags,args_for_listing_forwards_positional_and_filterset,args_for_listing_drops_short_and_joined_test_threads,args_for_listing_emptycover the denylist behavior including joined-form-j4,--test-threads=Nand the--jobsalias.Closes #39 — automated triage