diff --git a/.github/scripts/check_coverage_map_health.py b/.github/scripts/check_coverage_map_health.py index f4dddc983..f8307b524 100644 --- a/.github/scripts/check_coverage_map_health.py +++ b/.github/scripts/check_coverage_map_health.py @@ -1,11 +1,10 @@ """Fail loudly if the committed coverage map is stale or under-covers. Used by coverage-health.yml.""" import datetime -import subprocess import sys from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "toolchain")) -from mfc.test.coverage import COVERAGE_MAP_PATH, load_map, map_health # noqa: E402 +from mfc.test.coverage import COVERAGE_MAP_PATH, _git, load_map, map_health # noqa: E402 from mfc.test.cases import list_cases # noqa: E402 (returns the current test list) MAX_AGE_DAYS = 10 @@ -40,7 +39,7 @@ def verified_sha(cwd=None): caller must read that as undeterminable and fall back to the wall-clock age rule, not as a failure -- an absent ref is not evidence of a broken refresh. """ - rev = subprocess.run(["git", "rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], capture_output=True, text=True, check=False, cwd=cwd) + rev = _git(["rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], cwd) return rev.stdout.strip() or None @@ -53,10 +52,10 @@ def verified_after_last_change(git_sha, cwd=None): """ if not git_sha: return None - last = subprocess.run(["git", "log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], capture_output=True, text=True, check=False, cwd=cwd) + last = _git(["log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], cwd) if last.returncode != 0 or not last.stdout.strip(): return None # shallow clone or no such commit -> fall back to the age rule - ancestor = subprocess.run(["git", "merge-base", "--is-ancestor", last.stdout.strip(), git_sha], capture_output=True, check=False, cwd=cwd) + ancestor = _git(["merge-base", "--is-ancestor", last.stdout.strip(), git_sha], cwd) return {0: True, 1: False}.get(ancestor.returncode) # anything else -> None (unknown sha, shallow history) diff --git a/toolchain/mfc/test/coverage.py b/toolchain/mfc/test/coverage.py index 26d664aff..e279fc4b9 100644 --- a/toolchain/mfc/test/coverage.py +++ b/toolchain/mfc/test/coverage.py @@ -216,8 +216,14 @@ def select_tests(cases, coverage_map, changed_files): return to_run, skipped, f"selected {len(to_run)}/{len(cases)} by coverage overlap" +def _env_without_git(): + # Git exports GIT_DIR and GIT_INDEX_FILE to hooks, and neither cwd nor `git -C` overrides them: + # under the pre-commit hook every call below would otherwise act on the committing repository. + return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + + def _git(args, cwd, timeout=60): - return subprocess.run(["git", *args], capture_output=True, text=True, cwd=cwd, timeout=timeout, check=False) + return subprocess.run(["git", *args], capture_output=True, text=True, cwd=cwd, timeout=timeout, check=False, env=_env_without_git()) def _merge_base(cwd, branch): diff --git a/toolchain/mfc/test/test_coverage_unit.py b/toolchain/mfc/test/test_coverage_unit.py index 24f52ec1a..5e9ad49e3 100644 --- a/toolchain/mfc/test/test_coverage_unit.py +++ b/toolchain/mfc/test/test_coverage_unit.py @@ -6,7 +6,7 @@ from pathlib import Path from unittest.mock import patch -from mfc.test.coverage import canonicalize_param_paths, entries_equal, format_summary, get_changed_files, is_always_run_all, load_map, map_health, param_hash, save_map, select_tests +from mfc.test.coverage import _env_without_git, canonicalize_param_paths, entries_equal, format_summary, get_changed_files, is_always_run_all, load_map, map_health, param_hash, save_map, select_tests def test_param_hash_is_order_independent(): @@ -462,17 +462,6 @@ def test_health_fails_immediately_when_no_refresh_ran_since_last_source_change() CHANGED_SCRIPT = Path(__file__).resolve().parents[3] / ".github" / "scripts" / "coverage_map_changed.py" -def _env_without_git(): - """The environment minus every GIT_* variable. - - `git -C ` changes directory but does NOT override an inherited GIT_DIR or - GIT_INDEX_FILE. Git exports both when it runs a hook, and MFC's pre-commit hook runs - precheck, which runs this suite -- so without this scrub the commits below are made - against the real repository instead of the throwaway one. - """ - return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} - - def _repo_with_committed_map(d, entries): """A throwaway git repo whose HEAD holds `entries` as the coverage map.""" repo = Path(d)