Merge pull request #7523 from jotitadev/fix/tools-check-python-test-registration

fix(tools): fail when a test_*.py script is never run

CMakeLists.txt

@@ -180,6 +180,8 @@ if(BUILD_TESTING)
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_check_conventions_debt_length.py)
         add_test(NAME test_registration COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_test_registration.py)
+        add_test(NAME test_script_registration COMMAND ${Python3_EXECUTABLE}
+            ${CMAKE_CURRENT_SOURCE_DIR}/tools/check_test_registration.py --scripts)
         add_test(NAME build_eligibility COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_build_eligibility.py)
         add_test(NAME check_skips COMMAND ${Python3_EXECUTABLE}

tools/check_test_registration.py

@@ -11,6 +11,7 @@ LEXICAL = re.compile(r'\\.|"(?:\\.|[^"\\])*"|\[(?P<literal_equals>=*)\[.*?\](?P=
                      r'|(?P<comment>\#\[(?P<comment_equals>=*)\[.*?\](?P=comment_equals)\]|\#[^\n]*)', re.S)
 
 KNOWN_UNRUN = set()
+SCRIPT = re.compile(r"(?:[\w.-]+/)*test_\w+\.py")
 
 
 def built(build):
@@ -125,11 +126,52 @@ def registered(build):
     return names
 
 
+def listed(root):
+    output = subprocess.run(["git", "-c", "core.quotePath=false", "-C", str(root), "ls-files", "-z", "--cached", "--others",
+                             "--exclude-standard"], capture_output=True, check=True).stdout.decode("utf-8", errors="replace")
+    return sorted(set(filter(None, output.split("\0"))))
+
+
+def run_scripts(root, files):
+    run = set()
+    for name in files:
+        if name.endswith("CMakeLists.txt") or name.endswith(".cmake"):
+            content = (Path(root) / name).read_text(encoding="utf-8", errors="replace")
+            for command, arguments in commands(content):
+                if command == "add_test":
+                    run.update(argument.rpartition("}")[2].lstrip("/") for argument in arguments)
+        elif name.startswith(".github/workflows/") and name.endswith((".yml", ".yaml")):
+            for line in (Path(root) / name).read_text(encoding="utf-8", errors="replace").splitlines():
+                if line.lstrip().startswith("#"):
+                    continue
+                line = re.split(r"\s#", line)[0]
+                if "python" in line:
+                    run.update(token[2:] if token.startswith("./") else token for token in SCRIPT.findall(line))
+    return {name for name in run if SCRIPT.fullmatch(name)}
+
+
+def unrun_scripts(root):
+    files = listed(root)
+    run = run_scripts(root, files)
+    scripts = [name for name in files if re.match(r"^test_\w+\.py$", name.rpartition("/")[2])]
+    return scripts, [name for name in scripts if not any(name == path or name.endswith("/" + path) for path in run)]
+
+
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(description="fail when a test executable is built that no ctest test runs")
     parser.add_argument("build", type=Path, nargs="?", default=Path("build"), help="configured build directory")
     parser.add_argument("--list", action="store_true", help="print every test executable and whether ctest runs it")
+    parser.add_argument("--scripts", action="store_true", help="instead check that an add_test or a workflow runs every test_*.py of the source tree")
     args = parser.parse_args()
+    if args.scripts:
+        scripts, unrun = unrun_scripts(Path(__file__).resolve().parents[1])
+        for name in unrun:
+            print(f"error: {name} is a test script that no add_test or workflow runs")
+        if unrun:
+            print(f"{len(unrun)} test script(s) would never run: register each with add_test, or run it from a workflow")
+            raise SystemExit(1)
+        print(f"{len(scripts)} test script(s), each run by an add_test or a workflow")
+        raise SystemExit
     try:
         every = sorted(built(args.build))
         run = registered(args.build)

tools/tests/test_test_registration.py

@@ -294,5 +294,54 @@ class RegistrationTests(unittest.TestCase):
         self.assertIn("requires a Ninja generator", result.stderr)
         self.assertNotIn("Traceback", result.stderr)
 
+class ScriptRegistrationTests(unittest.TestCase):
+    def unrun(self, files):
+        with tempfile.TemporaryDirectory() as directory:
+            root = Path(directory)
+            check_test_registration.subprocess.run(["git", "init", "-q", str(root)], check=True)
+            for name, content in files.items():
+                (root / name).parent.mkdir(parents=True, exist_ok=True)
+                (root / name).write_text(content, encoding="utf-8")
+            return check_test_registration.unrun_scripts(root)[1]
+
+    def test_script_registered_by_a_nested_add_test_runs(self):
+        self.assertEqual(self.unrun({
+            "core/relinker/CMakeLists.txt": "if(WIN32)\n    add_test(NAME a COMMAND ${Python3_EXECUTABLE}\n        ${CMAKE_CURRENT_SOURCE_DIR}/relinker/tests/test_a.py)\nendif()\n",
+            "core/relinker/relinker/tests/test_a.py": ""}), [])
+
+    def test_unregistered_script_is_reported(self):
+        self.assertEqual(self.unrun({
+            "CMakeLists.txt": "add_test(NAME a COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_a.py)\n",
+            "tools/tests/test_a.py": "", "tools/tests/test_b.py": "", "tools/tests/helper.py": ""}), ["tools/tests/test_b.py"])
+
+    def test_commented_add_test_does_not_run_a_script(self):
+        self.assertEqual(self.unrun({
+            "CMakeLists.txt": "#add_test(NAME a COMMAND python ${CMAKE_CURRENT_SOURCE_DIR}/test_a.py)\n#[[\nadd_test(NAME a COMMAND python test_a.py)\n]]\n",
+            "test_a.py": ""}), ["test_a.py"])
+
+    def test_registration_names_its_directory(self):
+        self.assertEqual(self.unrun({
+            "CMakeLists.txt": "add_test(NAME a COMMAND python ${CMAKE_CURRENT_SOURCE_DIR}/a/tests/test_a.py)\n",
+            "a/tests/test_a.py": "", "b/tests/test_a.py": ""}), ["b/tests/test_a.py"])
+
+    def test_workflow_run_counts_and_a_paths_filter_does_not(self):
+        self.assertEqual(self.unrun({
+            ".github/workflows/pages.yml": "jobs:\n  web:\n    steps:\n      - run: python3 tools/test_web.py --build build-wasm\n",
+            ".github/workflows/web.yml": "on:\n  pull_request:\n    paths:\n      - tools/test_paths.py\n",
+            "tools/test_web.py": "", "tools/test_paths.py": ""}), ["tools/test_paths.py"])
+
+    def test_commented_workflow_line_does_not_run_a_script(self):
+        self.assertEqual(self.unrun({
+            ".github/workflows/ci.yml": "# python3 tools/tests/test_f.py\njobs:\n  t:\n    steps:\n      # python3 tools/tests/test_a.py\n"
+                                        "      - run: |\n          #python3 tools/tests/test_b.py\n"
+                                        "          \t# python3 tools/tests/test_c.py\n"
+                                        "          echo done\t# python3 tools/tests/test_d.py\n"
+                                        "          python3 tools/tests/test_e.py\n",
+            "tools/tests/test_a.py": "", "tools/tests/test_b.py": "", "tools/tests/test_c.py": "",
+            "tools/tests/test_d.py": "", "tools/tests/test_e.py": "", "tools/tests/test_f.py": ""}),
+            ["tools/tests/test_a.py", "tools/tests/test_b.py", "tools/tests/test_c.py", "tools/tests/test_d.py",
+             "tools/tests/test_f.py"])
+
+
 if __name__ == "__main__":
     unittest.main()