Skip to content

fix(tests): use cross-platform pytest-timeout method#39881

Merged
austinpickett merged 1 commit into
NousResearch:mainfrom
rodboev:pr/pytest-timeout-thread
Jun 9, 2026
Merged

fix(tests): use cross-platform pytest-timeout method#39881
austinpickett merged 1 commit into
NousResearch:mainfrom
rodboev:pr/pytest-timeout-thread

Conversation

@rodboev

@rodboev rodboev commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Windows pytest runs inherited --timeout-method=signal from pyproject.toml, and pytest-timeout implements that method with SIGALRM. Windows does not expose signal.SIGALRM, so local suite runs crashed with INTERNALERROR: AttributeError: module 'signal' has no attribute 'SIGALRM' before reaching normal test failures.

This switches the per-test timeout method to thread, which is pytest-timeout's cross-platform mode. The repo still has the outer per-file subprocess timeout from scripts/run_tests_parallel.py, so process-level hang protection remains in place while direct Windows pytest runs can execute.

Fixes #39880

Changes

  • pyproject.toml: updates the pytest-timeout comment and changes --timeout-method=signal to --timeout-method=thread in [tool.pytest.ini_options] addopts.

Validation

Scenario Before After
Windows full-suite pytest Crashed during collection with AttributeError: module 'signal' has no attribute 'SIGALRM' Reached normal test execution without SIGALRM or INTERNALERROR
Per-test timeout behavior POSIX-only signal alarm Cross-platform thread timeout
Outer process timeout Unchanged Unchanged, scripts/run_tests_parallel.py still owns per-file process isolation

Test plan

  • $env:BROWSER='echo'; C:\Apps\hermes\hermes-agent\venv\Scripts\python.exe -m pytest tests/ -q --tb=line 2>&1 | Select-String 'SIGALRM' before the change reproduced the crash with AttributeError: module 'signal' has no attribute 'SIGALRM'.
  • $env:BROWSER='echo'; C:\Apps\hermes\hermes-agent\venv\Scripts\python.exe -m pytest tests/ -q --tb=line after the change reached ordinary test execution and stopped on a Python-level pytest-timeout in tests/cli/test_surrogate_sanitization.py; no SIGALRM or INTERNALERROR appeared in the captured log.

@rodboev rodboev requested a review from a team June 5, 2026 14:40
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists type/test Test coverage or test infrastructure labels Jun 5, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed by Hermes Agent — Approve

Correct cross-platform fix. --timeout-method=signal uses SIGALRM, which doesn't exist on Windows (signal.SIGALRMAttributeError), so Windows pytest runs crash with INTERNALERROR before any test runs. Switching to thread is pytest-timeout's documented cross-platform method. I verified --timeout-method=thread runs cleanly here (timeout method: thread, tests pass), and the change correctly keeps the 30s cap and updates the explanatory comment rather than dropping it.

Cross-ref: this is the proper, scoped home for this change. PR #41754 (security hardening) had snuck the same signal→thread switch in as an unrelated hunk — but bumped the timeout to 60s and deleted the comment. I've asked #41754 to drop that hunk in favor of this PR. LGTM here.

@austinpickett austinpickett merged commit 5750d05 into NousResearch:main Jun 9, 2026
26 checks passed
wachoo pushed a commit to wachoo/hermes-agent that referenced this pull request Jun 10, 2026
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists type/bug Something isn't working type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pytest-timeout SIGALRM crashes entire suite on Windows

3 participants