diff --git a/.github/workflows/dead-code-hook.yml b/.github/workflows/dead-code-hook.yml index 9fb7940..8befa24 100644 --- a/.github/workflows/dead-code-hook.yml +++ b/.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 diff --git a/check-dead-code.sh b/check-dead-code.sh index 5ad1db0..a2a71a6 100755 --- a/check-dead-code.sh +++ b/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)" diff --git a/tests/test-check-dead-code.sh b/tests/test-check-dead-code.sh index 606b7f8..b16d5e9 100755 --- a/tests/test-check-dead-code.sh +++ b/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