-
Notifications
You must be signed in to change notification settings - Fork 0
hooks: full-suite pre-push test gate (both unittest roots, git-env sanitized) #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,52 @@ | ||
| #!/bin/bash | ||
| # Pre-push hook wrapper | ||
| # Pre-push hook wrapper. | ||
| # | ||
| # Gate 1 — full-suite test gate: run BOTH unittest roots (tests/ and scripts/, | ||
| # mirroring CI's exact discovery invocations) before any push from a feature | ||
| # branch. Motivated by ledger pattern partial.suite.run.hides.ci.failure: a | ||
| # session ran one complete root not knowing the second existed and pushed a | ||
| # red-CI commit. Both roots run in well under a minute in this repository. | ||
| # - Opt out of the TEST GATE ONLY with exactly AGENT_COLLAB_PREPUSH_TESTS=0 | ||
| # (fail-closed: unset, empty, or any other value still runs the tests). | ||
| # - An affirmatively detected 'main' branch skips the test gate (direct main | ||
| # pushes are branch-protection-blocked anyway); detached HEAD or detection | ||
| # failure runs the tests. | ||
| # | ||
| # Gate 2 — compliance-trace check (scripts/hook-pre-push.py). Always runs, | ||
| # including under the test-gate opt-out; exec'd so the hook's stdin ref data | ||
| # stays available to it. | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| python3 "$SCRIPT_DIR/../scripts/hook-pre-push.py" | ||
| REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" | ||
|
|
||
| run_tests=1 | ||
| if [ "${AGENT_COLLAB_PREPUSH_TESTS-}" = "0" ]; then | ||
| echo "pre-push: WARNING — test gate SKIPPED (AGENT_COLLAB_PREPUSH_TESTS=0); compliance check still runs" >&2 | ||
| run_tests=0 | ||
| fi | ||
|
|
||
| branch="$(git -C "$REPO_ROOT" symbolic-ref --quiet --short HEAD 2>/dev/null)" | ||
| if [ "$branch" = "main" ]; then | ||
| run_tests=0 | ||
| fi | ||
|
|
||
| if [ "$run_tests" = "1" ]; then | ||
| echo "pre-push: running both unittest roots (set AGENT_COLLAB_PREPUSH_TESTS=0 to skip the test gate)" >&2 | ||
| # git exports GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE into hook processes; | ||
| # tests that spawn git in temp directories then silently target THIS repo | ||
| # and fail (23 errors observed under a linked worktree's absolute GIT_DIR). | ||
| # Sanitize the suite subshells only; the exec'd compliance checker below | ||
| # operates on this repo and keeps the hook environment. | ||
| ( cd "$REPO_ROOT" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX && \ | ||
| python3 -m unittest discover -s tests -t . -q < /dev/null ) || { | ||
|
Comment on lines
+40
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the working tree contains uncommitted changes, these commands test the mutable checkout rather than the Useful? React with 👍 / 👎. |
||
| echo "pre-push: BLOCKED — tests/ suite failed" >&2 | ||
| exit 1 | ||
| } | ||
| ( cd "$REPO_ROOT" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX && \ | ||
| python3 -m unittest discover -s scripts -p 'test_*.py' -q < /dev/null ) || { | ||
| echo "pre-push: BLOCKED — scripts/ suite failed" >&2 | ||
| exit 1 | ||
| } | ||
| fi | ||
|
|
||
| exec python3 "$SCRIPT_DIR/../scripts/hook-pre-push.py" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ### Changed | ||
|
|
||
| - The tracked `.githooks/pre-push` wrapper now runs BOTH unittest roots | ||
| (`tests/` and `scripts/`, mirroring CI's exact discovery invocations) as a | ||
| fail-fast gate before the existing compliance-trace check. Fail-closed | ||
| opt-out for the test gate only via exactly `AGENT_COLLAB_PREPUSH_TESTS=0` | ||
| (loud skip warning; compliance check always runs, now `exec`'d so the | ||
| hook's stdin ref data reaches it); an affirmatively detected `main` branch | ||
| skips the test gate while detached HEAD or detection failure runs it. | ||
| Wrapper-level tests added (`scripts/test_hook_pre_push_wrapper.py`, 8 | ||
| cases, including sanitization of git's exported hook environment — | ||
| GIT_DIR et al. — from the suite subshells, which otherwise poisons | ||
| tests that spawn git in temp directories). Repository tooling only — no distributed content, no version bump. | ||
| Motivation: ledger `partial.suite.run.hides.ci.failure` (recurred on #83). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| #!/usr/bin/env python3 | ||
| """Wrapper-level tests for .githooks/pre-push (the test gate + compliance chain). | ||
|
|
||
| The wrapper is exercised as bash against a temp copy of the hook with stub | ||
| `python3` and `git` executables on PATH, so the tests assert the wrapper's | ||
| control flow (which suites ran, in what order, what blocked) without running | ||
| the real suites. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import stat | ||
| import subprocess | ||
| import tempfile | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
| REPO_ROOT = Path(__file__).resolve().parents[1] | ||
| HOOK_SRC = REPO_ROOT / ".githooks" / "pre-push" | ||
|
|
||
|
|
||
| class PrePushWrapperTests(unittest.TestCase): | ||
| def setUp(self) -> None: | ||
| self.tmp = tempfile.TemporaryDirectory() | ||
| self.root = Path(self.tmp.name) | ||
| (self.root / ".githooks").mkdir() | ||
| (self.root / "scripts").mkdir() | ||
| (self.root / "tests").mkdir() | ||
| self.hook = self.root / ".githooks" / "pre-push" | ||
| self.hook.write_text(HOOK_SRC.read_text()) | ||
| self.hook.chmod(self.hook.stat().st_mode | stat.S_IXUSR) | ||
| (self.root / "scripts" / "hook-pre-push.py").write_text("# stub\n") | ||
| self.log = self.root / "calls.log" | ||
| self.bin = self.root / "stubbin" | ||
| self.bin.mkdir() | ||
| self._write_stub( | ||
| "python3", | ||
| '#!/bin/bash\necho "python3 $* GIT_DIR=${GIT_DIR-unset}" >> "$STUB_LOG"\n' | ||
| 'case "$*" in\n' | ||
| ' *"-s tests"*) exit "${FAIL_TESTS_SUITE:-0}";;\n' | ||
| ' *"-s scripts"*) exit "${FAIL_SCRIPTS_SUITE:-0}";;\n' | ||
| " *) exit 0;;\n" | ||
| "esac\n", | ||
| ) | ||
| self._write_stub( | ||
| "git", | ||
| '#!/bin/bash\nif [ -n "${GIT_FAIL:-}" ]; then exit 1; fi\n' | ||
| 'echo "${GIT_BRANCH:-feature-x}"\n', | ||
| ) | ||
|
|
||
| def tearDown(self) -> None: | ||
| self.tmp.cleanup() | ||
|
|
||
| def _write_stub(self, name: str, body: str) -> None: | ||
| p = self.bin / name | ||
| p.write_text(body) | ||
| p.chmod(p.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
| def _run(self, **env_over: str) -> subprocess.CompletedProcess: | ||
| env = dict(os.environ) | ||
| env.pop("AGENT_COLLAB_PREPUSH_TESTS", None) | ||
| env["PATH"] = f"{self.bin}:{env['PATH']}" | ||
| env["STUB_LOG"] = str(self.log) | ||
| env.update(env_over) | ||
| return subprocess.run( | ||
| ["bash", str(self.hook)], | ||
| capture_output=True, | ||
| text=True, | ||
| env=env, | ||
| check=False, | ||
| ) | ||
|
|
||
| def _calls(self) -> list[str]: | ||
| if not self.log.exists(): | ||
| return [] | ||
| return [line for line in self.log.read_text().splitlines() if line] | ||
|
|
||
| def test_both_suites_run_in_order_then_compliance(self) -> None: | ||
| res = self._run() | ||
| self.assertEqual(res.returncode, 0, res.stderr) | ||
| calls = self._calls() | ||
| self.assertEqual(len(calls), 3) | ||
| self.assertIn("-s tests", calls[0]) | ||
| self.assertIn("-s scripts", calls[1]) | ||
| self.assertIn("hook-pre-push.py", calls[2]) | ||
|
|
||
| def test_first_suite_failure_blocks_before_compliance(self) -> None: | ||
| res = self._run(FAIL_TESTS_SUITE="1") | ||
| self.assertEqual(res.returncode, 1) | ||
| self.assertIn("tests/ suite failed", res.stderr) | ||
| self.assertEqual(len(self._calls()), 1) | ||
|
|
||
| def test_second_suite_failure_blocks_before_compliance(self) -> None: | ||
| res = self._run(FAIL_SCRIPTS_SUITE="1") | ||
| self.assertEqual(res.returncode, 1) | ||
| self.assertIn("scripts/ suite failed", res.stderr) | ||
| self.assertEqual(len(self._calls()), 2) | ||
|
|
||
| def test_exact_opt_out_skips_tests_but_runs_compliance(self) -> None: | ||
| res = self._run(AGENT_COLLAB_PREPUSH_TESTS="0") | ||
| self.assertEqual(res.returncode, 0, res.stderr) | ||
| self.assertIn("SKIPPED", res.stderr) | ||
| calls = self._calls() | ||
| self.assertEqual(len(calls), 1) | ||
| self.assertIn("hook-pre-push.py", calls[0]) | ||
|
|
||
| def test_non_exact_opt_out_values_still_run_tests(self) -> None: | ||
| for value in ("", "false", "no", "1"): | ||
| self.log.unlink(missing_ok=True) | ||
| res = self._run(AGENT_COLLAB_PREPUSH_TESTS=value) | ||
| self.assertEqual(res.returncode, 0, (value, res.stderr)) | ||
| self.assertEqual(len(self._calls()), 3, value) | ||
|
|
||
| def test_main_branch_skips_tests_but_runs_compliance(self) -> None: | ||
| res = self._run(GIT_BRANCH="main") | ||
| self.assertEqual(res.returncode, 0, res.stderr) | ||
| calls = self._calls() | ||
| self.assertEqual(len(calls), 1) | ||
| self.assertIn("hook-pre-push.py", calls[0]) | ||
|
|
||
| def test_branch_detection_failure_runs_tests(self) -> None: | ||
| res = self._run(GIT_FAIL="1") | ||
| self.assertEqual(res.returncode, 0, res.stderr) | ||
| self.assertEqual(len(self._calls()), 3) | ||
|
|
||
| def test_suite_subshells_are_sanitized_of_hook_git_env(self) -> None: | ||
| res = self._run(GIT_DIR="/some/repo/.git/worktrees/x") | ||
| self.assertEqual(res.returncode, 0, res.stderr) | ||
| calls = self._calls() | ||
| self.assertEqual(len(calls), 3) | ||
| self.assertIn("GIT_DIR=unset", calls[0]) | ||
| self.assertIn("GIT_DIR=unset", calls[1]) | ||
| # the exec'd compliance checker keeps the hook environment | ||
| self.assertIn("GIT_DIR=/some/repo/.git/worktrees/x", calls[2]) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
HEADismain, an explicitgit push origin feature-xorgit push --allreaches this condition and disables the test gate even though a feature ref is being sent. The pre-push contract provides every pushed local and remote ref on stdin, so the exemption should be based on those records rather than the currently checked-out branch; otherwise this bypasses the fail-closed gate without the opt-out warning.Useful? React with 👍 / 👎.