Harden the dead-code gate's own test coverage - #4
Merged
Merged
Conversation
Mutation testing (ClickUp 86cb417ty follow-up) showed the 9-test suite passed unchanged against several real regressions: exit 127 becoming exit 2 for a missing vulture, and a bad TARGET falling through to the same exit 1 as "dead code found" (both undetected). - Tighten t_missing_vulture_fails_closed to assert exactly 127, the gate's whole fail-closed contract, not just non-zero. - Give cd a real failure path: a bad TARGET now exits 2 (this script's existing bad-usage code) and names the target, instead of silently reusing exit 1 via `set -e`, which is indistinguishable from a dead-code finding. Add a test for it. - Add a two-halves vulture_whitelist.py test (fails unwhitelisted, passes once whitelisted) for real regression coverage of that path, though note in the report that it can't discriminate the specific "delete the pickup line" mutation — that mutation turns out to be behaviorally inert given vulture's own directory traversal, verified against vulture 2.14's source. - Replace the CI try-repo step, which pointed at this repo's own zero .py files and thus proved nothing, with a positive/negative control against a throwaway repo with planted dead code. Suite: 9 -> 12 passing assertions. No change to the gate's external exit-code contract other than the deliberate bad-target 1 -> 2 move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V
Follow-up to the previous hardening commit: mutation testing showed deleting check-dead-code.sh:65 doesn't change any test outcome. Verified (and independently re-verified by team-lead) that this isn't a coverage gap — vulture already walks "." and picks up vulture_whitelist.py on its own, so passing it a second time as an explicit CLI arg has no observable effect for any case reachable through this script's usage pattern. Leave the line in place (removing it is safe but is a deliberate cleanup needing a version bump to reach consumers, not part of this test-hardening PR) and record the finding in a comment so the next reader doesn't re-derive the "this line is why six repos are green" belief that the mutation-testing review's report had gotten wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V
Closes the last real gap the mutation-testing review found: breaking --ignore-decorators (check-dead-code.sh:63) passed the suite at 12/0. Unlike the vulture_whitelist.py pickup, this one is a genuine gap, not a behaviour-preserving mutation — confirmed by direct experiment that an @app.route-decorated, otherwise-unreferenced handler is reported as dead without the flag and suppressed with it. Tower-Finder's backend is the Flask/FastAPI app this flag protects; if it broke, CI would light up with dozens of false findings on live route handlers, risking someone "fixing" it by deleting or bulk-whitelisting real code. t_ignore_decorators_suppresses_handlers follows the same two-halves shape as the whitelist test. The script has no flag to disable its own --ignore-decorators, so the first half calls vulture directly with the script's other options (same EXCLUDE, same --min-confidence, no --ignore-decorators) to prove the finding is real; the second half runs the actual script and proves the flag suppresses it. Verified by mutation: DECORATORS="" now fails exactly this test (13 passed, 1 failed), script restored after. Suite: 12 -> 14 passing assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V
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.
Follow-up to #1–#3, acting on a mutation-testing review of that work. The gate itself was sound; its tests proved less than they appeared to.
What mutation testing found
Each mutation was applied to
check-dead-code.shand the 9-test suite re-run:exit 127→exit 2cd "$TARGET"(no guard)DECORATORS=""vulture_whitelist.pypickupEXCLUDE=""Suite goes 9 → 14.
Changes
Assert exit 127 exactly. 127 is this gate's entire contract and was only asserted as "non-zero", so changing it to 2 passed cleanly.
Guard the
cd. A barecd "$TARGET"exits 1 underset -ewhen the target is missing or is a file — the same code as "dead code found". A typo'dargs: [backedn]in a consumer would surface in CI as if dead code had been found. Now exits 2 (this script's existing "bad usage" code) and names the target.Make
try-repoa real test.opscontains zero.pyfiles, sopre-commit try-repo . dead-code --all-filesscanned nothing — it provedentry:/language:resolve but could not distinguish "ran" from "no-opped". Replaced with a positive control: planted dead code must make the hook FAIL and name the symbol, then pass once removed.Two behaviour tests with concrete consumers.
vulture_whitelist.pysuppression, and--ignore-decorators— the latter because Tower-Finder's backend is the Flask/FastAPI app here, and@app.*,@bp.*,@router.*is the only thing stopping every route handler reading as dead. If that broke, its CI would fill with false findings, and the plausible human response — deleting "unused" handlers — is worse than the gate failing outright.Two findings worth recording rather than fixing
The whitelist pickup is inert, not untested. The review called
check-dead-code.sh:65"the single line keeping six consumers green" because deleting it failed no test. That premise is wrong — vulture already picks upvulture_whitelist.pywhen it walks., so passing it explicitly changes nothing:The mutation is behaviour-preserving, so no test should fail. The line keeps an explanatory comment rather than being removed — it is harmless, and changing the shared script for zero behavioural gain would need a version bump to reach six consumers. A mutation that no test catches is either a coverage gap or proof the code does nothing; the two are not distinguishable without checking whether behaviour changed.
EXCLUDE=""is caught for the wrong reason. vulture'sprepare_pattern()turns an empty exclude into**, which matches everything — so all four failures share one symptom (nothing gets scanned at all) rather than four tests independently validating exclude semantics. Losing a single token such asbuildhas a real, independent effect and would not be caught. Left open deliberately and recorded here so it is not rediscovered as news.Consumer impact
None. All six pin
dead-code-v1.0; this lands onmainahead of that tag and reaches them at the next deliberate version bump. No tag is created here.🤖 Generated with Claude Code
https://claude.ai/code/session_013ZcazeseXVpu8XrYA2tE1V