fix(tests): make the test suite pass on native Windows, run the full suite in Windows CI - #96458
Merged
ethernet8023 merged 19 commits intoSep 24, 2026
Merged
Conversation
alt-glitch
added
type/bug
ethernet8023
force-pushed
the
ethie/windows-tests
branch
from
August 30, 2026 18:13
11b510f to
347766c
Compare
ethernet8023
force-pushed
the
ethie/windows-tests
branch
from
August 30, 2026 18:27
347766c to
0efabf2
Compare
ethernet8023
changed the title
ethernet8023
force-pushed
the
ethie/windows-tests
branch
2 times, most recently
from
August 31, 2026 20:10
1b855f3 to
4868b5c
Compare
Gate the POSIX-only and symlink-only tests with the linux_only and require_symlinks markers. Fix the real cross-platform bugs: - file_operations: use the translate_path flag, send snippets as base64, and use sys.executable (the MS Store python3 stub and the list2cmdline backslash collapse both broke snippets) - approval: treat backslash as a Windows path separator, not an escape - registry, browser_registry, secret_sources, plugins: normalize scope-key case - checkpoint_manager, plugins_cmd: clear read-only bits before delete - deadline: make MAX_SAFE_TIMEOUT_S fit the Windows limit - image_routing, acp, cua_backend, daytona: fix Windows and POSIX paths - hermes_state: match backslash in the retag LIKE clause - kanban, disk-cleanup: match drive paths and split command arguments - scripts: emit host separators through as_posix 207 test files are gated or isolated.
- dashboard_auth_gate: bind port 0, not 9119 (the pre-bind probe fails on any machine with a live gateway on the default port) - dashboard_unified_launch: cover the Windows subprocess.Popen reexec path - hermes_state FTS5: force the non-WAL path so the trace callback on db._conn observes the enrichment query
hermes_home_key already resolved None to the default home. Make it treat an empty string the same way, then remove the now-redundant guards at the call sites: - scope or hermes_home_key() -> hermes_home_key(scope) - the two ternary forms -> hermes_home_key(scope) The scope=None sites that mean "global fallback" are unchanged.
…only Fold the OS-specific lanes into tests.yml's test job as a three-OS matrix. Linux and Windows run the full suite (conftest skips foreign-OS markers). macOS keeps macos_only for now. Delete tests-os.yml.
…test gaps The scope-normalization sweep only fixed the read paths (list_providers/get_provider/registry_generation) in most registries; the write paths (register/snapshot/restore) still stored under the raw scope string while reads resolved through hermes_home_key — so a scoped registration was written under one key and read under another (agent/terminal_env_registry.py test_scoped_registration_isolated failed on both Linux and Windows CI). - add hermes_constants.normalize_scope(scope): hermes_home_key for non-None, None preserved for the process-global layer — one named function instead of the expression repeated at ~20 call sites - apply it on both sides of all nine scoped registries (browser, terminal_env, image_gen, transcription, tts, video_gen, web_search, secret_sources, dashboard_auth, tools.registry) - agent/deadline: MAX_SAFE_TIMEOUT_S on win32 leaves 60s headroom under the DWORD-ms ceiling so margin-adding consumers (human-wait +60s) stay platform-safe; tools/approval fail-closed fallback mirrors it - tools/approval: strip shell escapes everywhere except inside drive-anchored paths (the Windows shell is bash, so r\m still spells rm; only C:\... backslashes must survive deobfuscation) - consolidate the duplicated _rmtree_force into hermes_cli/fs_utils and repoint both callers - hermes_cli/kanban + disk-cleanup: shlex.split(posix=False) keeps wrapping quotes in tokens; strip them before dispatch/matching - test gating: SIGKILL durability conformance cells are linux_only (TerminateProcess is not SIGKILL), macOS TCC anchor symlink-tree classes are linux_only, Linux chromium profile tests resolve the fixture home via expanduser patch + posix-joined expecteds so they run identically on every host
…ndows CI normalize_scope unified the two scope contracts that live on the same registries, which was wrong: WRITE paths (register/snapshot/restore) treat None as the process-global layer, but READ paths (get_provider/ list_providers/registry_generation) treat None as 'resolve to the active home's scope' — hermes_home_key(None) is the active default home. Routing reads through normalize_scope hid every scoped registration from ambient-home lookups: 52 linux failures across plugin discovery, secret-source profile isolation, web-search/browser backend selection (the CI run on e66a627aa5). - read sites go back to hermes_home_key(scope); write/slot sites keep normalize_scope (None preserved); the docstring now spells out both contracts so the unification bug can't be reintroduced blind - run_tests_parallel.py default worker count: cpu_count*2 -> cpu_count. Oversubscription made the 32-core windows runner run 64 pytest subprocesses; the suite's own linux measurement showed one worker per core is the flat optimum, and windows CI showed the cost — 160 files past the 300s per-file cap (they pass in seconds on an idle box), each burn a kill + retry cycle, and the churn dominated the lane's wall clock. docker.yml's explicit nproc pin now just documents its own intent instead of fighting a *2 default - windows CI lane: HERMES_TEST_FILE_TIMEOUT=900 (via a matrix field) so contended-but-passing files stop getting killed at 300s
At 60 minutes the lane hit the job ceiling at 88.6% (run 33338310764) with only 9 files actually hitting the per-file cap — the 900s file timeout worked, but the lane's long tail of server/socket/subprocess files that run minutes-per-file on the CI runner (seconds on an idle dev box) needed more wall, not more file headroom. 120 min fits the observed curve (76 files >300s; ~11.4% of work left at the 60-min mark) with margin, and the per-file cap still bounds any genuinely hung file. Linux and macOS keep 30-minute budgets.
Process-spawn-heavy test files pay the real-time scan on every spawn and temp write; that inflated the lane's slow tail to minutes-per-file (they pass in seconds on an idle dev box). The runner is ephemeral and single-purpose — scanning it protects nothing. Best-effort step: if a runner image refuses the preference change, the lane still passes, just slower.
Two of the three failure classes on the windows-2025 runner are environmental, not test bugs: - bash resolved to the WSL stub in System32 (no distro installed), so every bash-harness test got the UTF-16 banner and exit 1. Prepend Git bin to PATH. - 16 large files hit the 900s per-file cap under 64-worker contention (1802s = cap x 2 attempts). Pin workers to 32 and raise the cap to 2400s, lane budget to 150 min.
…nner The per-file model pays a spawn+import wall of ~0.5-1.5s per file x ~3400 files on the Windows runner — a ~6-minute floor before any test runs (measured: bare spawn 140ms serial / 531ms under 32-way contention; the full pytest boot 1.4-1.8s per file; one-process collection of the whole tree takes only 43s, so the architecture itself is the ~8x multiplier). xdist with --dist loadfile pays that wall once per worker and keeps each file's tests on ONE worker. Cross-file state pollution is bounded to files co-scheduled on a worker; failures from that are stateful-test bugs to fix (the same class PR NousResearch#29016 fixed when it moved the other way). Linux stays on the per-file runner: its spawn floor is ~15ms, so isolation is nearly free there. Add pytest-xdist==3.8.0 to the dev extras and scripts/run_xdist.sh. Windows-only experiment; the local 6h36m xdist run on a polluted dev box (MSIX PYTHONPATH + SAC) is not representative — CI renders the real verdict.
The first xdist run died with 'runner lost communication' (run 33389773498, 1h15m, no logs flushed). Cause: the per-file runner launched every pytest with start_new_session, so each file's process-tree-killing tests could only reach their own children. xdist puts 32 workers and the controller in one process family, and the live_system_guard_bypass tests run REAL kill sweeps (install.ps1 -Stage venv does per-PID taskkill /F /T against every python.exe it enumerates) — a sweep under shared workers can take out a sibling worker or the runner agent itself. Two phases now: xdist loadfile for the bulk; the 15 process-killer files run serially, one plain pytest per file, so a sweep's only reachable victims are its own children.
…mmed CLIs test_command_token_source: the key_cmd fixtures were POSIX shell commands (printf/echo/date) run through _mint's shell=True — cmd.exe on Windows, where none of those behave the same. New _pycmd() helper base64-wraps a python -c payload so the command is pure printable ASCII that both cmd.exe and POSIX sh run unchanged. doctor: _gh_authenticated() crashed the whole doctor run with WinError 5 when the PATH gh resolves to a Windows Store/MSIX reparse-point shim (CreateProcess denies those outside the package context). Treat an unspawnable gh as not-authenticated — a diagnostic probe must never take down the run. The shared test helper also stops doctor from probing the ambient gh at all (the gh-specific behaviors have their own mocked tests).
… tests - test_web_ui_build: the mock seam drifted — _build_web_ui resolves npm through _resolve_node_runtime_npm (hermes_constants' launchable-variant scan), not shutil.which; patch the actual seam. On Windows the old mock resolved None and the build path bailed before the assertions under test - test_update_wedged_gateway: TestLoopTickWitness binds real AF_UNIX sockets, which Windows CPython builds don't expose — gate the class to the POSIX lanes that run that transport - test_browser_real_profile: the SingletonLock fixture symlink falls back to a placeholder file on Windows without the symlink privilege (same exclusion contract); the two POSIX-mode assertions (0600/0700 chmod bits are not expressible by os.chmod on Windows) gate linux_only; test_relaunch_path_does_snapshot now mocks Popen (the real-binary launch) like its siblings instead of only subprocess.run
…rocess machinery scripts/run_tests.sh now runs pytest-xdist -n <N> --dist loadfile as the single canonical path on every OS (Linux and Windows CI lanes both use it). The per-file subprocess model (run_tests_parallel.py, and the interim run_xdist.sh experiment) is deleted along with its two self-tests: persistent xdist workers pay the interpreter+import wall once per worker instead of once per file (~0.5-1.5s x ~3400 files was a ~6-minute floor on Windows), and --dist loadfile pins a file's tests to ONE worker, bounding state pollution to co-scheduled files — which is exactly the class of flake we are now committing to fix properly. The serial process-killer quarantine phase is dropped too. It existed to keep process-tree-sweep tests from killing sibling xdist workers; the durable fix belongs in those tests (sweeps must target their own children, not enumerate every python process), and keeping a divergent two-phase path would hide that work. Kept from the old wrapper: hermetic env -i scrubbing, Windows location-var forwarding, venv probing, bytecode pre-compile, -m 'not integration', and the HERMES_TEST_IMAGE docker-knob allowlist. HERMES_TEST_FILE_TIMEOUT/FILE_RETRIES/SLICE go away with the runner they parameterized; -j/HERMES_TEST_WORKERS now map to xdist -n (Linux CI pins 96, Windows 32, default auto). Docs updated to match: AGENTS.md (runner contract, flake policy, isolation section), CONTRIBUTING.md, tests/conftest.py comments, classify_changes docstring + its lane expectation (a .sh runner no longer trips the supply-chain scan lane), comfyui README, hermes-agent contributor guide, debugpy skill and its website doc.
A process created a directory literally named '%SystemDrive%' in the repo root (unexpanded env-var path) and the previous commit's git add -A swept its Defender cache .db files in. Not part of the tree.
The fixed linux_only/macos_only/windows_only trio can only say 'one OS,
no qualifiers' — it cannot express 'anything except macOS', 'Windows
but only arm64', or 'POSIX-family behaviour'. The new platforms marker
takes any number of spec strings (any-of) plus optional arch filters:
@pytest.mark.platforms("linux")
@pytest.mark.platforms("not macos")
@pytest.mark.platforms("windows", arch="arm64")
@pytest.mark.platforms("posix")
Specs: linux / macos / windows / posix / any and 'not <spec>'.
arch matches platform.machine() with alias normalization
(amd64→x86_64, aarch64→arm64); arch_negate inverts it. Unknown specs
and stray keyword arguments are hard UsageErrors — a silent typo would
mean a test skipped on every host, which is the exact green-zero-
coverage failure the marker machinery exists to prevent.
The legacy trio remains accepted as aliases routing through the same
skip path (a mechanical rewrite of the ~500 existing call sites is a
separate sweep); tests/hermes_cli/test_linux_desktop_entry.py converts
as the reference usage. list_os_marked_tests.py now accepts both the
short platform names (matching quoted platforms() specs, including
negated ones) and the legacy _only names, and returns nonzero when a
marker selects nothing. The macOS CI lane passes -m macos.
Also documents the module-mark stacking trap: module-level
platforms/_only plus a per-test host marker trips the conftest's
double-mark hard reject — AGENTS.md now says so. Verified: the earlier
xdist INTERNALERROR crash on test_linux_desktop_entry +
test_browser_real_profile was exactly that stacked double mark, gone
with per-test platforms() marks (76 passed, 44 skipped on Windows).
…/windows_only trio
No shims, one marker system. The full mechanical sweep:
- 165 test files converted: every @pytest.mark.<os>_only decorator and
pytestmark assignment is now platforms("<os>"). Docstring/comment
prose mentioning the trio rewritten to the platforms() vocabulary.
- conftest: _OS_MARKS, the legacy skip loop, the double-mark reject for
the trio, and the selector-tagging shim are deleted. The single
platforms() gate does all host gating; _reject_contradictory_platform_
marks replaces the old reject (one platforms() marker per test — a
module-level pytestmark stacked on a per-test marker is the historic
skipped-everywhere-green-everywhere failure and stays a hard error).
- pyproject: only the platforms marker is registered.
- list_os_marked_tests.py: rewritten for the single vocabulary — takes
a platform name (linux/macos/windows), matches quoted platforms(...)
specs including negated and any-of forms, exits nonzero on empty
selection. Its test file rewritten to match (bare identifiers must
not match; the spec must appear inside the string literal).
- tests.yml macOS lane: marker: macos feeds the lister; the pytest
selection is -m "platforms and not integration" — the marker name is
the selector, the conftest's per-test host skips are the gate.
- test_os_marker_gating.py rewritten for the new reject (the old file
tested deleted machinery).
- AGENTS.md / CONTRIBUTING.md updated to the single vocabulary.
Verified: full-tree compile; marker unit tests (36); lister tests;
macOS-lane simulation (21 files, 180 collected / 429 deselected on a
windows host); broad xdist slices through scripts/run_tests.sh
(367 + 1674 passed, zero refactor-attributable failures — the 6 red
tests in the second slice fail identically with the changes stashed).
…, xdist on Windows
xdist-as-standard was the wrong shape for Linux: the per-file subprocess
model ran the full suite there in 3m45s with zero cross-file failures,
while the first xdist run took 7m51s and failed 111 tests. The two
costs are structural opposites:
* xdist multiplies full-tree collection by the worker count — every
worker imports the whole suite and pays pytest's per-item
fixture-closure machinery (profiled: ~98M function calls to
collect 42k items against the conftest's autouse fixtures —
_matchfactories alone 1.37M calls, traverse_fixture_closure 783k).
On the 96-core runner that front-loads 106s before any test runs.
* The per-file model pays a spawn+import wall per file: ~15ms on
Linux (nothing), 0.5-1.5s on Windows (~6-min floor across 3400
files — the dominant cost of that lane).
So scripts/run_tests.sh now dispatches on host: run_tests_parallel.py
(restored verbatim from b8696aedea^, with its two self-tests) on POSIX,
pytest-xdist --dist loadfile on Windows. Both paths share one hermetic
env contract (env -i scrub, TZ=UTC, PYTHONHASHSEED=0, venv probing,
bytecode pre-compile). -j/HERMES_TEST_WORKERS feeds either backend.
The 111 xdist co-scheduling failures on Linux were the per-file
model's isolation guarantee surfacing as test bugs — with the POSIX
lane back on per-file, that class disappears where it never existed;
the Windows lane keeps loadfile (with its remaining co-scheduling
hazards as fix-at-the-test work).
Workflow comments, AGENTS.md, CONTRIBUTING.md, conftest comments and
the classify test follow the dual-path truth.
New dev extra exact-pins pytest-xdist==3.8.0; the every-exact-pin-exempt contract test caught the missing whitelist entry.
ethernet8023
force-pushed
the
ethie/windows-tests
branch
from
August 31, 2026 22:00
4868b5c to
65ce9f4
Compare
This was referenced Sep 2, 2026
salch-cred
added a commit
to salch-cred/hermes-agent
that referenced
this pull request
Sep 3, 2026
The rg-gated tests evaluated subprocess.run(["which", "rg"]) inside the skipif decorator arguments at module import. On Windows there is no which executable, so that raises FileNotFoundError during collection and the whole 10-test module is lost instead of two tests skipping. Gate the rg tests on shutil.which (resolved once at import, no subprocess), and POSIX-skip the find/grep fallback tests whose shell command strings only exist on Unix. Windows now collects 10 tests (2 cross-platform pass, 8 platform-skips); Linux CI is unchanged — every test still runs there. Not covered by NousResearch#96458's 305-file set.
This was referenced Sep 3, 2026
This was referenced Sep 5, 2026
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes the test suite pass on a native Windows host, with no regression on
Linux. It also turns on the full suite for Windows in CI.
The Windows baseline was 227 failing test files and about 1290 failing nodes.
Most failures had two causes: tests that use POSIX-only or symlink-only
behavior, and real cross-platform bugs in the production code. This PR fixes
both. The CI change then makes Windows run the full suite on every PR, the
same as Linux, so the fixes stay fixed.
Related Issue
N/A (no tracked issue).
Type of Change
Changes Made
Production fixes (19 files)
use sys.executable (the MS Store python3 stub and the list2cmdline
backslash collapse both broke snippets)
listings and their footer examples use forward slashes on every OS)
Test changes (about 210 files)
hardcoded port.
CI (tests.yml, ci.yaml; tests-os.yml deleted)
scripts/run_tests.sh. The conftest OS-marker skip does the platform gating.
exit-5 guard as before.
scope or hermes_home_key()guards at the call sites are gone.How to Test
scripts/run_tests.sh <file>for any file this PRtouches. It passes.
on the foreign OS, and the rest must pass.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — I ran each touched file throughscripts/run_tests.sh; CI owns the full runDocumentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A