From df9061743db56b0421be7e2ceb7421896f77adf4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:46:07 -0700 Subject: [PATCH] test(hermes_cli): scope concurrent-gate fixture to Windows, fix race at source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/hermes_cli/conftest.py | 44 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/tests/hermes_cli/conftest.py b/tests/hermes_cli/conftest.py index 2848a8a9aa23..4673479d0de9 100644 --- a/tests/hermes_cli/conftest.py +++ b/tests/hermes_cli/conftest.py @@ -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",