From 3e7a11ca2e5fb9a1faa4866dad493b20fc30b0a4 Mon Sep 17 00:00:00 2001 From: SmokeDev Date: Wed, 29 Jul 2026 20:15:43 -0700 Subject: [PATCH] fix(test-runner): native Windows venv probe + glyph-safe stdio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Windows fixes for the canonical test runner, salvaged from #66496: - scripts/run_tests.sh: probe the native Windows venv layout (Scripts/activate → Scripts/python.exe) alongside bin/activate, adapted to main's pytest-import-guarded loop with SKIPPED_VENVS reporting. Without it a python -m venv / uv venv on Git Bash/MSYS is never found and the runner refuses to start. - scripts/run_tests_parallel.py: _make_stdio_glyph_safe() reconfigures stdout/stderr to UTF-8 (errors=replace fallback) so the ✓/✗ progress glyphs cannot crash a cp1252 console when the runner is invoked directly (run_tests.sh's PYTHONUTF8=1 only covers the wrapped path). No-op on UTF-8 stdio. Ships 3 OS-independent cp1252 tests plus encoding=utf-8 in the runner-subprocess assertions. Dropped from the original PR: the USERPROFILE/HOMEDRIVE/HOMEPATH/ SYSTEMROOT env-forwarding hunk — main's WIN_ENV loop already forwards a superset (#67385/#70813). --- contributors/emails/degensmoke@gmail.com | 2 + scripts/run_tests.sh | 16 +++++- scripts/run_tests_parallel.py | 26 +++++++++ tests/test_run_tests_parallel.py | 12 ++++- tests/test_run_tests_parallel_stdio.py | 69 ++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 contributors/emails/degensmoke@gmail.com create mode 100644 tests/test_run_tests_parallel_stdio.py diff --git a/contributors/emails/degensmoke@gmail.com b/contributors/emails/degensmoke@gmail.com new file mode 100644 index 00000000000..328aaa00f89 --- /dev/null +++ b/contributors/emails/degensmoke@gmail.com @@ -0,0 +1,2 @@ +TheSmokeDev +# PR #66496 salvage diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 635852ad9f5..0db80ffb13e 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -49,11 +49,25 @@ REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" # reported "0 tests passed" (which reads green at a glance even though the # exit code is 1). Skip such a venv and keep probing instead. VENV="" +VENV_PYTHON="" SKIPPED_VENVS="" for candidate in "$REPO_ROOT/.venv" "$REPO_ROOT/venv" "$HOME/.hermes/hermes-agent/venv"; do if [ -f "$candidate/bin/activate" ]; then if "$candidate/bin/python" -c 'import pytest' 2>/dev/null; then VENV="$candidate" + VENV_PYTHON="$candidate/bin/python" + break + fi + SKIPPED_VENVS="$SKIPPED_VENVS $candidate" + fi + # Native Windows venv layout: python.exe and activate live under + # Scripts/, and there is no bin/. Anyone running this script from + # Git Bash / MSYS with a `python -m venv`- or uv-created venv hits + # this branch — without it the canonical runner refuses to start. + if [ -f "$candidate/Scripts/activate" ]; then + if "$candidate/Scripts/python.exe" -c 'import pytest' 2>/dev/null; then + VENV="$candidate" + VENV_PYTHON="$candidate/Scripts/python.exe" break fi SKIPPED_VENVS="$SKIPPED_VENVS $candidate" @@ -67,7 +81,7 @@ if [ -n "$SKIPPED_VENVS" ]; then fi if [ -n "$VENV" ]; then - PYTHON="$VENV/bin/python" + PYTHON="$VENV_PYTHON" elif [ -n "${HERMES_PYTHON:-}" ] && [ -x "$HERMES_PYTHON" ] \ && "$HERMES_PYTHON" -c 'import pytest' 2>/dev/null; then # Guard with an import check: HERMES_PYTHON may point at the RELEASE diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 9deb46cae04..1b34c74b4d6 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -648,7 +648,33 @@ def _slice_files( return target +def _make_stdio_glyph_safe() -> None: + """Keep status glyphs from killing the runner on narrow console encodings. + + On native Windows, piped or legacy-console stdio defaults to a locale + codec (usually cp1252) that cannot encode the ✓/✗ progress glyphs — the + first per-file status line then dies with UnicodeEncodeError before a + single test result is reported. Declare the runner's own output UTF-8 + (what CI and every modern terminal already are), with errors="replace" + as the can't-crash backstop; where the encoding can't be changed, fall + back to errors="replace" alone so glyphs degrade to "?" instead of + killing the run. On already-UTF-8 stdio this is a no-op. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is None: + continue + try: + reconfigure(encoding="utf-8", errors="replace") + except Exception: + try: + reconfigure(errors="replace") + except Exception: + pass + + def main() -> int: + _make_stdio_glyph_safe() parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter, diff --git a/tests/test_run_tests_parallel.py b/tests/test_run_tests_parallel.py index fc2ae66d7b3..563d9f09573 100644 --- a/tests/test_run_tests_parallel.py +++ b/tests/test_run_tests_parallel.py @@ -146,7 +146,11 @@ def test_grandchild_leak_is_killed_by_runner(tmp_path: Path) -> None: cwd=repo_root, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - text=True, + # The runner declares its stdio UTF-8 (see _make_stdio_glyph_safe); + # decode the same way so ✓-glyph assertions hold on Windows, where + # text=True alone would decode with the locale codec (cp1252). + encoding="utf-8", + errors="replace", timeout=60, ) @@ -220,7 +224,11 @@ def _run_runner(probe_dir: Path, *extra: str) -> subprocess.CompletedProcess: cwd=repo_root, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - text=True, + # The runner declares its stdio UTF-8 (see _make_stdio_glyph_safe); + # decode the same way so ✓-glyph assertions hold on Windows, where + # text=True alone would decode with the locale codec (cp1252). + encoding="utf-8", + errors="replace", timeout=60, ) diff --git a/tests/test_run_tests_parallel_stdio.py b/tests/test_run_tests_parallel_stdio.py new file mode 100644 index 00000000000..2add57c3e25 --- /dev/null +++ b/tests/test_run_tests_parallel_stdio.py @@ -0,0 +1,69 @@ +"""The runner's status glyphs must not crash narrow console encodings. + +On native Windows, piped or legacy-console stdio defaults to cp1252, which +cannot encode the runner's ✓/✗ progress glyphs — before the fix, the first +per-file status line killed the whole run with UnicodeEncodeError. The +failure depends only on the stream's encoding, so these tests pin it on +every OS by building a cp1252 stream explicitly. +""" + +from __future__ import annotations + +import importlib.util +import io +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +_RUNNER_PATH = REPO_ROOT / "scripts" / "run_tests_parallel.py" + + +def _load_runner(): + spec = importlib.util.spec_from_file_location("run_tests_parallel", _RUNNER_PATH) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def _cp1252_stream() -> tuple[io.TextIOWrapper, io.BytesIO]: + raw = io.BytesIO() + return io.TextIOWrapper(raw, encoding="cp1252", errors="strict"), raw + + +def test_cp1252_stream_reproduces_the_crash_without_the_fix() -> None: + # Baseline for the bug: a strict cp1252 stream cannot take the glyph. + stream, _raw = _cp1252_stream() + try: + stream.write("✓") + except UnicodeEncodeError: + return + raise AssertionError("expected UnicodeEncodeError on strict cp1252") + + +def test_glyph_safe_stdio_survives_cp1252(monkeypatch) -> None: + mod = _load_runner() + stream, raw = _cp1252_stream() + monkeypatch.setattr(sys, "stdout", stream) + monkeypatch.setattr(sys, "stderr", stream) + + mod._make_stdio_glyph_safe() + print("✓ tests/foo.py (3 tests, 1.2s) ✗") + sys.stdout.flush() + + out = raw.getvalue() + assert "✓".encode("utf-8") in out, "stream should now carry UTF-8 glyphs" + assert b"tests/foo.py (3 tests, 1.2s)" in out, "line content must survive" + + +def test_glyph_safe_stdio_noop_without_reconfigure(monkeypatch) -> None: + # Streams without .reconfigure (e.g. pytest's capture buffers, plain + # StringIO) must pass through untouched instead of raising. + mod = _load_runner() + plain = io.StringIO() + monkeypatch.setattr(sys, "stdout", plain) + monkeypatch.setattr(sys, "stderr", plain) + + mod._make_stdio_glyph_safe() + print("✓ still fine") + + assert "✓ still fine" in plain.getvalue()