diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 1b34c74b4d64d..4fd1da44ced73 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -32,7 +32,9 @@ Usage: Environment: HERMES_TEST_WORKERS Override worker count (default: os.cpu_count()) - HERMES_TEST_PATHS Override discovery roots (colon-sep, default: 'tests') + HERMES_TEST_PATHS Override discovery roots (colon-sep; on Windows + ';' also works and drive letters are handled; + default: 'tests') Exit code: 0 if every file's pytest exited 0; 1 otherwise. """ @@ -100,6 +102,41 @@ _DEFAULT_FILE_RETRIES = 1 _DURATIONS_FILE = "test_durations.json" +def _split_pathspec(value: str) -> List[str]: + """Split a separator-joined path list (``--paths``/``--files``/ + ``HERMES_TEST_PATHS``) into individual paths. + + POSIX: ``:``-separated, as documented. + + Windows: ``;`` (``os.pathsep``) and ``:`` are both accepted as + separators, but a ``:`` that forms a drive letter (``C:\\...`` or + ``C:/...``) stays glued to its path — a naive ``split(":")`` turns + ``C:\\repo\\tests`` into ``['C', '\\repo\\tests']``, where the bogus + ``C`` becomes a phantom discovery root and the rooted remainder only + resolves by accident of ``Path.__truediv__`` re-anchoring it onto + ``repo_root``'s drive. + """ + if sys.platform != "win32": + return [p for p in value.split(":") if p.strip()] + parts: List[str] = [] + for chunk in value.split(";"): + raw = chunk.split(":") + i = 0 + while i < len(raw): + part = raw[i] + if ( + len(part) == 1 + and part.isalpha() + and i + 1 < len(raw) + and raw[i + 1][:1] in ("\\", "/") + ): + part = f"{part}:{raw[i + 1]}" + i += 1 + parts.append(part) + i += 1 + return [p for p in parts if p.strip()] + + def _approximately_count_tests( files: List[Path], repo_root: Path ) -> dict[Path, int]: @@ -689,7 +726,11 @@ def main() -> int: parser.add_argument( "--paths", default=os.environ.get("HERMES_TEST_PATHS", ":".join(_DEFAULT_ROOTS)), - help="Colon-separated discovery roots (default: 'tests')", + help=( + "Colon-separated discovery roots (default: 'tests'). On " + "Windows, ';' also separates and drive letters (C:\\...) are " + "kept intact." + ), ) parser.add_argument( "--include-integration", @@ -748,9 +789,10 @@ def main() -> int: "--files", metavar="LIST", help=( - "Explicit colon-separated list of test files to run. Bypasses " - "discovery entirely — used by CI matrix jobs that receive their " - "file list from the generate job." + "Explicit colon-separated list of test files to run (on " + "Windows, ';' also separates and drive letters are kept " + "intact). Bypasses discovery entirely — used by CI matrix " + "jobs that receive their file list from the generate job." ), ) parser.add_argument( @@ -885,7 +927,7 @@ def main() -> int: # --files: explicit file list from the CI generate job — skip discovery. if args.files: - files = [repo_root / f for f in args.files.split(":") if f.strip()] + files = [repo_root / f for f in _split_pathspec(args.files)] roots = [] else: # Resolve discovery roots: positional path args override --paths if any @@ -893,7 +935,7 @@ def main() -> int: if args.paths_positional: roots = [repo_root / p for p in args.paths_positional] else: - roots = [repo_root / p for p in args.paths.split(":") if p] + roots = [repo_root / p for p in _split_pathspec(args.paths)] if args.include_integration: # Caller takes responsibility — typically used via explicit -k filter. diff --git a/tests/test_run_tests_parallel.py b/tests/test_run_tests_parallel.py index 563d9f09573eb..c1f3926fb5a54 100644 --- a/tests/test_run_tests_parallel.py +++ b/tests/test_run_tests_parallel.py @@ -267,7 +267,7 @@ def test_positional_path_not_treated_as_flag(tmp_path: Path) -> None: [sys.executable, str(runner), str(probe_dir), "-j", "1", "--file-timeout", "30", "-q"], cwd=repo_root, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - text=True, timeout=60, + encoding="utf-8", errors="replace", timeout=60, ) assert proc.returncode == 0, proc.stdout # Discovery found the probe file (2 tests), proving the positional path @@ -378,3 +378,48 @@ def test_explicit_k_wins_over_node_id_inference(tmp_path: Path) -> None: # -k test_beta wins: one test ran, and it wasn't filtered to nothing. assert proc.returncode == 0, proc.stdout assert "1 tests passed" in proc.stdout + + +def test_multiple_absolute_paths_split_on_pathsep(tmp_path: Path) -> None: + """``--paths`` accepts ``os.pathsep``-joined absolute paths. + + On Windows the absolute paths contain drive-letter colons, so a naive + ``split(":")`` shreds them into phantom roots and only one (or neither) + of the two probe dirs would be discovered. + """ + dir_a = _make_probe_dir(tmp_path) + dir_b = tmp_path / "probe_b" + dir_b.mkdir() + (dir_b / "test_flagprobe_b.py").write_text( + "def test_gamma():\n assert True\n" + ) + repo_root = Path(__file__).resolve().parent.parent + runner = repo_root / "scripts" / "run_tests_parallel.py" + proc = subprocess.run( + [sys.executable, str(runner), + "--paths", os.pathsep.join([str(dir_a), str(dir_b)]), + "-j", "1", "--file-timeout", "30", "-q"], + cwd=repo_root, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + encoding="utf-8", errors="replace", timeout=60, + ) + assert proc.returncode == 0, proc.stdout + assert "Discovered 2 test files" in proc.stdout, proc.stdout + + +@pytest.mark.skipif(sys.platform != "win32", reason="drive-letter paths") +def test_drive_letter_colon_is_not_a_path_separator(tmp_path: Path) -> None: + """An absolute ``--paths`` value stays one root on Windows. + + The naive split used to produce a phantom relative root ``'C'`` (the + drive letter) alongside the real path; discovery only worked by the + accident of ``repo_root / '\\rooted\\rest'`` re-anchoring onto the + repo's drive. + """ + probe_dir = _make_probe_dir(tmp_path) + proc = _run_runner(probe_dir, "-q") + assert proc.returncode == 0, proc.stdout + drive = str(probe_dir)[0] + assert f"['{drive}', " not in proc.stdout, ( + f"drive letter split off as a phantom root:\n{proc.stdout}" + ) + assert "Discovered 1 test files" in proc.stdout, proc.stdout