Skip to content

chore(test-lib): delete 15 orphaned test files that are byte-identical copies of tests already running (#2473) - #2498

Open
noahgift wants to merge 2 commits into
mainfrom
chore/delete-dead-duplicate-test-files
Open

chore(test-lib): delete 15 orphaned test files that are byte-identical copies of tests already running (#2473)#2498
noahgift wants to merge 2 commits into
mainfrom
chore/delete-dead-duplicate-test-files

Conversation

@noahgift

@noahgift noahgift commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Closes #2473.

This PR now carries three commits (#2501 was merged into this branch rather than stacking on it):

  1. 8acebab delete the 15 orphaned files (below)
  2. bbdceaf drop the now-dead browser_tests.rs exclusion from .pmat-gates.toml
  3. 870ceb4 add the guard so the class cannot recur (feat(ci): guard against source files that nothing declares (refs #2473) #2501, second half of this description)

Part 1 — the deletion

What #2473 got right, and what it got wrong

Right: 15 of the 16 *_tests.rs files in aprender-test-lib are wired into nothing. The crate contains zero include!() and zero #[path], and exactly one file-level declaration exists — mock/mod.rs:29: mod falsification_tests;.

Wrong: the claim that ~1,741 test functions "have NEVER COMPILED" and are therefore lost coverage. Those tests all compile and run today, from byte-identical copies that live inline in their parent modules. Net coverage lost was zero.

Deadness proven by mutation, not inspection

Appended an invalid Rust line to each file, re-ran cargo check, captured exit status directly to a variable (never through a pipe):

Subject Exit Verdict
control src/mock/wasm_runtime.rs (known-live) 101 harness can turn RED
src/mock/falsification_tests.rs 101 LIVE — kept
the 15 files deleted here 0 DEAD

They are duplicates, not orphans

Each file diffed against the body of the #[cfg(test)] mod tests { … } block in its parent:

  • 12 of 15 byte-identical, zero differing lines.
  • 3 of 15 differ only by clippy modernizations the inline copy received later — the deleted copy is strictly the older one:
File Lines Difference
llm/score_tests.rs 9 inline has .keys(), dropped redundant .clone()
validators_tests.rs 8 inline has if let vs match … _ => {}
playbook/runner_tests.rs 2 inline has !contains_key(…) vs get(…).is_none()

Per-file #[test] counts match exactly — 274/274 browser, 216/216 locator, 204/204 validators, … — totalling the 1,741 the issue counted.

Not written against a missing API

Two wirings were measured before deciding to delete:

Wiring Result
Correct — mounted as a submodule of the parent, so super resolves as the fragments expect 0 compile errors, 1544/1544 tests green, test-name set identical to the inline modules
Naive — mounted as siblings 562 errors: E0433 ×442, E0425 ×87, E0422 ×32, E0405 ×1

Not one API-shape error in 562 diagnostics — zero E0599, E0308, E0061, E0609. Every failure is scope, never shape. Top hits are the parent's own use std::… imports (Duration ×206, Arc ×20, Instant ×16) and its private items (DockerTestRunner ×33, RequestRecord ×29). Also of note: zero references to MockDriver in any of the 15 files; driver.rs, where it lives, has no _tests.rs companion at all.

So the choice was between adding 1,544 duplicate test executions for zero assurance, or deleting ~26k lines of dead copy. This PR deletes.

Provenance

All 15 arrived already orphaned in 8bd4ce5ad — a 17,830-file, 13.7M-line APR-MONO vendoring blob titled after an unrelated FFN falsifier. A bad bulk import, not a lapse in normal practice.

Verification

cargo test -p aprender-test-lib --lib --features browser,docker,llm,proptest,derive,compute-blocks
  before: 6458 passed; 0 failed
  after:  6458 passed; 0 failed
  test-name sets identical (diff exit 0; only the summary wall-clock line differs)

cargo check -p aprender-test-lib --all-targets --features browser,docker,llm,proptest,derive,compute-blocks  exit 0
cargo check -p aprender-test-lib --all-targets (default features)                                            exit 0
cargo fmt --all -- --check                                                                                   exit 0
scripts/check_include_files.sh      exit 0  (1771 include!() files tracked)
scripts/check_package_includes.sh   exit 0

No test was deleted to make a build green, and no #[ignore] was added — every one of the 1,544 tests in these files keeps running from its inline copy.

Pre-existing, deliberately not fixed here

cargo clippy -p aprender-test-lib --all-targets with that feature set fails with 8 errors — on the base commit too. I measured the base rather than assuming: the error sets before and after this deletion are byte-identical, in files this PR never opens (docker.rs, llm/report.rs, llm/score.rs, runtime.rs, tui/brick.rs, tui/compute_block.rs). This is the known "clean under default features, broken behind a non-default feature" trap. Left for a separate PR.

Recommended follow-up


Part 2 — the guard (was #2501)

Why

#2473 deleted 15 orphaned files. Nothing had noticed them for 15 months, because nothing looks.

A .rs file under src/ that no mod / include!() / #[path] declares is never compiled — and every gate we run is blind to it. fmt skips it. clippy skips it. The test count does not move. Coverage cannot report on a file that was never built. The only signal is that mutating it does not turn the build RED, which no gate was asking.

This adds that gate.

What it found

The class is much wider than #2473: 91 orphaned .rs files across 12 crates.

Crate Orphans
aprender-serve 65
aprender-shell 7
aprender-test-cli 3
aprender-present-terminal 3
apr-cli 3
7 others 1 each

Eight are *_tests.rs. One is literally named tests_conversion_stats_orphan.rs.

These are recorded as a baseline, not fixed here. Each needs its own read to decide wire-up vs. delete, and bulk-deleting 91 files on inference is exactly the move this guard exists to prevent.

A sample was verified by mutation, not inspection — invalid Rust appended, cargo check re-run, exit status captured directly to a variable:

File Exit Verdict
crates/aprender-test-cli/src/main_tests.rs 0 DEAD
crates/aprender-test-cli/src/commands_tests.rs 0 DEAD
crates/aprender-shell/src/cli_dispatch.rs 0 DEAD
src/bin/apr.rs 101 LIVE

That last row is why src/bin/** is exempt — auto-discovered binary targets are claimed by cargo, never by a mod line. The exemption is a measurement, not an assumption.

How it is proven

Against the real defect: on the pre-deletion tree the guard reports exactly the 15 files of #2473 and nothing else. Restoring them turns it RED; the state this PR sits on is GREEN.

Self-test — an 11-case must-flag / must-not-flag table over a hermetic fixture, covering every claim form (mod, pub(crate) mod, pub (in path) mod, include!, #[path], Cargo.toml path=, mod.rs, src/bin) and the regex's known confusables: a commented-out // mod ghost; and an inline mod ghost { } with no semicolon must not claim their files. Plus three ratchet mutations proving the baseline comparison fails on a missing entry and on a stale one.

The self-test was itself mutated three ways — dropping the regex's line-start anchor, removing the src/bin exemption, disabling the stale-baseline check. Each turns it RED with the correct specific message. The implementation was later restructured (associative arrays → sorted-file join), so all three were re-run against the new code rather than assuming the old proof transferred.

Two bugs were caught this way that reading would not have caught:

  1. The first version re-invoked itself as "$0" after scan_tree had cd'd elsewhere, and the script was not executable. Two of the three ratchet mutations were passing on Permission denied, not on the ratchet working.
  2. local scratch + trap … EXIT produced scratch: unbound variable — the exact pitfall documented in check_contract_test_binding.sh. Now a checked global, with the note repeated at the site.

Vacuity

Refuses to pass unless it scanned ≥ MIN_FILES files across ≥ MIN_CRATES crates, so a collapsed universe cannot read as a clean tree. Its universe is built by find over the filesystem, and the defect — a missing mod line — cannot remove a file from that universe, which is the correct side to build it from.

Known limits, stated plainly

This is a syntactic reachability check, one level deep. It proves a file is named somewhere, not that the namer is itself reachable from the crate root — a cluster of orphans that all declare each other would still pass. Deliberate trade: cheap, runs on every PR, catches the class that actually occurs. True reachability needs the compiler, and the compiler's answer is the mutation test, which cannot run in a gate.

Wiring

ci.yml guard-runner-labels job (self-test first, then the tree) and make tier3. Text-only, no build, ~1.5s on 6965 files across 93 crates. bashrs lint: 0 errors.

🤖 Generated with Claude Code

@noahgift
noahgift enabled auto-merge August 15, 2026 15:55
noahgift added a commit that referenced this pull request Aug 15, 2026
This reverts bad1103, which is not a retraction: the finding and the
evidence stand exactly as committed. The deletion simply belongs in its
own PR rather than buried under a driver change.

#2498 now carries it standalone, and a 26,205-line deletion is far easier
to review on its own than mixed into ~800 lines of new driver and
executor code where the diffstat hides it.

The .pmat-gates.toml exclusion for browser_tests.rs comes back with the
file, since it only becomes dead once the file is gone -- it moves to
#2498 with the deletion it depends on.

Refs #2473, #2498
noahgift and others added 2 commits August 18, 2026 19:13
…l copies of tests already running (#2473)

Issue #2473 reported that 15 of 16 *_tests.rs files in aprender-test-lib are
wired into no mod declaration and no include!(), so ~1,741 test functions had
"NEVER COMPILED". The wiring half of that is exactly right. The damage half is
not: those tests all compile and run today, from byte-identical copies that live
inline in their parent modules. Net coverage lost was zero.

Evidence:

1. Wiring. The crate contains zero include!() and zero #[path]. Exactly one
   file-level declaration exists: mock/mod.rs:29 mod falsification_tests;.

2. Deadness proven by mutation, not inspection. Appending an invalid Rust line
   to each file and re-running cargo check (exit status captured directly to a
   variable, never through a pipe):
     - control, src/mock/wasm_runtime.rs, a known-live file: exit 101,
       so the harness can turn RED
     - src/mock/falsification_tests.rs: exit 101, LIVE
     - all 15 files deleted here: exit 0, DEAD

3. They are duplicates, not orphans. Diffed against the body of the
   #[cfg(test)] mod tests block in each parent:
     - 12 of 15 byte-identical, zero differing lines
     - 3 of 15 differ only by clippy modernizations the inline copy received
       later, so the deleted copy is strictly the older one:
         llm/score_tests.rs      9 lines  (.keys(), dropped redundant .clone())
         validators_tests.rs     8 lines  (if let vs match with a no-op arm)
         playbook/runner_tests.rs 2 lines (!contains_key vs get().is_none())
   Per-file #[test] counts match exactly: 274/274 browser, 216/216 locator,
   204/204 validators, and so on, totalling the 1,741 the issue counted.

4. Not written against a missing API. Mounted correctly, as a submodule of the
   parent so that super resolves as the fragments expect, all 15 compile with
   ZERO errors and run 1544/1544 green, with a test-name set identical to the
   inline modules. Mounted naively as siblings instead, they produce 562
   resolution errors (E0433 442, E0425 87, E0422 32, E0405 1) and not one
   API-shape error: zero E0599, E0308, E0061, E0609. Every failure is scope,
   never shape.

Provenance: all 15 arrived already orphaned in commit 8bd4ce5, a 17,830-file
13.7M-line APR-MONO vendoring blob titled after an unrelated FFN falsifier.
This was a bad bulk import, not a lapse in normal practice.

Verification of this deletion:
  cargo test -p aprender-test-lib --lib --features browser,docker,llm,proptest,derive,compute-blocks
    before: 6458 passed, 0 failed
    after:  6458 passed, 0 failed
    test-name sets identical (diff exit 0; only the summary wall-clock differs)
  cargo check --all-targets, with those features and with defaults: exit 0
  cargo fmt --all --check: exit 0
  scripts/check_include_files.sh: exit 0 (1771 include!() files tracked)
  scripts/check_package_includes.sh: exit 0

Pre-existing and untouched: cargo clippy -p aprender-test-lib --all-targets with
that feature set fails with 8 errors on the base commit too. The error sets
before and after this deletion are byte-identical, in files this PR never opens
(docker.rs, llm/report.rs, llm/score.rs, runtime.rs, tui/brick.rs,
tui/compute_block.rs). Filed separately rather than fixed here.

No test was deleted to make a build green and no #[ignore] was added: every one
of the 1,544 tests in these files keeps running from its inline copy.

Closes #2473

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With crates/aprender-test-lib/src/browser_tests.rs deleted by this PR,
`**/browser_tests.rs` in .pmat-gates.toml's file_health exclusions matches
nothing anywhere in the repo (verified: `find . -name browser_tests.rs`
returns nothing outside target/).

A dead exclusion is a rule that looks like it is protecting something and
is not -- the same shape as the 20 of 29 dead advisory exemptions in
#2491.
@noahgift
noahgift force-pushed the chore/delete-dead-duplicate-test-files branch from 870ceb4 to 0558f6d Compare August 18, 2026 17:14
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.

probar has no browser driver and 1,741 tests that never compile — the Playwright-killer claim is not supported

1 participant