Skip to content

fix(tests): use the platform temp dir for the isolation-probe handoff - #74083

Open
hyqqx wants to merge 1 commit into
NousResearch:mainfrom
hyqqx:fix/windows-test-collection
Open

fix(tests): use the platform temp dir for the isolation-probe handoff#74083
hyqqx wants to merge 1 commit into
NousResearch:mainfrom
hyqqx:fix/windows-test-collection

Conversation

@hyqqx

@hyqqx hyqqx commented Jul 29, 2026

Copy link
Copy Markdown

What does this PR do?

tests/test_run_tests_parallel.py builds its handoff directory from
os.environ.get("TMPDIR", "/tmp") and mkdir()s it at module scope. On Windows
TMPDIR is normally unset — the platform uses TEMP/TMP — so the fallback resolves to a
non-existent \tmp on the current drive and raises during collection:

tests\test_run_tests_parallel.py:38: in <module>
    _HANDOFF_DIR.mkdir(exist_ok=True)
E   FileNotFoundError: [WinError 3] The system cannot find the path specified:
    '\tmp\hermes-isolation-probe'

pytest aborts the whole run on a collection error, so this single line stops the entire
suite on native Windows.

The module already declares the right intent

Its docstring says "POSIX-only … Marked accordingly" and the probe carries
@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only probe"). The marker never
gets a chance to apply, because the mkdir runs at import time — before pytest evaluates
any marker.

tempfile.gettempdir() is the cross-platform primitive for this, and it still honours
$TMPDIR first, so POSIX behaviour is byte-identical. Nothing else changes: _HANDOFF_DIR
is read only by _handoff_path_for, whose sole caller is the skipped POSIX probe.

Scope change since the first review

This PR originally carried two collection-time fixes. The import pty half landed
independently on main
as 05504bd9f (test(gateway): make test_gateway collectable on
Windows
, @iso2kx, 2026-07-30) while this was open — same approach, same call site. That
half is dropped here and the branch is rebased onto current main, addressing
@monerostar's note that the head was ~600 commits behind. What remains is the one blocker
still live on main today.

How to test

From PowerShell or cmd (not Git Bash — MSYS sets TMPDIR, which masks this entirely):

python -m pytest tests/test_run_tests_parallel.py --collect-only -q

Before, on current main (710b02663):

E   FileNotFoundError: [WinError 3] ... '\tmp\hermes-isolation-probe'
Interrupted: 1 error during collection
no tests collected, 1 error

After:

7 tests collected in 0.12s        # exit 0

Running the file: 3 passed, 1 skipped, 3 failed. The three failures are pre-existing
Windows gaps in the runner itself (No test files to run from
scripts/run_tests_parallel.py) — none of them reference _HANDOFF_DIR or
_handoff_path_for, and before this change they could not run at all.

Platforms tested

Native Windows 11 Home, build 26200 · Python 3.11.6 · PowerShell (TMPDIR unset).
No production code is touched; on POSIX gettempdir() returns $TMPDIR exactly as the old
expression did.

Why CI never caught this

All CI jobs run on ubuntu-latest (the only other runner in the tree is
ubuntu-24.04-arm, in docker.yml). There is no Windows runner, and Git Bash — the shell
most Windows contributors reach for — sets TMPDIR and hides the failure.

Related

@hyqqx hyqqx changed the title What does this PR do? fix(tests): make the suite collectable on native Windows Jul 29, 2026
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 29, 2026
@monerostar

Copy link
Copy Markdown
Contributor

Native Windows 11 verification (monerostar)

Could not leave a formal Approve (fork collaborator scope) — posting review-as-comment with live evidence.

Verified on a live Windows 11 Pro host. The import pty collection fix is still needed on current main and applies cleanly.

Host evidence

Probe Result
systeminfo OS Name Microsoft Windows 11 Pro
OS Version 10.0.26200 Build 26200
Python 3.11.15 (win32) via Hermes venv
Shell for this run Git Bash (MSYS) — note TMPDIR=/tmp is set here

tests/hermes_cli/test_gateway.py (the important half)

Current main (broken collection):

ERROR collecting tests/hermes_cli/test_gateway.py
  import pty → tty → termios
E ModuleNotFoundError: No module named 'termios'
Interrupted: 1 error during collection

This PR cherry-picked onto today's main (381a6b8c4 → clean auto-merge):

14 tests collected in 0.41s

Run (after cherry-pick):

9 passed, 4 skipped, 1 failed in 0.64s

The single failure is pre-existing platform gap, not introduced by this diff:

test_systemd_install_checks_linger_status
E AttributeError: module 'os' has no attribute 'getuid'
  at hermes_cli/gateway.py:1882

Matches the PR note that remaining failures are separate Windows/systemd gaps (same family as #48986), not the collection-time pty import.

tests/test_run_tests_parallel.py

Cherry-pick also applied cleanly. On this Git Bash shell both before and after collect 7 testsTMPDIR=/tmp is set by MSYS, so the old Path(os.environ.get("TMPDIR", "/tmp")) path does not blow up here.

The tempfile.gettempdir() swap is still the right fix for PowerShell / cmd contributors where TMPDIR is unset (PR description). Under this shell, gettempdir()C:\Users\Admin\AppData\Local\Temp and mkdir succeeds. I did not re-run a pure PowerShell collection in this session; treating that half as documented by the author and orthogonal to the gateway fix which I fully reproduced.

Rebase note

PR head is ~600 commits behind main. Cherry-pick of 381a6b8c4 onto current main was clean (auto-merge both files, no conflicts). A maintainer rebase should be low-friction.

Related

LGTM on the gateway collection fix from native Win11; recommend rebase + merge when convenient.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows collection fix. Current main still imports pty at tests/hermes_cli/test_gateway.py:5, before the Windows skip marker at line 55 can apply; the only pty.openpty() use is inside that skipped test at line 114. It also still creates _HANDOFF_DIR from the "/tmp" fallback at tests/test_run_tests_parallel.py:37, before that file's Windows skip marker at line 66.

The PR moves the first import into the POSIX-only execution path and replaces the second fallback with tempfile.gettempdir(). A repository-wide search found no other top-level import pty in tests/ and no second os.environ.get("TMPDIR", ...) handoff-path construction. The diff is appropriately limited to these two collection-time blockers.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
tests/test_run_tests_parallel.py builds its handoff directory from
`os.environ.get("TMPDIR", "/tmp")` and mkdir()s it at module scope. On
Windows TMPDIR is normally unset -- the platform uses TEMP/TMP -- so the
fallback resolves to a non-existent \tmp on the current drive and raises
during collection:

  tests\test_run_tests_parallel.py:38: in <module>
      _HANDOFF_DIR.mkdir(exist_ok=True)
  E   FileNotFoundError: [WinError 3] The system cannot find the path
      specified: '\tmp\hermes-isolation-probe'

pytest aborts the whole run on a collection error, so this one line stops
the entire suite on native Windows.

The module already declares the right intent -- its docstring says
"POSIX-only ... Marked accordingly" and the probe carries
`@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only probe")`.
The marker never gets a chance to apply, because the mkdir runs at import
time.

`tempfile.gettempdir()` is the cross-platform primitive for this, and it
still honours $TMPDIR first, so POSIX behaviour is byte-identical. Nothing
else changes: `_HANDOFF_DIR` is read only by `_handoff_path_for`, whose
sole caller is the skipped POSIX probe.

Verified on native Windows 11 (build 26200), Python 3.11.6, from a
PowerShell prompt where TMPDIR is unset:

  before:  Interrupted: 1 error during collection, no tests collected
  after:   7 tests collected, exit 0

Note for reviewers running Git Bash: MSYS sets TMPDIR, which masks this
failure entirely -- it only reproduces from PowerShell or cmd.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hyqqx
hyqqx force-pushed the fix/windows-test-collection branch from 381a6b8 to 2350f21 Compare August 2, 2026 17:49
@hyqqx hyqqx changed the title fix(tests): make the suite collectable on native Windows fix(tests): use the platform temp dir for the isolation-probe handoff Aug 2, 2026
@hyqqx

hyqqx commented Aug 2, 2026

Copy link
Copy Markdown
Author

Rebased onto current main and narrowed the scope. Thanks @monerostar and the sweeper for the review — both points are addressed.

The import pty half landed independently. 05504bd9f (test(gateway): make test_gateway collectable on Windows, @iso2kx, 2026-07-30) made the same change at the same call site while this was open. I dropped it from this branch rather than carry a conflicting duplicate — the rebase conflicted on exactly that hunk, which is how I found it.

Rebase done. The head was ~600 commits behind; it is now one commit on top of main (710b02663), so there is nothing for a maintainer to untangle.

What remains is still broken on main today. Re-verified on native Windows 11 (build 26200, Python 3.11.6), from PowerShell:

$ python -m pytest tests/test_run_tests_parallel.py --collect-only -q
E   FileNotFoundError: [WinError 3] The system cannot find the path specified:
    '\tmp\hermes-isolation-probe'
Interrupted: 1 error during collection
no tests collected, 1 error in 0.50s

With this commit: 7 tests collected in 0.12s, exit 0.

Worth restating @monerostar's observation, since it explains why this survived so long: Git Bash sets TMPDIR, so the failure is invisible there. It only reproduces from PowerShell or cmd — which is the shell the Windows install guide tells users to open.

@tneemo tneemo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent verification — #74083 (Windows: fix collection abort from /tmp fallback)

Verified the PR head (2350f21b, 1 file) on a real Windows checkout.

The bug is real and reproduced: tests/test_run_tests_parallel.py builds its handoff dir from os.environ.get("TMPDIR", "/tmp") and mkdir()s it at module scope. On native Windows (PowerShell) TMPDIR is unset, so the fallback resolves to a non-existent \tmp on the current drive → FileNotFoundError at import time, before the skipif(win32) marker can apply → pytest aborts collection for the entire suite (~50k tests). The PR description's traceback is accurate.

Verification results:

  • ✅ Head fetched; parses clean (386 lines)
  • tempfile.gettempdir() replaces the /tmp fallback; _HANDOFF_DIR.mkdir still at module scope but now targets the real platform temp dir
  • Reproduced the critical case on this box: with TMPDIR/TMP/TEMP all removed (pure PowerShell scenario), tempfile.gettempdir()C:\Users\tneemo\AppData\Local\Temp and the module imports cleanly — collection no longer aborts
  • ✅ Module tests: 5 passed, 1 skipped; the 1 failure (test_file_retry_self_heals_and_prints_both_attempts) is a pre-existing Windows subprocess/PATH issue that fails identically on main without the PR (verified by checkout) — not a regression
  • ✅ POSIX behavior unchanged: gettempdir() honors TMPDIR first, so Linux/macOS runs behave as before

Design notes:

  • Correct diagnosis: the marker can't help when the crash is at import time — moving to a cross-platform primitive is the right fix, not more skip markers
  • One-line change, zero behavior drift on POSIX

Verdict: Ready to land. Real Windows blocker (entire test suite uncollectable from PowerShell), minimal fix, verified on the actual failure mode.

This was generated by AI, Review is declarative

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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.

5 participants