Skip to content

fix(tests): make canonical run_tests.sh work on native Windows - #71591

Closed
monerostar wants to merge 1 commit into
NousResearch:mainfrom
monerostar:fix/windows-run-tests-runner
Closed

fix(tests): make canonical run_tests.sh work on native Windows#71591
monerostar wants to merge 1 commit into
NousResearch:mainfrom
monerostar:fix/windows-run-tests-runner

Conversation

@monerostar

Copy link
Copy Markdown
Contributor

Summary

On native Windows + Git Bash, scripts/run_tests.sh (the runner CONTRIBUTING requires before every PR) was unusable:

  1. env -i + HOME only — CPython on Windows ignores HOME; Path.home() raises RuntimeError: Could not determine home directory ([Bug]: run_tests.sh cannot run tests touching Path.home() on native Windows — env -i strips USERPROFILE #67385)
  2. Progress glyphs crashes under legacy console code pages (UnicodeEncodeError cp1252)
  3. Venv layout — Windows venvs put python.exe under Scripts/, not bin/

What this PR does

Combines the approaches already validated in community PRs #67387 and #66496, plus #70813 location vars, after live verification on Win11:

Change Source
Scripts/python.exe probe (+ pytest import check, matching existing bin/ gate) #67387 / #66496
Forward USERPROFILE HOMEDRIVE HOMEPATH SYSTEMROOT TEMP TMP #67387
Also LOCALAPPDATA APPDATA #70813
PYTHONUTF8=1 #67387
_make_stdio_glyph_safe() + UTF-8 decode for pytest child output #66496 / #70813
tests/test_run_tests_parallel_stdio.py (cp1252) #66496

All Windows env vars are :+-guarded — Linux/macOS env stays equivalent (no extra keys when unset).

Live verification (monerostar, Win11)

# main (broken)
env -i … HOME=… only → Path.home() RuntimeError
run_tests.sh → UnicodeEncodeError on ✓ (cp1252)

# this branch
env -i … + Windows location vars → Path.home() == C:\Users\Admin
pytest tests/test_run_tests_parallel_stdio.py → 2 passed

Related

Test plan

  • pytest tests/test_run_tests_parallel_stdio.py → 2 passed
  • Path.home() under the new env -i allowlist
  • CI full suite
  • Optional: bash scripts/run_tests.sh tests/test_hermes_constants.py on native Windows (symlink tests may still fail without Developer Mode — separate from this fix)

Type

  • Bug fix
  • Tests

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #67387 and #66496 repair the same native-Windows runner family. This patch combines the venv/profile work with stdio coverage and adds app-data variables, so it is a competing consolidation rather than a duplicate.

@monerostar

Copy link
Copy Markdown
Contributor Author

Overnight refresh (monerostar, native Win11 build 26200)

Rebased locally onto current main (1dfe781ed) — still applies cleanly (1 commit). Could not push the rebase: GitHub rejected the update with OAuth App ... workflow scope (fork token lacks workflow, and fast-forwarding the PR branch onto a main that touched .github/workflows/* trips that check). Branch on the PR remains the older base until a token with workflow can push, or a maintainer rebases.

Live evidence the bug is still real on main

Hermes install venv is native Windows layout only:

VENV=…/hermes-agent/venv
VENV_PYTHON=…/venv/Scripts/python.exe   # no bin/activate

env -i without USERPROFILE (what unpatched run_tests.sh still does relative to Windows needs):

Path.home() → RuntimeError: Could not determine home directory.

With USERPROFILE (+ SYSTEMROOT/TEMP) preserved: home_ok C:\Users\Admin.

Local tests on rebased tree

pytest tests/test_run_tests_parallel_stdio.py -o addopts=
→ 2 passed (glyph-safe stdio / cp1252)

No code change in this comment — status only.

@monerostar

Copy link
Copy Markdown
Contributor Author

Overnight status (monerostar, Win11 build 26200)

Bug still real on current main (015718066)

env -i PATH=… HOME=… PYTHONHASHSEED=0 python -c "from pathlib import Path; print(Path.home())"
→ RuntimeError: Could not determine home directory.

env -i … + USERPROFILE + SYSTEMROOT + TEMP/TMP …
→ home_ok C:\Users\Admin

Hermes install venv remains native layout only: …/hermes-agent/venv/Scripts/python.exe (no bin/).

Rebase

Rebased cleanly onto upstream/main locally (58badb7ca, 1 commit). Push still blocked: GitHub rejects updating the PR branch because the new base history touches .github/workflows/* and the monerostar OAuth token lacks workflow scope (refusing to allow an OAuth App to create or update workflow … without workflow scope). Same constraint as the prior overnight note.

No code change in this comment — status + fresh main evidence only. Maintainer rebase (or a token with workflow) still needed to refresh the PR head.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the native-Windows investigation and the concrete reproduction details. This is an automated hermes-sweeper review; the requested behavior is now implemented on main.

  • 66c4c9c0b1f6450f6443b9d644f0a9653355f558 added the hermetic WIN_ENV forwarding loop plus PYTHONUTF8=1; current scripts/run_tests.sh:111-124 forwards the requested location variables through env -i.
  • 3e7a11ca2e5fb9a1faa4866dad493b20fc30b0a4 added the native venv probe at scripts/run_tests.sh:63-74, using Scripts/python.exe with the existing pytest import guard.
  • The same commit added _make_stdio_glyph_safe() at scripts/run_tests_parallel.py:651-677 and the strict-cp1252 coverage in tests/test_run_tests_parallel_stdio.py:33-69.
  • The latter commit is an ancestor of current main; its commit message records that it salvaged this runner family while retaining main's already-landed Windows environment forwarding.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 30, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: run_tests.sh cannot run tests touching Path.home() on native Windows — env -i strips USERPROFILE

3 participants