Skip to content

fix: don't hand nextest a phantom-only filterset in run - #81

Merged
max-sixty merged 1 commit into
mainfrom
fix/run-all-phantom-selection-exit4
Sep 17, 2026
Merged

max-sixty merged 1 commit into
mainfrom
fix/run-all-phantom-selection-exit4

Conversation

@cargo-affected-bot

Copy link
Copy Markdown
Collaborator

Deleting a test makes cargo affected run exit 4 with error: no tests to run.

Deleting a test function produces a diff hunk over exactly the lines its own stored coverage range covers, so the deleted test is selected as affected — and if the deletion is the only change, it is the whole selection. It is also gone from the current nextest list, so the generated default-filter matches nothing, and nextest's "no tests to run" exit propagates through run_tests. A stale cache row is reported to the caller as a test failure.

collect --diff has never had this problem: it passes --no-tests=warn and uses DiffPlan::live_selected_count afterwards to tell an all-phantom rerun from a runner-shim failure. run had no equivalent.

Repro

Against the two-module scratch project the functional suite uses, with the whole test_multiply block deleted and nothing else changed:

1 tests to run (1 affected + 0 config + 0 new + 0 stranded, 1 skipped of 2 reachable-known)
running 1 tests with nextest
    Starting 0 tests across 1 binary (1 test skipped via profile.default.default-filter)
     Summary [   0.000s] 0 tests run: 0 passed, 1 skipped
error: no tests to run

Exit code 4.

Change

Selection::live_selected() (new, in selection.rs) is selected() restricted to tests still in the listing. run hands nextest that subset instead of the full selection, and short-circuits with exit 0 when it is empty — there is no run to make, and asking nextest to run a phantom-only filterset can only produce the exit above. Both paths emit a note naming the count and pointing at cargo affected collect to drop the rows.

status gets the same note, in the conditional tense. Without it the dry run would predict a test that run now visibly skips, which is the invariant #71 established.

collect --diff is untouched — it keeps phantoms in its filterset deliberately, and prunes their rows afterwards.

Tests

run_with_all_phantom_selection_exits_zero (functional) collects, deletes test_multiply, and asserts exit 0 plus the two new messages. Verified failing against pre-fix src/: run with an all-phantom selection must exit 0, got Some(4), with error: no tests to run in the captured output.

Two unit tests in selection.rs pin live_selected()'s filtering and the notice's singular/plural agreement.

cargo test is green (114 unit + 42 functional); cargo clippy --all-targets is clean.

Relationship to #39 / #41

Same exit-4 failure class, different trigger, disjoint fix. #41 keeps filter-mismatched tests (positional filters, -E, #[ignore]) out of the selection by keying off filter-match.status; its own comment notes that phantoms are deliberately not in listing.excluded and stay in affected. This handles that remaining source. The two should merge cleanly enough — #41 edits the top of compute and the imports in run.rs; this edits impl Selection and the tail of run().

Deleting a test produces a hunk over exactly its own stored range, so it
lands in `affected` — and when nothing else changed, it is the whole
selection. It is also gone from `nextest list`, so the generated
default-filter matches nothing and nextest's exit 4 propagates: a stale
cache row reported as a test failure.

`run` now hands nextest `Selection::live_selected()` and short-circuits
with exit 0 when that is empty, noting the count either way. `status`
renders the same note in the conditional tense so its prediction still
matches what `run` does. `collect --diff` is untouched: it keeps
phantoms deliberately and prunes their rows afterwards.
@max-sixty
max-sixty merged commit 8c34ffe into main Sep 17, 2026
6 checks passed
@max-sixty
max-sixty deleted the fix/run-all-phantom-selection-exit4 branch September 17, 2026 16:06
@max-sixty max-sixty mentioned this pull request Sep 18, 2026
max-sixty added a commit that referenced this pull request Sep 18, 2026
Three fixes to `run` and to the output, with no new CLI surface, no report-JSON
schema change, and no change to the grep-able `cargo-affected:` summary line —
so a patch.

Deleting a test no longer makes `run` exit 4 (#81). The deletion's diff hunk
covers exactly the lines of the deleted test's own stored coverage range, so the
test selects itself, and nextest is then handed a filterset matching nothing.
`run` now restricts the filterset to tests still in the listing and exits 0 with
a note when that leaves nothing; `status` predicts the same.

A change committed since the last collect that nothing covers now reports as
uncovered (#92). The empty-selection message keyed off the working tree while
selection diffs against each reachable `collect_sha`, so the change read as "no
uncommitted changes ... nothing to run" directly under the "N commit(s) since
collect" notice.

Count nouns in `collect` and `run` output agree with their numbers (#82): "1
binary" rather than "1 binaryy", "1 test to run" rather than "1 tests to run".

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
max-sixty pushed a commit to max-sixty/worktrunk that referenced this pull request Sep 20, 2026
Weekly CI pin bump for the two third-party tool pins that drifted this
week.

- **cargo-affected** `=0.4.0` → `=0.4.1` — `affected.yaml`, both call
sites
- **cargo-nextest** `=0.9.144` → `=0.9.145` — `coverage.yaml`,
`.github/actions/test-setup/action.yaml`,
`.github/actions/tend-setup/action.yaml`, and `.codex/cloud.sh`, which
the weekly checklist keeps level with `test-setup`

Toolchain compatibility against the pinned `1.97.0`: `cargo-affected`
0.4.1 declares `rust-version = 1.94`, `cargo-nextest` 0.9.145 declares
`1.91`. Both build under `baptiste0928/cargo-install`. The
`.codex/cloud.sh` tarball URL resolves at the new version —
`cargo-nextest-0.9.145-x86_64-unknown-linux-gnu.tar.gz` is published on
the `cargo-nextest-0.9.145` tag.

The `worktrunk` pin also drifted this week and is in its own PR — it is
the `wt` that drives `wt hook pre-merge` on all three platforms, so a
red matrix there shouldn't hold back these two.

<details><summary>What the two releases change, against this repo's
config</summary>

**cargo-affected 0.4.1** carries two `run` fixes, both landing on the
advisory `affected` legs: a committed uncovered change is now reported
as uncovered rather than as "no changes"
([max-sixty/cargo-affected#92](max-sixty/cargo-affected#92)),
and `run` no longer hands nextest a phantom-only filterset
([#81](max-sixty/cargo-affected#81)).

**cargo-nextest 0.9.145**'s headline changes are profile-inheritance
fixes — overrides defined in intermediate profiles are now applied, and
a profile without `default-filter` inherits from its nearest ancestor
rather than always from `profile.default`. Those are no-ops here:
`.config/nextest.toml` defines only `profile.default` (plus its `junit`
sub-table), sets no `default-filter`, and CI passes no
`--tool-config-file`.

The change that does reach this suite is
[nextest-rs/nextest#3553](nextest-rs/nextest#3553):
on Apple platforms, where the standard library can't create pipes with
`FD_CLOEXEC` atomically, a concurrently-spawned test could inherit a
sibling's capture pipe and the sibling was then reported as having
leaked handles after exit. Nextest now creates capture pipes itself.
That is a macOS-leg flake source removed, not a behavior change the
suite has to absorb.

</details>

Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants