mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-10-20 05:15:31 +00:00
test(hermes_cli): scope concurrent-gate fixture to Windows, fix race at source
The autouse _suppress_concurrent_hermes_gate fixture imported and monkeypatched hermes_cli.main for EVERY test in the package — which is what raced a partially-initialized main module under pytest's per-test spawn isolation (the AttributeError flake hardened with raising=False in the prior PR). But _detect_concurrent_hermes_instances already short-circuits to [] via 'not _is_windows()' on every non-Windows host, so the stub does nothing useful on Linux CI or macOS — it only matters for a Windows dev running the suite via hermes itself. Gate the whole fixture behind sys.platform=='win32' so CI never imports or mutates main here, removing the race at its source while preserving the Windows-dev behavior the fixture exists for. Keep raising=False as defense-in-depth on the Windows path. Verified: config_validation (the file that flaked), the real_concurrent_gate windows tests, and cmd_update/autostash update tests all pass on Linux — the update tests rely on the real helper's natural [] return, confirming the stub was redundant off-Windows.
This commit is contained in:
parent
f8adefdebf
commit
df9061743d
1 changed files with 26 additions and 18 deletions
|
|
@ -2,6 +2,8 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
|
|
@ -21,33 +23,39 @@ def all_assignees_spawnable(monkeypatch):
|
|||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _suppress_concurrent_hermes_gate(request, monkeypatch):
|
||||
"""Default ``_detect_concurrent_hermes_instances`` to ``[]`` for every test.
|
||||
"""Default ``_detect_concurrent_hermes_instances`` to ``[]`` on Windows hosts.
|
||||
|
||||
The Windows update path now refuses to proceed when another
|
||||
``hermes.exe`` is detected (issue #26670). On a developer's Windows
|
||||
machine running the test suite via ``hermes`` itself, this would
|
||||
flag the running agent as a concurrent instance and abort every
|
||||
``cmd_update`` test. Tests that want to exercise the gate explicitly
|
||||
re-patch ``_detect_concurrent_hermes_instances`` with their own
|
||||
return value — autouse here gives a clean default without touching
|
||||
the rest of the suite.
|
||||
The Windows update path refuses to proceed when another ``hermes.exe`` is
|
||||
detected (issue #26670). On a developer's Windows machine running the test
|
||||
suite via ``hermes`` itself, this would flag the running agent as a
|
||||
concurrent instance and abort every ``cmd_update`` test. This fixture
|
||||
stubs the helper to ``[]`` so those tests run cleanly.
|
||||
|
||||
Tests that need to call the REAL function (e.g. unit tests for the
|
||||
helper itself) opt out with ``@pytest.mark.real_concurrent_gate``.
|
||||
Scope: the helper short-circuits to ``[]`` via ``not _is_windows()`` on
|
||||
every non-Windows host (Linux CI, macOS), so there is nothing to suppress
|
||||
there — and importing + monkeypatching ``hermes_cli.main`` for every test
|
||||
in the package is exactly what raced a partially-initialized module under
|
||||
pytest's per-test spawn isolation (the AttributeError flake). Gating the
|
||||
whole fixture behind ``sys.platform == "win32"`` means CI never imports or
|
||||
mutates ``main`` here, removing the race at its source while preserving the
|
||||
Windows-dev behavior the fixture exists for.
|
||||
|
||||
Tests that need to call the REAL function (e.g. unit tests for the helper
|
||||
itself, or that force ``_is_windows`` True) opt out with
|
||||
``@pytest.mark.real_concurrent_gate``.
|
||||
"""
|
||||
if sys.platform != "win32":
|
||||
return
|
||||
if request.node.get_closest_marker("real_concurrent_gate"):
|
||||
return
|
||||
try:
|
||||
from hermes_cli import main as _cli_main
|
||||
except Exception:
|
||||
return
|
||||
# raising=False: under pytest's per-test spawn isolation, a concurrent
|
||||
# xdist worker importing a module that transitively touches hermes_cli.main
|
||||
# can briefly expose a partially-initialized module object here — one where
|
||||
# _detect_concurrent_hermes_instances isn't defined yet. A bare setattr
|
||||
# would raise AttributeError and error the (unrelated) test. The attribute
|
||||
# always exists once main.py finishes importing, so a no-op when it's
|
||||
# transiently absent is the correct, race-free default.
|
||||
# raising=False: defense-in-depth against a transiently partial
|
||||
# hermes_cli.main module under spawn isolation. The attribute always
|
||||
# exists once main.py finishes importing, so a no-op when it's briefly
|
||||
# absent is the correct, race-free default.
|
||||
monkeypatch.setattr(
|
||||
_cli_main,
|
||||
"_detect_concurrent_hermes_instances",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue