Skip to content

fix(test): support Windows virtualenvs in canonical runner - #63642

Closed
lucamoretti09 wants to merge 1 commit into
NousResearch:mainfrom
lucamoretti09:contrib/windows-test-runner
Closed

fix(test): support Windows virtualenvs in canonical runner#63642
lucamoretti09 wants to merge 1 commit into
NousResearch:mainfrom
lucamoretti09:contrib/windows-test-runner

Conversation

@lucamoretti09

Copy link
Copy Markdown

What

Makes the canonical scripts/run_tests.sh work with both virtualenv layouts:

  • POSIX: .venv/bin/python
  • native Windows: .venv/Scripts/python.exe

It also checks both .venv and venv under the repository and installed Hermes checkout, and preserves USERPROFILE plus PYTHONUTF8=1 inside the runner's hermetic env -i environment.

The bundled Hermes skill documentation is updated to distinguish a supported Windows venv layout from the smaller end-user venv that may intentionally omit pytest.

Verification

  • 2 passedtests/scripts/test_run_tests_windows_venv.py
  • bash -n scripts/run_tests.sh using Git for Windows Bash
  • git diff --check
  • staged secret-pattern scan: 0 hits

Scope

This only changes interpreter discovery and the clean environment. POSIX behavior and test-runner arguments remain unchanged.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure platform/windows Native Windows-specific behavior or breakage P3 Low — cosmetic, nice to have sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing native Windows test-runner discovery. The underlying defect is present on current main: scripts/run_tests.sh:42-54 only accepts bin/activate and derives bin/python, so Scripts/python.exe venvs cannot run.

Problems

  • tests/scripts/test_run_tests_windows_venv.py:10 reads run_tests.sh and asserts source strings. This is a banned source-snapshot pattern under AGENTS.md:1358-1411; it does not exercise interpreter resolution or the hermetic environment.
  • website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md:7 says the page is generated from skills/autonomous-ai-agents/hermes-agent/SKILL.md. The PR edits only the generated page, while the source skill still says the wrapper is POSIX-only at skills/autonomous-ai-agents/hermes-agent/SKILL.md:868 and :1067.

Suggested changes

  • Replace the snapshot test with a behavioral runner test using a temporary fixture and real shell invocation; cover both interpreter layouts.
  • Edit the source SKILL.md and regenerate its website page.

This is an automated hermes-sweeper review.


def test_runner_supports_posix_and_windows_virtualenv_layouts():
script = RUNNER.read_text(encoding="utf-8")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test snapshots shell source rather than executing behavior, which AGENTS.md:1358-1411 bans. Please replace it with a behavioral invocation against a temporary fixture/venv so it proves the selected interpreter and clean environment work.


**`scripts/run_tests.sh` doesn't work as-is on Windows** — it looks for
POSIX venv layouts (`.venv/bin/activate`). The Hermes-installed venv at
**`scripts/run_tests.sh` supports native Windows venv layouts** — it probes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This page is generated from skills/autonomous-ai-agents/hermes-agent/SKILL.md (its header says so). Please update that source skill and regenerate this page; otherwise the shipped skill continues to say the wrapper is POSIX-only.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@lucamoretti09 lucamoretti09 closed this by deleting the head repository Jul 20, 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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

3 participants