diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 68c9423db673..811a79ea5e02 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -32,7 +32,7 @@ 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 (path-list, default: 'tests') Exit code: 0 if every file's pytest exited 0; 1 otherwise. """ @@ -364,6 +364,52 @@ def _format_file(file: Path, repo_root: Path) -> str: return str(file) +def _split_path_list(value: str | None) -> list[str]: + """Split a test path-list without shredding Windows drive letters. + + CI still passes repo-relative ``:``-joined paths, so keep supporting + colon as the portable generated format. On Windows, user-entered absolute + paths should use ``os.pathsep`` (``;``), and a single ``C:\\...`` path must + not split at the drive separator. + """ + if not value: + return [] + + if os.pathsep != ":" and os.pathsep in value: + return [part.strip() for part in value.split(os.pathsep) if part.strip()] + + parts: list[str] = [] + start = 0 + for idx, char in enumerate(value): + if char != ":": + continue + is_windows_drive = ( + idx == 1 + and value[0].isalpha() + and idx + 1 < len(value) + and value[idx + 1] in {"\\", "/"} + ) + if is_windows_drive: + continue + parts.append(value[start:idx]) + start = idx + 1 + parts.append(value[start:]) + return [part.strip() for part in parts if part.strip()] + + +def _print_line(msg: str = "", *, file=None) -> None: + """Print one line, replacing unencodable glyphs on legacy Windows pipes.""" + stream = sys.stdout if file is None else file + try: + print(msg, file=stream, flush=True) + except UnicodeEncodeError: + encoding = getattr(stream, "encoding", None) or "utf-8" + safe = msg.encode(encoding, errors="replace").decode( + encoding, errors="replace" + ) + print(safe, file=stream, flush=True) + + def _print_progress( tests_done: int, approx_total_tests: int, @@ -434,7 +480,7 @@ def _print_progress( msg = msg[: cols - 1] + "…" except OSError: pass - print(msg, flush=True) + _print_line(msg) def _print_inline_failure( @@ -606,7 +652,7 @@ 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="Path-list discovery roots (default: 'tests')", ) parser.add_argument( "--include-integration", @@ -652,7 +698,7 @@ def main() -> int: "--files", metavar="LIST", help=( - "Explicit colon-separated list of test files to run. Bypasses " + "Explicit path-list of test files to run. Bypasses " "discovery entirely — used by CI matrix jobs that receive their " "file list from the generate job." ), @@ -749,7 +795,7 @@ def _is_our_flag(tok: str) -> bool: # --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_path_list(args.files)] roots = [] else: # Resolve discovery roots: positional path args override --paths if any @@ -757,7 +803,7 @@ def _is_our_flag(tok: str) -> bool: 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_path_list(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 3cba46fab00d..82d54149bb87 100644 --- a/tests/test_run_tests_parallel.py +++ b/tests/test_run_tests_parallel.py @@ -21,12 +21,14 @@ from __future__ import annotations import json +import importlib.util import os import subprocess import sys import textwrap import time from pathlib import Path +from types import ModuleType import pytest @@ -38,6 +40,19 @@ _HANDOFF_DIR.mkdir(exist_ok=True) +def _load_runner_module() -> ModuleType: + repo_root = Path(__file__).resolve().parent.parent + runner = repo_root / "scripts" / "run_tests_parallel.py" + spec = importlib.util.spec_from_file_location( + "_hermes_run_tests_parallel", runner + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + def _handoff_path_for(nonce: str) -> Path: return _HANDOFF_DIR / f"grandchild-{nonce}.json" @@ -261,6 +276,55 @@ def test_explicit_double_dash_still_works(tmp_path: Path) -> None: assert "unrecognized arguments" not in proc.stdout +def test_path_list_preserves_windows_drive_letters(monkeypatch) -> None: + """Windows absolute paths are not split at the drive separator.""" + runner = _load_runner_module() + monkeypatch.setattr(runner.os, "pathsep", ";") + + assert runner._split_path_list(r"C:\Users\me\repo\tests") == [ + r"C:\Users\me\repo\tests" + ] + assert runner._split_path_list(r"C:\repo\tests;D:\other\tests") == [ + r"C:\repo\tests", + r"D:\other\tests", + ] + + +def test_path_list_keeps_ci_colon_joined_relative_files(monkeypatch) -> None: + """CI-generated repo-relative ``:`` lists keep working on every platform.""" + runner = _load_runner_module() + monkeypatch.setattr(runner.os, "pathsep", ";") + + assert runner._split_path_list("tests/a.py:tests/b.py") == [ + "tests/a.py", + "tests/b.py", + ] + + +def test_progress_print_replaces_unencodable_glyphs() -> None: + """Legacy Windows pipes should not drop progress callbacks.""" + runner = _load_runner_module() + + class LegacyPipe: + encoding = "cp1252" + + def __init__(self) -> None: + self.output = "" + + def write(self, text: str) -> int: + text.encode(self.encoding) + self.output += text + return len(text) + + def flush(self) -> None: + pass + + pipe = LegacyPipe() + runner._print_line("1✓", file=pipe) + + assert pipe.output == "1?\n" + + def test_positional_path_not_treated_as_flag(tmp_path: Path) -> None: """A positional path arg still overrides discovery (not routed to pytest).""" probe_dir = _make_probe_dir(tmp_path)