Fix check_patch_targets' own fail-open: put the repo root on sys.path
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
This commit is contained in:
Krzysztof Rudnicki 2026-08-15 21:27:24 +02:00
parent 5125a98980
commit cb23f3c09d
2 changed files with 23 additions and 1 deletions

View File

@ -2,7 +2,7 @@ name: diet_guard_app
description: "Companion phone app for diet_guard: log meals on the go."
publish_to: 'none'
version: 1.0.28+1
version: 1.0.29+1
environment:
sdk: ^3.12.2

View File

@ -31,6 +31,14 @@ 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."""
@ -78,6 +86,20 @@ def main() -> int:
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)