diet-guard/scripts/check_patch_targets.py
Krzysztof kuhy Rudnicki cb23f3c09d
Some checks are pending
file-length / file length <= 250 lines (push) Waiting to run
pre-commit / pre-commit (push) Waiting to run
Tests / test (3.10) (push) Waiting to run
Tests / test (3.11) (push) Waiting to run
Tests / test (3.12) (push) Waiting to run
release-apk / release (APK) (push) Waiting to run
Fix check_patch_targets' own fail-open: put the repo root on sys.path
The pre-commit CI job reported all 19 patch targets stale, including ones no
commit in this sweep touched -- the tell that the checker, not the targets,
was broken.

Running it as `python3 scripts/check_patch_targets.py` puts *scripts/* on
sys.path[0], not the repo root. `diet_guard` is not installed in the runner's
environment (CI installs requirements.txt only), so every `import_module` hit
the resolver's `except ImportError: continue` and every target reported
unresolvable. It passed locally because the dev venv has the package on the
path.

That is precisely the fail-open shape this script exists to prevent, in the
script itself: a green-or-red signal that reflects the environment rather than
the code. Two fixes:

- put the repo root on sys.path explicitly, so the checker resolves the same
  way whatever cwd or interpreter invokes it;
- import the package once up front and fail with a distinct message if that
  fails, so "the package will not import" can never again masquerade as
  "every patch target moved".

Reproduced in a clean venv built from requirements.txt: 19 stale before, exit 0
after, and still exactly 1 stale when a target is genuinely broken
(verified by renaming FOOD_LOG_FILE's target).

Verified: 708 Python tests / 100% branch coverage, pre-commit clean, checker
exit 0 under both the dev venv and the clean venv.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RA9ynjqXPnexRQnR5oAuCC
2026-08-15 21:27:24 +02:00

124 lines
4.5 KiB
Python
Executable File

#!/usr/bin/env python3
"""Fail if a test patches a dotted path that no longer resolves.
``unittest.mock.patch("diet_guard._state.FOOD_LOG_FILE", ...)`` fails *open*.
Once a refactor moves ``FOOD_LOG_FILE`` to another module, the old path still
imports, the patch redirects nothing, and the test keeps passing -- while the
suite writes to the real ``~/.local/share/diet_guard``, which
``diet-guard-sync.timer`` then publishes to every other device.
That is why this gate exists rather than a note in CLAUDE.md: the failure it
guards against is silent and green, so only a static check catches it.
Usage:
python3 scripts/check_patch_targets.py
"""
from __future__ import annotations
import ast
import importlib
import pathlib
import sys
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from collections.abc import Iterator
#: Only paths into this package are checked; a patch of `requests` or `tk` is
#: not ours to validate.
PACKAGE = "diet_guard"
TESTS_ROOT = pathlib.Path(PACKAGE) / "tests"
# Running this as `python3 scripts/check_patch_targets.py` puts *scripts/* on
# sys.path, not the repo root -- so `import diet_guard` fails unless the
# package happens to be installed. It is not, on a CI runner that only
# installs requirements.txt. Without this the resolver's `except ImportError`
# swallowed every lookup and the hook reported all 19 targets as stale, which
# is the exact fail-open shape this script exists to prevent.
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))
def patch_targets(root: pathlib.Path) -> Iterator[tuple[pathlib.Path, int, str]]:
"""Yield ``(file, lineno, dotted_target)`` for every literal patch target."""
for path in sorted(root.rglob("*.py")):
tree = ast.parse(path.read_text(), str(path))
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
name = getattr(node.func, "attr", None) or getattr(node.func, "id", None)
if name != "patch" or not node.args:
continue
first = node.args[0]
if (
isinstance(first, ast.Constant)
and isinstance(first.value, str)
and first.value.startswith(PACKAGE)
):
yield path, node.lineno, first.value
def resolves(dotted: str) -> bool:
"""Whether ``dotted`` names a real attribute reachable from a real module.
Tries the longest importable module prefix first, then walks the remaining
segments as attributes -- the same resolution order ``mock.patch`` uses.
"""
parts = dotted.split(".")
for split in range(len(parts), 0, -1):
try:
module = importlib.import_module(".".join(parts[:split]))
except ImportError:
continue
obj: object = module
for attr in parts[split:]:
if not hasattr(obj, attr):
return False
obj = getattr(obj, attr)
return True
return False
def main() -> int:
"""Report every unresolvable patch target; return 1 if any were found."""
if not TESTS_ROOT.is_dir():
sys.stderr.write(f"Error: {TESTS_ROOT} not found; run from the repo root.\n")
return 1
# Distinguish "this target moved" from "the package will not import at
# all". Without this the second case masquerades as the first: every
# target reports stale at once, which reads like a catastrophic refactor
# and hides the real error (a missing dependency, a syntax error).
try:
importlib.import_module(PACKAGE)
except ImportError as exc:
sys.stderr.write(
f"Error: cannot import {PACKAGE} ({exc}); every patch target would "
f"be reported stale. Fix the import first -- this is an environment "
f"problem, not a stale patch target.\n",
)
return 1
stale = [
(path, lineno, target)
for path, lineno, target in patch_targets(TESTS_ROOT)
if not resolves(target)
]
for path, lineno, target in stale:
sys.stderr.write(f"{path}:{lineno}: patch target does not resolve: {target}\n")
if stale:
plural = "" if len(stale) == 1 else "s"
sys.stderr.write(
f"\n{len(stale)} stale patch target{plural}. A patch that does not "
f"resolve silently redirects nothing -- the test passes while writing "
f"to real user state. Update the path to where the name now lives.\n"
)
return 1
return 0
if __name__ == "__main__":
raise SystemExit(main())