tests: replace dead shim tripwires with a live no-coverage check

[cargo-affected-bot]

Problem

Five assertions across the two basename-collision scenarios grep stderr for strings the code no longer emits, so they are permanently true:

$ rg 'failed to resolve binary_id|basename fallback ambiguous' src/
$   # no matches

Both strings belonged to the pre-NEXTEST_BINARY_ID shim, which resolved binaries by path and probed byte markers to disambiguate colliding basenames. That resolver is gone — the shim now reads NEXTEST_BINARY_ID straight from the env (src/shim.rs#L115-L120) — but the assertions that pinned its behavior stayed behind. tests/CLAUDE.md calls silent under-selection "the one failure mode this tool cannot detect downstream"; these two scenarios are supposed to be part of that guard and currently aren't.

The gap is not theoretical, because a failure to resolve a binary_id is a soft failure. That test lands as TestOutcome::Skipped, and as long as some other binary produced rows, collect still exits 0 (src/collect.rs#L376-L379). So the surviving collect.status.success() assertion doesn't cover it either: a green collect whose database is missing one of the two colliding binaries is exactly the shape both scenarios exist to catch.

Fix

Point each assertion at "produced no coverage" — the line collect prints whenever any test is skipped — and drop the now-redundant second basename fallback ambiguous check. The module docs' used to bail with … history is untouched; those describe the old shim in the past tense and are accurate.

Verification

Per tests/CLAUDE.md ("a test written for a bug must be seen failing against the pre-fix behavior"), each new assertion was run against an injected break — a temporary branch in shim.rs returning Skipped for one of the two colliding binaries, simulating a resolution failure on exactly one target. Both scenarios failed on the new assertion; neither would have failed on the old one.

Injected-break output

Break applied in src/shim.rs (not committed):

let outcome = if binary_id.contains("wt_perf") {
    TestOutcome::Skipped { reason: "TEMP BREAK for tripwire verification".into() }
} else {
    extract(&dir, Path::new(binary), &env)
};

duplicate_basename_with_stripped_debuginfo_resolves_correctly — nextest reports both tests passing, collect exits 0, the database gets one test's rows instead of two, and the old string appears nowhere in stderr:

    Starting 2 tests across 4 binaries
        PASS [   0.007s] (1/2) dup_strip_wt_perf::builds builds
        PASS [   0.012s] (2/2) dup_strip_mock_stub::builds builds
     Summary [   0.012s] 2 tests run: 2 passed, 0 skipped
  skipped dup_strip_wt_perf::builds::builds: TEMP BREAK for tripwire verification
1 test produced no coverage
storing coverage for 1 tests (3 ranges)...
done. 1 tests, 3 ranges stored in target/affected/coverage.db (0.6s total)

panicked at tests/functional/duplicate_target_names.rs:87:5:
duplicate-basename binaries must not cost a test its coverage

lib_bin_same_basename_resolves_via_nextest_binary_id with the break narrowed to bin/wt-perf (both of its targets share the wt_perf_collide package, so the broader predicate tripped the hard mappings.is_empty() bail instead):

    Starting 2 tests across 2 binaries
        PASS [   0.007s] (1/2) wt_perf_collide::bin/wt-perf tests::bin_test_invokes_lib
        PASS [   0.012s] (2/2) wt_perf_collide tests::lib_test_double
     Summary [   0.012s] 2 tests run: 2 passed, 0 skipped
  skipped wt_perf_collide::bin/wt-perf::tests::bin_test_invokes_lib: TEMP BREAK …
1 test produced no coverage
storing coverage for 1 tests (3 ranges)...

With the break reverted, cargo test is green: 41 passed, 0 failed. cargo fmt --check and cargo clippy --all-targets both clean.

Follow-up commit: the same drift, in the prose next to it (a56181c)

The nightly survey drew duplicate_target_names.rs and found two more artifacts of the same pre-NEXTEST_BINARY_ID shim, in the lines immediately around the hunks above. Folded in here rather than opened as a third PR against this file — #99 is already the second.

The rebuild comment named the wrong mechanism. It credited "the pre-run listing refreshes paths, so the lookup remains exact" for the partial-rebuild case surviving. Paths are not what saves it, and the shim's own module doc says so directly (src/shim.rs#L38-L48): attribution comes from NEXTEST_BINARY_ID, and the map is looked up by the binary's file name (map_path), which a rebuild preserves — "cargo's hash suffix tracks build metadata rather than contents — a rebuilt binary keeps its name." What actually rejects a stale map after the rebuild this scenario forces is the BinaryStamp check in load_function_map. The comment now names both halves. A second comment further down ("pre-run listing must still produce a working binary_map") had the same error and is corrected to what the second collect actually has to do: re-export a map whose stamp matches the rebuilt binary.

A trailing restore that restores nothing. The scenario ended with git checkout -- mock-stub/src/lib.rs, but the edit it looks like it's undoing was committed ~15 lines earlier, so the checkout wrote the edited content back over itself — a no-op, on a tempdir that drops on the next line either way. Removed.

Both are comment/dead-code changes with no behavioral effect, so there's no regression test to add; the existing scenario is the coverage. Verified with cargo clippy --all-targets clean and the full cargo test suite green (41 passed, 0 failed) after provisioning llvm-tools and cargo-nextest, which the agent sandbox doesn't ship — the gap #96 fixes.

Third commit: the last "probe path" line in this file (469ee12)

The review pass over a56181c found the same artifact three lines below a hunk this PR already rewrites, in lib_bin_collision.rs this time: "A second collect drives the pre-run listing through the same probe path again". There is no probe, and the pre-run listing is not what attributes a test to a target. The comment now names what the second collect actually re-exercises — NEXTEST_BINARY_ID for attribution, and a function map filed under the binary's own file name (map_path), whose -<hash> suffix differs per target even though the wt_perf stem is what collides in this scenario. Comment-only; cargo fmt --check and cargo check --tests clean.

Overlap with open PRs

This section previously read that #31's and #63's non-message hunks were "independent and unaffected". That is wrong — most of their hunks land on lines this PR rewrote:

• docs: drop stale "marker probe" wording in lib_bin_collision test #63 (2 hunks) is moot in full. Hunk 1 rewords the message on !stderr.contains("basename fallback ambiguous"), an assertion 67e96ec deleted. Hunk 2 rewords the recollect comment 469ee12 just rewrote, and carries "expected bin binary_id in {ids:?}", plus its ); as leading context — a block that became the assert_both_binary_ids(dir, "after the first collect") call. So both hunks conflict, not just the message one.
• tests: refresh duplicate-basename comments for NEXTEST_BINARY_ID shim #31 (6 hunks) keeps two. Hunks 3 and 4 edit the two duplicate_target_names.rs comments a56181c corrected; hunk 5 is docs: drop stale "marker probe" wording in lib_bin_collision test #63's deleted assertion and hunk 6 its recollect comment. Only its module-doc hunk and its cargo_affected_stripped doc-comment hunk are independent of this PR.
• tests: delegate cargo_affected_stripped to the shared cargo_affected_with_env #99 touches duplicate_target_names.rs imports and the cargo_affected_stripped helper — no overlap with anything here, though it does sit on the same function as tests: refresh duplicate-basename comments for NEXTEST_BINARY_ID shim #31's surviving hunk.

Every one of those conflicts is "the stale line this PR rewords is already gone", so none is a reason to hold anything; the point is only that whoever sequences the three shouldn't plan around the earlier reading.

[cargo-affected-bot]

Self-review. CI is green on all three platforms, and the core claim checks out: rg finds neither failed to resolve binary_id nor basename fallback ambiguous anywhere in src/, and Skipped is only ever constructed from real extraction failures in shim.rs — never for a test that merely executes nothing — so the new assertions can't false-positive on the empty builds() test.

One observation beyond the inline nit. The replacements are still string-greps against a collect message, the same shape as the two they retire: a future reword of eprintln!("{skipped} test{s} produced no coverage") in collect.rs puts all four assertions back in the permanently-true state this PR exists to fix, silently. On the first collect in both scenarios that's cushioned — the SELECT DISTINCT binary_id assertions below would fail independently if a target were dropped. The uncushioned spot is the second collect in each scenario (recollect): neither re-queries test_regions afterwards, so the stderr grep is the sole guard there and also the only genuinely new coverage this PR adds. Re-running the same binary_id query after the recollect would pin the property against message edits as well as against a dropped target.

[cargo-affected-bot]

Self-review of the two commits since the last review. 56406d7 closes the gap the previous review raised — both scenarios now re-query test_regions after the second collect, and because plain collect takes the store_coverage path (DELETE FROM test_regions WHERE env_fingerprint = ?1 before insert), those re-queries can't be satisfied by the first collect's rows. a56181c's comment corrections check out against the source: map_path looks the map up by binary.file_name(), and the BinaryStamp comparison in load_function_map is what rejects a stale map, so NEXTEST_BINARY_ID + stamp is the right pair to name. The dropped git checkout -- mock-stub/src/lib.rs is a genuine no-op — the edit it appears to undo is committed ~15 lines above — and git is still used by that commit, so the import stays.

One correction to the PR body's overlap section, which is now stale in a way that could misinform the merge order. It says #63's non-message hunks are "independent and unaffected". #63's second hunk carries "expected bin binary_id in {ids:?}", and the ); after it as leading context, and this PR replaced that whole block with the assert_both_binary_ids(dir, "after the first collect") call — that string no longer exists anywhere in lib_bin_collision.rs at this head, so #63 will conflict on both of its hunks, not just the message one. Worth rewording so whoever sequences the two isn't planning around a stale reading.

One inline note on the residual prose in the same file.