From 1ec58f5704fb840e13422ef27fa9116385f171d8 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 23:52:21 -0700 Subject: [PATCH] gate: a branch taken per operating system needs a test that injects one A guard's screen-lock and idle probes returned early on anything but macOS. The suite was green on a Mac and red on Linux CI with "unexpectedly None", because the branch under test only ever ran on the author's machine. The coverage gate now asks any detector that reads sys.platform for a test that passes a platform in. Backtested against the tests as first written: fires. Against the fixed ones: silent. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KU2pPKob4MJ1NqjsfTNyYJ Change-Id: I0ebec990b0153ad5e64e4725cbe045d4cd428ed4 --- scripts/check_hook_test_coverage.py | 42 +++++++++++++++++++++++++ tests/test_check_hook_test_coverage.py | 43 ++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/scripts/check_hook_test_coverage.py b/scripts/check_hook_test_coverage.py index 6918d931..6c56b217 100755 --- a/scripts/check_hook_test_coverage.py +++ b/scripts/check_hook_test_coverage.py @@ -61,6 +61,8 @@ "corrupt", "malformed", ) +PLATFORM_BRANCH_RE = ("sys.platform", "platform.system") +PLATFORM_INJECTION_RE = ("platform=", 'platform="', "platform='", "sys.platform", "platform.system") READS_INPUT_RE = ( "open(", "read(", @@ -150,6 +152,39 @@ def reads_external_input(detector_path: str) -> bool: return any(token in source for token in READS_INPUT_RE) +def tests_inject_a_platform(tests_dir: str) -> bool: + """True when some test hands the detector a platform instead of the host's.""" + try: + names = sorted(os.listdir(tests_dir)) + except OSError: + return False + for fname in names: + if not (fname.startswith("test_") and fname.endswith(".py")): + continue + try: + with open(os.path.join(tests_dir, fname), encoding="utf-8") as handle: + source = handle.read() + except OSError: + continue + if any(token in source for token in PLATFORM_INJECTION_RE): + return True + return False + + +def branches_on_platform(detector_path: str) -> bool: + """True when a detector takes a different path per operating system. + + Such a branch only ever runs on the host that runs it, so a suite green + on a developer's machine says nothing about the branch CI takes. + """ + try: + with open(detector_path, encoding="utf-8") as handle: + source = handle.read() + except OSError: + return False + return any(token in source for token in PLATFORM_BRANCH_RE) + + def hooks_with_detector() -> list[str]: if not os.path.isdir(HOOKS_DIR): return [] @@ -181,6 +216,13 @@ def check_hook(hook_dir: str) -> list[str]: "that proves the detector stays silent on a clean case)" ) detector = os.path.join(hook_dir, "detect.py") + if branches_on_platform(detector) and not tests_inject_a_platform(tests_dir): + problems.append( + f"{name}: detect.py branches on the operating system but no test injects a " + "platform. Pass the platform in rather than reading the host's, so both " + "branches run wherever the suite runs; a branch that only executes on the " + "author's machine is untested on the one CI uses." + ) if reads_external_input(detector) and not any( pat in n.lower() for n in names for pat in UNCHECKED_RE ): diff --git a/tests/test_check_hook_test_coverage.py b/tests/test_check_hook_test_coverage.py index 13a05d95..0397b9a6 100644 --- a/tests/test_check_hook_test_coverage.py +++ b/tests/test_check_hook_test_coverage.py @@ -78,6 +78,49 @@ def test_missing_detector_file_fails_open_rather_than_erroring(self): self.assertFalse(chtc.reads_external_input("/nonexistent/detect.py")) +PLATFORM_DETECTOR = ( + "import sys\n\n" + "def screen_is_locked(platform=None):\n" + " if (platform or sys.platform) != 'darwin':\n" + " return None\n" + " return False\n" +) +PLATFORM_INJECTING_TEST = ( + "\n\ndef test_no_hit_off_macos():\n" + " assert screen_is_locked(platform='linux') is None\n" +) + + +class TestPlatformBranchRule(unittest.TestCase): + """A branch taken per operating system only runs on the host running it. + + A suite green on a developer's machine says nothing about the branch CI + takes, which is how a macOS-only probe shipped and went red on Linux. + """ + + def test_platform_branch_without_an_injecting_test_fails(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook(Path(tmp), "probe", PLATFORM_DETECTOR, FIRES_AND_SILENT) + problems = [p for p in chtc.check_hook(str(hook_dir)) if "platform" in p] + self.assertEqual(len(problems), 1, problems) + self.assertIn("injects a platform", problems[0]) + + def test_platform_branch_with_an_injecting_test_passes(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook( + Path(tmp), "probe", PLATFORM_DETECTOR, + FIRES_AND_SILENT + PLATFORM_INJECTING_TEST, + ) + problems = [p for p in chtc.check_hook(str(hook_dir)) if "platform" in p] + self.assertEqual(problems, []) + + def test_a_detector_with_no_platform_branch_is_not_asked(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook(Path(tmp), "inline", INLINE_DETECTOR, FIRES_AND_SILENT) + problems = [p for p in chtc.check_hook(str(hook_dir)) if "platform" in p] + self.assertEqual(problems, []) + + class TestExistingRulesStillHold(unittest.TestCase): def test_no_positive_test_still_fails(self): with tempfile.TemporaryDirectory() as tmp: