From 0c52b2a0b00faeb50a4fde5b32831b4ecff6260b Mon Sep 17 00:00:00 2001 From: Rujul Talekar <113801876+Roojool@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:34:07 +0530 Subject: [PATCH 1/2] fix(tools): use configured CMAKE_MAKE_PROGRAM for test registration audit tools/check_test_registration.py invoked bare ninja from PATH, so the audit failed with FileNotFoundError when the configured Ninja lived outside PATH. Read CMAKE_MAKE_PROGRAM from the build directory CMakeCache.txt and fail honestly when it is missing. Adapt the mocked built tests to a configured build and add a regression that runs a real Ninja from a path with spaces outside PATH. --- tools/check_test_registration.py | 12 +++++- tools/tests/test_test_registration.py | 59 +++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/tools/check_test_registration.py b/tools/check_test_registration.py index 0fb47521a..7b00d94d3 100644 --- a/tools/check_test_registration.py +++ b/tools/check_test_registration.py @@ -11,7 +11,17 @@ KNOWN_UNRUN = set() def built(build): - listed = subprocess.run(["ninja", "-C", str(build), "-t", "targets", "all"], + program, generator = None, None + for line in (Path(build) / "CMakeCache.txt").read_text(encoding="utf-8").splitlines(): + if line.startswith("CMAKE_MAKE_PROGRAM:"): + program = line.partition("=")[2] + elif line.startswith("CMAKE_GENERATOR:"): + generator = line.partition("=")[2] + if generator not in ("Ninja", "Ninja Multi-Config"): + raise ValueError(f"test registration audit requires a Ninja generator, got {generator!r}") + if not program or program.endswith("-NOTFOUND"): + raise FileNotFoundError(f"CMAKE_MAKE_PROGRAM is not set in {Path(build) / 'CMakeCache.txt'}") + listed = subprocess.run([program, "-C", str(build), "-t", "targets", "all"], capture_output=True, text=True, check=True).stdout names = set() for line in listed.splitlines(): diff --git a/tools/tests/test_test_registration.py b/tools/tests/test_test_registration.py index 3d91bf0d6..e69004676 100644 --- a/tools/tests/test_test_registration.py +++ b/tools/tests/test_test_registration.py @@ -1,4 +1,6 @@ from pathlib import Path +import os +import shutil import sys import tempfile import unittest @@ -36,13 +38,21 @@ class RegistrationTests(unittest.TestCase): def test_hyphenated_executable_is_discovered(self): output = "tests/buffer-bounds_tests: CXX_EXECUTABLE_LINKER\n" - with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)): - self.assertEqual(check_test_registration.built("build"), {"buffer-bounds_tests"}) + with tempfile.TemporaryDirectory() as directory: + build = Path(directory) + (build / "CMakeCache.txt").write_text("CMAKE_MAKE_PROGRAM:FILEPATH=/configured/ninja\nCMAKE_GENERATOR:INTERNAL=Ninja\n", encoding="utf-8") + with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)) as run: + self.assertEqual(check_test_registration.built(build), {"buffer-bounds_tests"}) + self.assertEqual(run.call_args[0][0][0], "/configured/ninja") def test_nested_test_output_is_discovered(self): output = "tests/gpu/buffer_bounds_tests: CXX_EXECUTABLE_LINKER\n" - with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)): - self.assertEqual(check_test_registration.built("build"), {"buffer_bounds_tests"}) + with tempfile.TemporaryDirectory() as directory: + build = Path(directory) + (build / "CMakeCache.txt").write_text("CMAKE_MAKE_PROGRAM:FILEPATH=/configured/ninja\nCMAKE_GENERATOR:INTERNAL=Ninja Multi-Config\n", encoding="utf-8") + with patch.object(check_test_registration.subprocess, "run", return_value=SimpleNamespace(stdout=output)) as run: + self.assertEqual(check_test_registration.built(build), {"buffer_bounds_tests"}) + self.assertEqual(run.call_args[0][0][0], "/configured/ninja") def test_quoted_cmake_path_is_unescaped(self): names = self.registered(r'add_test("bounds" "/build/tests/a\"b_tests")' + '\n') @@ -118,7 +128,48 @@ class RegistrationTests(unittest.TestCase): (external / "CTestTestfile.cmake").write_text( 'add_test("bounds" "/build/tests/buffer_bounds_tests")\n', encoding="utf-8") self.assertEqual(check_test_registration.registered(build), {"buffer_bounds_tests"}) + def test_built_uses_configured_ninja_outside_path(self): + source = shutil.which("ninja") + self.assertIsNotNone(source) + with tempfile.TemporaryDirectory() as directory: + tool = Path(directory) / "tool dir with spaces" + tool.mkdir() + name = "ninja.exe" if os.name == "nt" else "ninja" + configured = tool / name + shutil.copy2(source, configured) + if os.name != "nt": + configured.chmod(0o755) + build = Path(directory) / "build" + build.mkdir() + (build / "CMakeCache.txt").write_text("CMAKE_MAKE_PROGRAM:FILEPATH=" + str(configured) + "\nCMAKE_GENERATOR:INTERNAL=Ninja\n", encoding="utf-8") + (build / "build.ninja").write_text("rule CXX_EXECUTABLE_LINKER\n command = true\nbuild tests/buffer_bounds_tests: CXX_EXECUTABLE_LINKER\n", encoding="utf-8") + empty = Path(directory) / "empty" + empty.mkdir() + with patch.dict(os.environ, {"PATH": str(empty)}): + self.assertEqual(check_test_registration.built(build), {"buffer_bounds_tests"}) + def test_unsupported_generator_never_runs_build_tool(self): + for generator in ("Unix Makefiles", "Visual Studio 17 2022", ""): + with self.subTest(generator=generator), tempfile.TemporaryDirectory() as directory: + build = Path(directory) + (build / "CMakeCache.txt").write_text("CMAKE_MAKE_PROGRAM:FILEPATH=make\nCMAKE_GENERATOR:INTERNAL=" + generator + "\n", encoding="utf-8") + with patch.object(check_test_registration.subprocess, "run") as run: + with self.assertRaisesRegex(ValueError, "requires a Ninja generator"): + check_test_registration.built(build) + run.assert_not_called() + + def test_missing_make_program_never_runs_build_tool(self): + for program in (None, "", "CMAKE_MAKE_PROGRAM-NOTFOUND"): + with self.subTest(program=program), tempfile.TemporaryDirectory() as directory: + build = Path(directory) + cache = "CMAKE_GENERATOR:INTERNAL=Ninja\n" + if program is not None: + cache += "CMAKE_MAKE_PROGRAM:FILEPATH=" + program + "\n" + (build / "CMakeCache.txt").write_text(cache, encoding="utf-8") + with patch.object(check_test_registration.subprocess, "run") as run: + with self.assertRaises(FileNotFoundError): + check_test_registration.built(build) + run.assert_not_called() if __name__ == "__main__": unittest.main() From 290675799e62bc1a57c27b7b719d0a8e7305c6fe Mon Sep 17 00:00:00 2001 From: Rujul Talekar <113801876+Roojool@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:51:04 +0530 Subject: [PATCH 2/2] test(tools): drop the Ninja-on-PATH requirement from the registration suite Remove test_built_uses_configured_ninja_outside_path per review: it copied whatever ninja was on PATH and asserted one existed, so the suite failed on exactly the setup this PR fixes. The rewritten tests already verify the configured CMAKE_MAKE_PROGRAM is the program that runs; the suite now passes with and without Ninja on PATH on Windows and Linux. --- tools/tests/test_test_registration.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tools/tests/test_test_registration.py b/tools/tests/test_test_registration.py index e69004676..e9862c2bc 100644 --- a/tools/tests/test_test_registration.py +++ b/tools/tests/test_test_registration.py @@ -128,26 +128,6 @@ class RegistrationTests(unittest.TestCase): (external / "CTestTestfile.cmake").write_text( 'add_test("bounds" "/build/tests/buffer_bounds_tests")\n', encoding="utf-8") self.assertEqual(check_test_registration.registered(build), {"buffer_bounds_tests"}) - def test_built_uses_configured_ninja_outside_path(self): - source = shutil.which("ninja") - self.assertIsNotNone(source) - with tempfile.TemporaryDirectory() as directory: - tool = Path(directory) / "tool dir with spaces" - tool.mkdir() - name = "ninja.exe" if os.name == "nt" else "ninja" - configured = tool / name - shutil.copy2(source, configured) - if os.name != "nt": - configured.chmod(0o755) - build = Path(directory) / "build" - build.mkdir() - (build / "CMakeCache.txt").write_text("CMAKE_MAKE_PROGRAM:FILEPATH=" + str(configured) + "\nCMAKE_GENERATOR:INTERNAL=Ninja\n", encoding="utf-8") - (build / "build.ninja").write_text("rule CXX_EXECUTABLE_LINKER\n command = true\nbuild tests/buffer_bounds_tests: CXX_EXECUTABLE_LINKER\n", encoding="utf-8") - empty = Path(directory) / "empty" - empty.mkdir() - with patch.dict(os.environ, {"PATH": str(empty)}): - self.assertEqual(check_test_registration.built(build), {"buffer_bounds_tests"}) - def test_unsupported_generator_never_runs_build_tool(self): for generator in ("Unix Makefiles", "Visual Studio 17 2022", ""): with self.subTest(generator=generator), tempfile.TemporaryDirectory() as directory: