Harden the dead-code gate's own test coverage

.github/workflows/dead-code-hook.yml

@@ -29,7 +29,38 @@ jobs:
       # in .pre-commit-hooks.yaml — that would only surface in a consumer
       # repo, after the tag is published. try-repo runs the hook exactly as
       # a consumer's pre-commit would, without needing a published tag.
+      #
+      # `ops` itself has zero .py files, so pointing try-repo at it proves
+      # nothing — it can't tell "ran and found nothing" from "no-opped". Both
+      # steps below build a throwaway git repo (try-repo needs a real repo
+      # with a resolvable rev) with planted content and check the hook's
+      # actual verdict on it.
       - run: pip install pre-commit==4.6.2
 
-      - name: Exercise the hook manifest through pre-commit
-        run: pre-commit try-repo . dead-code --all-files
+      - name: Exercise the hook manifest through pre-commit (positive control)
+        run: |
+          set -euo pipefail
+          tmp="$(mktemp -d)"
+          cd "$tmp"
+          git init -q
+          git config user.email test@example.com
+          git config user.name test
+          printf 'def used():\n    return 1\n\n\ndef planted_orphan():\n    return 2\n\n\nprint(used())\n' > main.py
+          git add -A
+          if pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files 2>&1 | tee out.txt; then
+              echo "FAIL: hook did not catch planted dead code" >&2
+              exit 1
+          fi
+          grep -q planted_orphan out.txt || { echo "FAIL: hook ran but did not name the finding" >&2; exit 1; }
+
+      - name: Exercise the hook manifest through pre-commit (negative control)
+        run: |
+          set -euo pipefail
+          tmp="$(mktemp -d)"
+          cd "$tmp"
+          git init -q
+          git config user.email test@example.com
+          git config user.name test
+          printf 'def used():\n    return 1\n\n\nprint(used())\n' > main.py
+          git add -A
+          pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files

check-dead-code.sh

@@ -47,7 +47,7 @@ for arg in "$@"; do
         *)      TARGET="$arg" ;;
     esac
 done
-cd "$TARGET"
+cd "$TARGET" || { echo "check-dead-code: cannot enter target: $TARGET" >&2; exit 2; }
 
 # Fail closed (ops README convention 4: fail loudly). A missing tool is not a
 # clean scan, and this gate used to report one as the other: the shell's
@@ -62,6 +62,10 @@ EXCLUDE=".venv,scripts,htmlcov,__pycache__,node_modules,build,dist,*.egg-info"
 # Framework-dispatched handlers: Flask (@bp/@app) and FastAPI (@router/@app).
 DECORATORS="@app.*,@bp.*,@router.*"
 WHITELIST=""
+# Redundant in practice: vulture already picks this up when it walks ".", so
+# passing it explicitly changes nothing (verified against vulture 2.14). Kept
+# as an explicit statement of intent; removing it is safe but would need a
+# version bump to reach consumers.
 [ -f vulture_whitelist.py ] && WHITELIST="vulture_whitelist.py"
 
 stderr_file="$(mktemp)"

tests/test-check-dead-code.sh

@@ -59,17 +59,30 @@ mkbroken() {
     printf '%s' "$dir"
 }
 
+# A decorated, otherwise-unreferenced handler — the Flask/FastAPI dispatch
+# pattern --ignore-decorators exists for (Tower-Finder's backend is exactly
+# this shape). vulture can't see the framework's dispatch wiring, so without
+# the flag this reads as dead.
+mkdirty_decorated() {
+    local dir; dir="$(mktemp -d)"
+    printf 'def used():\n    return 1\n\n\n@app.route("/foo")\ndef handler_endpoint():\n    return "hi"\n\n\nprint(used())\n' >"$dir/main.py"
+    printf '%s' "$dir"
+}
+
 # --- tests that run whether or not vulture is installed ---------------------
 
 t_missing_vulture_fails_closed() {
     local dir out rc
     dir="$(mkclean)"
     out="$(cd "$dir" && PATH="$NO_VULTURE_PATH" bash "$SCRIPT" 2>&1)"; rc=$?
-    if [ "$rc" -eq 0 ]; then
-        bad "missing vulture exits non-zero" \
-            "exited 0 — reported a scan that never happened"
+    # Assert the exact code, not just non-zero: 127 is this gate's whole
+    # fail-closed contract, and other paths (findings, bad usage) also exit
+    # non-zero, so a loose check can't tell them apart.
+    if [ "$rc" -eq 127 ]; then
+        ok "missing vulture exits exactly 127"
     else
-        ok "missing vulture exits non-zero (got $rc)"
+        bad "missing vulture exits exactly 127" \
+            "exited $rc — reported a scan that never happened, or used the wrong code"
     fi
     case "$out" in
         *vulture*) ok "missing vulture names the tool" ;;
@@ -78,6 +91,22 @@ t_missing_vulture_fails_closed() {
     rm -rf "$dir"
 }
 
+# cd failure must not be mistaken for "dead code found" (both exit 1 under a
+# bare `cd`). A bad target is a usage error, so it gets exit 2 — the code
+# this script already uses for other usage errors — and must name the target
+# so a typo'd path in a consumer's pre-commit config is diagnosable from CI
+# output alone.
+t_bad_target_exits_2() {
+    local target out rc
+    target="$(mktemp -u)/does-not-exist"
+    out="$(PATH="$NO_VULTURE_PATH" bash "$SCRIPT" "$target" 2>&1)"; rc=$?
+    if [ "$rc" -eq 2 ] && [[ "$out" == *"$target"* ]]; then
+        ok "nonexistent target exits 2 and names the target"
+    else
+        bad "nonexistent target exits 2 and names the target" "rc=$rc out=$out"
+    fi
+}
+
 t_unknown_option_rejected() {
     local dir out rc
     dir="$(mkclean)"
@@ -155,6 +184,60 @@ t_vulture_failure_propagates() {
     rm -rf "$dir"
 }
 
+# vulture_whitelist.py pickup (check-dead-code.sh:65) is what keeps every
+# consumer repo green — all six have one. Both halves are required: checking
+# only "whitelisted symbol passes" would also pass if the scan found nothing
+# at all, so this first proves the fixture fails without a whitelist, then
+# adds a whitelist for exactly that symbol and proves it now passes.
+t_whitelist_suppresses_finding() {
+    local dir out rc
+    dir="$(mkdirty)"
+    out="$(cd "$dir" && bash "$SCRIPT" 2>&1)"; rc=$?
+    if [ "$rc" -eq 1 ] && [[ "$out" == *"orphan_function"* ]]; then
+        ok "whitelist test: unwhitelisted dead code fails first"
+    else
+        bad "whitelist test: unwhitelisted dead code fails first" "rc=$rc out=$out"
+    fi
+
+    printf 'from main import orphan_function\norphan_function\n' >"$dir/vulture_whitelist.py"
+    out="$(cd "$dir" && bash "$SCRIPT" 2>&1)"; rc=$?
+    if [ "$rc" -eq 0 ] && [[ "$out" == *"no dead code found"* ]]; then
+        ok "vulture_whitelist.py suppresses the whitelisted symbol"
+    else
+        bad "vulture_whitelist.py suppresses the whitelisted symbol" "rc=$rc out=$out"
+    fi
+    rm -rf "$dir"
+}
+
+# --ignore-decorators (check-dead-code.sh:63) is what stops every Flask/
+# FastAPI route handler reading as dead — Tower-Finder's backend is exactly
+# this shape and depends on it directly. 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 establish the finding is real; the second half
+# runs the actual script and proves the flag suppresses it. As with the
+# whitelist test, both halves are required — the second half alone would
+# also pass if the scan found nothing at all.
+t_ignore_decorators_suppresses_handlers() {
+    local dir out rc
+    dir="$(mkdirty_decorated)"
+    out="$(cd "$dir" && vulture . --min-confidence 60 \
+        --exclude ".venv,scripts,htmlcov,__pycache__,node_modules,build,dist,*.egg-info" 2>&1)"; rc=$?
+    if [ "$rc" -eq 3 ] && [[ "$out" == *"handler_endpoint"* ]]; then
+        ok "decorators test: scan without --ignore-decorators finds the handler dead"
+    else
+        bad "decorators test: scan without --ignore-decorators finds the handler dead" "rc=$rc out=$out"
+    fi
+
+    out="$(cd "$dir" && bash "$SCRIPT" 2>&1)"; rc=$?
+    if [ "$rc" -eq 0 ] && [[ "$out" == *"no dead code found"* ]]; then
+        ok "--ignore-decorators suppresses the decorated handler"
+    else
+        bad "--ignore-decorators suppresses the decorated handler" "rc=$rc out=$out"
+    fi
+    rm -rf "$dir"
+}
+
 t_positional_target_scopes_scan() {
     local root out rc
     root="$(mktemp -d)"
@@ -175,6 +258,7 @@ t_positional_target_scopes_scan() {
 echo "fail-closed:"
 t_missing_vulture_fails_closed
 t_unknown_option_rejected
+t_bad_target_exits_2
 
 echo "behaviour:"
 if command -v vulture >/dev/null 2>&1; then
@@ -184,6 +268,8 @@ if command -v vulture >/dev/null 2>&1; then
     t_test_files_filtered
     t_positional_target_scopes_scan
     t_vulture_failure_propagates
+    t_whitelist_suppresses_finding
+    t_ignore_decorators_suppresses_handlers
 elif [ "${REQUIRE_VULTURE:-}" = "1" ]; then
     # A skipped suite reporting success is the same class of bug as the gate
     # this repo hosts: silence read as a clean result. CI sets