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: