mirror of
https://github.com/kuhyx/diet-guard.git
synced 2026-11-10 20:27:11 +01:00
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
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
124 lines
4.5 KiB
Python
Executable File
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())
|