mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(test-runner): native Windows venv probe + glyph-safe stdio
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).
This commit is contained in:
parent
0860b9804f
commit
3e7a11ca2e
5 changed files with 122 additions and 3 deletions
2
contributors/emails/degensmoke@gmail.com
Normal file
2
contributors/emails/degensmoke@gmail.com
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
TheSmokeDev
|
||||
# PR #66496 salvage
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
|
|
|||
69
tests/test_run_tests_parallel_stdio.py
Normal file
69
tests/test_run_tests_parallel_stdio.py
Normal file
|
|
@ -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()
|
||||
Loading…
Add table
Add a link
Reference in a new issue