Files
hermes-agent/tests/scripts/test_code_health_cli.py
Teknium a6274687fd fix(code-health): parse ENFORCEMENT with ast and read it from the main tip locally
unsupportedpastels m3: (a) the switch was found by a regex that accepted only the
exact `ENFORCEMENT = "..."` spelling, so `ENFORCEMENT: str = "off"` silently stayed
blocking; (b) local runs and both hooks read it from the merge-base with
origin/main, so a flip on main reached a branch only after a rebase, contrary to
"CI and local hooks alike".

(a) parse_switch() reads the module-level plain or annotated assignment with ast
(last one wins, as at runtime); missing, unparseable or unknown values are
blocking. (b) Without --base, the base is still the merge-base of the head (a
--head commit, else HEAD) but the switch comes from origin/main when it resolves,
falling back to the base. An explicit --base pins both (CI passes HEAD^1, the
target tip). scripts/check --commit now hands the commit itself to the ratchet so
it derives that commit's merge-base. The head under test never supplies the switch.
(c) A test pins enforcement() on this tree to config.ENFORCEMENT.
2026-10-04 21:29:39 -07:00

260 lines
11 KiB
Python

"""scripts/check runner, git hooks, CLI output modes and regex-rule scope, on real git repos.
Fixture repos carry a copy of this checkout's engine (scripts/check, scripts/code_health, the
guard scripts, pyproject's ruff pin) so the runner and the hooks execute exactly as they would
in a clone, against tiny trees that keep every check fast.
"""
from __future__ import annotations
import json
import os
import shutil
import subprocess
import sys
from collections.abc import Mapping
from pathlib import Path
import pytest
from scripts.code_health import cli
from scripts.code_health.config import ENFORCEMENT
REPO = Path(__file__).resolve().parents[2]
_GUARD_SCRIPTS = (
"scripts/check-windows-footguns.py", "scripts/check_bash_shebangs.py",
"scripts/check_no_tmp_literals.py", "scripts/check_config_yaml_writers.py",
"scripts/ci/check_os_marker_fakes.py", "scripts/check-case-collisions.py",
"scripts/ci/check_lazy_deps_imports.py", "scripts/ci/check_profile_archive_boundary.py",
)
_ENGINE = ("scripts/check", "scripts/ci/profile_scope_patterns.json", *_GUARD_SCRIPTS)
_LEGACY = "def legacy(x):\n" + "".join(f" if x == {i}:\n return {i}\n" for i in range(21))
_GROWN = _LEGACY + " if x == 99:\n return 99\n"
_ENV_COPY = "import os\n\n\ndef child_env():\n env = os.environ.copy()\n return env\n"
_SWITCH = "scripts/code_health/config.py"
def _env() -> dict[str, str]:
"""The caller's env minus anything that would point git at another repo or config."""
env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
env.update(GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_NOSYSTEM="1")
return env
def _sh(cwd: Path, *argv: str, env: dict[str, str] | None = None) -> subprocess.CompletedProcess:
return subprocess.run(list(argv), cwd=cwd, env=env or _env(), capture_output=True, text=True,
encoding="utf-8", errors="replace", stdin=subprocess.DEVNULL,
timeout=600, check=False)
def _git(repo: Path, *args: str, env: dict[str, str] | None = None) -> str:
proc = _sh(repo, "git", *args, env=env)
assert proc.returncode == 0, (args, proc.stdout, proc.stderr)
return proc.stdout.strip()
def _write(repo: Path, files: Mapping[str, str | None]) -> None:
for rel, text in files.items():
path = repo / rel
if text is None:
path.unlink()
else:
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(text, encoding="utf-8")
def _commit(repo: Path, files: Mapping[str, str | None], env: dict[str, str] | None = None) -> str:
_write(repo, files)
_git(repo, "add", "--all", "--", *files)
_git(repo, "commit", "-q", "-m", "step", env=env)
return _git(repo, "rev-parse", "HEAD")
def _init(path: Path) -> Path:
path.mkdir(parents=True)
_git(path, "init", "-q", "-b", "main")
_git(path, "config", "user.email", "t@example.com")
_git(path, "config", "user.name", "t")
(path / ".gitignore").write_text(".venv\n", encoding="utf-8")
shutil.copy(REPO / "pyproject.toml", path / "pyproject.toml")
return path
def _add_engine(repo: Path) -> list[str]:
for rel in _ENGINE:
(repo / rel).parent.mkdir(parents=True, exist_ok=True)
shutil.copy2(REPO / rel, repo / rel)
shutil.copytree(REPO / "scripts/code_health", repo / "scripts/code_health",
ignore=shutil.ignore_patterns("__pycache__"), dirs_exist_ok=True)
return ["scripts"]
def _engine_repo(tmp_path: Path) -> Path:
"""A repo whose first commit already carries the checker, with one clean module."""
repo = _init(tmp_path / "repo")
_write(repo, {"pkg/a.py": "def a():\n return 1\n"})
_add_engine(repo)
_git(repo, "add", "--all", "--", ".gitignore", "pyproject.toml", "pkg", "scripts")
_git(repo, "commit", "-q", "-m", "base")
return repo
def _check(repo: Path, *args: str, env: dict[str, str] | None = None) -> subprocess.CompletedProcess:
return _sh(repo, sys.executable, str(repo / "scripts/check"), *args, env=env)
def _ratchet_repo(tmp_path: Path, switch: str | None = None) -> tuple[Path, str]:
repo = _init(tmp_path / "repo")
(repo / "scripts/ci").mkdir(parents=True)
shutil.copy2(REPO / "scripts/ci/profile_scope_patterns.json", repo / "scripts/ci/")
files: dict[str, str | None] = {"pkg/a.py": _LEGACY, **({_SWITCH: switch} if switch else {})}
_write(repo, files)
_git(repo, "add", "--all", "--", ".gitignore", "pyproject.toml", "scripts", "pkg")
_git(repo, "commit", "-q", "-m", "base")
return repo, _git(repo, "rev-parse", "HEAD")
# --- F22: selectors are validated before anything runs ----------------------------------------
@pytest.mark.parametrize("selector", ["heath", ",", "", "heath,rof", "health,rof"])
def test_unknown_or_empty_selector_is_a_usage_error(selector):
proc = _sh(REPO, sys.executable, str(REPO / "scripts/check"), "--only", selector)
assert proc.returncode == 2, proc.stdout + proc.stderr
assert "health" in proc.stderr and "shebangs" in proc.stderr # lists the valid names
assert "checks, ok" not in proc.stdout
def test_valid_selector_runs_its_check_and_keeps_its_status(tmp_path):
repo = _engine_repo(tmp_path)
_write(repo, {"pkg/c.py": _ENV_COPY})
_git(repo, "add", "--", "pkg/c.py")
proc = _check(repo, "--staged", "--only", "health,shebangs", "--base", "HEAD")
assert proc.returncode == 1 and "2 checks, FAILED: health" in proc.stdout, proc.stdout
# --- JSON output on every return path ----------------------------------------------------------
@pytest.mark.parametrize("case, switch, files, code, findings", [
("no measured change", None, {"README.md": "docs\n"}, 0, 0),
("enforcement off", 'ENFORCEMENT = "off"\n', {"pkg/a.py": _GROWN}, 0, 0),
("findings", None, {"pkg/a.py": _GROWN}, 1, 1),
("clean measured diff", None, {"pkg/b.py": "def b():\n return 2\n"}, 0, 0),
])
def test_json_stdout_is_json_on_every_path(tmp_path, capsys, case, switch, files, code, findings):
repo, base = _ratchet_repo(tmp_path, switch)
head = _commit(repo, files)
assert cli.run(repo, base, head, as_json=True) == code, case
out, err = capsys.readouterr()
data = json.loads(out)
assert isinstance(data, list) and len(data) == findings, (case, out)
if case in ("no measured change", "enforcement off"):
assert "code health:" in err # the human explanation moves to stderr
# --- m3: the ENFORCEMENT switch ---------------------------------------------------------------
@pytest.mark.parametrize("switch, code", [
('ENFORCEMENT: str = "off"\n', 0),
('ENFORCEMENT: str = "advisory"\n', 0),
('ENFORCEMENT = "warn"\n', 1), # an unknown value is not a way to relax the check
('ENFORCEMENT = "off" # until the burn-down lands\n', 0),
])
def test_enforcement_switch_parses_annotated_and_unknown_values(tmp_path, capsys, switch, code):
repo, base = _ratchet_repo(tmp_path, switch)
head = _commit(repo, {"pkg/a.py": _GROWN})
assert cli.run(repo, base, head) == code, capsys.readouterr().out
def test_enforcement_reads_this_trees_own_switch():
assert cli.enforcement(REPO, "HEAD") == ENFORCEMENT
text = (REPO / _SWITCH).read_text(encoding="utf-8")
assert cli.parse_switch(text) == ENFORCEMENT
def _branch_behind_main(tmp_path: Path, main_switch: str, branch_files: dict[str, str]) -> Path:
repo, base = _ratchet_repo(tmp_path, 'ENFORCEMENT = "blocking"\n')
_git(repo, "checkout", "-q", "-b", "feature")
_commit(repo, branch_files)
_git(repo, "checkout", "-q", "main")
tip = _commit(repo, {_SWITCH: main_switch, "NEWS.md": "main moved on\n"})
_git(repo, "update-ref", "refs/remotes/origin/main", tip)
_git(repo, "reset", "-q", "--hard", base) # local main is stale; only origin/main moved
_git(repo, "checkout", "-q", "feature")
return repo
def test_local_runs_take_the_switch_from_the_main_tip(tmp_path, monkeypatch, capsys):
repo = _branch_behind_main(tmp_path, 'ENFORCEMENT = "advisory"\n', {"pkg/a.py": _GROWN})
monkeypatch.chdir(repo)
assert cli.main([]) == 0, capsys.readouterr().out # the flip on main reaches the branch
head = _git(repo, "rev-parse", "HEAD")
assert cli.main(["--head", head]) == 0
out = capsys.readouterr().out
assert "CC 23 > 22" in out and "advisory mode" in out
def test_a_branch_cannot_relax_its_own_switch(tmp_path, monkeypatch, capsys):
grown_and_off = {"pkg/a.py": _GROWN, _SWITCH: 'ENFORCEMENT = "off"\n'}
repo = _branch_behind_main(tmp_path, 'ENFORCEMENT = "blocking"\n', grown_and_off)
monkeypatch.chdir(repo)
assert cli.main([]) == 1, capsys.readouterr().out
# --- F21: profile regex rules see executable code, not prose -----------------------------------
_FUNC = "import os\n\n\ndef build(cmd):\n{} return cmd\n"
@pytest.mark.parametrize("body, blocks", [
(" # Avoid env = os.environ.copy(); use the scoped builder.\n", False),
(' """Avoid env = os.environ.copy(); use the scoped builder."""\n', False),
(' """Builder.\n\n Never env = os.environ.copy() here.\n """\n', False),
(" # don't read os.getenv(\"DISCORD_TOKEN\") directly\n", False),
(" cmd = [cmd] # not os.environ.get('SLACK_TOKEN')\n", False),
(" 'inert example: env = os.environ.copy()'\n", False),
# positive controls: the real operations, including a literal env-key argument
(" env = os.environ.copy()\n cmd = (cmd, env)\n", True),
(" token = os.getenv(\"DISCORD_TOKEN\")\n cmd = (cmd, token)\n", True),
(" env = dict(os.environ) # copy it\n cmd = (cmd, env)\n", True),
])
def test_profile_regex_rules_ignore_comments_and_docstrings(tmp_path, capsys, body, blocks):
repo, base = _ratchet_repo(tmp_path)
head = _commit(repo, {"pkg/c.py": _FUNC.format(body)})
code = cli.run(repo, base, head)
out = capsys.readouterr().out
assert code == (1 if blocks else 0), out
assert ("PS-P05" in out or "PS-P06" in out) == blocks, out
# --- allow comments may list several rules ----------------------------------------------------
@pytest.mark.parametrize("rules", ["BLE001 S110", "BLE001, S110", "BLE001,S110"])
def test_allow_comment_lists_rules_by_comma_or_space(tmp_path, capsys, rules):
repo, base = _ratchet_repo(tmp_path)
swallow = ("def other():\n try:\n pass\n"
f" except Exception: # health: allow {rules} -- boundary\n pass\n")
head = _commit(repo, {"pkg/b.py": swallow})
assert cli.run(repo, base, head) == 0, capsys.readouterr().out
# --- measurement tooling failures are errors, not findings ------------------------------------
def test_tooling_failure_exits_2_without_a_traceback(tmp_path, monkeypatch, capsys):
repo, base = _ratchet_repo(tmp_path)
lock = {"packages": {"node_modules/typescript": {"version": "0.0.0-not-published"}}}
_commit(repo, {"package-lock.json": json.dumps(lock), "web/a.ts": "export const a = 1;\n"})
if not shutil.which("npm") or not shutil.which("node"):
pytest.skip("needs node + npm to reach the npm install path")
monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache"))
monkeypatch.setenv("npm_config_offline", "true") # a real npm failure, without the network
monkeypatch.chdir(repo)
assert cli.main(["--base", base, "--head", "HEAD"]) == 2
err = capsys.readouterr().err
assert err.startswith("code health:") and "npm" in err