diff --git a/AGENTS.md b/AGENTS.md index 95f3985c4b..e5391fc870 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1369,6 +1369,44 @@ Any test that reads or asserts about `package.json`, `package-lock.json`, `tsconfig.json`, `.ts`/`.tsx`/`.js`/`.mjs`/`.cjs` source files configuration belongs in the JS (vitest) test suite, not in `tests/*.py`. +### Don't fake the host OS + +Hermes supports Linux, macOS and native Windows, and plenty of its behaviour +genuinely differs per host. Those differences are tested by running on the +host, not by patching `sys.platform`. + +```python +@pytest.mark.linux_only +@pytest.mark.macos_only +@pytest.mark.windows_only +``` + +Things that are host-independent can stay unmarked: + +- **Pure functions that take a platform as data** — + `hidden_windows_child_options(opts, is_windows=True)` is input→output, not a + fake host. (Contrast: setting a module-level `IS_WINDOWS` flag and then + calling `windows_detach_flags()` *is* a fake.) +- **Declaration/packaging invariants** — "pyproject declares `tzdata` with a + `sys_platform == 'win32'` marker" asserts about a file, not about runtime. + +The line: **if the test needs the interpreter to believe it is on another OS +in order to pass, it belongs on that OS.** +When one test body walks several platforms in sequence, split it. +Keep the host-native arm on the Linux lane and move the other arm into its own marked test. + +**Use the marker, never a bare `skipif`.** `scripts/ci/list_os_marked_tests.py` +decides which files the macOS/Windows lanes import by grepping for the marker +*name*, and the lane then filters with `-m `. A test gated with +`@pytest.mark.skipif(sys.platform != "win32")` therefore skips on Linux AND is +never imported on the Windows lane — it runs on no host at all, silently. The +same trap catches a file-local alias (`windows_only = pytest.mark.skipif(...)`): +the grep matches the name, so the file *is* listed, but `-m windows_only` +deselects every test in it and the lane reports green over zero coverage. +Equally, don't `pytest.skip()` the non-host rows of a `@parametrize` over +platforms — split it into one marked test per OS, or only the host's row ever +executes. + ### Don't write change-detector tests A test is a **change-detector** if it fails whenever data that is **expected diff --git a/pyproject.toml b/pyproject.toml index 5c6ea039d9..52fd270462 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -418,6 +418,9 @@ markers = [ "requires_wal: needs the runtime to actually enable SQLite WAL mode (skipped where Hermes falls back to journal_mode=DELETE)", "no_isolate: opt out of per-file subprocess isolation (tests share mutable module-level state)", "ssh: marks tests requiring a reachable SSH server (skipped in normal CI)", + "linux_only: exercises Linux-specific behaviour; skipped on other hosts", + "macos_only: exercises macOS-specific behaviour; skipped on other hosts", + "windows_only: exercises native-Windows behaviour; skipped on other hosts", ] # integration tests take way too long to run in the normal CI environments addopts = "-m 'not integration'" diff --git a/tests/agent/lsp/test_install_and_lint_fixes.py b/tests/agent/lsp/test_install_and_lint_fixes.py index bfe28f3527..b614cee50f 100644 --- a/tests/agent/lsp/test_install_and_lint_fixes.py +++ b/tests/agent/lsp/test_install_and_lint_fixes.py @@ -61,8 +61,15 @@ def test_install_npm_works_without_extras(tmp_path, monkeypatch): +@pytest.mark.windows_only def test_install_pip_finds_windows_scripts_launcher(tmp_path, monkeypatch): - """pip console scripts can land in Scripts/ on native Windows.""" + """pip console scripts can land in Scripts/ on native Windows. + + ``windows_only``: the ``Scripts/`` layout and the ``.exe`` launcher are + what pip actually produces on Windows. Faking ``_is_windows()`` on Linux + made the test assert against a directory tree the test itself created, on + a host where pip would never lay it out that way. + """ monkeypatch.setenv("HERMES_HOME", str(tmp_path)) from agent.lsp import install as install_mod @@ -75,7 +82,6 @@ def test_install_pip_finds_windows_scripts_launcher(tmp_path, monkeypatch): launcher.chmod(0o755) return MagicMock(returncode=0, stderr="") - monkeypatch.setattr(install_mod, "_is_windows", lambda: True) monkeypatch.setattr(install_mod.subprocess, "run", fake_run) resolved = install_mod._install_pip("fake-lsp", "fake-language-server") diff --git a/tests/agent/test_anthropic_keychain.py b/tests/agent/test_anthropic_keychain.py index 397610ff32..d54a92c0d1 100644 --- a/tests/agent/test_anthropic_keychain.py +++ b/tests/agent/test_anthropic_keychain.py @@ -17,22 +17,29 @@ from agent.anthropic_adapter import ( pytestmark = pytest.mark.allow_macos_keychain +@pytest.mark.macos_only class TestReadClaudeCodeCredentialsFromKeychain: - """Bug 4: macOS Keychain support for Claude Code >=2.1.114.""" + """Bug 4: macOS Keychain support for Claude Code >=2.1.114. + + ``macos_only``: the reader is gated on ``platform.system() == "Darwin"`` + and shells out to the ``security`` CLI. Faking Darwin on Linux selected + the branch but proved nothing about the host it exists for; on the real + macOS runner only ``subprocess.run`` is mocked (via the + ``allow_macos_keychain`` opt-out of the suite-wide guard), so no real + Keychain is ever touched. + """ def test_returns_none_when_security_command_not_found(self): """OSError from missing security binary must be handled gracefully.""" - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run", + with patch("agent.anthropic_adapter.subprocess.run", side_effect=OSError("security not found")): assert _read_claude_code_credentials_from_keychain() is None def test_returns_none_on_nonzero_exit_code(self): """security returns non-zero when the Keychain entry doesn't exist.""" - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="") assert _read_claude_code_credentials_from_keychain() is None @@ -42,6 +49,7 @@ class TestReadClaudeCodeCredentialsFromKeychain: +@pytest.mark.macos_only class TestReadClaudeCodeCredentialsPriority: """Bug 4: Keychain must be checked before the JSON file.""" @@ -60,8 +68,7 @@ class TestReadClaudeCodeCredentialsPriority: monkeypatch.setattr("agent.anthropic_adapter.Path.home", lambda: tmp_path) # Mock Keychain to return a "newer" token - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: mock_run.return_value = MagicMock( returncode=0, stdout=json.dumps({ @@ -93,8 +100,7 @@ class TestReadClaudeCodeCredentialsPriority: })) monkeypatch.setattr("agent.anthropic_adapter.Path.home", lambda: tmp_path) - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: # Simulate Keychain entry not found mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="") creds = read_claude_code_credentials() @@ -107,14 +113,14 @@ class TestReadClaudeCodeCredentialsPriority: """No credentials anywhere — must return None cleanly.""" monkeypatch.setattr("agent.anthropic_adapter.Path.home", lambda: tmp_path) - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="") creds = read_claude_code_credentials() assert creds is None +@pytest.mark.macos_only class TestReadClaudeCodeCredentialsDesync: """Reconciliation when Keychain and JSON file disagree. @@ -162,8 +168,7 @@ class TestReadClaudeCodeCredentialsDesync: ``No Anthropic credentials found`` error.) """ self._setup(tmp_path, monkeypatch, file_expires_at=self._FRESH, file_token="fresh-file-token") - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: mock_run.return_value = self._keychain_payload( access_token="stale-keychain-token", expires_at=self._EXPIRED, ) @@ -181,8 +186,7 @@ class TestReadClaudeCodeCredentialsDesync: succeed at the OAuth refresh endpoint. """ self._setup(tmp_path, monkeypatch, file_expires_at=self._EXPIRED + 5, file_token="newer-expired-file") - with patch("agent.anthropic_adapter.platform.system", return_value="Darwin"), \ - patch("agent.anthropic_adapter.subprocess.run") as mock_run: + with patch("agent.anthropic_adapter.subprocess.run") as mock_run: mock_run.return_value = self._keychain_payload( access_token="older-expired-keychain", expires_at=self._EXPIRED, ) diff --git a/tests/agent/test_prompt_builder.py b/tests/agent/test_prompt_builder.py index c545220dcf..d02871db13 100644 --- a/tests/agent/test_prompt_builder.py +++ b/tests/agent/test_prompt_builder.py @@ -749,11 +749,14 @@ class TestEnvironmentHints: def test_build_environment_hints_suppresses_host_on_docker_backend(self, monkeypatch): - """Docker/remote backends must hide host info — the agent can only touch the backend.""" + """Docker/remote backends must hide host info — the agent can only touch the backend. + + Host-independent: suppression is a property of the remote-backend + branch, so instead of faking a Windows host we assert no host line of + any kind is emitted. + """ import agent.prompt_builder as _pb - import sys monkeypatch.setattr(_pb, "is_wsl", lambda: False) - monkeypatch.setattr(sys, "platform", "win32") monkeypatch.setenv("TERMINAL_ENV", "docker") # Force the probe to fail so we exercise the static fallback path # deterministically (the live probe would try to spin up docker). @@ -761,7 +764,7 @@ class TestEnvironmentHints: _pb._clear_backend_probe_cache() result = _pb.build_environment_hints() # Host suppression: none of the local-backend lines should appear. - assert "Host: Windows" not in result + assert "Host:" not in result assert "User home directory:" not in result assert "PowerShell" not in result # Backend info must appear instead. diff --git a/tests/agent/test_skill_utils.py b/tests/agent/test_skill_utils.py index 0a3ea3bb60..a1a71e093b 100644 --- a/tests/agent/test_skill_utils.py +++ b/tests/agent/test_skill_utils.py @@ -2,6 +2,8 @@ from unittest.mock import patch +import pytest + from agent.skill_utils import ( extract_skill_config_vars, extract_skill_conditions, @@ -252,16 +254,20 @@ class TestParseFrontmatterBOM: def test_bom_platform_gating_regression(self): - # The concrete harm: a macOS-only skill must stay hidden on non-macOS + # The concrete harm: a macOS-only skill must be gated identically # whether or not the file carries a BOM. Empty frontmatter (the bug) - # reads as "no platform restriction" and leaks the skill everywhere. - with patch("agent.skill_utils.sys.platform", "win32"), patch( - "agent.skill_utils.is_termux", return_value=False - ): + # reads as "no platform restriction" and leaks the skill everywhere, + # i.e. it would answer True on every host. Compare against the real + # host's verdict instead of faking Windows — the fake only stood in + # for "some non-macOS host", which the CI host already is. + import sys + + expected = sys.platform == "darwin" + with patch("agent.skill_utils.is_termux", return_value=False): plain_fm, _ = parse_frontmatter(self.SKILL) bom_fm, _ = parse_frontmatter("\ufeff" + self.SKILL) - assert skill_matches_platform(plain_fm) is False - assert skill_matches_platform(bom_fm) is False + assert skill_matches_platform(plain_fm) is expected + assert skill_matches_platform(bom_fm) is expected def test_real_file_read_path(self, tmp_path): diff --git a/tests/cli/test_cli_file_drop.py b/tests/cli/test_cli_file_drop.py index 00c0fd5d5f..e08e37f95f 100644 --- a/tests/cli/test_cli_file_drop.py +++ b/tests/cli/test_cli_file_drop.py @@ -1,7 +1,6 @@ """Tests for _detect_file_drop — file path detection that prevents dragged/pasted absolute paths from being mistaken for slash commands.""" -import os import pytest @@ -169,7 +168,10 @@ class TestEscapedSpaces: assert result["remainder"] == "what is this?" - @pytest.mark.skipif(os.name != "nt", reason="Windows drive-letter URI contract") + # ``windows_only`` rather than ``skipif(os.name != "nt")``: the Windows CI + # job selects ``-m windows_only``, so a bare skipif would leave this + # skipped on Linux AND unselected there — dead on every host. + @pytest.mark.windows_only def test_windows_drive_letter_file_uri_drops_url_leading_slash(self, tmp_path): image = tmp_path / "drive-uri.png" image.write_bytes(b"\x89PNG\r\n\x1a\n") diff --git a/tests/cli/test_cli_init.py b/tests/cli/test_cli_init.py index 76757c0858..f3fe883f74 100644 --- a/tests/cli/test_cli_init.py +++ b/tests/cli/test_cli_init.py @@ -6,6 +6,8 @@ import sys from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest + def _make_cli(env_overrides=None, config_overrides=None, **kwargs): @@ -153,11 +155,18 @@ class TestPromptToolkitTerminalCompatibility: On a bare local POSIX TTY (no SSH/WSL/WT/Ghostty) we keep c-j → submit so Enter works on thin PTYs (docker exec, certain ssh configurations). - On Windows, WSL, SSH sessions, Windows Terminal, and Ghostty we leave c-j + In WSL, SSH sessions, Windows Terminal, and Ghostty we leave c-j unbound here so it can be used as the Ctrl+Enter newline keystroke without conflicting with submit. See issue #22379. + + The native-Windows arm of this behaviour is + ``test_windows_leaves_ctrl_j_unbound`` below — it has to run on a real + Windows host, because ``_bind_prompt_submit_keys`` delegates to + ``_preserve_ctrl_enter_newline()``, which short-circuits on + ``sys.platform == "win32"``. Faking that here would assert the literal + in the ``if`` and nothing about how prompt_toolkit actually delivers + keys on a Windows console. """ - import sys as _sys import os as _os from unittest.mock import patch as _patch from prompt_toolkit.key_binding import KeyBindings @@ -168,8 +177,7 @@ class TestPromptToolkitTerminalCompatibility: return None # Bare local POSIX (no SSH/WSL markers): both enter and c-j submit. - with _patch.object(_sys, "platform", "linux"), \ - _patch.dict(_os.environ, {}, clear=True), \ + with _patch.dict(_os.environ, {}, clear=True), \ _patch("builtins.open", side_effect=OSError("no /proc")): kb = KeyBindings() _bind_prompt_submit_keys(kb, submit_handler) @@ -179,8 +187,7 @@ class TestPromptToolkitTerminalCompatibility: # POSIX over SSH: c-j stays free so Ctrl+Enter (sent as LF by # Windows Terminal / Kitty / mintty over SSH) inserts a newline. - with _patch.object(_sys, "platform", "linux"), \ - _patch.dict(_os.environ, {"SSH_CONNECTION": "1.2.3.4 5 6.7.8.9 22"}, clear=True), \ + with _patch.dict(_os.environ, {"SSH_CONNECTION": "1.2.3.4 5 6.7.8.9 22"}, clear=True), \ _patch("builtins.open", side_effect=OSError("no /proc")): kb = KeyBindings() _bind_prompt_submit_keys(kb, submit_handler) @@ -190,8 +197,7 @@ class TestPromptToolkitTerminalCompatibility: # Ghostty through tmux: TERM_PROGRAM is tmux, but Ghostty exports a # stable env marker. Keep c-j free so Ctrl+J inserts a newline. - with _patch.object(_sys, "platform", "linux"), \ - _patch.dict(_os.environ, {"TERM": "tmux-256color", "TERM_PROGRAM": "tmux", "GHOSTTY_RESOURCES_DIR": "/usr/share/ghostty"}, clear=True), \ + with _patch.dict(_os.environ, {"TERM": "tmux-256color", "TERM_PROGRAM": "tmux", "GHOSTTY_RESOURCES_DIR": "/usr/share/ghostty"}, clear=True), \ _patch("builtins.open", side_effect=OSError("no /proc")): kb = KeyBindings() _bind_prompt_submit_keys(kb, submit_handler) @@ -199,14 +205,22 @@ class TestPromptToolkitTerminalCompatibility: assert bindings[("c-m",)] is submit_handler assert ("c-j",) not in bindings - # Windows: only enter submits; c-j is free for the newline binding - # added separately in the prompt setup. - with _patch.object(_sys, "platform", "win32"): - kb = KeyBindings() - _bind_prompt_submit_keys(kb, submit_handler) - bindings = {tuple(key.value for key in binding.keys): binding.handler for binding in kb.bindings} - assert bindings[("c-m",)] is submit_handler - assert ("c-j",) not in bindings + @pytest.mark.windows_only + def test_windows_leaves_ctrl_j_unbound(self): + """On native Windows only enter submits; c-j is free for the newline + binding added separately in the prompt setup.""" + from prompt_toolkit.key_binding import KeyBindings + + from cli import _bind_prompt_submit_keys + + def submit_handler(event): + return None + + kb = KeyBindings() + _bind_prompt_submit_keys(kb, submit_handler) + bindings = {tuple(key.value for key in binding.keys): binding.handler for binding in kb.bindings} + assert bindings[("c-m",)] is submit_handler + assert ("c-j",) not in bindings def test_cpr_warning_callback_is_disabled(self): from cli import _disable_prompt_toolkit_cpr_warning @@ -220,25 +234,20 @@ class TestPromptToolkitTerminalCompatibility: - def test_cpr_gating_posix_local_and_windows_preserve(self, monkeypatch): - """POSIX suppresses CPR without SSH; native Windows keeps PT default. + def test_cpr_gating_posix_suppresses_without_ssh(self, monkeypatch): + """POSIX suppresses CPR without SSH. - Broader coverage (Application wiring + delayed-CPR PTY repro) lives in - ``tests/cli/test_cpr_local_leak.py``. + The native-Windows arm (``_terminal_may_leak_cpr() is False``, plus + the ``PROMPT_TOOLKIT_NO_CPR`` override that outranks it) lives in + ``tests/cli/test_cpr_local_leak.py`` under ``windows_only``, where it + runs against a real Windows console. """ - import sys as _sys - from cli import _terminal_may_leak_cpr for var in ("SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY", "PROMPT_TOOLKIT_NO_CPR"): monkeypatch.delenv(var, raising=False) - monkeypatch.setattr(_sys, "platform", "linux") assert _terminal_may_leak_cpr() is True - monkeypatch.setattr(_sys, "platform", "darwin") - assert _terminal_may_leak_cpr() is True - monkeypatch.setattr(_sys, "platform", "win32") - assert _terminal_may_leak_cpr() is False monkeypatch.setenv("PROMPT_TOOLKIT_NO_CPR", "1") assert _terminal_may_leak_cpr() is True diff --git a/tests/cli/test_cpr_local_leak.py b/tests/cli/test_cpr_local_leak.py index c1feaf1809..3d636e00b5 100644 --- a/tests/cli/test_cpr_local_leak.py +++ b/tests/cli/test_cpr_local_leak.py @@ -33,13 +33,13 @@ def _clear_cpr_env(monkeypatch): class TestClassicCliOutputSelection: - def test_windows_preserves_default_output_selection(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "win32") + @pytest.mark.windows_only + def test_windows_preserves_default_output_selection(self): assert _terminal_may_leak_cpr() is False assert _select_classic_cli_pt_output(sys.stdout) is None + @pytest.mark.windows_only def test_windows_honors_explicit_no_cpr(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "win32") monkeypatch.setenv("PROMPT_TOOLKIT_NO_CPR", "1") assert _terminal_may_leak_cpr() is True out = _select_classic_cli_pt_output(sys.stdout) diff --git a/tests/cli/test_ctrl_enter_newline.py b/tests/cli/test_ctrl_enter_newline.py index 9da1e531af..7249f9fa28 100644 --- a/tests/cli/test_ctrl_enter_newline.py +++ b/tests/cli/test_ctrl_enter_newline.py @@ -7,73 +7,76 @@ byte. Without environment-aware gating, binding c-j to submit means Ctrl+Enter submits instead of inserting a newline. These tests pin the gating predicate and the resulting binding behavior. + +``_preserve_ctrl_enter_newline()`` short-circuits to True on native Windows +before it ever looks at the environment, so the env-driven cases below are +POSIX assertions and run on the Linux job. The native-Windows short-circuit +is marked ``windows_only`` and asserted on the real host — patching +``sys.platform`` to ``"win32"`` here would only re-assert the literal in the +``if``, on an interpreter where none of the Windows terminal behaviour it +exists for is present. """ from __future__ import annotations import os -import sys from unittest.mock import patch +import pytest + +@pytest.mark.windows_only def test_native_windows_preserves_newline(): import cli as cli_mod - with patch.object(sys, "platform", "win32"): - assert cli_mod._preserve_ctrl_enter_newline() is True - + assert cli_mod._preserve_ctrl_enter_newline() is True def test_ssh_tty_alone_preserves_newline(): import cli as cli_mod - with patch.object(sys, "platform", "linux"): - # Strip out anything that might leak truth - with patch.dict(os.environ, {"SSH_TTY": "/dev/pts/0"}, clear=True): - assert cli_mod._preserve_ctrl_enter_newline() is True - - + # Strip out anything that might leak truth + with patch.dict(os.environ, {"SSH_TTY": "/dev/pts/0"}, clear=True): + assert cli_mod._preserve_ctrl_enter_newline() is True def test_windows_terminal_session_preserves_newline(): import cli as cli_mod - with patch.object(sys, "platform", "linux"): - with patch.dict(os.environ, {"WT_SESSION": "abc-def"}, clear=True): - assert cli_mod._preserve_ctrl_enter_newline() is True + with patch.dict(os.environ, {"WT_SESSION": "abc-def"}, clear=True): + assert cli_mod._preserve_ctrl_enter_newline() is True def test_ghostty_tmux_session_preserves_ctrl_j_newline(): """Ghostty-inherited env survives tmux even when TERM_PROGRAM becomes tmux.""" import cli as cli_mod - with patch.object(sys, "platform", "linux"): - with patch.dict( - os.environ, - {"TERM": "tmux-256color", "TERM_PROGRAM": "tmux", "GHOSTTY_RESOURCES_DIR": "/usr/share/ghostty"}, - clear=True, - ): - assert cli_mod._preserve_ctrl_enter_newline() is True - - + with patch.dict( + os.environ, + {"TERM": "tmux-256color", "TERM_PROGRAM": "tmux", "GHOSTTY_RESOURCES_DIR": "/usr/share/ghostty"}, + clear=True, + ): + assert cli_mod._preserve_ctrl_enter_newline() is True +@pytest.mark.linux_only def test_proc_version_microsoft_marker_preserves_newline(): - """WSL detection via /proc when env vars are scrubbed (sudo etc.).""" + """WSL detection via /proc when env vars are scrubbed (sudo etc.). + + ``linux_only``: the fallback reads ``/proc/version`` — a Linux-only + interface, and the WSL kernels it sniffs for are Linux kernels. + """ import cli as cli_mod from io import StringIO - with patch.object(sys, "platform", "linux"): - with patch.dict(os.environ, {}, clear=True): - real_open = open - def _fake_open(path, *args, **kwargs): - if "/proc/version" in str(path) or "/proc/sys/kernel/osrelease" in str(path): - return StringIO("Linux version 5.15.167.4-microsoft-standard-WSL2") - return real_open(path, *args, **kwargs) - with patch("builtins.open", side_effect=_fake_open): - assert cli_mod._preserve_ctrl_enter_newline() is True + with patch.dict(os.environ, {}, clear=True): + real_open = open + + def _fake_open(path, *args, **kwargs): + if "/proc/version" in str(path) or "/proc/sys/kernel/osrelease" in str(path): + return StringIO("Linux version 5.15.167.4-microsoft-standard-WSL2") + return real_open(path, *args, **kwargs) + + with patch("builtins.open", side_effect=_fake_open): + assert cli_mod._preserve_ctrl_enter_newline() is True # --------------------------------------------------------------------------- # install_ctrl_enter_alias() — ANSI sequence mappings for enhanced terminals # --------------------------------------------------------------------------- - - - - diff --git a/tests/cli/test_slash_confirm_windows.py b/tests/cli/test_slash_confirm_windows.py index f91bab3950..616f7e3617 100644 --- a/tests/cli/test_slash_confirm_windows.py +++ b/tests/cli/test_slash_confirm_windows.py @@ -8,15 +8,25 @@ Windows (#33961), where the earlier ``sys.platform == "win32"`` → raw ``input( fallback deadlocked the daemon thread against prompt_toolkit's stdin ownership. These tests verify: -1. Daemon-thread confirm uses the modal via the app loop on Linux AND native - Windows (#33961) — never the raw stdin fallback, never a hang. +1. Daemon-thread confirm uses the modal via the app loop — never the raw stdin + fallback, never a hang. Since the #33961 fix this path is platform-agnostic, + so it runs on the host as-is. 2. Main-thread confirm with a running app uses the modal. 3. The raw stdin fallback is kept ONLY for the safe cases: no running app, and (on win32, off-thread) a scheduling failure degrades to a clean cancel. 4. Empty choices returns None. + +**Why the Windows cases are ``windows_only`` rather than ``sys.platform`` +patches.** The deadlock #33961 fixed is a real property of the Windows console: +a raw ``input()`` off the main thread blocks forever against prompt_toolkit's +stdin ownership. On Linux the same call returns immediately (or EOFs), so a test +that patches ``sys.platform`` to ``"win32"`` proves only that we took the +``if``-branch — the hazard the branch exists to avoid isn't present on the host. +The one remaining platform-specific behaviour is the ``_stdin_fallback`` +win32-and-off-main-thread clean-cancel; those tests are marked and run on the +Windows CI job. """ -import sys import threading import time from unittest.mock import MagicMock, patch @@ -62,7 +72,7 @@ def _answer_modal_when_open(cli, response, stop=None): time.sleep(0.02) -def _run_on_daemon(call, cli, *, platform, response, schedule=None): +def _run_on_daemon(call, cli, *, response, schedule=None): """Invoke ``call`` on a daemon thread — as the process_loop does — answering the modal with ``response`` once it opens. @@ -76,8 +86,7 @@ def _run_on_daemon(call, cli, *, platform, response, schedule=None): def _worker(): try: - with patch.object(sys, "platform", platform), \ - patch.object(cli._app.loop, "call_soon_threadsafe", side_effect=schedule or (lambda cb: cb())), \ + with patch.object(cli._app.loop, "call_soon_threadsafe", side_effect=schedule or (lambda cb: cb())), \ patch.object(cli, "_prompt_text_input") as mock_stdin, \ patch.object(cli, "_invalidate"), \ patch.object(cli, "_capture_modal_input_snapshot", side_effect=lambda: outcome["capture"].append(1)), \ @@ -98,13 +107,13 @@ def _run_on_daemon(call, cli, *, platform, response, schedule=None): class TestModal: - """Behaviour of _prompt_text_input_modal across platforms and threads.""" + """Behaviour of _prompt_text_input_modal across threads.""" - @pytest.mark.parametrize("platform", ["linux", "win32"]) - def test_daemon_thread_uses_modal_via_app_loop(self, platform): + def test_daemon_thread_uses_modal_via_app_loop(self): """Off the process_loop daemon thread, the confirm uses the modal via - call_soon_threadsafe on every platform — including native Windows, where - the old win32 early-return deadlocked on raw input() (#33961).""" + call_soon_threadsafe. Since #33961 this path is the same on every + platform — native Windows no longer takes an early return into raw + input(), so there is nothing platform-specific left to fake here.""" cli = _make_cli() outcome = _run_on_daemon( lambda: cli._prompt_text_input_modal( @@ -114,7 +123,6 @@ class TestModal: timeout=5, ), cli, - platform=platform, response="once", ) assert outcome["stdin_called"] is False, "must use the modal, not raw input()" @@ -126,8 +134,7 @@ class TestModal: def test_main_thread_with_app_uses_modal(self): """On the main thread with a running app, the queue-based modal is used.""" cli = _make_cli() - with patch.object(sys, "platform", "darwin"), \ - patch.object(cli, "_capture_modal_input_snapshot"), \ + with patch.object(cli, "_capture_modal_input_snapshot"), \ patch.object(cli, "_restore_modal_input_snapshot"), \ patch.object(cli, "_invalidate"), \ patch.object(cli, "_prompt_text_input") as mock_stdin: @@ -144,8 +151,7 @@ class TestModal: mock_stdin.assert_not_called() assert result == "once" - - + @pytest.mark.windows_only def test_windows_scheduling_failure_clean_cancels(self): """win32 off the main thread: if marshaling onto the app loop fails, cancel cleanly (None) rather than fall to raw input() (which deadlocks on native @@ -163,7 +169,6 @@ class TestModal: timeout=5, ), cli, - platform="win32", response="once", schedule=_raise, ) @@ -172,10 +177,14 @@ class TestModal: assert cli._slash_confirm_state is None +class TestConfirmDestructiveSlash: + """End-to-end _confirm_destructive_slash on the process_loop daemon thread. - -class TestConfirmDestructiveSlashWindows: - """End-to-end _confirm_destructive_slash on the native-Windows daemon thread.""" + This is the flow bug #33961 froze on native Windows. The fix made it + platform-agnostic (modal via the app loop), so the assertion holds on + whichever host runs it — including the Windows CI job, where it is the + genuine regression guard. + """ def _make_interactive_cli(self): cli = _make_cli() @@ -194,9 +203,9 @@ class TestConfirmDestructiveSlashWindows: "response, expected", [("once", "once"), ("cancel", None)], ) - def test_confirm_destructive_slash_uses_modal_on_windows(self, response, expected): - """On native Windows, the bare /new confirm drives the modal (not stdin) - and returns the chosen outcome — the bug #33961 froze this path.""" + def test_confirm_destructive_slash_uses_modal(self, response, expected): + """The bare /new confirm drives the modal (not stdin) and returns the + chosen outcome — bug #33961 froze this path on native Windows.""" cli = self._make_interactive_cli() with patch("cli.load_cli_config", return_value={"approvals": {"destructive_slash_confirm": True}}): outcome = _run_on_daemon( @@ -205,7 +214,6 @@ class TestConfirmDestructiveSlashWindows: "This starts a fresh session.\nThe current conversation history will be discarded.", ), cli, - platform="win32", response=response, ) @@ -213,6 +221,7 @@ class TestConfirmDestructiveSlashWindows: assert outcome["result"] == expected +@pytest.mark.windows_only class TestNativeWindowsNoRawInputDeadlock: """Anti-regression guard exercising the REAL ``_prompt_text_input``. @@ -227,6 +236,11 @@ class TestNativeWindowsNoRawInputDeadlock: ``input()`` and assert the worker thread never hangs. They fail on the pre-#33961 code (win32 → ``_prompt_text_input`` → off-main ``input()``) and pass once the modal path / clean-cancel fallback is in place. + + ``windows_only``: the deadlock is a property of the Windows console's stdin + ownership. Running this with a patched ``sys.platform`` on Linux exercises + a blocking ``input()`` that does not actually deadlock there, so it could + never have caught the regression it is named for. """ def test_win32_daemon_thread_never_blocks_on_real_input(self): @@ -251,8 +265,7 @@ class TestNativeWindowsNoRawInputDeadlock: def _worker(): try: - with patch.object(sys, "platform", "win32"), \ - patch("builtins.input", side_effect=_blocking_input), \ + with patch("builtins.input", side_effect=_blocking_input), \ patch.object(cli, "_capture_modal_input_snapshot"), \ patch.object(cli, "_restore_modal_input_snapshot"), \ patch.object(cli, "_invalidate"): @@ -306,8 +319,7 @@ class TestNativeWindowsNoRawInputDeadlock: outcome = {} def _worker(): - with patch.object(sys, "platform", "win32"), \ - patch("builtins.input", side_effect=_tracking_input), \ + with patch("builtins.input", side_effect=_tracking_input), \ patch.object(cli, "_invalidate"): outcome["result"] = cli._prompt_text_input_modal( title="/new", diff --git a/tests/computer_use/test_cua_no_overlay.py b/tests/computer_use/test_cua_no_overlay.py index 02959bd89b..73af0fd965 100644 --- a/tests/computer_use/test_cua_no_overlay.py +++ b/tests/computer_use/test_cua_no_overlay.py @@ -10,9 +10,10 @@ probe), not specific config snapshots. """ import os -import sys from unittest.mock import mock_open, patch +import pytest + from tools.computer_use import cua_backend @@ -28,11 +29,27 @@ class TestNoOverlayFlag: assert cua_backend._cua_no_overlay() is True - def test_config_load_failure_falls_through_to_auto_detect(self): - """Unreadable config => auto-detect (macOS defaults to disabled).""" + @pytest.mark.macos_only + def test_config_load_failure_falls_through_to_auto_detect_macos(self): + """Unreadable config => auto-detect (macOS defaults to overlay off). + + macOS-only: the auto-detect verdict IS ``sys.platform == "darwin"``, + so a patched platform would only re-assert the patch. + """ with patch("hermes_cli.config.load_config", - side_effect=RuntimeError("boom")), \ - patch.object(sys, "platform", "darwin"): + side_effect=RuntimeError("boom")): + assert cua_backend._cua_no_overlay() is True + + @pytest.mark.linux_only + def test_config_load_failure_falls_through_to_auto_detect_linux(self, monkeypatch): + """Unreadable config must not raise; headless Linux auto-detects off. + + Linux-only: the auto-detect branch here keys off ``DISPLAY`` and + ``/proc/version``, neither of which exists to be probed elsewhere. + """ + monkeypatch.delenv("DISPLAY", raising=False) + with patch("hermes_cli.config.load_config", + side_effect=RuntimeError("boom")): assert cua_backend._cua_no_overlay() is True diff --git a/tests/computer_use/test_cua_spawn_env_sanitization.py b/tests/computer_use/test_cua_spawn_env_sanitization.py index e737eefe98..038f052e1c 100644 --- a/tests/computer_use/test_cua_spawn_env_sanitization.py +++ b/tests/computer_use/test_cua_spawn_env_sanitization.py @@ -48,7 +48,15 @@ def _assert_sanitized(captured): def _patch_windows_hide_flags(monkeypatch, module): - monkeypatch.setattr(module, "IS_WINDOWS", True, raising=False) + """Pin the ``windows_hide_flags()`` seam so the console-hiding assertion + is host-independent. + + ``windows_hide_flags`` is our own platform probe (CREATE_NO_WINDOW on + Windows, ``0`` elsewhere). Patching that seam — rather than lying to the + interpreter about ``sys.platform`` — keeps the real subject of these + tests (does the spawn site forward its result to ``creationflags=``?) + covered on every host. + """ monkeypatch.setattr( module, "windows_hide_flags", lambda: CREATE_NO_WINDOW, raising=False ) diff --git a/tests/computer_use/test_permissions_resolution.py b/tests/computer_use/test_permissions_resolution.py index c51fc9987f..932ee107c8 100644 --- a/tests/computer_use/test_permissions_resolution.py +++ b/tests/computer_use/test_permissions_resolution.py @@ -22,11 +22,11 @@ def test_status_finds_user_local_driver_when_path_omits_it(tmp_path, monkeypatch monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setenv("PATH", "/usr/bin:/bin:/usr/sbin:/sbin") - with patch("tools.computer_use.permissions.sys.platform", "darwin"), \ - patch("tools.computer_use.cua_backend.sys.platform", "darwin"), \ - patch.object(permissions, "_run", return_value=MagicMock(stdout="0.0.0")), \ - patch.object(permissions, "_doctor", return_value={"ok": True, "checks": []}), \ - patch.object(permissions, "_mac_permissions"): + # No platform faking: ``~/.local/bin/cua-driver`` is a POSIX resolution + # candidate on Linux exactly as on macOS, so the regression reproduces on + # the host we actually run on. + with patch.object(permissions, "_run", return_value=MagicMock(stdout="0.0.0")), \ + patch.object(permissions, "_doctor", return_value={"ok": True, "checks": []}): status = permissions.computer_use_status() assert status["installed"] is True diff --git a/tests/conftest.py b/tests/conftest.py index 54298d70e3..d7e746d397 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1023,6 +1023,64 @@ def _wal_is_usable() -> bool: _AUDIO_GUARD_BYPASS_MARK = "real_audio_playback" _ALLOW_MACOS_KEYCHAIN_MARK = "allow_macos_keychain" +# --------------------------------------------------------------------------- +# OS gating +# +# Hermes runs on Linux, macOS and native Windows, and a lot of its behaviour +# genuinely differs per host: PTY vs pywinpty, taskkill vs SIGTERM, launchd +# vs systemd, Keychain vs libsecret, ``%LOCALAPPDATA%`` vs ``~/.hermes``. +# +# Historically those code paths were tested by *faking* the host — patching +# ``sys.platform`` to ``"win32"`` inside a Linux CI job. That gives a green +# test on a machine where the code under test could not actually run: the +# fake covers the ``if sys.platform == "win32"`` branch selection but nothing +# underneath it (``msvcrt`` still isn't importable, ``taskkill`` still isn't +# on PATH, paths are still POSIX, ``signal.SIGKILL`` still exists). The +# result was tests that pass on Linux and tell us nothing about Windows. +# +# So: a test whose subject is genuinely OS-specific declares the OS it +# belongs to and runs there for real — +# +# @pytest.mark.windows_only → only on native Windows (``sys.platform == "win32"``) +# @pytest.mark.macos_only → only on macOS (``sys.platform == "darwin"``) +# @pytest.mark.linux_only → only on Linux (``sys.platform.startswith("linux")``) +# +# Elsewhere the test is skipped, not faked. CI runs a dedicated macOS job +# (``-m macos_only``) and a dedicated Windows job (``-m windows_only``) so +# those markers are actually exercised on their own host rather than +# quietly skipped everywhere. +# +# This does NOT mean every mention of another platform must be gated. Two +# things are legitimately host-independent and stay on the Linux runner: +# +# • Pure functions that TAKE a platform as data — e.g. +# ``hidden_windows_child_options(opts, is_windows=True)`` or a +# ``resolve_launcher(platform_name)`` helper. Passing "win32" as an +# argument is not faking the host; the function's whole contract is +# that it maps input to output. +# • Declaration/packaging invariants — e.g. "pyproject declares tzdata +# with a ``sys_platform == 'win32'`` marker". That's an assertion about +# a file, not about runtime behaviour. +# +# The line is: if the test needs the interpreter to BELIEVE it is on +# another OS in order to pass, it belongs on that OS. +# --------------------------------------------------------------------------- + +_OS_MARKS = { + "linux_only": ( + lambda: sys.platform.startswith("linux"), + "Linux", + ), + "macos_only": ( + lambda: sys.platform == "darwin", + "macOS", + ), + "windows_only": ( + lambda: sys.platform == "win32", + "native Windows", + ), +} + def pytest_configure(config): # noqa: D401 — pytest hook """Register markers used by hermetic conftest.""" @@ -1055,6 +1113,12 @@ def pytest_configure(config): # noqa: D401 — pytest hook "created in the current environment (needs admin/developer mode " "on Windows).", ) + # NOTE: linux_only / macos_only / windows_only are declared in + # pyproject.toml's ``markers`` list, not here — they are part of the + # project's public marker vocabulary (``pytest --markers``, and the CI + # lanes select on them), whereas the marks above are conftest-internal + # guards. Declaring them in both places just meant two descriptions that + # could drift apart. # The pyproject addopts pin ``--timeout-method=signal`` relies on # ``signal.SIGALRM``, which does not exist on Windows — pytest-timeout @@ -1096,12 +1160,27 @@ def pytest_runtest_setup(item): def pytest_collection_modifyitems(config, items): # noqa: D401 — pytest hook - """Skip ``requires_wal`` tests when the linked SQLite can't use WAL. + """Apply host-OS gating, then skip ``requires_wal`` where WAL is unusable. - Cheaper and more honest than each test hand-rolling a version check: the - reason string names the actual linked version so the skip is diagnosable - rather than mysterious. + OS gating: a test marked ``linux_only`` / ``macos_only`` / + ``windows_only`` runs only on that host. See the ``_OS_MARKS`` block + comment above for why these tests are skipped rather than run against a + patched ``sys.platform``. + + WAL gating is cheaper and more honest than each test hand-rolling a + version check: the reason string names the actual linked version so the + skip is diagnosable rather than mysterious. """ + for mark_name, (is_host, label) in _OS_MARKS.items(): + if is_host(): + continue + skip_os = pytest.mark.skip( + reason=f"{label}-only test (marked {mark_name}); host is {sys.platform}" + ) + for item in items: + if item.get_closest_marker(mark_name) is not None: + item.add_marker(skip_os) + if _wal_is_usable(): return diff --git a/tests/cron/test_cron_script.py b/tests/cron/test_cron_script.py index dbe6bf9f6b..32642e0341 100644 --- a/tests/cron/test_cron_script.py +++ b/tests/cron/test_cron_script.py @@ -130,7 +130,11 @@ class TestRunJobScript: assert success is True assert output == "ABSENT" + @pytest.mark.windows_only def test_windows_uv_venv_python_script_bypasses_launcher(self, cron_env, tmp_path, monkeypatch): + # Windows-only: the fake ``sys.platform`` could not reproduce the + # ``Scripts/python.exe`` launcher layout or the CREATE_NO_WINDOW + # creationflags this branch exists for. from cron import scheduler as sched_mod from cron.scheduler import _run_job_script @@ -157,9 +161,7 @@ class TestRunJobScript: captured["kwargs"] = kwargs return SimpleNamespace(returncode=0, stdout="ok\n", stderr="") - monkeypatch.setattr(sched_mod.sys, "platform", "win32") monkeypatch.setattr(sched_mod.sys, "executable", str(venv_python)) - monkeypatch.setattr(sched_mod, "windows_hide_flags", lambda: 0x08000000) monkeypatch.setattr(sched_mod.subprocess, "run", fake_run) success, output = _run_job_script("probe.py") @@ -167,13 +169,14 @@ class TestRunJobScript: assert success is True assert output == "ok" assert captured["argv"] == [str(base_python), str(script.resolve())] - assert captured["kwargs"]["creationflags"] == 0x08000000 + assert captured["kwargs"]["creationflags"] == sched_mod.windows_hide_flags() env = captured["kwargs"]["env"] assert env["VIRTUAL_ENV"] == str(venv) assert str(site_packages) in env["PYTHONPATH"] def test_non_windows_script_preserves_default_text_decoding(self, cron_env, monkeypatch): + # No platform patching: the Linux CI host already takes this branch. from cron import scheduler as sched_mod from cron.scheduler import _run_job_script @@ -187,7 +190,6 @@ class TestRunJobScript: captured["kwargs"] = kwargs return SimpleNamespace(returncode=0, stdout="ok\n", stderr="") - monkeypatch.setattr(sched_mod.sys, "platform", "linux") monkeypatch.setattr(sched_mod.subprocess, "run", fake_run) success, output = _run_job_script("probe.py") diff --git a/tests/gateway/test_replace_child_reap.py b/tests/gateway/test_replace_child_reap.py index 6f7817abbc..969a3b1c69 100644 --- a/tests/gateway/test_replace_child_reap.py +++ b/tests/gateway/test_replace_child_reap.py @@ -66,7 +66,6 @@ def _fake_psutil(monkeypatch, *, wait_gone=None, wait_alive=None): class TestReapGatewayChildren: def test_reaps_orphaned_children_sigterm_then_wait(self, monkeypatch): - monkeypatch.setattr(status, "_IS_WINDOWS", False) fake = _fake_psutil(monkeypatch) orphans = [_FakeChild(101, ppid=1), _FakeChild(102, ppid=1)] @@ -78,7 +77,6 @@ class TestReapGatewayChildren: fake.wait_procs.assert_called_once() def test_survivors_of_sigterm_get_sigkill(self, monkeypatch): - monkeypatch.setattr(status, "_IS_WINDOWS", False) stubborn = _FakeChild(103, ppid=1) _fake_psutil(monkeypatch, wait_gone=[], wait_alive=[stubborn]) @@ -91,7 +89,6 @@ class TestReapGatewayChildren: class TestSnapshotGatewayChildren: def test_snapshot_walks_descendants_recursively(self, monkeypatch): - monkeypatch.setattr(status, "_IS_WINDOWS", False) fake = _fake_psutil(monkeypatch) kids = [_FakeChild(201), _FakeChild(202)] fake.Process.return_value.children.return_value = kids diff --git a/tests/gateway/test_restart_drain.py b/tests/gateway/test_restart_drain.py index a44644ddde..7304f93581 100644 --- a/tests/gateway/test_restart_drain.py +++ b/tests/gateway/test_restart_drain.py @@ -220,15 +220,18 @@ async def test_run_restart_excluded_from_stop_cancel_loop(): ) +@pytest.mark.windows_only @pytest.mark.asyncio async def test_windows_detached_restart_scrubs_gateway_marker(monkeypatch, tmp_path): + """Faking sys.platform="win32" on Linux could not reach the real Windows + detach branch (msvcrt/creationflags spawn, Lib/site-packages venv layout); + this runs on the Windows CI job instead.""" runner, _adapter = make_restart_runner() popen_calls = [] venv_dir = tmp_path / "venv" site_packages = venv_dir / "Lib" / "site-packages" site_packages.mkdir(parents=True) - monkeypatch.setattr(gateway_run.sys, "platform", "win32") monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["hermes"]) monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321) monkeypatch.setenv("_HERMES_GATEWAY", "1") @@ -260,19 +263,23 @@ async def test_windows_detached_restart_scrubs_gateway_marker(monkeypatch, tmp_p assert kwargs["stderr"] is subprocess.DEVNULL +@pytest.mark.windows_only @pytest.mark.asyncio async def test_windows_detached_restart_watcher_keeps_console_python(monkeypatch, tmp_path): """The restart watcher must run sys.executable (console python) under the hidden-console detach kwargs — NOT swap in GUI-subsystem pythonw.exe, which would leave the watcher console-less and make its descendants - flash visible conhosts (#54220/#56747).""" + flash visible conhosts (#54220/#56747). + + Faking sys.platform on Linux could not enter the Windows-only watcher + spawn branch this asserts on, so it runs on the Windows CI job. + """ runner, _adapter = make_restart_runner() popen_calls = [] venv_dir = tmp_path / "venv" site_packages = venv_dir / "Lib" / "site-packages" site_packages.mkdir(parents=True) - monkeypatch.setattr(gateway_run.sys, "platform", "win32") monkeypatch.setattr(gateway_run.sys, "executable", r"C:\venv\Scripts\python.exe") monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["hermes"]) monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321) diff --git a/tests/gateway/test_status.py b/tests/gateway/test_status.py index d777dcf69d..eea8e05a0a 100644 --- a/tests/gateway/test_status.py +++ b/tests/gateway/test_status.py @@ -7,6 +7,8 @@ import time from pathlib import Path from types import SimpleNamespace +import pytest + from gateway import status @@ -310,9 +312,12 @@ class TestGetProcessStartTime: class TestTerminatePid: + @pytest.mark.windows_only def test_force_uses_taskkill_on_windows(self, monkeypatch): + # Faking _IS_WINDOWS on POSIX could not reproduce the real + # CREATE_NO_WINDOW creationflags value that windows_hide_flags() + # returns only on Windows (it is 0 elsewhere). calls = [] - monkeypatch.setattr(status, "_IS_WINDOWS", True) def fake_run(cmd, capture_output=False, text=False, timeout=None, creationflags=0, **kwargs): calls.append((cmd, capture_output, text, timeout, creationflags)) @@ -324,8 +329,6 @@ class TestTerminatePid: # taskkill is spawned with the no-window flag so the windowless # pythonw.exe backend doesn't flash a conhost window on force-kill. - # windows_hide_flags() is 0 on the POSIX test host (a valid no-op - # creationflags value); on real Windows it is CREATE_NO_WINDOW. from hermes_cli._subprocess_compat import windows_hide_flags assert calls == [ @@ -334,7 +337,11 @@ class TestTerminatePid: class TestScopedLocks: + @pytest.mark.windows_only def test_windows_file_lock_uses_high_offset(self, tmp_path, monkeypatch): + # Faking _IS_WINDOWS on POSIX could not reproduce the msvcrt + # byte-range locking path at all: msvcrt does not exist off Windows, + # so the stub below had to invent the module as well as the host. lock_path = tmp_path / "gateway.lock" handle = open(lock_path, "a+", encoding="utf-8") fd = handle.fileno() @@ -343,7 +350,6 @@ class TestScopedLocks: def fake_locking(fd, mode, size): calls.append((fd, mode, size, handle.tell())) - monkeypatch.setattr(status, "_IS_WINDOWS", True) monkeypatch.setattr( status, "msvcrt", @@ -865,7 +871,6 @@ class TestReadProcessCmdlinePsFallback: def test_ps_fallback_when_proc_unavailable(self, monkeypatch): monkeypatch.setattr(status.Path, "read_bytes", lambda self: (_ for _ in ()).throw(FileNotFoundError)) - monkeypatch.setattr(status, "_IS_WINDOWS", False) monkeypatch.setattr( status.subprocess, "run", lambda args, **kwargs: SimpleNamespace(returncode=0, stdout="/usr/libexec/bluetoothuserd\n"), diff --git a/tests/gateway/test_whatsapp_connect.py b/tests/gateway/test_whatsapp_connect.py index f43582a4c0..12ee1e00c2 100644 --- a/tests/gateway/test_whatsapp_connect.py +++ b/tests/gateway/test_whatsapp_connect.py @@ -312,7 +312,11 @@ class TestBridgeRuntimeFailure: class TestKillPortProcess: """Verify _kill_port_process uses platform-appropriate commands.""" + @pytest.mark.windows_only def test_uses_netstat_and_taskkill_on_windows(self): + """``windows_only``: netstat/taskkill are Windows binaries. The old + ``_IS_WINDOWS`` patch selected this branch on Linux, where neither + exists, so the mocked argv was the only thing under test.""" from plugins.platforms.whatsapp.adapter import _kill_port_process netstat_output = ( @@ -330,8 +334,7 @@ class TestKillPortProcess: return mock_taskkill return MagicMock() - with patch("plugins.platforms.whatsapp.adapter._IS_WINDOWS", True), \ - patch("plugins.platforms.whatsapp.adapter.subprocess.run", side_effect=run_side_effect) as mock_run: + with patch("plugins.platforms.whatsapp.adapter.subprocess.run", side_effect=run_side_effect) as mock_run: _kill_port_process(3000) # netstat called @@ -345,6 +348,7 @@ class TestKillPortProcess: ) + @pytest.mark.linux_only def test_kills_only_listeners_on_linux(self): """POSIX path SIGTERMs only LISTENer PIDs (never clients) — the #43846 fix. @@ -352,12 +356,14 @@ class TestKillPortProcess: matched client sockets sharing the port number, which closed unrelated processes (a browser tab on the same port). The implementation now resolves listeners via ``_listener_pids_on_port`` and signals only those. + + ``linux_only``: asserts the POSIX ``os.kill``/SIGTERM path, which is + genuinely selected here without patching ``_IS_WINDOWS``. """ from plugins.platforms.whatsapp import adapter as wa kills = [] - with patch("plugins.platforms.whatsapp.adapter._IS_WINDOWS", False), \ - patch("plugins.platforms.whatsapp.adapter._listener_pids_on_port", + with patch("plugins.platforms.whatsapp.adapter._listener_pids_on_port", return_value=[55555]) as mock_listeners, \ patch("plugins.platforms.whatsapp.adapter.os.kill", side_effect=lambda pid, sig: kills.append((pid, sig))): @@ -375,8 +381,13 @@ class TestHttpSessionLifecycle: """Verify persistent aiohttp.ClientSession is created and cleaned up.""" @pytest.mark.asyncio + @pytest.mark.windows_only async def test_disconnect_uses_taskkill_tree_on_windows(self): - """Windows disconnect should target the bridge process tree, not just the parent PID.""" + """Windows disconnect should target the bridge process tree, not just the parent PID. + + ``windows_only``: ``taskkill /T`` is the Windows tree-kill primitive; + on Linux the branch was reachable only by faking ``_IS_WINDOWS``. + """ adapter = _make_adapter() mock_proc = MagicMock() mock_proc.pid = 12345 @@ -387,8 +398,7 @@ class TestHttpSessionLifecycle: adapter._running = True adapter._session_lock_identity = None - with patch("plugins.platforms.whatsapp.adapter._IS_WINDOWS", True), \ - patch("plugins.platforms.whatsapp.adapter.subprocess.run", return_value=MagicMock(returncode=0)) as mock_run, \ + with patch("plugins.platforms.whatsapp.adapter.subprocess.run", return_value=MagicMock(returncode=0)) as mock_run, \ patch("plugins.platforms.whatsapp.adapter.asyncio.sleep", new_callable=AsyncMock): await adapter.disconnect() diff --git a/tests/hermes_cli/test_auth_ssl_macos.py b/tests/hermes_cli/test_auth_ssl_macos.py index 48e5a615c9..9d9abc1dbf 100644 --- a/tests/hermes_cli/test_auth_ssl_macos.py +++ b/tests/hermes_cli/test_auth_ssl_macos.py @@ -13,7 +13,6 @@ bundle and refuses stubs. import shutil import ssl import subprocess -import sys from pathlib import Path import pytest @@ -49,15 +48,14 @@ def real_bundle_file(tmp_path: Path) -> str: class TestDefaultVerify: - def test_returns_ssl_context_on_darwin(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "darwin") + @pytest.mark.macos_only + def test_returns_ssl_context_on_darwin(self): result = _default_verify() assert isinstance(result, ssl.SSLContext) + @pytest.mark.macos_only def test_darwin_falls_back_to_true_when_certifi_missing(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "darwin") - real_import = __import__ def fake_import(name, *args, **kwargs): @@ -73,8 +71,8 @@ class TestResolveVerifyIntegration: """_resolve_verify should defer to _default_verify in the no-CA path.""" + @pytest.mark.linux_only def test_no_ca_uses_default_verify_on_linux(self, monkeypatch): - monkeypatch.setattr(sys, "platform", "linux") for var in ("HERMES_CA_BUNDLE", "SSL_CERT_FILE", "REQUESTS_CA_BUNDLE"): monkeypatch.delenv(var, raising=False) assert _resolve_verify() is True diff --git a/tests/hermes_cli/test_clipboard_text_write.py b/tests/hermes_cli/test_clipboard_text_write.py index 85d28a8f36..379ab5b798 100644 --- a/tests/hermes_cli/test_clipboard_text_write.py +++ b/tests/hermes_cli/test_clipboard_text_write.py @@ -17,15 +17,16 @@ def _completed(returncode=0): return subprocess.CompletedProcess(args=[], returncode=returncode) +@pytest.mark.macos_only def test_darwin_uses_pbcopy(): - with patch.object(clip.sys, "platform", "darwin"), \ - patch.object(clip.subprocess, "run", return_value=_completed()) as run: + with patch.object(clip.subprocess, "run", return_value=_completed()) as run: assert clip.write_clipboard_text("hello") is True argv = run.call_args[0][0] assert argv == ["pbcopy"] assert run.call_args[1]["input"] == b"hello" +@pytest.mark.linux_only def test_linux_falls_through_backends_until_success(): calls = [] @@ -34,8 +35,7 @@ def test_linux_falls_through_backends_until_success(): # xclip fails, xsel succeeds return _completed(returncode=0 if argv[0] == "xsel" else 1) - with patch.object(clip.sys, "platform", "linux"), \ - patch.object(clip, "_is_wsl", return_value=False), \ + with patch.object(clip, "_is_wsl", return_value=False), \ patch.dict(clip.os.environ, {}, clear=False), \ patch.object(clip.os.environ, "get", lambda k, d=None: None), \ patch.object(clip.subprocess, "run", side_effect=fake_run): diff --git a/tests/hermes_cli/test_dep_ensure.py b/tests/hermes_cli/test_dep_ensure.py index e550fdc732..14d1681607 100644 --- a/tests/hermes_cli/test_dep_ensure.py +++ b/tests/hermes_cli/test_dep_ensure.py @@ -1,16 +1,20 @@ from unittest.mock import patch +import pytest - +@pytest.mark.linux_only def test_find_install_script_from_checkout(tmp_path): - """_find_install_script finds scripts/install.sh in a git checkout.""" + """_find_install_script finds scripts/install.sh in a git checkout. + + ``linux_only``: the POSIX arm picks ``install.sh`` + ``bash``, which is + already what ``_IS_WINDOWS`` reports here — nothing needs faking. + """ from hermes_cli.dep_ensure import _find_install_script scripts_dir = tmp_path / "scripts" scripts_dir.mkdir() (scripts_dir / "install.sh").write_text("#!/bin/bash", encoding="utf-8") - with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): - path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) assert path is not None assert path.name == "install.sh" assert shell == "bash" @@ -22,13 +26,16 @@ def test_find_install_script_from_checkout(tmp_path): +@pytest.mark.windows_only def test_ensure_dependency_uses_powershell_on_windows(tmp_path): + """``windows_only``: the assertion is that we shell out to a real + PowerShell. Faking ``_IS_WINDOWS`` on Linux also required faking + ``shutil.which`` into inventing a powershell.exe that isn't there.""" from hermes_cli.dep_ensure import ensure_dependency scripts_dir = tmp_path / "scripts" scripts_dir.mkdir(parents=True) (scripts_dir / "install.ps1").write_text("# fake") - with patch("hermes_cli.dep_ensure._IS_WINDOWS", True), \ - patch("hermes_cli.dep_ensure._DEP_CHECKS", {"node": lambda: False}), \ + with patch("hermes_cli.dep_ensure._DEP_CHECKS", {"node": lambda: False}), \ patch("hermes_cli.dep_ensure._find_install_script", return_value=(scripts_dir / "install.ps1", "powershell")), \ patch("hermes_cli.dep_ensure.shutil") as mock_shutil, \ patch("hermes_constants.get_hermes_home", return_value=tmp_path / "fakehome"), \ diff --git a/tests/hermes_cli/test_desktop_exe_integrity.py b/tests/hermes_cli/test_desktop_exe_integrity.py index c91fe410dc..6e9d3dd06e 100644 --- a/tests/hermes_cli/test_desktop_exe_integrity.py +++ b/tests/hermes_cli/test_desktop_exe_integrity.py @@ -138,26 +138,36 @@ def _fake_windll( return _windll -def test_native_machine_reports_os_arch_not_process_arch(monkeypatch): +@pytest.mark.windows_only +def test_native_machine_reports_os_arch_not_process_arch(): """The #69179 WoA regression: x64 Python under ARM64 emulation must report ARM64 (the OS), not AMD64 (the process) — otherwise the integrity gate - rejects the correct ARM64 rebuild.""" + rejects the correct ARM64 rebuild. + + ``windows_only``: the probe under test is a ``ctypes.WinDLL("kernel32")`` + call to ``IsWow64Process2``. A patched ``sys.platform`` only got the branch + entered — there is no kernel32 to bind on Linux, so nothing below the + branch (the HANDLE typing that #71218 was about) was ever executed. + """ import ctypes - monkeypatch.setattr(cli_main.sys, "platform", "win32") # WinDLL only exists on Windows; create=True so Linux/macOS CI can stub it. with patch.object(ctypes, "WinDLL", _fake_windll(PE_ARM64), create=True), \ patch("platform.machine", return_value="AMD64"): assert cli_main._windows_native_machine() == "ARM64" +@pytest.mark.windows_only def test_expected_machines_prefers_user_runnable_api_over_arch_name(monkeypatch): """GetMachineTypeAttributes answers "can this host load PE machine X?" directly, so a WoA host that reports AMD64 everywhere else still accepts an - ARM64 exe.""" + ARM64 exe. + + ``windows_only``: ``GetMachineTypeAttributes`` is a real kernel32 export + the fake host could not provide. + """ import ctypes - monkeypatch.setattr(cli_main.sys, "platform", "win32") monkeypatch.setenv("PROCESSOR_ARCHITECTURE", "AMD64") monkeypatch.delenv("PROCESSOR_ARCHITEW6432", raising=False) with patch.object( @@ -225,8 +235,11 @@ def test_rollback_restores_backup_and_keeps_corrupt_copy(tmp_path): -def test_gate_fails_clearly_without_backup(tmp_path, monkeypatch, capsys): - monkeypatch.setattr(cli_main.sys, "platform", "win32") +@pytest.mark.windows_only +def test_gate_fails_clearly_without_backup(tmp_path, capsys): + """``windows_only``: ``_ensure_desktop_exe_launchable`` is a documented + no-op off Windows, so the fake was the only reason the gate ran at all. + """ desktop_dir, exe = _win_tree(tmp_path) fake = exe fake.parent.mkdir(parents=True) @@ -261,16 +274,21 @@ def _ns(**kw): return argparse.Namespace(**defaults) +@pytest.mark.windows_only def test_build_only_fails_when_pack_produces_corrupt_exe(tmp_path, monkeypatch, capsys): """The updater chain's contract: a rebuild whose Hermes.exe cannot launch must exit nonzero (so hermes-setup's retry-once kicks in) and must restore - the previous working build instead of leaving the corrupt one.""" + the previous working build instead of leaving the corrupt one. + + ``windows_only``: the whole chain is Windows-gated — ``win-unpacked`` + candidate discovery in ``_desktop_packaged_executable`` and the integrity + gate itself both short-circuit off Windows. + """ root = tmp_path / "hermes-agent" desktop_dir = root / "apps" / "desktop" desktop_dir.mkdir(parents=True) (desktop_dir / "package.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) - monkeypatch.setattr(cli_main.sys, "platform", "win32") exe = desktop_dir / "release" / "win-unpacked" / "Hermes.exe" make_pe(exe, PE_AMD64, truncate_to=0x300) # what the failed pack produced @@ -280,6 +298,7 @@ def test_build_only_fails_when_pack_produces_corrupt_exe(tmp_path, monkeypatch, pack_ok = subprocess.CompletedProcess(["npm", "run", "pack"], 0) with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._resolve_node_runtime_npm", return_value="npm.cmd"), \ patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok), \ patch("hermes_cli.main._desktop_build_needed", return_value=True), \ patch("hermes_cli.main._stop_desktop_processes_locking_build", return_value=[]), \ diff --git a/tests/hermes_cli/test_ensure_hermes_home_uid_34107.py b/tests/hermes_cli/test_ensure_hermes_home_uid_34107.py index 798bcde1ec..ceb9bfb382 100644 --- a/tests/hermes_cli/test_ensure_hermes_home_uid_34107.py +++ b/tests/hermes_cli/test_ensure_hermes_home_uid_34107.py @@ -36,7 +36,11 @@ class TestResolveHermesUidGid: assert gid == 911 - @pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific") + # ``windows_only`` rather than ``skipif(sys.platform != "win32")``: the + # Windows CI job selects ``-m windows_only``, so a bare skipif would leave + # this test skipped on Linux AND unselected on the Windows lane — dead on + # every host. + @pytest.mark.windows_only def test_windows_returns_none_none(self, monkeypatch): monkeypatch.setenv("HERMES_UID", "1000") monkeypatch.setenv("HERMES_GID", "911") diff --git a/tests/hermes_cli/test_gateway_platform_gating.py b/tests/hermes_cli/test_gateway_platform_gating.py index f6dec1b5f1..e1c4dabe25 100644 --- a/tests/hermes_cli/test_gateway_platform_gating.py +++ b/tests/hermes_cli/test_gateway_platform_gating.py @@ -12,23 +12,42 @@ Currently: Windows path that works. """ +import pytest + class TestMatrixHiddenOnWindows: - def test_matrix_present_on_linux(self, monkeypatch): - """Sanity: matrix is still in the picker on Linux/macOS.""" + @pytest.mark.linux_only + def test_matrix_present_on_linux(self): + """Sanity: matrix is still in the picker on Linux. + + Linux-gated because the assertion is the negative of the Windows + gate — it only means anything when the host really is not Windows. + """ import hermes_cli.gateway as gateway_mod - monkeypatch.setattr(gateway_mod.sys, "platform", "linux") platforms = gateway_mod._all_platforms() keys = {p["key"] for p in platforms} assert "matrix" in keys, "matrix must be available on Linux" + @pytest.mark.windows_only + def test_matrix_absent_on_windows(self): + """The gate itself: matrix must be dropped on a real Windows host. - def test_other_platforms_unaffected_on_windows(self, monkeypatch): + A patched ``sys.platform`` proved only that the ``if`` branch runs; + on native Windows this also proves the picker the user actually sees + omits the platform whose dependency cannot build here. + """ + import hermes_cli.gateway as gateway_mod + + platforms = gateway_mod._all_platforms() + keys = {p["key"] for p in platforms} + assert "matrix" not in keys, "matrix must be hidden on Windows" + + @pytest.mark.windows_only + def test_other_platforms_unaffected_on_windows(self): """Gating must only drop matrix, not collateral damage.""" import hermes_cli.gateway as gateway_mod - monkeypatch.setattr(gateway_mod.sys, "platform", "win32") platforms = gateway_mod._all_platforms() keys = {p["key"] for p in platforms} # A representative sample of platforms that have no Windows diff --git a/tests/hermes_cli/test_gateway_proc_fallback.py b/tests/hermes_cli/test_gateway_proc_fallback.py index ab5cf1b898..7872ec2b46 100644 --- a/tests/hermes_cli/test_gateway_proc_fallback.py +++ b/tests/hermes_cli/test_gateway_proc_fallback.py @@ -9,6 +9,8 @@ See: NousResearch/hermes-agent#7622 import os from unittest.mock import MagicMock, patch +import pytest + import hermes_cli.gateway as gateway_mod @@ -51,8 +53,15 @@ def _fake_proc_dir(entries: dict): # --------------------------------------------------------------------------- +@pytest.mark.linux_only class TestProcFallback: - """_scan_gateway_pids reads /proc when available, skips ps.""" + """_scan_gateway_pids reads /proc when available, skips ps. + + Linux-only: ``/proc//cmdline`` is the subject. The non-Windows arm of + ``_scan_gateway_pids`` is selected by the real host here, so the previous + ``is_windows`` stub is gone — only the /proc filesystem itself is faked so + the scan sees deterministic PIDs. + """ def test_detects_gateway_pid_via_proc(self): my_pid = os.getpid() @@ -64,7 +73,6 @@ class TestProcFallback: _isdir, _listdir, _open = _fake_proc_dir(entries) with ( - patch("hermes_cli.gateway.is_windows", return_value=False), patch("os.path.isdir", side_effect=_isdir), patch("os.listdir", side_effect=_listdir), patch("builtins.open", side_effect=_open), @@ -93,7 +101,6 @@ class TestProcFallback: raise PermissionError("no access") with ( - patch("hermes_cli.gateway.is_windows", return_value=False), patch("os.path.isdir", side_effect=_isdir), patch("os.listdir", side_effect=_listdir), patch("builtins.open", side_effect=_open), diff --git a/tests/hermes_cli/test_gateway_service.py b/tests/hermes_cli/test_gateway_service.py index a5d9ad6dee..9b5aa9942a 100644 --- a/tests/hermes_cli/test_gateway_service.py +++ b/tests/hermes_cli/test_gateway_service.py @@ -235,15 +235,20 @@ class TestGeneratedSystemdUnits: class TestGatewayStopCleanup: + @pytest.mark.linux_only def test_stop_only_kills_current_profile_by_default(self, tmp_path, monkeypatch): """Without --all, stop uses systemd (if available) and does NOT call - the global kill_gateway_processes().""" + the global kill_gateway_processes(). + + Linux-gated: the routing under test is the systemd arm, and it is only + reached when the host really isn't macOS/Windows (the old + ``is_macos → False`` stub is gone). + """ unit_path = tmp_path / "hermes-gateway.service" unit_path.write_text("unit\n", encoding="utf-8") monkeypatch.setattr(gateway_cli, "supports_systemd_services", lambda: True) monkeypatch.setattr(gateway_cli, "is_termux", lambda: False) - monkeypatch.setattr(gateway_cli, "is_macos", lambda: False) monkeypatch.setattr(gateway_cli, "get_systemd_unit_path", lambda system=False: unit_path) service_calls = [] @@ -737,12 +742,17 @@ class TestGatewaySystemServiceRouting: + @pytest.mark.macos_only def test_gateway_restart_does_not_fallback_to_foreground_when_launchd_restart_fails(self, tmp_path, monkeypatch): + """macOS-gated: the branch under test is ``elif is_macos() and + get_launchd_plist_path().exists()``. Faking the platform flags on Linux + left ``supports_systemd_services()`` / ``launchctl`` semantics untested; + on a real macOS host only ``launchd_restart`` is stubbed (it would touch + the user's real launchd domain). + """ plist_path = tmp_path / "ai.hermes.gateway.plist" plist_path.write_text("plist\n", encoding="utf-8") - monkeypatch.setattr(gateway_cli, "is_linux", lambda: False) - monkeypatch.setattr(gateway_cli, "is_macos", lambda: True) monkeypatch.setattr(gateway_cli, "get_launchd_plist_path", lambda: plist_path) monkeypatch.setattr( gateway_cli, diff --git a/tests/hermes_cli/test_gateway_windows.py b/tests/hermes_cli/test_gateway_windows.py index 616fbac42a..b8c4a5c5b3 100644 --- a/tests/hermes_cli/test_gateway_windows.py +++ b/tests/hermes_cli/test_gateway_windows.py @@ -26,10 +26,17 @@ def test_schtasks_encoding_falls_back_to_utf8(monkeypatch): +@pytest.mark.windows_only def test_build_gateway_argv_keeps_venv_console_python_for_uv_venv(monkeypatch, tmp_path): """No pythonw / base-interpreter detour: the venv console python.exe is launched hidden (CREATE_NO_WINDOW) so descendants inherit its hidden - console instead of flashing their own (#54220/#56747).""" + console instead of flashing their own (#54220/#56747). + + Windows-only: ``_build_gateway_argv()`` asserts the host is Windows and the + argv/env overlay it returns is built from real Windows path separators and + ``Scripts/python.exe`` layout — a patched ``sys.platform`` covered the + branch but not any of that. + """ project = tmp_path / "project" scripts = project / "venv" / "Scripts" @@ -53,7 +60,6 @@ def test_build_gateway_argv_keeps_venv_console_python_for_uv_venv(monkeypatch, t import hermes_cli.gateway as gateway - monkeypatch.setattr(gateway_windows.sys, "platform", "win32") monkeypatch.setattr(gateway, "PROJECT_ROOT", project) monkeypatch.setattr(gateway, "get_python_path", lambda: str(venv_python)) monkeypatch.setattr(gateway, "_profile_arg", lambda hermes_home: "") @@ -123,10 +129,17 @@ def _arrange_startup_fallback(monkeypatch, tmp_path, running_pids): +@pytest.mark.windows_only def test_elevated_gateway_command_uses_hidden_console_python(monkeypatch): """UAC handoff launches console python with SW_HIDE — a single hidden console, not console-less pythonw (#54220/#56747), and no visible - elevated cmd.exe window left open.""" + elevated cmd.exe window left open. + + Windows-only: the code path runs behind ``_assert_windows()`` and goes + through ``ctypes.windll.shell32``, neither of which exists on a faked + host. ShellExecuteW itself stays mocked — it would raise a real UAC + prompt — but the host identity is genuine. + """ calls = [] class FakeShell32: @@ -137,7 +150,6 @@ def test_elevated_gateway_command_uses_hidden_console_python(monkeypatch): class FakeWindll: shell32 = FakeShell32() - monkeypatch.setattr(gateway_windows, "_assert_windows", lambda: None) monkeypatch.setattr(gateway_windows, "_current_profile_cli_args", lambda: ["--profile", "alice"]) monkeypatch.setattr(gateway_windows.sys, "executable", r"C:\Hermes\venv\Scripts\python.exe") monkeypatch.setattr(gateway_windows.ctypes, "windll", FakeWindll(), raising=False) @@ -154,12 +166,16 @@ def test_elevated_gateway_command_uses_hidden_console_python(monkeypatch): def test_install_scheduled_task_recreates_instead_of_change(monkeypatch, tmp_path): - """Install must delete+create so stale minute-repeat task settings are not preserved.""" + """Install must delete+create so stale minute-repeat task settings are not preserved. + + Host-agnostic on purpose: ``_install_scheduled_task`` only renders the task + XML and shells out through ``_exec_schtasks`` (mocked here as the genuine + external dependency), so no platform fake is needed. + """ calls = [] script_path = tmp_path / "Hermes_Gateway_alice.cmd" xml_seen = {} - monkeypatch.setattr(gateway_windows, "_assert_windows", lambda: None) monkeypatch.setattr(gateway_windows, "_resolve_task_user", lambda: r"DOMAIN\\alice") def fake_schtasks(args): diff --git a/tests/hermes_cli/test_gateway_wsl.py b/tests/hermes_cli/test_gateway_wsl.py index bedec3ad6d..6e7ff37932 100644 --- a/tests/hermes_cli/test_gateway_wsl.py +++ b/tests/hermes_cli/test_gateway_wsl.py @@ -59,9 +59,13 @@ class TestWslSystemdOperational: class TestSupportsSystemdServicesWSL: """Test that supports_systemd_services() handles WSL correctly.""" + @pytest.mark.linux_only def test_wsl_with_systemd(self, monkeypatch): - """WSL + working systemd → True.""" - monkeypatch.setattr(gateway, "is_linux", lambda: True) + """WSL + working systemd → True. + + Linux-gated: ``supports_systemd_services()`` short-circuits on + ``is_linux()``, so off Linux this asserted nothing about systemd. + """ monkeypatch.setattr(gateway, "is_termux", lambda: False) monkeypatch.setattr( gateway.shutil, "which", lambda _name: "/usr/bin/systemctl" @@ -70,9 +74,13 @@ class TestSupportsSystemdServicesWSL: monkeypatch.setattr(gateway, "_wsl_systemd_operational", lambda: True) assert gateway.supports_systemd_services() is True + @pytest.mark.linux_only def test_termux_still_excluded(self, monkeypatch): - """Termux → False regardless of WSL status.""" - monkeypatch.setattr(gateway, "is_linux", lambda: True) + """Termux → False regardless of WSL status. + + Linux-gated: off Linux the ``not is_linux()`` arm returns False first, + so the Termux exclusion itself would never be exercised. + """ monkeypatch.setattr(gateway, "is_termux", lambda: True) assert gateway.supports_systemd_services() is False @@ -84,22 +92,20 @@ class TestSupportsSystemdServicesWSL: class TestGatewayCommandWSLMessages: """Test that WSL users see appropriate guidance.""" + @pytest.mark.linux_only def test_install_wsl_no_systemd(self, monkeypatch, capsys): - """hermes gateway install on WSL without systemd shows guidance.""" - monkeypatch.setattr(gateway, "is_linux", lambda: True) + """hermes gateway install on WSL without systemd shows guidance. + + Linux-gated: WSL *is* a Linux host, and the guidance branch sits after + the macOS/Windows arms in ``gateway_command``. Reaching it on another + host previously required stubbing ``is_macos``/``is_windows`` — on a + real Windows host the unstubbed version would have run + ``gateway_windows.install()`` against the user's real Startup folder. + """ monkeypatch.setattr(gateway, "is_termux", lambda: False) monkeypatch.setattr(gateway, "is_wsl", lambda: True) monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) - monkeypatch.setattr(gateway, "is_macos", lambda: False) monkeypatch.setattr(gateway, "is_managed", lambda: False) - # CRITICAL: also stub is_windows. Without this, running this test on a - # real Windows host falls through to the is_windows() branch *before* - # the WSL guidance branch, invoking gateway_windows.install() which - # writes a Startup-folder .cmd into the real user's Startup folder - # (NOT tmp_path) pointing at a now-vanished pytest fixture path. - # The user then sees a broken Hermes_Gateway.cmd flash a cmd.exe - # window on every login. See fix/windows-gateway-reliability. - monkeypatch.setattr(gateway, "is_windows", lambda: False) args = SimpleNamespace( gateway_command="install", force=False, system=False, @@ -116,15 +122,16 @@ class TestGatewayCommandWSLMessages: assert "tmux" in out + @pytest.mark.linux_only def test_status_wsl_running_manual(self, monkeypatch, capsys): - """hermes gateway status on WSL with manual process shows WSL note.""" + """hermes gateway status on WSL with manual process shows WSL note. + + Linux-gated for the same reason as the install case: the WSL note is + printed only after the macOS/Windows service branches decline. + """ monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) - monkeypatch.setattr(gateway, "is_macos", lambda: False) monkeypatch.setattr(gateway, "is_termux", lambda: False) monkeypatch.setattr(gateway, "is_wsl", lambda: True) - # Stub is_windows so a Windows host running this test does NOT take - # the Windows status branch (which reads gateway_windows.is_installed()). - monkeypatch.setattr(gateway, "is_windows", lambda: False) monkeypatch.setattr(gateway, "find_gateway_pids", lambda: [12345]) monkeypatch.setattr(gateway, "_runtime_health_lines", lambda: []) # Stub out the systemd unit path check diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index cde1ee5b50..96b968c5f9 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -13,6 +13,24 @@ import pytest from hermes_cli import main as cli_main +@pytest.fixture(autouse=True) +def _isolate_xdg_data_home(tmp_path, monkeypatch): + """Keep desktop-entry writes out of the developer's real home directory. + + ``cmd_gui`` registers an XDG launcher entry, and ``desktop_entry_path()`` + resolves it under ``XDG_DATA_HOME`` (falling back to ``~/.local/share``). + While these tests faked the host as darwin the Linux-only registration + never ran, so nothing escaped. Running them on their real host makes that + call live, and on a Linux dev box it wrote a ``hermes.desktop`` pointing + ``Exec=`` at the test's throwaway npm stub into the user's actual + applications menu. + + The hermetic conftest deliberately does NOT redirect ``HOME`` (subprocesses + depend on it being stable), so this has to be pinned per-file. + """ + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "xdg-data")) + + def _ns(**kw): defaults = dict( skip_build=False, @@ -36,17 +54,30 @@ def _make_desktop_tree(tmp_path: Path) -> Path: return root -def _make_packaged_executable(root: Path, monkeypatch, platform: str = "darwin") -> Path: - monkeypatch.setattr(cli_main.sys, "platform", platform) +def _make_packaged_executable(root: Path, monkeypatch) -> Path: + """Create the packaged-app path layout electron-builder emits on THIS host. + + The layout is keyed off the real ``sys.platform`` rather than a caller- + supplied override: ``cmd_gui`` resolves the executable through the same + branch, so faking the platform here only proved the test and the code + agreed about a host neither was running on. + + Note the Linux arm also lays down ``chrome-sandbox``. ``cmd_gui`` refuses to + launch without it (Electron's setuid sandbox helper), which the old + darwin-by-default fake concealed — on Linux the packaged tree genuinely has + to include it. + """ desktop_dir = root / "apps" / "desktop" - if platform == "darwin": + if sys.platform == "darwin": exe = desktop_dir / "release" / "mac-arm64" / "Hermes.app" / "Contents" / "MacOS" / "Hermes" - elif platform == "win32": + elif sys.platform == "win32": exe = desktop_dir / "release" / "win-unpacked" / "Hermes.exe" else: exe = desktop_dir / "release" / "linux-unpacked" / "hermes" - exe.parent.mkdir(parents=True) + exe.parent.mkdir(parents=True, exist_ok=True) exe.write_text("", encoding="utf-8") + if sys.platform not in ("darwin", "win32"): + (exe.parent / "chrome-sandbox").write_text("", encoding="utf-8") return exe @@ -65,6 +96,8 @@ def test_gui_installs_packages_and_launches_desktop_app(tmp_path, monkeypatch): patch("hermes_cli.main._desktop_build_needed", return_value=True), \ patch("hermes_cli.main._write_desktop_build_stamp"), \ patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \ + patch("hermes_cli.main._desktop_linux_sandbox_fixup", return_value=True), \ + patch("hermes_cli.main._register_linux_desktop_entry"), \ patch("hermes_cli.main.subprocess.run", side_effect=[pack_ok, launch_ok]) as mock_run, \ pytest.raises(SystemExit) as exc: cli_main.cmd_gui(_ns()) @@ -95,7 +128,7 @@ def test_gui_install_env_prepends_managed_node_on_bare_path(tmp_path, monkeypatc root = _make_desktop_tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) - _make_packaged_executable(root, monkeypatch, platform="win32") + _make_packaged_executable(root, monkeypatch) # A managed Node tree on disk so with_hermes_node_path() actually prepends it. home = tmp_path / "hermes-home" @@ -107,11 +140,19 @@ def test_gui_install_env_prepends_managed_node_on_bare_path(tmp_path, monkeypatc install_ok = subprocess.CompletedProcess(["npm", "ci"], 0) launch_ok = subprocess.CompletedProcess(["hermes"], 0) + # A plain return_value rather than a fixed side_effect list: this test only + # cares about the env handed to the npm install, and pinning an exact + # sequence of subprocess.run calls makes it fail (StopIteration) whenever + # cmd_gui legitimately shells out one extra time — e.g. the Linux sandbox + # fixup, which fires on hosts where chrome-sandbox isn't already + # root-owned+4755. Assert on the install env, not on a call count. with patch("hermes_cli.main._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \ patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok) as mock_install, \ patch("hermes_cli.main._desktop_build_needed", return_value=True), \ patch("hermes_cli.main._write_desktop_build_stamp"), \ - patch("hermes_cli.main.subprocess.run", side_effect=[subprocess.CompletedProcess([], 0), launch_ok]), \ + patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \ + patch("hermes_cli.main._desktop_linux_sandbox_fixup", return_value=True), \ + patch("hermes_cli.main.subprocess.run", return_value=launch_ok), \ pytest.raises(SystemExit): cli_main.cmd_gui(_ns(skip_build=False)) @@ -196,7 +237,7 @@ def test_gui_does_not_retry_after_packaged_executable_exists(tmp_path, monkeypat root = _make_desktop_tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) # Executable EXISTS at failure time → late failure, not a corrupt download. - _make_packaged_executable(root, monkeypatch, platform="darwin") + _make_packaged_executable(root, monkeypatch) monkeypatch.delenv("ELECTRON_MIRROR", raising=False) install_ok = subprocess.CompletedProcess(["npm", "ci"], 0) @@ -224,25 +265,74 @@ def test_gui_does_not_retry_after_packaged_executable_exists(tmp_path, monkeypat # ── electronDist (re)download helper tests (#47266) ─────────────────── -@pytest.mark.parametrize( - "platform,rel", - [ - ("linux", "dist/electron"), - ("win32", "dist/electron.exe"), - ("darwin", "dist/Electron.app/Contents/MacOS/Electron"), - ], -) -def test_electron_dist_ok_per_platform(tmp_path, monkeypatch, platform, rel): - monkeypatch.setattr(cli_main.sys, "platform", platform) - electron = tmp_path / "node_modules" / "electron" - # A dist dir that exists but lacks the binary is NOT ok (partial extraction). - (electron / "dist").mkdir(parents=True) - assert cli_main._electron_dist_ok(tmp_path) is False +def test_electron_dist_ok_on_this_host(): + """A dist dir that exists but lacks the binary is NOT ok (partial extraction). - binp = electron / rel - binp.parent.mkdir(parents=True, exist_ok=True) - binp.write_text("", encoding="utf-8") - assert cli_main._electron_dist_ok(tmp_path) is True + The binary's basename is per-OS (``electron`` / ``electron.exe`` / + ``Electron.app/…/Electron``), and ``_electron_dist_binary()`` picks it from + the real ``sys.platform``. Asking the implementation for the path it + expects — instead of hardcoding one and faking the platform to match — + makes this a genuine round-trip on whichever lane runs it. + """ + import tempfile + + with tempfile.TemporaryDirectory() as td: + root = Path(td) + electron = root / "node_modules" / "electron" + (electron / "dist").mkdir(parents=True) + assert cli_main._electron_dist_ok(root) is False + + binp = cli_main._electron_dist_binary(root) + # The resolved binary must live under the dist dir we just created. + assert (electron / "dist") in binp.parents + binp.parent.mkdir(parents=True, exist_ok=True) + binp.write_text("", encoding="utf-8") + assert cli_main._electron_dist_ok(root) is True + + +@pytest.mark.linux_only +def test_electron_dist_binary_basename_linux(): + """``dist/electron`` on Linux — asserted against the live function. + + Split per-OS rather than parametrized over a platform table: the old + ``@parametrize(("linux", …), ("win32", …), ("darwin", …))`` skipped the two + non-host rows, so outside the Linux lane those two branches were asserted + nowhere at all. One marked test per OS puts each row on the lane that can + actually execute it. + """ + root = Path("/tmp/does-not-need-to-exist") + assert cli_main._electron_dist_binary(root) == ( + root / "node_modules" / "electron" / "dist" / "electron" + ) + + +@pytest.mark.windows_only +def test_electron_dist_binary_basename_windows(): + """``dist/electron.exe`` on Windows — the ``.exe`` suffix is the whole point.""" + root = Path("C:/does-not-need-to-exist") + assert cli_main._electron_dist_binary(root) == ( + root / "node_modules" / "electron" / "dist" / "electron.exe" + ) + + +@pytest.mark.macos_only +def test_electron_dist_binary_basename_macos(): + """``dist/Electron.app/Contents/MacOS/Electron`` on macOS. + + The nested ``.app`` bundle path is why #47266's "dist exists but the + binary doesn't" check can't just stat the dist directory. + """ + root = Path("/tmp/does-not-need-to-exist") + assert cli_main._electron_dist_binary(root) == ( + root + / "node_modules" + / "electron" + / "dist" + / "Electron.app" + / "Contents" + / "MacOS" + / "Electron" + ) @@ -346,14 +436,19 @@ def test_desktop_macos_local_codesign_signs_native_binaries(tmp_path, monkeypatc +@pytest.mark.macos_only def test_relaunchable_fixup_falls_back_to_legacy_adhoc_on_failure(tmp_path, monkeypatch, capsys): - """A failing stable sign must still leave a launchable (deep ad-hoc) bundle.""" + """A failing stable sign must still leave a launchable (deep ad-hoc) bundle. + + ``macos_only``: the subject is ``codesign`` against a real ``.app`` bundle + layout (``exe.parents[2]``), which only the macOS packaged tree produces. + """ root = _make_desktop_tree(tmp_path) desktop_dir = root / "apps" / "desktop" monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) monkeypatch.delenv("CSC_LINK", raising=False) monkeypatch.delenv("APPLE_SIGNING_IDENTITY", raising=False) - exe = _make_packaged_executable(root, monkeypatch, platform="darwin") + exe = _make_packaged_executable(root, monkeypatch) app = exe.parents[2] calls = [] @@ -387,11 +482,12 @@ def test_relaunchable_fixup_falls_back_to_legacy_adhoc_on_failure(tmp_path, monk # --- Linux launcher entry registration ------------------------------------ +@pytest.mark.linux_only def test_gui_registers_linux_desktop_entry_before_launch(tmp_path, monkeypatch): """`hermes desktop` gives the app a launcher presence on Linux.""" root = _make_desktop_tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) - packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + packaged_exe = _make_packaged_executable(root, monkeypatch) registered: list[Path] = [] monkeypatch.setattr("hermes_cli.linux_desktop_entry.is_supported", lambda: True) @@ -412,11 +508,12 @@ def test_gui_registers_linux_desktop_entry_before_launch(tmp_path, monkeypatch): assert registered == [root] +@pytest.mark.linux_only def test_gui_launches_even_when_desktop_entry_install_fails(tmp_path, monkeypatch): """Launcher plumbing is a convenience — it must never block the app.""" root = _make_desktop_tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) - packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + packaged_exe = _make_packaged_executable(root, monkeypatch) def boom(_project_root): raise OSError("read-only /home") @@ -437,10 +534,11 @@ def test_gui_launches_even_when_desktop_entry_install_fails(tmp_path, monkeypatc assert mock_run.call_args.args[0] == [str(packaged_exe)] +@pytest.mark.macos_only def test_gui_skips_desktop_entry_off_linux(tmp_path, monkeypatch): root = _make_desktop_tree(tmp_path) monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) - packaged_exe = _make_packaged_executable(root, monkeypatch, platform="darwin") + packaged_exe = _make_packaged_executable(root, monkeypatch) monkeypatch.setattr("hermes_cli.linux_desktop_entry.is_supported", lambda: False) diff --git a/tests/hermes_cli/test_install_cua_driver.py b/tests/hermes_cli/test_install_cua_driver.py index b768758700..55d82408a4 100644 --- a/tests/hermes_cli/test_install_cua_driver.py +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -29,7 +29,23 @@ import pytest class TestInstallCuaDriverUpgrade: + # ``install_cua_driver`` supports macOS, Windows AND Linux. For everything + # below except the two unsupported-platform cases, the Linux host takes a + # byte-identical path to macOS — same ``fetch_tool`` ("curl"), same + # ``_cua_install_target_writable()`` verdict, same branch — so the old + # ``patch("platform.system", return_value="Darwin")`` bought nothing but a + # fake host. Dropped, and the names no longer claim macOS. + def test_upgrade_on_unsupported_platform_is_silent_noop(self): + """The one branch no CI runner can reach for real. + + ``platform.system`` is still faked here, deliberately and narrowly: we + run Linux/macOS/Windows lanes, and every one of them is a *supported* + platform, so the refusal path is unreachable on all three. The fake is + sound because the function returns before touching any OS facility — + no subprocess, no path handling, no import — so there is nothing + underneath the branch for a real host to falsify. + """ from hermes_cli import tools_config with patch.object(tools_config, "_print_warning") as warn, \ @@ -38,6 +54,7 @@ class TestInstallCuaDriverUpgrade: warn.assert_not_called() def test_non_upgrade_on_unsupported_platform_warns(self): + """Same narrow exception as above — see that test's docstring.""" from hermes_cli import tools_config with patch.object(tools_config, "_print_warning") as warn, \ @@ -45,11 +62,10 @@ class TestInstallCuaDriverUpgrade: assert tools_config.install_cua_driver(upgrade=False) is False warn.assert_called() - def test_upgrade_on_macos_with_binary_runs_installer(self): + def test_upgrade_with_binary_present_runs_installer(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/local/bin/" + n if n in {"cua-driver", "curl"} else None), \ patch.object(tools_config, "_run_cua_driver_installer", @@ -60,19 +76,20 @@ class TestInstallCuaDriverUpgrade: kwargs = runner.call_args.kwargs assert kwargs.get("verbose") is False - def test_upgrade_on_macos_without_binary_runs_installer(self): + def test_upgrade_without_binary_runs_installer(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ patch.object(tools_config, "_run_cua_driver_installer", return_value=True) as runner: assert tools_config.install_cua_driver(upgrade=True) is True runner.assert_called_once() + @pytest.mark.linux_only def test_quiet_refresh_prints_single_contextual_progress_line(self): - import subprocess + """``linux_only``: reaches Popen through the POSIX download-then-exec + branch, which this lane takes for real.""" from unittest.mock import MagicMock from hermes_cli import tools_config @@ -82,8 +99,7 @@ class TestInstallCuaDriverUpgrade: fake_proc.returncode = 0 fake_proc.communicate.return_value = ("", None) - with patch("platform.system", return_value="Linux"), \ - patch( + with patch( "subprocess.run", return_value=MagicMock(returncode=0, stderr=""), ), \ @@ -104,7 +120,9 @@ class TestInstallCuaDriverUpgrade: "→ Refreshing cua-driver (Computer Use)..." ) + @pytest.mark.linux_only def test_quiet_refresh_can_suppress_progress_line(self): + """``linux_only``: same POSIX Popen path as the test above.""" from unittest.mock import MagicMock from hermes_cli import tools_config @@ -114,8 +132,7 @@ class TestInstallCuaDriverUpgrade: fake_proc.returncode = 0 fake_proc.communicate.return_value = ("", None) - with patch("platform.system", return_value="Linux"), \ - patch( + with patch( "subprocess.run", return_value=MagicMock(returncode=0, stderr=""), ), \ @@ -138,8 +155,7 @@ class TestInstallCuaDriverUpgrade: def test_upgrade_can_suppress_installer_progress(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object( + with patch.object( tools_config.shutil, "which", side_effect=lambda name: ( @@ -161,11 +177,10 @@ class TestInstallCuaDriverUpgrade: assert runner.call_args.kwargs["show_progress"] is False - def test_upgrade_on_macos_non_writable_applications_skips_refresh(self): + def test_upgrade_non_writable_install_target_skips_refresh(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/local/bin/" + n if n in {"cua-driver", "curl"} else None), \ patch.object(tools_config, "_cua_install_target_writable", @@ -179,11 +194,10 @@ class TestInstallCuaDriverUpgrade: for call in info.call_args_list ) - def test_fresh_install_on_macos_non_writable_applications_skips_install(self): + def test_fresh_install_non_writable_install_target_skips_install(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ patch.object(tools_config, "_cua_install_target_writable", return_value=False), \ @@ -196,11 +210,29 @@ class TestInstallCuaDriverUpgrade: for call in info.call_args_list ) - def test_non_upgrade_on_macos_with_binary_skips_install(self): + @pytest.mark.macos_only + def test_install_target_writability_is_probed_for_real_on_macos(self): + """The ``_cua_install_target_writable`` seam the two tests above patch. + + ``macos_only``: ``/Applications`` is the only install target Hermes + checks, and the probe short-circuits to True on every other platform — + so this is the one host where the real filesystem answer means + anything. + """ + import os + from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + writable = tools_config._cua_install_target_writable() + if os.path.isdir("/Applications"): + assert writable is os.access("/Applications", os.W_OK) + else: + assert writable is True + + def test_non_upgrade_with_binary_skips_install(self): + from hermes_cli import tools_config + + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/local/bin/" + n if n in {"cua-driver", "curl"} else None), \ patch.object(tools_config, "_run_cua_driver_installer") as runner, \ @@ -208,11 +240,10 @@ class TestInstallCuaDriverUpgrade: assert tools_config.install_cua_driver(upgrade=False) is True runner.assert_not_called() - def test_non_upgrade_on_macos_without_binary_runs_installer(self): + def test_non_upgrade_without_binary_runs_installer(self): from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ patch.object(tools_config, "_run_cua_driver_installer", return_value=True) as runner: @@ -233,18 +264,25 @@ class TestRequireConfirmedUpdate: still reinstall when the check can't answer. """ - def _install(self, system, check_state, require_confirmed): + def _install(self, check_state, require_confirmed): + """Drive ``install_cua_driver`` on the host, whatever it is. + + The old signature took a ``system`` string and faked + ``platform.system`` with it, so callers picked "Windows"/"Darwin" + arbitrarily. Nothing in the confirmed-update gate is + platform-dependent — it's ``check-update`` state plus a flag — so the + fake only decided which lie the test told itself. ``which`` answers + for every fetch tool so the host's own branch resolves cleanly. + """ from unittest.mock import MagicMock from hermes_cli import tools_config - exe = "cua-driver" + (".exe" if system == "Windows" else "") - with patch("platform.system", return_value=system), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/x/" + n if n in {"cua-driver", "curl", "powershell"} else None), \ patch.object(tools_config, "_resolved_cua_driver_cmd", - return_value="/x/" + exe), \ + return_value="/x/cua-driver"), \ patch.object(tools_config, "_cua_install_target_writable", return_value=True), \ patch("tools.computer_use.cua_backend.cua_driver_update_check", @@ -262,7 +300,7 @@ class TestRequireConfirmedUpdate: return ok, runner, info def test_indeterminate_check_keeps_installed_version(self): - ok, runner, info = self._install("Windows", None, require_confirmed=True) + ok, runner, info = self._install(None, require_confirmed=True) assert ok is True runner.assert_not_called() assert any( @@ -271,7 +309,7 @@ class TestRequireConfirmedUpdate: ) def test_indeterminate_check_points_at_force_path(self): - ok, runner, info = self._install("Darwin", None, require_confirmed=True) + ok, runner, info = self._install(None, require_confirmed=True) assert ok is True runner.assert_not_called() assert any( @@ -282,21 +320,21 @@ class TestRequireConfirmedUpdate: def test_confirmed_update_still_runs_installer(self): state = {"current_version": "0.5.0", "latest_version": "0.6.0", "update_available": True} - ok, runner, _ = self._install("Windows", state, require_confirmed=True) + ok, runner, _ = self._install(state, require_confirmed=True) assert ok is True runner.assert_called_once() def test_up_to_date_short_circuits(self): state = {"current_version": "0.6.0", "latest_version": "0.6.0", "update_available": False} - ok, runner, _ = self._install("Windows", state, require_confirmed=True) + ok, runner, _ = self._install(state, require_confirmed=True) assert ok is True runner.assert_not_called() def test_explicit_upgrade_still_falls_through_on_indeterminate(self): # `hermes computer-use install --upgrade` (default flag): the old # behaviour — indeterminate check re-runs the installer. - ok, runner, _ = self._install("Darwin", None, require_confirmed=False) + ok, runner, _ = self._install(None, require_confirmed=False) assert ok is True runner.assert_called_once() @@ -309,7 +347,7 @@ class TestUpdateCheckTimeoutDefaults: full reinstall fall-through during `hermes update`. """ - def _captured_timeout(self, platform_name): + def _captured_timeout(self): from unittest.mock import MagicMock from tools.computer_use import cua_backend @@ -323,17 +361,23 @@ class TestUpdateCheckTimeoutDefaults: with patch("tools.computer_use.cua_backend.resolve_cua_driver_cmd", return_value="/x/cua-driver"), \ - patch("tools.computer_use.cua_backend.sys.platform", platform_name), \ patch("tools.computer_use.cua_backend.subprocess.run", side_effect=fake_run): cua_backend.cua_driver_update_check() return captured.get("timeout") + @pytest.mark.windows_only def test_windows_default_is_generous(self): - assert self._captured_timeout("win32") == 25.0 + """``windows_only``: the 25s default exists because a real Windows + first-spawn is delayed by Defender/SmartScreen scanning — a faked + platform asserted the constant, never the host it is chosen for. + """ + assert self._captured_timeout() == 25.0 def test_posix_default_unchanged(self): - assert self._captured_timeout("linux") == 8.0 + # Unmarked: the POSIX default is what this (Linux) host already picks, + # so no platform faking is involved. + assert self._captured_timeout() == 8.0 def test_explicit_timeout_wins(self): from unittest.mock import MagicMock @@ -389,9 +433,11 @@ class TestArchProbeRemoval: """ from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", - side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ + # No platform fake: "does Python hit the GitHub API?" is host-agnostic, + # and ``which`` is stubbed so the host's own fetch tool resolves. + with patch.object(tools_config.shutil, "which", + side_effect=lambda n: "/usr/bin/" + n + if n in ("curl", "powershell") else None), \ patch("urllib.request.urlopen") as urlopen, \ patch.object(tools_config, "_run_cua_driver_installer", return_value=True) as runner: @@ -409,10 +455,9 @@ class TestArchProbeRemoval: """ from hermes_cli import tools_config - with patch("platform.system", return_value="Darwin"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/usr/local/bin/" + n - if n in ("cua-driver", "curl") else None), \ + if n in ("cua-driver", "curl", "powershell") else None), \ patch("urllib.request.urlopen") as urlopen, \ patch("subprocess.run"), \ patch.object(tools_config, "_run_cua_driver_installer", @@ -490,11 +535,15 @@ class TestPosixStaleInstallLockClear: class TestWindowsStaleInstallLockClearDispatch: + @pytest.mark.windows_only def test_windows_branch_uses_file_lock_probe(self): + """``windows_only``: which lock protocol applies IS the host fact under + test — on Linux the faked platform asserted the dispatch and skipped + the ``.install.lock.d`` directory that really exists here. + """ from hermes_cli import tools_config - with patch.object(tools_config.sys, "platform", "win32"), \ - patch.object( + with patch.object( tools_config, "_clear_stale_windows_cua_install_lock" ) as clear_windows: tools_config._clear_stale_cua_install_lock() @@ -502,10 +551,10 @@ class TestWindowsStaleInstallLockClearDispatch: clear_windows.assert_called_once_with() -@pytest.mark.skipif( - sys.platform != "win32", - reason="requires native Win32 FileShare semantics", -) +# ``windows_only`` rather than ``skipif(sys.platform != "win32")``: the +# dedicated Windows CI job selects ``-m windows_only``, so a bare skipif left +# these real-CreateFileW tests running on no host at all. +@pytest.mark.windows_only class TestWindowsStaleInstallLockClear: def _make_lock(self, tmp_path): import os @@ -572,10 +621,16 @@ class TestWindowsStaleInstallLockClear: class TestInstallerTimeoutKillsProcessGroup: """On timeout the whole installer process group must be killed, so the - `curl | bash` grandchildren can't survive holding the install lock.""" + `curl | bash` grandchildren can't survive holding the install lock. - def test_timeout_kills_process_group_and_returns_false(self, tmp_path): - import os + The POSIX cases drop the old ``platform.system`` → "Linux" fake: this lane + IS Linux, so the branch is selected for real. The Windows cases are + ``windows_only`` — the psutil tree-kill only runs when ``is_windows``, and + on Linux the fake picked that branch on a host with no such process model. + """ + + @pytest.mark.linux_only + def test_timeout_kills_process_group_and_returns_false(self): import signal import subprocess from unittest.mock import MagicMock @@ -596,9 +651,7 @@ class TestInstallerTimeoutKillsProcessGroup: killed["pgid"] = pgid killed["sig"] = sig - with patch("platform.system", return_value="Linux"), \ - patch.object(signal, "SIGKILL", sigkill, create=True), \ - patch("subprocess.run", return_value=MagicMock(returncode=0, stderr="")), \ + with patch("subprocess.run", return_value=MagicMock(returncode=0, stderr="")), \ patch("subprocess.Popen", return_value=fake_proc), \ patch.object( tools_config.os, "getpgid", return_value=99999, create=True @@ -623,8 +676,8 @@ class TestInstallerTimeoutKillsProcessGroup: # lock; our ceiling must give that window room to complete. assert tools_config._CUA_INSTALLER_TIMEOUT > tools_config._CUA_LOCK_STALE_AFTER - def test_installer_runs_in_new_session_on_posix(self, tmp_path): - import subprocess + @pytest.mark.linux_only + def test_installer_runs_in_new_session_on_posix(self): from unittest.mock import MagicMock from hermes_cli import tools_config @@ -638,8 +691,7 @@ class TestInstallerTimeoutKillsProcessGroup: captured.update(kwargs) return fake_proc - with patch("platform.system", return_value="Linux"), \ - patch("subprocess.run", return_value=MagicMock(returncode=0, stderr="")), \ + with patch("subprocess.run", return_value=MagicMock(returncode=0, stderr="")), \ patch("subprocess.Popen", side_effect=fake_popen), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ patch.object(tools_config, "_print_warning"), \ @@ -648,6 +700,7 @@ class TestInstallerTimeoutKillsProcessGroup: assert captured.get("start_new_session") is True + @pytest.mark.windows_only def test_windows_timeout_kills_descendants_and_parent(self): import subprocess from unittest.mock import MagicMock @@ -664,8 +717,7 @@ class TestInstallerTimeoutKillsProcessGroup: ("", None), ] - with patch("platform.system", return_value="Windows"), \ - patch("subprocess.Popen", return_value=fake_proc), \ + with patch("subprocess.Popen", return_value=fake_proc), \ patch("psutil.Process", return_value=parent), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ patch.object(tools_config, "_print_warning"), \ @@ -681,6 +733,7 @@ class TestInstallerTimeoutKillsProcessGroup: fake_proc.kill.assert_not_called() assert fake_proc.communicate.call_count == 2 + @pytest.mark.windows_only def test_windows_tree_enumeration_failure_falls_back_to_direct_kill(self): import psutil import subprocess @@ -697,8 +750,7 @@ class TestInstallerTimeoutKillsProcessGroup: ("", None), ] - with patch("platform.system", return_value="Windows"), \ - patch("subprocess.Popen", return_value=fake_proc), \ + with patch("subprocess.Popen", return_value=fake_proc), \ patch("psutil.Process", return_value=parent), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ patch.object(tools_config, "_print_warning"), \ @@ -712,14 +764,19 @@ class TestInstallerTimeoutKillsProcessGroup: assert fake_proc.communicate.call_count == 2 +@pytest.mark.linux_only class TestInstallerNoShell: """The POSIX installer path must not use shell=True or command substitution: the script is downloaded to a mkstemp file and exec'd as a plain argv list (salvage of #34974's intent, without the fixed - /tmp path TOCTOU that PR introduced).""" + /tmp path TOCTOU that PR introduced). + + ``linux_only``: the download-then-exec argv IS the POSIX branch, and this + lane already takes it — the old ``platform.system`` → "Linux" fake was + asserting a branch the host had already selected. + """ def _run(self, download_rc=0): - import subprocess from unittest.mock import MagicMock from hermes_cli import tools_config @@ -740,8 +797,7 @@ class TestInstallerNoShell: calls.append(("popen", cmd, kw)) return fake_proc - with patch("platform.system", return_value="Linux"), \ - patch("subprocess.run", side_effect=fake_run), \ + with patch("subprocess.run", side_effect=fake_run), \ patch("subprocess.Popen", side_effect=fake_popen), \ patch.object(tools_config.shutil, "which", return_value="/usr/local/bin/cua-driver"), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ @@ -771,10 +827,9 @@ class TestInstallerNoShell: assert ok is False assert not [c for c in calls if c[0] == "popen"] - def test_temp_script_removed_after_run(self, tmp_path): + def test_temp_script_removed_after_run(self): import os captured = {} - import subprocess from unittest.mock import MagicMock from hermes_cli import tools_config @@ -791,8 +846,7 @@ class TestInstallerNoShell: captured["script"] = cmd[1] return fake_proc - with patch("platform.system", return_value="Linux"), \ - patch("subprocess.run", side_effect=fake_run), \ + with patch("subprocess.run", side_effect=fake_run), \ patch("subprocess.Popen", side_effect=fake_popen), \ patch.object(tools_config.shutil, "which", return_value="/usr/local/bin/cua-driver"), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ @@ -818,16 +872,22 @@ class TestConfirmedVersionPinning: """ def _install(self, check_state): + """Host-agnostic: version pinning is string handling, not an OS branch. + + The old ``platform.system`` → "Windows" fake was incidental — the + pin flows into ``CUA_DRIVER_RS_VERSION`` identically on every host + (both upstream installers honour it), and this test never reaches the + installer anyway because ``_run_cua_driver_installer`` is mocked. + """ from unittest.mock import MagicMock from hermes_cli import tools_config - with patch("platform.system", return_value="Windows"), \ - patch.object(tools_config.shutil, "which", + with patch.object(tools_config.shutil, "which", side_effect=lambda n: "/x/" + n if n in {"cua-driver", "curl", "powershell"} else None), \ patch.object(tools_config, "_resolved_cua_driver_cmd", - return_value="/x/cua-driver.exe"), \ + return_value="/x/cua-driver"), \ patch.object(tools_config, "_cua_install_target_writable", return_value=True), \ patch("tools.computer_use.cua_backend.cua_driver_update_check", @@ -872,9 +932,16 @@ class TestConfirmedVersionPinning: assert runner.call_args.kwargs.get("pin_version") is None +@pytest.mark.linux_only class TestRunInstallerPinEnv: """_run_cua_driver_installer(pin_version=...) exports CUA_DRIVER_RS_VERSION - into the installer child env; unpinned runs leave it untouched.""" + into the installer child env; unpinned runs leave it untouched. + + ``linux_only``: the helper reaches Popen through the POSIX + download-then-exec branch, which this lane takes for real — no + ``platform.system`` fake needed. The pin itself is host-agnostic + (``TestConfirmedVersionPinning`` covers the caller side unmarked). + """ def _run(self, pin_version): from unittest.mock import MagicMock @@ -895,8 +962,7 @@ class TestRunInstallerPinEnv: m = MagicMock(); m.returncode = 0; m.stderr = "" return m - with patch("platform.system", return_value="Linux"), \ - patch("subprocess.run", side_effect=fake_run), \ + with patch("subprocess.run", side_effect=fake_run), \ patch("subprocess.Popen", side_effect=fake_popen), \ patch.object(tools_config, "_cua_driver_env", return_value={"PATH": "/usr/bin"}), \ @@ -918,7 +984,12 @@ class TestRunInstallerPinEnv: class TestWindowsAutostartRepair: + @pytest.mark.windows_only def test_existing_task_skips_elevated_powershell_repair(self): + """``windows_only``: ``_repair_cua_driver_autostart_windows`` returns + True unconditionally off Windows, so only the fake made the schtasks + probe run at all. + """ from hermes_cli import tools_config calls = [] @@ -927,8 +998,7 @@ class TestWindowsAutostartRepair: calls.append((cmd, kwargs)) return SimpleNamespace(returncode=0) - with patch.object(tools_config.sys, "platform", "win32"), \ - patch("subprocess.run", side_effect=fake_run), \ + with patch("subprocess.run", side_effect=fake_run), \ patch.object(tools_config.shutil, "which") as which: ok = tools_config._repair_cua_driver_autostart_windows( "cua-driver", verbose=False @@ -940,7 +1010,11 @@ class TestWindowsAutostartRepair: ] which.assert_not_called() + @pytest.mark.windows_only def test_windows_installer_runs_autostart_repair_after_success(self): + """``windows_only``: the PowerShell install argv and the autostart + repair hook are both inside the ``is_windows`` branch, so on Linux the + fake selected a branch whose `powershell` doesn't exist on PATH.""" from unittest.mock import MagicMock from hermes_cli import tools_config @@ -960,8 +1034,7 @@ class TestWindowsAutostartRepair: return r"C:\Users\Ha Trung\AppData\Local\Programs\Cua\cua-driver\bin\cua-driver.exe" return None - with patch("platform.system", return_value="Windows"), \ - patch.object(tools_config.shutil, "which", side_effect=fake_which), \ + with patch.object(tools_config.shutil, "which", side_effect=fake_which), \ patch("subprocess.Popen", side_effect=fake_popen), \ patch.object(tools_config, "_clear_stale_cua_install_lock"), \ patch.object(tools_config, "_repair_cua_driver_autostart_windows", return_value=True) as repair, \ @@ -978,7 +1051,11 @@ class TestWindowsAutostartRepair: ] repair.assert_called_once_with("cua-driver", verbose=False) + @pytest.mark.windows_only def test_autostart_repair_quotes_username_space_path_via_file_path(self): + """``windows_only``: same early return off Windows — the elevated + PowerShell command string is only built on a real Windows host. + """ from hermes_cli import tools_config calls = [] @@ -1000,8 +1077,7 @@ class TestWindowsAutostartRepair: return SimpleNamespace(returncode=1) return SimpleNamespace(returncode=0, stdout="", stderr="") - with patch.object(tools_config.sys, "platform", "win32"), \ - patch.object(tools_config.shutil, "which", side_effect=fake_which), \ + with patch.object(tools_config.shutil, "which", side_effect=fake_which), \ patch("subprocess.run", side_effect=fake_run), \ patch.object(tools_config, "_print_warning"), \ patch.object(tools_config, "_print_info"): diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 7de9b4c0b7..ce254bf9f5 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -49,20 +49,28 @@ def _init_git_repo(repo: Path) -> None: +@pytest.mark.windows_only def test_cross_process_init_lock_uses_windows_byte_range_lock(tmp_path, monkeypatch): """Windows must use a real (non-blocking) process lock, not a no-op open. The init lock acquires with LK_NBLCK in a bounded retry loop (#36644) so a wedged holder can never block connect() forever; a clean acquire takes the lock once and releases it once. + + ``windows_only``: ``msvcrt`` does not exist off Windows, so faking + ``_IS_WINDOWS`` on Linux meant injecting a fake ``msvcrt`` module too — + the test then asserted against its own stub rather than the byte-range + locking API. Here the platform is real; only ``msvcrt.locking`` is + instrumented so the call sequence is observable. """ calls: list[tuple[int, int, int]] = [] + import msvcrt as _msvcrt + fake_msvcrt = types.SimpleNamespace( - LK_NBLCK=3, - LK_UNLCK=2, + LK_NBLCK=_msvcrt.LK_NBLCK, + LK_UNLCK=_msvcrt.LK_UNLCK, locking=lambda fd, mode, nbytes: calls.append((fd, mode, nbytes)), ) - monkeypatch.setattr(kb, "_IS_WINDOWS", True) monkeypatch.setitem(sys.modules, "msvcrt", fake_msvcrt) db_path = tmp_path / "kanban.db" diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 1c60866101..21d1d3f4fc 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -74,9 +74,11 @@ def _make_runtime_install( # --------------------------------------------------------------------------- class TestManagedUvPath: + # POSIX arm of the name mapping; the Windows arm (uv.exe) is exercised + # for real by TestEnsureUvWindowsSafe on the Windows lane. + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only: bin/uv name") def test_posix(self, tmp_path): - with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"): + with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path): from hermes_cli.managed_uv import managed_uv_path assert managed_uv_path() == tmp_path / "bin" / "uv" @@ -157,6 +159,8 @@ class TestEnsureUv: assert observed == [repair] +@pytest.mark.skipif(sys.platform == "win32", + reason="POSIX-only: the _UvResult dual contract is not offered on Windows") class TestEnsureUvUpdateBoundary: """``ensure_uv()`` must answer to both the single-value and the legacy ``(path, fresh_bootstrap)`` call conventions — **on POSIX**. @@ -172,15 +176,15 @@ class TestEnsureUvUpdateBoundary: 2-tuple, in both the success and failure cases. The dual contract is intentionally **not** offered on Windows — see - ``TestEnsureUvWindowsSafe`` for why — so these tests pin ``platform.system`` - to a POSIX value. + ``TestEnsureUvWindowsSafe`` for why — so these tests are POSIX-only: the + host's real ``platform.system()`` selects the wrapper branch, nothing is + faked. """ def test_success_usable_as_single_value(self, tmp_path): _make_executable(tmp_path / "bin" / "uv") with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"): + patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")): from hermes_cli.managed_uv import ensure_uv uv_bin = ensure_uv() assert uv_bin == str(tmp_path / "bin" / "uv") @@ -189,8 +193,7 @@ class TestEnsureUvUpdateBoundary: def test_success_unpacks_as_legacy_two_tuple(self, tmp_path): _make_executable(tmp_path / "bin" / "uv") with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"): + patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")): from hermes_cli.managed_uv import ensure_uv uv_bin, fresh = ensure_uv() # old: uv_bin, fresh_bootstrap = ensure_uv() assert uv_bin == str(tmp_path / "bin" / "uv") @@ -199,7 +202,6 @@ class TestEnsureUvUpdateBoundary: def test_failure_unpacks_without_raising(self, tmp_path): with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ patch("hermes_cli.managed_uv._install_uv", side_effect=RuntimeError("network down")): from hermes_cli.managed_uv import ensure_uv uv_bin, fresh = ensure_uv() @@ -232,13 +234,17 @@ class TestEnsureUvWindowsSafe: with pytest.raises(TypeError): subprocess.list2cmdline([_UvResult("C:\\hermes\\uv.exe"), "pip"]) + @pytest.mark.windows_only def test_windows_returns_plain_str_safe_for_subprocess(self, tmp_path): + """``windows_only``: the subject is the real Windows opt-out branch and + ``subprocess.list2cmdline`` — the faked ``platform.system`` only ever + proved the branch existed, not that the field crash was fixed on the + host that reported it.""" import subprocess - # On (mocked) Windows the managed binary is uv.exe. + # On Windows the managed binary is uv.exe. _make_executable(tmp_path / "bin" / "uv.exe") with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Windows"): + patch("hermes_cli.managed_uv.repair_vulnerable_runtime", return_value=_RRR("not-applicable")): from hermes_cli.managed_uv import _UvResult, ensure_uv uv_bin = ensure_uv() assert type(uv_bin) is str and not isinstance(uv_bin, _UvResult) @@ -366,14 +372,15 @@ class TestManagedPythonStore: assert base_env["PYTHONHOME"] == "/poison/home" +@pytest.mark.skipif(sys.platform == "win32", + reason="POSIX-only: fixtures build the bin/ (not Scripts/) venv layout") class TestRuntimeRepair: def test_safe_runtime_is_a_noop(self, tmp_path): from hermes_cli.managed_uv import repair_vulnerable_runtime root, live, sentinel = _make_runtime_install(tmp_path) current = _runtime_info(live / "bin" / "python", (3, 53, 1)) - with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ - patch( + with patch( "hermes_cli.managed_uv.probe_sqlite_runtime", return_value=current, ), \ @@ -410,8 +417,7 @@ class TestRuntimeRepair: patch( "hermes_cli.managed_uv._smoke_candidate_venv", return_value=(True, "", None), - ), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"): + ): candidate = _stage_candidate_venv( "uv", project_root=root, @@ -446,8 +452,7 @@ class TestRuntimeRepair: candidate_python.write_text("candidate interpreter", encoding="utf-8") fixed = _runtime_info(candidate_python, (3, 53, 1)) - with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ - patch( + with patch( "hermes_cli.managed_uv.probe_sqlite_runtime", side_effect=[current, current], ), \ @@ -491,8 +496,7 @@ class TestRuntimeRepair: (fresh_backup / "bin").mkdir(parents=True) current = _runtime_info(live / "bin" / "python", (3, 53, 1)) - with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ - patch( + with patch( "hermes_cli.managed_uv.probe_sqlite_runtime", return_value=current, ): @@ -521,8 +525,7 @@ class TestRuntimeRepair: "candidate venv interpreter", encoding="utf-8" ) - with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ - patch( + with patch( "hermes_cli.managed_uv.probe_sqlite_runtime", side_effect=[current, current], ), \ @@ -876,33 +879,36 @@ class TestRefreshManagedUvCatalog: def test_version_change_reports_true(self, tmp_path): import hermes_cli.managed_uv as managed_uv - uv_path = tmp_path / "bin" / "uv" - _make_executable(uv_path) versions = iter(["uv 0.1.0", "uv 0.2.0"]) with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ patch("hermes_cli.managed_uv._install_uv"), \ patch( "hermes_cli.managed_uv._uv_version_string", side_effect=lambda _uv: next(versions), ): + # Host-native path: the refresh only acts on the managed binary, + # so the fixture must live at the real host's managed_uv_path() + # (uv on POSIX, uv.exe on Windows) — no platform fake needed. + uv_path = managed_uv.managed_uv_path() + _make_executable(uv_path) assert managed_uv._refresh_managed_uv_catalog(str(uv_path)) is True def test_installer_failure_reports_false(self, tmp_path): import hermes_cli.managed_uv as managed_uv - uv_path = tmp_path / "bin" / "uv" - _make_executable(uv_path) with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \ - patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ patch( "hermes_cli.managed_uv._install_uv", side_effect=RuntimeError("network down"), ): + uv_path = managed_uv.managed_uv_path() + _make_executable(uv_path) assert managed_uv._refresh_managed_uv_catalog(str(uv_path)) is False +@pytest.mark.skipif(sys.platform == "win32", + reason="POSIX-only: fixtures build the bin/ (not Scripts/) venv layout") class TestRepairRetriesAfterUvRefresh: def _run_repair(self, tmp_path, *, refresh_result, second_attempt): """Drive repair with the first provisioning attempt failing.""" @@ -919,8 +925,7 @@ class TestRepairRetriesAfterUvRefresh: return None return second_attempt(project_root) - with patch("hermes_cli.managed_uv.platform.system", return_value="Linux"), \ - patch( + with patch( "hermes_cli.managed_uv.probe_sqlite_runtime", return_value=current, ), \ @@ -1054,11 +1059,12 @@ class TestVenvPythonUpdateBoundary: # exactly as on an install that booted the pre-upgrade checkout. The # file on disk is the current one, so a reload recovers the real helper. monkeypatch.delattr(hermes_constants, "venv_python_path", raising=False) - monkeypatch.setattr("platform.system", lambda: "Linux") - assert _venv_python(Path("/opt/hermes/venv")) == Path( - "/opt/hermes/venv/bin/python" - ) + # Host-native: the subject is the reload-recovery seam, not the + # bin/Scripts mapping — assert whatever layout the real host resolves. + expected = Path("/opt/hermes/venv/Scripts/python.exe") \ + if sys.platform == "win32" else Path("/opt/hermes/venv/bin/python") + assert _venv_python(Path("/opt/hermes/venv")) == expected def test_recovery_uses_the_shared_helper_not_a_second_copy(self, monkeypatch): """The reload must resolve through hermes_constants, not open-code it. @@ -1071,7 +1077,6 @@ class TestVenvPythonUpdateBoundary: from hermes_cli.managed_uv import _venv_python monkeypatch.delattr(hermes_constants, "venv_python_path", raising=False) - monkeypatch.setattr("platform.system", lambda: "Linux") sentinel = Path("/sentinel/from/shared/helper") real_reload = __import__("importlib").reload @@ -1094,9 +1099,8 @@ class TestVenvPythonUpdateBoundary: raise AssertionError("reload must not run when the import succeeds") monkeypatch.setattr("importlib.reload", _no_reload) - monkeypatch.setattr("platform.system", lambda: "Linux") - assert _venv_python(Path("/opt/hermes/venv")) == Path( - "/opt/hermes/venv/bin/python" - ) + expected = Path("/opt/hermes/venv/Scripts/python.exe") \ + if sys.platform == "win32" else Path("/opt/hermes/venv/bin/python") + assert _venv_python(Path("/opt/hermes/venv")) == expected diff --git a/tests/hermes_cli/test_mem_trim.py b/tests/hermes_cli/test_mem_trim.py index 3c94f23f07..575a4a8dcd 100644 --- a/tests/hermes_cli/test_mem_trim.py +++ b/tests/hermes_cli/test_mem_trim.py @@ -55,7 +55,9 @@ def test_default_config_declares_memory_trim_controls(): def test_collect_memory_snapshot_parses_linux_proc_status(monkeypatch): - monkeypatch.setattr(mem_trim.sys, "platform", "linux") + # No ``sys.platform`` pin: the only platform check lives inside + # ``_read_proc_status``, which is replaced below — the subject here is + # the /proc/self/status parser, which is host-independent. monkeypatch.setattr( mem_trim, "_read_proc_status", diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index cb5e81d548..576396b49c 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -403,8 +403,8 @@ class TestAliasCollision: - def test_windows_checks_bat_extension(self, profile_env, monkeypatch): - monkeypatch.setattr("sys.platform", "win32") + @pytest.mark.windows_only + def test_windows_checks_bat_extension(self, profile_env): wrapper_dir = profile_env / ".local" / "bin" wrapper_dir.mkdir(parents=True, exist_ok=True) bat_path = wrapper_dir / "mybot.bat" @@ -433,7 +433,6 @@ class TestWrapperScript: """Tests for create_wrapper_script() and remove_wrapper_script().""" def test_creates_sh_on_posix(self, profile_env, monkeypatch): - monkeypatch.setattr("sys.platform", "darwin") monkeypatch.setattr("hermes_cli.profiles.shutil.which", lambda name: "/opt/hermes/bin/hermes") from hermes_cli.profiles import create_wrapper_script wrapper = create_wrapper_script("mybot") @@ -444,8 +443,8 @@ class TestWrapperScript: assert "exec /opt/hermes/bin/hermes -p mybot" in content - def test_remove_finds_bat_on_windows(self, profile_env, monkeypatch): - monkeypatch.setattr("sys.platform", "win32") + @pytest.mark.windows_only + def test_remove_finds_bat_on_windows(self, profile_env): from hermes_cli.profiles import create_wrapper_script, remove_wrapper_script wrapper = create_wrapper_script("mybot") assert wrapper is not None @@ -492,16 +491,14 @@ class TestWrapperScriptSecurity: class TestFindAliasForProfile: """Tests for find_alias_for_profile() and alias display in list/show.""" - def test_profile_named_alias(self, profile_env, monkeypatch): - monkeypatch.setattr("sys.platform", "darwin") + def test_profile_named_alias(self, profile_env): from hermes_cli.profiles import create_wrapper_script, find_alias_for_profile create_wrapper_script("steve") assert find_alias_for_profile("steve") == "steve" - def test_ignores_unrelated_files(self, profile_env, monkeypatch): + def test_ignores_unrelated_files(self, profile_env): # ~/.local/bin commonly holds unrelated binaries; they must not match. - monkeypatch.setattr("sys.platform", "darwin") from hermes_cli.profiles import _get_wrapper_dir, find_alias_for_profile wrapper_dir = _get_wrapper_dir() wrapper_dir.mkdir(parents=True, exist_ok=True) @@ -509,8 +506,7 @@ class TestFindAliasForProfile: assert find_alias_for_profile("steve") is None - def test_list_profiles_surfaces_custom_alias(self, profile_env, monkeypatch): - monkeypatch.setattr("sys.platform", "darwin") + def test_list_profiles_surfaces_custom_alias(self, profile_env): from hermes_cli.profiles import ( create_profile, create_wrapper_script, diff --git a/tests/hermes_cli/test_relaunch.py b/tests/hermes_cli/test_relaunch.py index f58b6dea81..ffb611950e 100644 --- a/tests/hermes_cli/test_relaunch.py +++ b/tests/hermes_cli/test_relaunch.py @@ -114,13 +114,26 @@ class TestRelaunch: assert calls == [("/usr/bin/hermes", ["/usr/bin/hermes", "--resume", "abc"])] + @pytest.mark.windows_only def test_windows_uses_subprocess_not_execvp(self, monkeypatch): """On Windows, os.execvp raises OSError "Exec format error" when the target is a .cmd shim or console-script wrapper (both common for hermes). relaunch() must detect win32 and use subprocess.run + - sys.exit instead.""" - monkeypatch.setattr(relaunch_mod.sys, "platform", "win32") + sys.exit instead. + + ``windows_only``: the bug is that ``os.execvp`` cannot exec a Windows + console-script shim. On Linux ``execvp`` works fine, so a patched + platform only re-asserted the branch we wrote, never the constraint + that motivated it. + """ monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: r"C:\Users\test\hermes.exe") + # Pin sys.argv: relaunch() preserves inherited flags from the LIVE + # argv, so under pytest it happily inherited the runner's own + # "-m 'windows_only and not integration'" and the assertion below saw + # them in the child argv. Nothing to do with Windows — it only showed + # up here because this is the first lane that actually executes the + # test, and -m is how that lane selects it. + monkeypatch.setattr(relaunch_mod.sys, "argv", [r"C:\Users\test\hermes.exe"]) import subprocess as _subprocess @@ -150,9 +163,9 @@ class TestRelaunch: assert execvp_calls == [] assert captured_argv == [[r"C:\Users\test\hermes.exe", "chat"]] + @pytest.mark.windows_only def test_windows_propagates_child_exit_code(self, monkeypatch): """A non-zero exit from the child should flow through to sys.exit.""" - monkeypatch.setattr(relaunch_mod.sys, "platform", "win32") monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: r"C:\hermes.exe") import subprocess as _subprocess @@ -176,8 +189,13 @@ class TestResolveHermesBinWindowsPyGuard: PATHEXT includes .py when the Python launcher is installed — but subprocess.run can't actually exec a .py directly, so the relaunch would fail with the cryptic "%1 is not a valid Win32 application" error. + + The Windows cases are ``windows_only``: the PATHEXT-driven ``os.access`` + result the guard defends against simply does not occur on POSIX, so a + faked ``sys.platform`` could never reproduce the hazard. """ + @pytest.mark.windows_only def test_windows_rejects_py_argv0_falls_through_to_path(self, monkeypatch, tmp_path): """On Windows, if sys.argv[0] is a .py file, we must skip the argv[0] fast-path and fall through to PATH / python -m.""" @@ -185,7 +203,6 @@ class TestResolveHermesBinWindowsPyGuard: script = tmp_path / "main.py" script.write_text("# stub") - monkeypatch.setattr(relaunch_mod.sys, "platform", "win32") monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) # Force PATH lookup to return a hermes.exe so the test doesn't # exercise the None-fallback path (that's a separate test). @@ -198,18 +215,18 @@ class TestResolveHermesBinWindowsPyGuard: # Must NOT be the .py — must be the hermes.exe PATH entry. assert bin_path == r"C:\venv\Scripts\hermes.exe" + @pytest.mark.linux_only def test_posix_still_accepts_py_argv0(self, monkeypatch, tmp_path): """POSIX behaviour unchanged: argv[0] pointing at an executable script (including .py with a shebang + chmod +x) is fine to return because POSIX exec can route through the shebang line.""" - if sys.platform == "win32": - pytest.skip("POSIX semantics") script = tmp_path / "hermes" script.write_text("#!/usr/bin/env python3\n") script.chmod(0o755) monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) assert relaunch_mod.resolve_hermes_bin() == str(script) + @pytest.mark.windows_only def test_windows_py_argv0_with_no_hermes_on_path_returns_none(self, monkeypatch, tmp_path): """Bulletproof fallback: if argv0 is .py on Windows AND hermes.exe isn't on PATH, return None so the caller falls back to @@ -217,7 +234,6 @@ class TestResolveHermesBinWindowsPyGuard: script = tmp_path / "main.py" script.write_text("# stub") - monkeypatch.setattr(relaunch_mod.sys, "platform", "win32") monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"]) monkeypatch.setattr(relaunch_mod.shutil, "which", lambda name: None) diff --git a/tests/hermes_cli/test_update_gateway_launcher_refresh.py b/tests/hermes_cli/test_update_gateway_launcher_refresh.py index ae17efb2b6..be92a79a3d 100644 --- a/tests/hermes_cli/test_update_gateway_launcher_refresh.py +++ b/tests/hermes_cli/test_update_gateway_launcher_refresh.py @@ -12,8 +12,10 @@ forever" gap: update`` regenerates the installed Scheduled Task / Startup launcher scripts instead of leaving install-time artifacts stale forever. -Windows-specific paths are exercised via ``_is_windows`` patching so they -run on any host (same approach as test_update_venv_health). +``_resolve_detached_python`` is a pure path helper and runs on any host. +``windowless_gateway_restart_spec`` returns its argv unchanged off Windows, +so the test that exercises the rewrite is ``windows_only`` rather than run +against a faked ``sys.platform``. """ from __future__ import annotations @@ -21,6 +23,8 @@ from __future__ import annotations from pathlib import Path from unittest import mock +import pytest + import hermes_cli.gateway_windows as gateway_windows import hermes_cli.main as cli_main @@ -53,21 +57,20 @@ def test_resolve_detached_python_swaps_legacy_pythonw_for_console_sibling(tmp_pa +@pytest.mark.windows_only def test_restart_spec_normalizes_legacy_pythonw_argv(tmp_path): """A pre-rework Scheduled Task argv snapshot (leading pythonw.exe) must be respawned through the console python + hidden-console launch, with every - argument after the interpreter preserved verbatim.""" + argument after the interpreter preserved verbatim. + + ``windows_only``: ``windowless_gateway_restart_spec`` returns the argv + untouched off Windows, so the fake was the only thing making the rewrite + (and its ``Scripts/``-layout venv derivation) run at all. + """ pythonw, python = _make_venv(tmp_path, with_console_python=True) - # Pre-import so the function's lazy imports resolve from sys.modules - # instead of re-importing under the win32 platform patch (see the - # TestWindowlessGatewayRestartSpec comment in - # tests/tools/test_windows_native_support.py). - import hermes_cli.config # noqa: F401 - import hermes_cli.gateway # noqa: F401 - argv = [str(pythonw), "-m", "hermes_cli.main", "gateway", "run"] - with mock.patch.object(gateway_windows.sys, "platform", "win32"), mock.patch.object( + with mock.patch.object( gateway_windows, "_stable_gateway_working_dir", return_value=str(tmp_path) ), mock.patch("hermes_cli.config.get_hermes_home", return_value=str(tmp_path)): new_argv, cwd, env = gateway_windows.windowless_gateway_restart_spec(list(argv)) diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index e61fe8acbb..547b4f887a 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -196,8 +196,12 @@ class TestKillStaleDashboardPosix: class TestKillStaleDashboardWindows: """Kill path on Windows: taskkill /F.""" - def test_taskkill_invoked_for_each_pid(self, monkeypatch, capsys): - monkeypatch.setattr(sys, "platform", "win32") + @pytest.mark.windows_only + def test_taskkill_invoked_for_each_pid(self, capsys): + """``windows_only``: ``taskkill.exe`` only exists on Windows, and the + faked platform also silently skipped the POSIX-only cgroup/argv + snapshot the real Windows path must not take. + """ def fake_run(args, *a, **kw): # taskkill returns 0 on success @@ -249,11 +253,16 @@ class TestWindowsWmicEncoding: `hermes update` on non-UTF-8 system locales (e.g. cp936 on zh-CN). """ - def test_wmic_invoked_with_utf8_ignore_errors(self, monkeypatch): + @pytest.mark.windows_only + def test_wmic_invoked_with_utf8_ignore_errors(self): """The wmic subprocess.run call must pass encoding='utf-8' and errors='ignore' so the subprocess reader thread cannot raise - UnicodeDecodeError on non-UTF-8 wmic output.""" - monkeypatch.setattr(sys, "platform", "win32") + UnicodeDecodeError on non-UTF-8 wmic output. + + ``windows_only``: the branch also imports ``windows_hide_flags()`` and + the crash it guards is a real cp936/wmic decode — neither reproducible + with a patched ``sys.platform`` on Linux. + """ with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock( returncode=0, @@ -421,7 +430,11 @@ class TestCmdlineCapture: assert argv == ["hermes", "serve", "--port", "8300"] - def test_returns_none_on_windows(self, monkeypatch): + @pytest.mark.windows_only + def test_returns_none_on_windows(self): + """``windows_only``: the contract is "no graceful-argv capture on a + real Windows host" — asserting it against a faked platform only + restated the branch condition. + """ live = self._live() - monkeypatch.setattr(live.sys, "platform", "win32") assert live._dashboard_cmdline_for_pid(123) is None diff --git a/tests/hermes_cli/test_verify_core_dependencies.py b/tests/hermes_cli/test_verify_core_dependencies.py index 51827a854e..0404d2bb0e 100644 --- a/tests/hermes_cli/test_verify_core_dependencies.py +++ b/tests/hermes_cli/test_verify_core_dependencies.py @@ -17,6 +17,7 @@ The verification step: from __future__ import annotations import subprocess +import sys import textwrap from pathlib import Path from unittest.mock import MagicMock, patch @@ -40,6 +41,7 @@ def temp_pyproject(tmp_path, monkeypatch): "pathspec==1.1.1", "pydantic==2.13.4", "ptyprocess>=0.7.0,<1; sys_platform != 'win32'", + "tzdata>=2024.1; sys_platform == 'win32'", ] """)) import hermes_cli.main as main_mod @@ -63,10 +65,17 @@ class TestVerifyCoreDependencies: def test_skips_deps_excluded_by_environment_markers(self, temp_pyproject, fake_venv_python): - """``ptyprocess ; sys_platform != 'win32'`` should NOT be reported as - missing on Windows. Without marker evaluation, the verification step - would false-positive on every cross-platform exclusion and chase its - tail forever trying to install something that can't apply here.""" + """A dep whose ``sys_platform`` marker excludes THIS host must not be + probed (and so never reported missing). Without marker evaluation the + verification step would false-positive on every cross-platform + exclusion and chase its tail installing something inapplicable here. + + Deliberately host-invariant rather than ``windows_only``: the subject + is ``packaging``'s marker *evaluation*, not any OS facility. The + pyproject fixture declares one dep gated to non-Windows and one gated + to Windows, so exactly one of the pair is filtered on any host — the + old ``patch("sys.platform", "win32")`` bought nothing but a fake host. + """ py, venv_root = fake_venv_python env = {"VIRTUAL_ENV": str(venv_root)} captured_argv: list[list[str]] = [] @@ -75,12 +84,9 @@ class TestVerifyCoreDependencies: captured_argv.append(list(cmd)) return MagicMock(returncode=0, stdout="", stderr="") - # Force sys.platform to look like Windows so the marker filters - # ptyprocess out. (We need the actual marker.evaluate() to see win32.) with patch("hermes_cli.main._resolve_install_target_python", return_value=py), \ patch("hermes_cli.main.subprocess.run", side_effect=fake_subprocess_run), \ - patch("hermes_cli.main._run_install_with_heartbeat"), \ - patch("sys.platform", "win32"): + patch("hermes_cli.main._run_install_with_heartbeat"): from hermes_cli.main import _verify_core_dependencies_installed _verify_core_dependencies_installed(["uv", "pip"], env=env) @@ -91,10 +97,16 @@ class TestVerifyCoreDependencies: None, ) assert probe is not None, "verification probe should have run" - # The dep names are tacked on after the -c script. - assert "ptyprocess" not in probe, ( - "ptyprocess is gated by sys_platform != 'win32' and must be filtered " - f"out on Windows; full probe argv was: {probe}" + # The dep names are tacked on after the -c script. Exactly one of the + # marker-gated pair applies to this host; the other must be filtered. + on_windows = sys.platform == "win32" + assert ("ptyprocess" in probe) is not on_windows, ( + "ptyprocess is gated by sys_platform != 'win32', so it must be " + f"probed off Windows and filtered on it; probe argv was: {probe}" + ) + assert ("tzdata" in probe) is on_windows, ( + "tzdata is gated by sys_platform == 'win32', so it must be probed " + f"on Windows and filtered elsewhere; probe argv was: {probe}" ) assert "pathspec" in probe, "core deps without markers must be checked" diff --git a/tests/hermes_cli/test_win_pty_bridge.py b/tests/hermes_cli/test_win_pty_bridge.py index e9a18c82f0..fff0da000d 100644 --- a/tests/hermes_cli/test_win_pty_bridge.py +++ b/tests/hermes_cli/test_win_pty_bridge.py @@ -25,10 +25,11 @@ import pytest # must never raise, otherwise the web_server import branch becomes a trap. from hermes_cli.win_pty_bridge import PtyUnavailableError, WinPtyBridge -windows_only = pytest.mark.skipif( - not sys.platform.startswith("win"), - reason="ConPTY bridge is Windows-only", -) +# ``pytest.mark.windows_only`` rather than a local ``skipif`` alias: the +# dedicated Windows CI job selects its files by grepping for the marker name +# and then filters with ``-m windows_only``. A file-local skipif alias matched +# the grep (so the file was listed) but carried no marker, so every test below +# was deselected — the lane looked like it covered ConPTY and ran none of it. def _read_until(bridge: WinPtyBridge, needle: bytes, timeout: float = 10.0) -> bytes: @@ -80,7 +81,7 @@ class TestWinPtyBridgeUnavailable: # --------------------------------------------------------------------------- -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeSpawn: def test_spawn_returns_bridge_with_pid(self): @@ -97,7 +98,7 @@ class TestWinPtyBridgeSpawn: WinPtyBridge.spawn([bogus]) -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeIO: def test_write_sends_to_child_stdin(self): @@ -136,7 +137,7 @@ class TestWinPtyBridgeIO: bridge.close() -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeResize: def test_resize_does_not_raise_on_live_child(self): # ConPTY exposes no ioctl-equivalent for reading the child's current @@ -164,7 +165,7 @@ class TestWinPtyBridgeResize: bridge.resize(cols=100, rows=40) -@windows_only +@pytest.mark.windows_only class TestClampDimension: """The clamp helper is the load-bearing piece — the dashboard sends untrusted winsize values straight from xterm.js, and pywinpty's @@ -186,7 +187,7 @@ class TestClampDimension: assert _clamp(float("inf"), _MAX_COLS) == 1 # type: ignore[arg-type] -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeClose: def test_close_terminates_long_running_child(self): @@ -208,7 +209,7 @@ class TestWinPtyBridgeClose: ) -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeEnv: def test_cwd_is_respected(self, tmp_path): bridge = WinPtyBridge.spawn( diff --git a/tests/run_agent/test_tool_batch_segmentation.py b/tests/run_agent/test_tool_batch_segmentation.py index 3a0fa30e13..c4bb16ec11 100644 --- a/tests/run_agent/test_tool_batch_segmentation.py +++ b/tests/run_agent/test_tool_batch_segmentation.py @@ -12,7 +12,6 @@ concurrently, barrier calls sequentially — while preserving: """ import json -import sys import threading import time import uuid @@ -711,10 +710,10 @@ class TestPathCanonicalization: ) - @pytest.mark.skipif( - sys.platform != "win32", - reason="normcase() case-folding only matters on Windows", - ) + # ``windows_only`` rather than ``skipif(sys.platform != "win32")``: the + # Windows CI job greps for the marker to decide which files to import, so + # a bare skipif leaves this running on no host at all. + @pytest.mark.windows_only def test_case_insensitive_paths_overlap_windows(self, tmp_path): """On Windows, FILE.txt and file.txt are the same file — they must be detected as overlapping after normcase() canonicalisation.""" diff --git a/tests/test_hermes_bootstrap.py b/tests/test_hermes_bootstrap.py index 61b0423269..b95b50d9ad 100644 --- a/tests/test_hermes_bootstrap.py +++ b/tests/test_hermes_bootstrap.py @@ -46,10 +46,7 @@ def _fresh_import(): class TestWindowsBehavior: """Windows: the bootstrap does its job.""" - @pytest.mark.skipif( - sys.platform != "win32", - reason="Windows-specific behavior", - ) + @pytest.mark.windows_only def test_env_vars_set_on_windows(self, monkeypatch): # Clear any pre-existing values and re-run bootstrap. monkeypatch.delenv("PYTHONUTF8", raising=False) @@ -60,10 +57,7 @@ class TestWindowsBehavior: assert os.environ.get("PYTHONIOENCODING") == "utf-8" assert hb._bootstrap_applied is True - @pytest.mark.skipif( - sys.platform != "win32", - reason="Windows-specific behavior", - ) + @pytest.mark.windows_only def test_stdout_reconfigured_to_utf8_on_windows(self): # The live process's stdout should now be UTF-8 (the Hermes CLI # runs on Windows with a pytest console that's cp1252 by default). @@ -83,10 +77,7 @@ class TestWindowsBehavior: "reconfigured it to UTF-8" ) - @pytest.mark.skipif( - sys.platform != "win32", - reason="Windows-specific behavior", - ) + @pytest.mark.windows_only def test_child_process_inherits_utf8_mode(self): """A subprocess spawned from this process should inherit PYTHONUTF8=1 and be able to print non-ASCII to stdout.""" @@ -119,10 +110,7 @@ class TestUserOptOut: """If the user has explicitly set PYTHONUTF8 / PYTHONIOENCODING in their environment, we respect that (setdefault, not overwrite).""" - @pytest.mark.skipif( - sys.platform != "win32", - reason="Only meaningful on Windows where we'd otherwise set these", - ) + @pytest.mark.windows_only def test_user_pythonutf8_zero_preserved(self, monkeypatch): monkeypatch.setenv("PYTHONUTF8", "0") _fresh_import() @@ -137,12 +125,13 @@ class TestPosixNoOp: stdio. The goal is that Linux/macOS behave identically before and after this module is imported.""" - def test_noop_on_fake_posix(self, monkeypatch): + def test_noop_on_posix_host(self, monkeypatch): """Even when imported, the bootstrap function must return False - and leave env untouched when _IS_WINDOWS is False.""" + and leave env untouched on a POSIX host (``_IS_WINDOWS`` is + genuinely False here — nothing is faked).""" hb = _fresh_import() - # Reset + fake POSIX - hb._IS_WINDOWS = False + # Reset the idempotence latch so the call below is not a no-op for + # the wrong reason. hb._bootstrap_applied = False monkeypatch.delenv("PYTHONUTF8", raising=False) monkeypatch.delenv("PYTHONIOENCODING", raising=False) @@ -174,11 +163,17 @@ class TestStdioReconfigureErrorHandling: don't support reconfigure (e.g. by a test harness), the bootstrap must degrade gracefully rather than crash.""" + @pytest.mark.windows_only def test_non_reconfigurable_stream_does_not_crash(self, monkeypatch): """Replace sys.stdout with a BytesIO (no reconfigure method), - then run the bootstrap and make sure it doesn't raise.""" + then run the bootstrap and make sure it doesn't raise. + + ``windows_only``: forcing ``_IS_WINDOWS = True`` on Linux was the only + thing that made the reconfigure block reachable — off Windows the + bootstrap returns before touching stdio, so the test proved nothing + about the guard it names. + """ hb = _fresh_import() - hb._IS_WINDOWS = True hb._bootstrap_applied = False fake = io.BytesIO() # no .reconfigure attribute @@ -341,20 +336,22 @@ class TestHardenImportPath: class TestSuppressPlatformVerConsole: """suppress_platform_ver_console: stub applied on Windows, no-op on POSIX.""" - def test_noop_on_posix(self, monkeypatch): + def test_noop_on_posix(self): import platform hb = _fresh_import() original = getattr(platform, "_syscmd_ver", None) - monkeypatch.setattr(hb, "_IS_WINDOWS", False) hb.suppress_platform_ver_console() assert getattr(platform, "_syscmd_ver", None) is original - def test_stub_applied_when_windows(self, monkeypatch): + @pytest.mark.windows_only + def test_stub_applied_when_windows(self): + # Faking _IS_WINDOWS on Linux asserted only that the stub was + # installed; the reason it exists — ``platform.win32_ver()`` shelling + # out ``cmd /c ver`` — has no counterpart off Windows. import platform hb = _fresh_import() original = getattr(platform, "_syscmd_ver", None) try: - monkeypatch.setattr(hb, "_IS_WINDOWS", True) hb.suppress_platform_ver_console() stubbed = platform._syscmd_ver assert stubbed is not original diff --git a/tests/test_hermes_constants.py b/tests/test_hermes_constants.py index a3164c2b2e..30ef95cc1f 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -54,13 +54,13 @@ class TestGetDefaultHermesRoot: monkeypatch.setenv("HERMES_HOME", str(profile)) assert get_default_hermes_root() == docker_root + @pytest.mark.windows_only def test_no_hermes_home_returns_localappdata_root_on_windows(self, tmp_path, monkeypatch): """Native Windows falls back to %LOCALAPPDATA%\\hermes, not ~/.hermes.""" local_appdata = tmp_path / "LocalAppData" monkeypatch.delenv("HERMES_HOME", raising=False) monkeypatch.setenv("LOCALAPPDATA", str(local_appdata)) monkeypatch.setattr(Path, "home", lambda: tmp_path / "Home") - monkeypatch.setattr(hermes_constants.sys, "platform", "win32") assert get_default_hermes_root() == local_appdata / "hermes" @@ -69,13 +69,13 @@ class TestGetDefaultHermesRoot: class TestGetHermesHome: """Tests for get_hermes_home() platform-aware fallback.""" + @pytest.mark.windows_only def test_windows_fallback_uses_localappdata(self, tmp_path, monkeypatch): """When HERMES_HOME is unset on Windows, use %LOCALAPPDATA%\\hermes.""" local_appdata = tmp_path / "LocalAppData" monkeypatch.delenv("HERMES_HOME", raising=False) monkeypatch.setenv("LOCALAPPDATA", str(local_appdata)) monkeypatch.setattr(Path, "home", lambda: tmp_path / "Home") - monkeypatch.setattr(hermes_constants.sys, "platform", "win32") monkeypatch.setattr(hermes_constants, "_profile_fallback_warned", False) assert get_hermes_home() == local_appdata / "hermes" @@ -98,24 +98,24 @@ class TestGetProcessHermesHome: class TestHermesManagedNode: + @pytest.mark.windows_only def test_windows_node_dir_prefers_portable_root(self, tmp_path, monkeypatch): home = tmp_path / "hermes" node_dir = home / "node" bin_dir = node_dir / "bin" node_dir.mkdir(parents=True) bin_dir.mkdir() - monkeypatch.setattr(hermes_constants.sys, "platform", "win32") monkeypatch.setenv("HERMES_HOME", str(home)) assert iter_hermes_node_dirs() == [node_dir, bin_dir] + @pytest.mark.windows_only def test_windows_finds_npm_cmd_before_path(self, tmp_path, monkeypatch): home = tmp_path / "hermes" node_dir = home / "node" node_dir.mkdir(parents=True) npm_cmd = node_dir / "npm.cmd" npm_cmd.write_text("@echo off\n") - monkeypatch.setattr(hermes_constants.sys, "platform", "win32") monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setattr(hermes_constants, "node_tool_runnable", lambda path: True) @@ -123,6 +123,7 @@ class TestHermesManagedNode: + @pytest.mark.windows_only def test_windows_skips_broken_managed_npm_without_path_fallback(self, tmp_path, monkeypatch): home = tmp_path / "hermes" managed_npm = home / "node" / "npm.cmd" @@ -132,7 +133,6 @@ class TestHermesManagedNode: bin_dir.mkdir() path_npm = bin_dir / "npm.cmd" path_npm.write_text("@echo off\n") - monkeypatch.setattr(hermes_constants.sys, "platform", "win32") monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setenv("PATH", str(bin_dir)) monkeypatch.setattr(hermes_constants, "_managed_node_heal_attempted", False) diff --git a/tests/test_hermes_logging.py b/tests/test_hermes_logging.py index 400b1cbb89..c0a9b03e9b 100644 --- a/tests/test_hermes_logging.py +++ b/tests/test_hermes_logging.py @@ -398,20 +398,27 @@ class TestWindowsConcurrentLogLockTimeout: logger.addHandler(handler) return logger, handler + @pytest.mark.windows_only def test_helper_only_matches_windows_concurrent_lock_timeout(self): - with patch.object(hermes_logging.sys, "platform", "win32"): - assert hermes_logging._is_windows_concurrent_log_lock_timeout( - RuntimeError("Cannot acquire lock after 20 attempts") - ) - assert not hermes_logging._is_windows_concurrent_log_lock_timeout( - RuntimeError("some other logging failure") - ) + # Windows-only: concurrent-log-handler (and therefore its cross-process + # lock timeout) is only installed on Windows — faking sys.platform + # exercised the string check without the handler that raises it. + assert hermes_logging._is_windows_concurrent_log_lock_timeout( + RuntimeError("Cannot acquire lock after 20 attempts") + ) + assert not hermes_logging._is_windows_concurrent_log_lock_timeout( + RuntimeError("some other logging failure") + ) - with patch.object(hermes_logging.sys, "platform", "linux"): - assert not hermes_logging._is_windows_concurrent_log_lock_timeout( - RuntimeError("Cannot acquire lock after 20 attempts") - ) + @pytest.mark.linux_only + def test_helper_never_matches_off_windows(self): + # On POSIX the suppression must stay inert: stdlib RotatingFileHandler + # is in use, so this RuntimeError text is never a CLH lock timeout. + assert not hermes_logging._is_windows_concurrent_log_lock_timeout( + RuntimeError("Cannot acquire lock after 20 attempts") + ) + @pytest.mark.windows_only def test_lock_timeout_routed_to_handle_error_is_suppressed(self, tmp_path, capsys): """Mirror CLH's real control flow. @@ -420,17 +427,20 @@ class TestWindowsConcurrentLogLockTimeout: RuntimeError raised in ``_do_lock()`` is caught *inside* CLH and routed to ``handleError`` with the exception live in ``sys.exc_info()``. We invoke ``handleError`` the same way CLH would and assert no traceback - reaches stderr (the slash-worker surface).""" + reaches stderr (the slash-worker surface). + + Windows-only: the suppression is keyed on the real host, and only on + Windows is the base handler CLH at all — the fake platform gave us the + branch without the handler that raises.""" logger, handler = self._make_logger_and_handler(tmp_path / "agent.log") record = logger.makeRecord( logger.name, logging.INFO, __file__, 0, "force rollover", (), None, ) try: - with patch.object(hermes_logging.sys, "platform", "win32"): - try: - raise RuntimeError("Cannot acquire lock after 20 attempts") - except RuntimeError: - handler.handleError(record) + try: + raise RuntimeError("Cannot acquire lock after 20 attempts") + except RuntimeError: + handler.handleError(record) captured = capsys.readouterr() assert "Cannot acquire lock after 20 attempts" not in captured.err diff --git a/tests/test_hermes_state_compression_locks.py b/tests/test_hermes_state_compression_locks.py index 43f68d70c8..710297a7df 100644 --- a/tests/test_hermes_state_compression_locks.py +++ b/tests/test_hermes_state_compression_locks.py @@ -91,11 +91,10 @@ def test_non_expired_lock_is_held(db: SessionDB) -> None: def test_non_expired_lock_from_dead_pid_is_reclaimed( db: SessionDB, monkeypatch: pytest.MonkeyPatch ) -> None: - # PID probing is POSIX-only by design (see - # test_windows_uses_ttl_only_without_pid_probe). Pin the platform so this - # exercises the probe branch on Windows dev machines too, instead of - # silently asserting the nt early-return. - monkeypatch.setattr(hermes_state.os, "name", "posix") + # No ``os.name`` pin: the probe below injects a fake ``psutil``, and + # ``_process_is_gone`` consults psutil *before* its POSIX/nt split — the + # nt early-return is unreachable here on any host, so faking the platform + # bought nothing. dead_holder = "pid=424242:tid=1:agent=abc:nonce=deadbeef" assert db.try_acquire_compression_lock( "sess1", dead_holder, ttl_seconds=300 diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 1dafba2dc5..2ca998519f 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -13010,7 +13010,10 @@ def test_browser_manage_connect_defaults_to_loopback(monkeypatch): def test_browser_manage_connect_default_local_reports_launch_hint(monkeypatch): monkeypatch.delenv("BROWSER_CDP_URL", raising=False) - monkeypatch.setattr("platform.system", lambda: "Linux") + # No ``platform.system`` fake: the resolved system string only flows into + # ``launch_chrome_debug`` / ``manual_chrome_debug_command`` / + # ``get_chrome_debug_candidates``, all of which are mocked below — the + # host's real value never reaches unmocked code. emitted: list[tuple[str, dict]] = [] monkeypatch.setattr( server, diff --git a/tests/test_web_server.py b/tests/test_web_server.py index e14d3151d9..55534e8b84 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -7,6 +7,7 @@ Config + Server + asyncio.run to capture kwargs without starting an event loop. import asyncio import contextlib +import pytest import uvicorn from hermes_cli import web_server @@ -146,6 +147,7 @@ def test_start_server_enables_ws_ping_for_half_open_detection(monkeypatch): assert captured["ws_ping_timeout"] >= captured["ws_ping_interval"] +@pytest.mark.windows_only def test_start_server_runs_on_uvicorns_loop_factory(monkeypatch): """The dashboard/desktop backend must serve uvicorn on the loop *uvicorn* selects, not the interpreter default. @@ -161,13 +163,12 @@ def test_start_server_runs_on_uvicorns_loop_factory(monkeypatch): This asserts the behavioral contract: on Windows the loop factory the runner receives is the one uvicorn's own Config produced, and bare ``asyncio.run`` is never the serve path when the loop-factory runner exists. + + Windows-only: faking ``sys.platform`` selected the branch but left the + proactor/selector loop policy this exists for entirely absent. """ _stub_uvicorn(monkeypatch) - # The fix only changes behavior on win32; simulate it so the Windows branch - # is actually exercised on a POSIX CI host. - monkeypatch.setattr(web_server.sys, "platform", "win32") - # The fake Config (installed by _stub_uvicorn) returns its ``_loop_factory`` # from get_loop_factory(). Pin a sentinel so we can assert it is threaded # through to the runner unchanged. @@ -212,9 +213,11 @@ def test_start_server_keeps_bare_asyncio_run_on_posix(monkeypatch): The #50641 fix is intentionally win32-scoped to keep the blast radius minimal — Python's default loop on POSIX is already a SelectorEventLoop (or uvloop), which is what uvicorn serves on, so there is nothing to fix. + + No platform patching: the Linux CI host is already POSIX, so this asserts + the real host's serve path. """ _stub_uvicorn(monkeypatch) - monkeypatch.setattr(web_server.sys, "platform", "linux") # If the Windows branch were taken, the loop-factory runner would fire. runner_called = {"hit": False} diff --git a/tests/test_windows_subprocess_no_window_flags.py b/tests/test_windows_subprocess_no_window_flags.py index 5ccc6b74dd..a3d67af7df 100644 --- a/tests/test_windows_subprocess_no_window_flags.py +++ b/tests/test_windows_subprocess_no_window_flags.py @@ -4,6 +4,8 @@ import subprocess from pathlib import Path from types import SimpleNamespace +import pytest + _CREATE_NO_WINDOW = 0x08000000 @@ -66,13 +68,20 @@ def _make_fake_popen(spawns, *, stdout="ok\n", returncode=0): return _FakePopen +@pytest.mark.windows_only def test_bounded_git_probe_fast_path_spawn_contract_windows(monkeypatch): """The normal-path spawn contract survives the run()->Popen rewrite: - PIPE/PIPE/DEVNULL, text + utf-8/replace, hidden-window flags on Windows.""" + PIPE/PIPE/DEVNULL, text + utf-8/replace, hidden-window flags on Windows. + + ``windows_only``: the ``creationflags`` assertion is the point, and + ``bounded_git_probe`` only sets that key when ``IS_WINDOWS`` — which the + helper caches from the real platform at import. ``windows_hide_flags`` is + still stubbed so the expected value is a fixed constant rather than + whatever bundle the helper currently returns. + """ from hermes_cli import _subprocess_compat spawns = [] - monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", True) monkeypatch.setattr(_subprocess_compat, "windows_hide_flags", lambda: _CREATE_NO_WINDOW) monkeypatch.setattr(_subprocess_compat.subprocess, "Popen", _make_fake_popen(spawns, stdout="main\n")) @@ -98,7 +107,6 @@ def test_bounded_git_probe_nonzero_returncode_returns_empty(monkeypatch): from hermes_cli import _subprocess_compat spawns = [] - monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", False) monkeypatch.setattr( _subprocess_compat.subprocess, "Popen", @@ -125,7 +133,6 @@ def test_bounded_git_probe_spawn_failure_returns_empty(monkeypatch): def boom(cmd, **kwargs): raise FileNotFoundError("git not found") - monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", False) monkeypatch.setattr(_subprocess_compat.subprocess, "Popen", boom) assert _subprocess_compat.bounded_git_probe(["git", "-C", "/repo", "status"], timeout=1.5) == "" @@ -149,7 +156,11 @@ def test_bounded_git_probe_spawn_failure_returns_empty(monkeypatch): +@pytest.mark.windows_only def test_shell_hooks_hide_hook_command_windows(monkeypatch): + """``windows_only``: ``shell_hooks._spawn`` only adds ``creationflags`` + under its module-level ``IS_WINDOWS``, so on Linux the flag patch was + what created the thing being asserted.""" from agent import shell_hooks captured = [] @@ -158,7 +169,6 @@ def test_shell_hooks_hide_hook_command_windows(monkeypatch): captured.append((cmd, kwargs)) return SimpleNamespace(returncode=0, stdout="{}", stderr="") - monkeypatch.setattr(shell_hooks, "IS_WINDOWS", True) monkeypatch.setattr(shell_hooks, "windows_hide_flags", lambda: _CREATE_NO_WINDOW) monkeypatch.setattr(shell_hooks.subprocess, "run", fake_run) @@ -192,9 +202,16 @@ def test_shell_hooks_hide_hook_command_windows(monkeypatch): def _patch_hide_flags(monkeypatch): + """Pin ``windows_hide_flags()`` to a known constant. + + The spawn sites these tests cover call ``windows_hide_flags()`` + unconditionally and pass the result straight through, so what is under + test is the WIRING — that the site threads the helper's value into + ``creationflags`` — not the platform. Stubbing only the helper keeps that + coverage on the Linux lane; no ``IS_WINDOWS`` fake is needed or wanted. + """ import hermes_cli._subprocess_compat as subprocess_compat - monkeypatch.setattr(subprocess_compat, "IS_WINDOWS", True) monkeypatch.setattr(subprocess_compat, "windows_hide_flags", lambda: _CREATE_NO_WINDOW) @@ -352,14 +369,20 @@ def test_lazy_deps_uv_install_hides_console_window(monkeypatch): +@pytest.mark.windows_only def test_suppress_platform_ver_console_stubs_syscmd_ver(monkeypatch): - """Simulated Windows: _syscmd_ver is replaced by an in-process echo stub - so win32_ver() takes its ValueError fallback instead of `cmd /c ver`.""" + """``_syscmd_ver`` is replaced by an in-process echo stub so win32_ver() + takes its ValueError fallback instead of shelling out to `cmd /c ver`. + + ``windows_only``: ``suppress_platform_ver_console()`` is a no-op unless + ``IS_WINDOWS``, and the console flash it prevents (``cmd /c ver``) only + exists on Windows — the old flag patch installed the stub on a host where + ``win32_ver`` is never consulted at all. + """ import platform from hermes_cli import _subprocess_compat - monkeypatch.setattr(_subprocess_compat, "IS_WINDOWS", True) # Register the original with monkeypatch so it gets restored after. monkeypatch.setattr(platform, "_syscmd_ver", platform._syscmd_ver) diff --git a/tests/tools/test_code_execution_windows_env.py b/tests/tools/test_code_execution_windows_env.py index 2963e24fcf..45d0058f4b 100644 --- a/tests/tools/test_code_execution_windows_env.py +++ b/tests/tools/test_code_execution_windows_env.py @@ -191,10 +191,11 @@ class TestScrubChildEnvPassthroughInteraction: assert "OPENAI_API_KEY" not in scrubbed -@pytest.mark.skipif( - sys.platform != "win32", - reason="Winsock-specific regression — only meaningful on Windows", -) +# ``windows_only`` rather than ``skipif(sys.platform != "win32")``: the +# dedicated Windows CI job selects its files by grepping for the marker, so a +# bare skipif is invisible to it — the file is never imported there and these +# tests run on no host at all. +@pytest.mark.windows_only class TestWindowsSocketSmokeTest: """Integration-ish smoke test: spawn a child Python with a scrubbed env and confirm it can create an AF_INET socket. This is the @@ -479,10 +480,7 @@ class TestSandboxWritesUtf8: finally: os.unlink(tmp_path) - @pytest.mark.skipif( - sys.platform != "win32", - reason="cp1252 default-encoding regression is Windows-specific", - ) + @pytest.mark.windows_only def test_windows_default_encoding_would_have_failed(self): """Negative control: prove that on Windows, writing the stub *without* ``encoding="utf-8"`` would corrupt the file. If this @@ -616,10 +614,7 @@ class TestChildStdioIsUtf8: assert "\u2192" in decoded, f"arrow missing from output: {decoded!r}" assert "\U0001f680" in decoded, f"emoji missing from output: {decoded!r}" - @pytest.mark.skipif( - sys.platform != "win32", - reason="cp1252 stdout default is Windows-specific", - ) + @pytest.mark.windows_only def test_windows_child_without_utf8_env_would_fail(self): """Negative control: spawn a Python child *without* our env overrides and prove that on Windows, printing non-ASCII fails. diff --git a/tests/tools/test_computer_use_cua_backend_linux.py b/tests/tools/test_computer_use_cua_backend_linux.py index 27ce2d56cd..f106ea14f0 100644 --- a/tests/tools/test_computer_use_cua_backend_linux.py +++ b/tests/tools/test_computer_use_cua_backend_linux.py @@ -4,6 +4,8 @@ from __future__ import annotations from unittest.mock import patch +import pytest + # Tied z_index=0 fixture from #58026 (ding ahead of real terminals). ISSUE_58026_WINDOWS = [ { @@ -83,12 +85,16 @@ def test_parse_xprop_net_active_window_standard_output(): assert _parse_xprop_net_active_window(raw) == 0x503000b +@pytest.mark.linux_only def test_default_capture_prefers_x11_active_window_when_z_index_tied(): + """The ``_NET_ACTIVE_WINDOW`` tie-break is a Linux/X11-only branch of + ``_select_capture_target``; run it where ``sys.platform`` really is + linux instead of patching the branch selector.""" from tools.computer_use.cua_backend import _select_capture_target windows = _normalized_windows() - with patch("tools.computer_use.cua_backend.sys.platform", "linux"), patch( + with patch( "tools.computer_use.cua_backend._linux_x11_active_window_id", return_value=84043449, ): @@ -98,13 +104,17 @@ def test_default_capture_prefers_x11_active_window_when_z_index_tied(): assert target["window_id"] == 84043449 +@pytest.mark.linux_only def test_default_capture_skips_desktop_helper_when_active_window_unknown(): - """Even without _NET_ACTIVE_WINDOW, ding/Desktop helpers must not win (#54173).""" + """Even without _NET_ACTIVE_WINDOW, ding/Desktop helpers must not win (#54173). + + Linux-only: the helper-skipping pool filter is inside the + ``sys.platform == "linux"`` branch.""" from tools.computer_use.cua_backend import _select_capture_target windows = _normalized_windows() - with patch("tools.computer_use.cua_backend.sys.platform", "linux"), patch( + with patch( "tools.computer_use.cua_backend._linux_x11_active_window_id", return_value=None, ): diff --git a/tests/tools/test_file_operations.py b/tests/tools/test_file_operations.py index 51df2b3ce7..37b5b08430 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -286,18 +286,19 @@ class TestShellFileOpsHelpers: assert file_ops._escape_shell_arg("hello") == "'hello'" - def test_escape_shell_arg_rewrites_forward_slash_native_paths(self, monkeypatch, file_ops): - import tools.environments.local as local_mod - - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + @pytest.mark.windows_only + def test_escape_shell_arg_rewrites_forward_slash_native_paths(self, file_ops): + """Windows-only: ``_bash_safe_path`` only rewrites drive paths to the + Git Bash form on Windows, where the MSYS path mangling it works around + actually happens.""" assert file_ops._escape_shell_arg( "C:/Users/alice/notes.txt" ) == "'/c/Users/alice/notes.txt'" - def test_read_file_uses_bash_safe_windows_paths(self, mock_env, monkeypatch): - import tools.environments.local as local_mod - - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + @pytest.mark.windows_only + def test_read_file_uses_bash_safe_windows_paths(self, mock_env): + """Windows-only: proves read_file's shell commands carry the MSYS path + form Git Bash needs — a translation that is a no-op off Windows.""" commands = [] def side_effect(command, **kwargs): diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index 74e6ae4dfc..d368385142 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -8,6 +8,8 @@ import json import logging from unittest.mock import MagicMock, patch +import pytest + from tools.file_tools import ( PATCH_SCHEMA, ) @@ -310,25 +312,28 @@ class TestSearchHandler: class TestWindowsMsysPathResolution: """File tools must translate Git Bash drive paths before Path resolution.""" + @pytest.mark.windows_only def test_absolute_msys_path_normalized_before_windows_resolve(self, monkeypatch): - import tools.environments.local as local_mod + """Windows-only: ``_resolve_path_for_task`` hands the translated path + to ``ntpath``/``Path``, and only a real Windows ``Path`` renders + ``C:\\Users\\...`` — faking ``sys.platform`` left PosixPath in place.""" import tools.file_tools as file_tools - monkeypatch.setattr(file_tools.sys, "platform", "win32") - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) monkeypatch.setattr(file_tools, "_uses_container_paths", lambda task_id="default": False) resolved = file_tools._resolve_path_for_task("/c/Users/Mark/project/app.py") assert str(resolved) == r"C:\Users\Mark\project\app.py" + @pytest.mark.windows_only def test_container_paths_skip_msys_translation(self, monkeypatch): - """WSL/docker Linux paths must not be rewritten as Windows drives.""" - import tools.environments.local as local_mod + """WSL/docker Linux paths must not be rewritten as Windows drives. + + Windows-only: the translation this guards against only happens when + the host really is Windows, so the negative is only meaningful there. + """ import tools.file_tools as file_tools - monkeypatch.setattr(file_tools.sys, "platform", "win32") - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) monkeypatch.setattr(file_tools, "_uses_container_paths", lambda task_id="default": True) monkeypatch.setattr( file_tools, diff --git a/tests/tools/test_find_shell.py b/tests/tools/test_find_shell.py index 4e29eb99c6..e84fdef0ce 100644 --- a/tests/tools/test_find_shell.py +++ b/tests/tools/test_find_shell.py @@ -8,7 +8,6 @@ when ``~/.bash_profile`` contained ``exec /bin/zsh -l``. import os import platform import subprocess -import sys from unittest.mock import patch import pytest @@ -66,13 +65,18 @@ class TestFindShellPrefersUserShell: class TestFindShellWindowsBehavior: """On Windows, _find_shell always delegates to _find_bash.""" + @pytest.mark.windows_only def test_windows_ignores_shell_env(self): - """On Windows, $SHELL is ignored — _find_shell delegates to _find_bash.""" - with patch("tools.environments.local._IS_WINDOWS", True): - # Even if SHELL is set, it should be ignored on Windows - with patch.dict(os.environ, {"SHELL": "/usr/bin/zsh"}): - result = _find_shell() - assert result == _find_bash() + """On Windows, $SHELL is ignored — _find_shell delegates to _find_bash. + + Windows-only: faking ``_IS_WINDOWS`` selected the branch but left + ``_find_bash`` resolving a POSIX bash, so the equality proved nothing + about Git-Bash resolution on the real host. + """ + # Even if SHELL is set, it should be ignored on Windows + with patch.dict(os.environ, {"SHELL": "/usr/bin/zsh"}): + result = _find_shell() + assert result == _find_bash() class TestFindShellReturnsString: @@ -101,10 +105,13 @@ class TestFindBashUnchanged: class TestFindBashSkipsBrokenCustomPath: """Stale HERMES_GIT_BASH_PATH must not brick Windows terminal startup.""" + @pytest.mark.windows_only def test_falls_through_to_portable_when_custom_fails_probe(self, tmp_path, monkeypatch): + """Windows-only: the candidate ladder (HERMES_GIT_BASH_PATH → + %LOCALAPPDATA%\\hermes\\git → Program Files) only exists in + ``_find_bash``'s Windows branch.""" import tools.environments.local as local_mod - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) local_mod._bash_starts_cache.clear() broken = tmp_path / "broken" / "bash.exe" @@ -129,6 +136,9 @@ class TestGitBashExternalProgramProbe: """The Windows health check must exercise MSYS child-process creation.""" def test_probe_runs_external_msys_programs(self, monkeypatch): + """``_bash_starts`` builds the same external-program probe argv on + every host, so this stays on the Linux runner with ``subprocess.run`` + mocked — no platform faking needed.""" import tools.environments.local as local_mod local_mod._bash_starts_cache.clear() @@ -140,14 +150,17 @@ class TestGitBashExternalProgramProbe: return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") monkeypatch.setattr(local_mod.subprocess, "run", fake_run) - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) assert local_mod._bash_starts(r"C:\Git\bin\bash.exe") is True assert calls[0][0][-1] == "/usr/bin/true; /usr/bin/cat --version >/dev/null" + @pytest.mark.windows_only def test_aslr_failure_surfaces_targeted_windows_command( self, tmp_path, monkeypatch ): + """Windows-only: the Mandatory-ASLR diagnostic is raised from + ``_find_bash``'s Windows candidate ladder and names PowerShell's + ``Set-ProcessMitigation`` — unreachable off Windows.""" import tools.environments.local as local_mod local_mod._bash_starts_cache.clear() @@ -156,7 +169,6 @@ class TestGitBashExternalProgramProbe: portable.parent.mkdir(parents=True) portable.write_text("", encoding="utf-8") - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) monkeypatch.setenv("HERMES_GIT_BASH_PATH", "") monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) monkeypatch.setenv("ProgramFiles", str(tmp_path / "empty-program-files")) @@ -181,8 +193,9 @@ class TestGitBashExternalProgramProbe: assert str(tmp_path / "hermes" / "git") in message +@pytest.mark.macos_only @pytest.mark.skipif( - not os.path.isfile("/bin/bash") or sys.platform != "darwin", + not os.path.isfile("/bin/bash"), reason="reproduces the macOS system-bash-3.2 login-shell swallow", ) class TestMacosLoginShellSwallowRegression: @@ -190,7 +203,7 @@ class TestMacosLoginShellSwallowRegression: invoked as a login shell (`-lic`) with stdin=/dev/null and a ~/.bash_profile that `exec`s zsh, silently swallows the command (exit 0, no output, no side effects). Prove (a) the bug exists with /bin/bash and - (b) the $SHELL (zsh) path _find_shell prefers does NOT swallow.""" + (b) the zsh path _find_shell prefers does NOT swallow.""" def _spawn_like_registry(self, shell, command, home, tmp_path): import subprocess @@ -212,7 +225,15 @@ class TestMacosLoginShellSwallowRegression: home.mkdir() (home / ".bash_profile").write_text("exec /bin/zsh -l\n") - zsh = os.environ.get("SHELL") or "/bin/zsh" + # Use /bin/zsh explicitly rather than $SHELL. The reported bug is + # specifically "system bash 3.2 swallows, zsh does not", and $SHELL is + # not zsh everywhere this runs: GitHub's macOS runner exports + # SHELL=/bin/bash, which silently turned the control arm into a SECOND + # bash arm. It then swallowed (correctly, per the bug!) and the + # assertion read as "the fix path is broken" when nothing was broken. + # /bin/zsh is the macOS default login shell since Catalina and is + # present on every supported version. + zsh = "/bin/zsh" if not os.path.isfile(zsh): pytest.skip("no zsh available") @@ -221,11 +242,11 @@ class TestMacosLoginShellSwallowRegression: # /bin/bash login shell: command is swallowed (file NOT created). self._spawn_like_registry("/bin/bash", f"echo x > {marker_bash}", home, tmp_path) - # zsh (the $SHELL _find_shell prefers): command runs (file created). + # zsh (what _find_shell prefers when $SHELL is zsh): command runs. self._spawn_like_registry(zsh, f"echo x > {marker_zsh}", home, tmp_path) # The FIX path (zsh) must run the command. - assert marker_zsh.exists(), "zsh ($SHELL) path must run the command" + assert marker_zsh.exists(), "zsh path must run the command" # Differential: when /bin/bash is the swallow-prone 3.x (macOS system # bash), the login-shell invocation must demonstrably FAIL to run the diff --git a/tests/tools/test_lazy_deps.py b/tests/tools/test_lazy_deps.py index dd23b16cee..774a5855bd 100644 --- a/tests/tools/test_lazy_deps.py +++ b/tests/tools/test_lazy_deps.py @@ -323,7 +323,19 @@ class TestRefreshActiveFeatures: def test_windows_matrix_refresh_is_skipped_before_pip(self, monkeypatch): # Matrix E2EE pulls python-olm, which has no native Windows wheel/build # path. `hermes update` must not retry that doomed install every run. - monkeypatch.setattr(ld.sys, "platform", "win32") + # + # The subject here is the *consumer* — refresh_active_features honouring + # the gate before pip — so we monkeypatch lazy_deps' own platform probe + # instead of faking the host, which keeps this covered on Linux too. + monkeypatch.setattr( + ld, + "_unsupported_feature_reason", + lambda feature: ( + "unsupported on Windows: Matrix E2EE depends on python-olm" + if feature == "platform.matrix" + else None + ), + ) monkeypatch.setattr(ld, "active_features", lambda: ["platform.matrix"]) monkeypatch.setattr(ld, "_is_satisfied", lambda spec: False) monkeypatch.setattr(ld, "_allow_lazy_installs", lambda: True) @@ -338,6 +350,14 @@ class TestRefreshActiveFeatures: assert result["platform.matrix"].startswith("skipped:") assert "unsupported on Windows" in result["platform.matrix"] + @pytest.mark.windows_only + def test_matrix_probe_reports_unsupported_on_real_windows(self): + # The probe itself keys off the real host: patching sys.platform only + # proved the string, never that Windows actually hits this gate. + assert "unsupported on Windows" in ( + ld._unsupported_feature_reason("platform.matrix") or "" + ) + def test_mixed_results_returns_per_feature_status(self, monkeypatch): monkeypatch.setattr(ld, "active_features", lambda: ["a.ok", "b.fail"]) diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 3431d63b4f..7a486bb367 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -558,11 +558,19 @@ class TestSanePathIncludesHomebrew: assert path_entries[:4] == ["/usr/bin", "/bin", "/usr/sbin", "/sbin"] + @pytest.mark.windows_only def test_make_run_env_preserves_windows_mixed_case_path_key(self, monkeypatch): + """Windows-only: ``_path_env_key`` looks for a case-insensitive PATH + key only on Windows, so the mixed-case ``Path`` preservation this + asserts is a genuinely Windows-native behaviour. + + The Git Bash dir prepend is neutralised so the assertion is about the + key casing alone (a real Windows box has those dirs). + """ from tools.environments import local as local_mod from tools.environments.local import _make_run_env windows_env = {"Path": r"C:\Windows\System32;C:\Program Files\Git\bin"} - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + monkeypatch.setattr(local_mod, "_git_bash_bin_dirs", lambda: []) with patch.object(local_mod.os, "environ", windows_env): result = _make_run_env({}) assert result["Path"] == windows_env["Path"] @@ -597,13 +605,15 @@ class TestHermesBinDirOnPath: local_mod._HERMES_BIN_DIR = None assert local_mod._prepend_hermes_bin_dir("/usr/bin:/bin") == "/usr/bin:/bin" - def test_make_run_env_injects_hermes_bin_dir(self, monkeypatch): - """A gateway env missing the hermes dir gets it back in the subshell PATH.""" + def test_make_run_env_injects_hermes_bin_dir(self): + """A gateway env missing the hermes dir gets it back in the subshell PATH. + + Platform-agnostic: ``_prepend_hermes_bin_dir`` uses ``os.pathsep`` on + every host, so no platform flag is faked here.""" from tools.environments import local as local_mod from tools.environments.local import _make_run_env self._reset_cache() local_mod._HERMES_BIN_DIR = "/opt/hermes/bin" - monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) with patch.dict(os.environ, {"PATH": "/usr/bin:/bin"}, clear=True): result = _make_run_env({}) entries = result["PATH"].split(os.pathsep) diff --git a/tests/tools/test_local_env_windows_msys.py b/tests/tools/test_local_env_windows_msys.py index 0d32178214..79a3edff0e 100644 --- a/tests/tools/test_local_env_windows_msys.py +++ b/tests/tools/test_local_env_windows_msys.py @@ -13,14 +13,27 @@ effect was a warning logged on every single terminal call: LocalEnvironment cwd '/c/Users/NVIDIA' is missing on disk; falling back to '/' so terminal commands keep working. -These tests fake the Windows env on Linux CI by patching ``_IS_WINDOWS`` -and ``os.path.isdir`` so the MSYS path tests as "missing" exactly like -on the real OS. +Platform gating +--------------- +These tests used to fake Windows on Linux CI by patching +``local_mod._IS_WINDOWS`` (and sometimes ``os.path.isdir``) so an MSYS +path tested as "missing" exactly like on the real OS. That inverted the +thing under test: the bug was that ``os.path.isdir("/c/Users/x")`` is +False *on Windows*, and the fake had to recreate that condition by hand +on a host where the path semantics, the drive letters, the path +separator, and Git Bash itself are all absent. + +So the Windows-behaviour tests are ``windows_only`` and run on the +Windows CI job against a real Git Bash layout. The "no-op off Windows" +cases assert genuine POSIX behaviour and are ``linux_only`` — on that +host ``_IS_WINDOWS`` is already False, so no patching is needed at all. """ import os from unittest.mock import patch +import pytest + from tools.environments.base import BaseEnvironment from tools.environments import local as local_mod from tools.environments.local import ( @@ -43,21 +56,20 @@ from tools.environments.local import ( # --------------------------------------------------------------------------- class TestMsysToWindowsPath: - def test_noop_on_non_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) + @pytest.mark.linux_only + def test_noop_on_non_windows(self): # On a non-Windows host the function must never rewrite the path # — POSIX-style paths are real paths there. assert _msys_to_windows_path("/c/Users/NVIDIA") == "/c/Users/NVIDIA" assert _msys_to_windows_path("/home/teknium") == "/home/teknium" - def test_translates_drive_path(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + @pytest.mark.windows_only + def test_translates_drive_path(self): assert _msys_to_windows_path("/c/Users/NVIDIA") == r"C:\Users\NVIDIA" assert _msys_to_windows_path("/d/Projects/foo bar") == r"D:\Projects\foo bar" - - def test_empty_string(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + @pytest.mark.windows_only + def test_empty_string(self): assert _msys_to_windows_path("") == "" @@ -66,13 +78,12 @@ class TestMsysToWindowsPath: # --------------------------------------------------------------------------- class TestWindowsToMsysPath: - def test_noop_on_non_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) + @pytest.mark.linux_only + def test_noop_on_non_windows(self): assert _windows_to_msys_path(r"C:\Users\NVIDIA") == r"C:\Users\NVIDIA" - - def test_does_not_translate_non_drive_path(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + @pytest.mark.windows_only + def test_does_not_translate_non_drive_path(self): assert _windows_to_msys_path("/tmp/foo") == "/tmp/foo" assert _windows_to_msys_path(r"\\server\share") == r"\\server\share" @@ -81,14 +92,12 @@ class TestWindowsToMsysPath: # _bash_safe_path / _quote_bash_path — shell-script interpolation # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestBashSafePath: - def test_native_windows_path_becomes_msys(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_native_windows_path_becomes_msys(self): assert _bash_safe_path(r"C:\Users\alice\notes.txt") == "/c/Users/alice/notes.txt" - - def test_quote_bash_path_quotes_mixed_windows_path(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_quote_bash_path_quotes_mixed_windows_path(self): quoted = _quote_bash_path( r"C:\Users\Alexander\AppData\Local\Temp\hermes-snap-abc.sh" ) @@ -100,36 +109,30 @@ class TestBashSafePath: # _resolve_safe_cwd — Windows fast path # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestResolveSafeCwdWindows: - def test_msys_path_resolves_to_native_when_native_exists( - self, monkeypatch, tmp_path, - ): + def test_msys_path_resolves_to_native_when_native_exists(self, tmp_path): """The whole point of this fix: a Git Bash ``/c/Users/x`` value should resolve to its native equivalent if that native dir exists, - WITHOUT falling back to the temp dir.""" - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + WITHOUT falling back to the temp dir. - # tmp_path is a real native dir on the test host. Build a fake - # MSYS form pointing at it and prove the resolver finds it. + ``tmp_path`` is a real native directory on the Windows runner, so + its MSYS spelling is a genuine round-trip rather than a stubbed + translation. + """ native = str(tmp_path) - # Construct a synthetic MSYS form for whatever tmp_path is. - # On Linux CI tmp_path is /tmp/... ; the resolver shouldn't even - # try to translate that (regex won't match), so emulate the - # mapping by pointing the translator at the real native dir. - with patch.object( - local_mod, "_msys_to_windows_path", return_value=native - ): - assert _resolve_safe_cwd("/c/whatever") == native + msys = _windows_to_msys_path(native) + assert msys != native, "expected a drive-letter path to translate" + assert _resolve_safe_cwd(msys) == native # --------------------------------------------------------------------------- -# End-to-end: _update_cwd via stdout marker (Windows simulation) +# End-to-end: _update_cwd via stdout marker # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestUpdateCwdWindowsMsys: - def test_marker_output_msys_path_stored_in_native_form( - self, monkeypatch, tmp_path, - ): + def test_marker_output_msys_path_stored_in_native_form(self, tmp_path): """When Git Bash emits ``/c/Users/x`` in the cwd marker on Windows, ``_update_cwd`` must translate to native form before validating and storing — otherwise ``os.path.isdir`` rejects a @@ -137,32 +140,24 @@ class TestUpdateCwdWindowsMsys: original = tmp_path / "starting" original.mkdir() - # Fake Windows for the test - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) - with patch.object( LocalEnvironment, "init_session", autospec=True, return_value=None ): env = LocalEnvironment(cwd=str(original), timeout=10) - # Pretend Git Bash wrote an MSYS path that maps to tmp_path/"next" new_dir = tmp_path / "next" new_dir.mkdir() marker = env._cwd_marker + # The real MSYS spelling of a real native dir — what Git Bash + # actually writes into the marker. + msys_new = _windows_to_msys_path(str(new_dir)) - # Translate the synthetic MSYS marker path to the real native dir. - def fake_translate(p): - if p == "/c/whatever/from/bash": - return str(new_dir) - return p - - with patch.object(local_mod, "_msys_to_windows_path", side_effect=fake_translate): - env._update_cwd( - { - "output": f"x\n{marker}/c/whatever/from/bash{marker}\n", - "returncode": 0, - } - ) + env._update_cwd( + { + "output": f"x\n{marker}{msys_new}{marker}\n", + "returncode": 0, + } + ) assert env.cwd == str(new_dir) @@ -171,58 +166,50 @@ class TestUpdateCwdWindowsMsys: # End-to-end: _extract_cwd_from_output rollback when marker is invalid # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestExtractCwdFromOutputWindowsMsys: - def test_stale_msys_marker_does_not_clobber_cwd(self, monkeypatch, tmp_path): + def test_stale_msys_marker_does_not_clobber_cwd(self, tmp_path): """When the cwd marker in stdout points at a non-existent path, ``LocalEnvironment._extract_cwd_from_output`` must roll back to the previous cwd instead of propagating a bad value.""" original = tmp_path / "starting" original.mkdir() - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) - with patch.object( LocalEnvironment, "init_session", autospec=True, return_value=None ): env = LocalEnvironment(cwd=str(original), timeout=10) marker = env._cwd_marker + gone = _windows_to_msys_path(str(tmp_path / "definitely-does-not-exist")) result = { - "output": f"some command output\n{marker}/c/no/such/path{marker}\n", + "output": f"some command output\n{marker}{gone}{marker}\n", "returncode": 0, } - # Translation produces a path that doesn't exist on disk → rollback. - with patch.object( - local_mod, - "_msys_to_windows_path", - return_value=str(tmp_path / "definitely-does-not-exist"), - ): - env._extract_cwd_from_output(result) + env._extract_cwd_from_output(result) assert env.cwd == str(original) - def test_valid_msys_marker_normalized_to_native(self, monkeypatch, tmp_path): + def test_valid_msys_marker_normalized_to_native(self, tmp_path): original = tmp_path / "starting" original.mkdir() new_dir = tmp_path / "next" new_dir.mkdir() - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) - with patch.object( LocalEnvironment, "init_session", autospec=True, return_value=None ): env = LocalEnvironment(cwd=str(original), timeout=10) marker = env._cwd_marker + msys_new = _windows_to_msys_path(str(new_dir)) result = { - "output": f"x\n{marker}/c/whatever{marker}\n", + "output": f"x\n{marker}{msys_new}{marker}\n", "returncode": 0, } - with patch.object(local_mod, "_msys_to_windows_path", return_value=str(new_dir)): - env._extract_cwd_from_output(result) + env._extract_cwd_from_output(result) assert env.cwd == str(new_dir) @@ -231,25 +218,21 @@ class TestExtractCwdFromOutputWindowsMsys: # MSYS_NO_PATHCONV — native Windows command flags (#56700) # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestWindowsMsysPathconvDefaults: - def test_make_run_env_sets_msys_no_pathconv_on_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_make_run_env_sets_msys_no_pathconv_on_windows(self): run_env = _make_run_env({}) assert run_env.get("MSYS_NO_PATHCONV") == "1" - def test_sanitize_subprocess_env_sets_msys_no_pathconv_on_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_sanitize_subprocess_env_sets_msys_no_pathconv_on_windows(self): env = _sanitize_subprocess_env({}) assert env.get("MSYS_NO_PATHCONV") == "1" - def test_hermes_subprocess_env_sets_msys_no_pathconv_on_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_hermes_subprocess_env_sets_msys_no_pathconv_on_windows(self): env = hermes_subprocess_env() assert env.get("MSYS_NO_PATHCONV") == "1" - - def test_msys2_arg_conv_excl_respects_user_override(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + def test_msys2_arg_conv_excl_respects_user_override(self): run_env = _make_run_env({"MSYS2_ARG_CONV_EXCL": "/custom"}) assert run_env.get("MSYS2_ARG_CONV_EXCL") == "/custom" @@ -264,8 +247,17 @@ class TestGitBashCoreutilsOnPath: existing = {e.replace("\\", "/") for e in existing} return lambda p: p.replace("\\", "/") in existing + @pytest.mark.windows_only def test_derives_dirs_from_portablegit_layout(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + """The PortableGit layout probe, run on the real OS. + + ``_find_bash`` and ``os.path.isdir`` are still stubbed — the point of + this test is the *derivation* (which sibling dirs we compute from a + bash path, and in what order), and hard-coding a fake tree keeps it + independent of which Git flavour the runner happens to have installed. + What is no longer faked is the host: the ``_IS_WINDOWS`` gate this + function opens with is genuinely True here. + """ monkeypatch.setattr(local_mod, "_git_bash_bin_dirs_cache", None) monkeypatch.setattr(local_mod, "_find_bash", lambda: "/pg/bin/bash.exe") existing = {"/pg/mingw64/bin", "/pg/usr/bin", "/pg/bin"} @@ -273,21 +265,25 @@ class TestGitBashCoreutilsOnPath: dirs = _git_bash_bin_dirs() + # Compare separator-agnostically: the derivation uses os.path.join, so + # on real Windows these come back with backslashes ("/pg\\usr\\bin"). + # The subject is WHICH dirs are derived and in what ORDER, not which + # separator the host's os.path uses. + norm = [d.replace("\\", "/") for d in dirs] + # usr/bin is the load-bearing coreutils dir; mingw64 precedes it. - assert "/pg/usr/bin" in dirs - assert dirs.index("/pg/mingw64/bin") < dirs.index("/pg/usr/bin") + assert "/pg/usr/bin" in norm + assert norm.index("/pg/mingw64/bin") < norm.index("/pg/usr/bin") # Non-existent dirs (mingw32, usr/local/bin) are excluded. - assert "/pg/mingw32/bin" not in dirs - + assert "/pg/mingw32/bin" not in norm + @pytest.mark.linux_only def test_empty_off_windows(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) monkeypatch.setattr(local_mod, "_git_bash_bin_dirs_cache", None) assert _git_bash_bin_dirs() == [] - + @pytest.mark.linux_only def test_make_run_env_noop_on_posix(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", False) monkeypatch.setattr(local_mod, "_git_bash_bin_dirs_cache", None) run_env = _make_run_env({"PATH": "/usr/bin:/bin"}) # No Windows git dirs injected on POSIX. @@ -298,10 +294,9 @@ class TestGitBashCoreutilsOnPath: # Command wrapping — native Windows cwd must be Git Bash-friendly for cd # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestWrapCommandWindowsNativeCwd: - def test_wrap_command_converts_native_cwd_for_builtin_cd(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) - + def test_wrap_command_converts_native_cwd_for_builtin_cd(self): with patch.object( LocalEnvironment, "init_session", autospec=True, return_value=None ): @@ -313,10 +308,7 @@ class TestWrapCommandWindowsNativeCwd: assert "builtin cd -- /c/Users/liush || exit 126" in wrapped assert r"builtin cd -- C:\Users\liush || exit 126" not in wrapped - def test_init_session_bootstrap_rewrites_backslash_snapshot_paths(self, monkeypatch): - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) - captured = {} def fake_run_bash(self, cmd_string, *, login=False, timeout=120, stdin_data=None): diff --git a/tests/tools/test_mcp_oauth.py b/tests/tools/test_mcp_oauth.py index 120123c168..c204fb15bc 100644 --- a/tests/tools/test_mcp_oauth.py +++ b/tests/tools/test_mcp_oauth.py @@ -1,7 +1,6 @@ """Tests for tools/mcp_oauth.py — OAuth 2.1 PKCE support for MCP servers.""" import json -import os import stat import sys from io import BytesIO @@ -159,10 +158,12 @@ class TestUtilities: assert _can_open_browser() is False def test_can_open_browser_true_with_display(self, monkeypatch): + # No ``os.name`` pin: on Linux this exercises the DISPLAY branch for + # real, and on macOS/Windows the function early-returns True anyway — + # the assertion holds on every host without faking one. monkeypatch.delenv("SSH_CLIENT", raising=False) monkeypatch.delenv("SSH_TTY", raising=False) monkeypatch.setenv("DISPLAY", ":0") - monkeypatch.setattr(os, "name", "posix") assert _can_open_browser() is True diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 878e11a6aa..0bd85fb542 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -144,8 +144,13 @@ def _wait_until(predicate, timeout: float = 5.0, interval: float = 0.05) -> bool return False -def test_write_stdin_uses_str_for_windows_pty(monkeypatch, registry): - """pywinpty expects str input; bytes raises a PyString conversion error.""" +@pytest.mark.windows_only +def test_write_stdin_uses_str_for_windows_pty(registry): + """pywinpty expects str input; bytes raises a PyString conversion error. + + Windows-only: the str-vs-bytes choice IS the ``_IS_WINDOWS`` branch, and + the real pty handle it must satisfy (pywinpty) does not exist elsewhere. + """ written = [] class _FakePty: @@ -155,7 +160,6 @@ def test_write_stdin_uses_str_for_windows_pty(monkeypatch, registry): session = _make_session(sid="pty-win") session._pty = _FakePty() registry._running[session.id] = session - monkeypatch.setattr("tools.process_registry._IS_WINDOWS", True) result = registry.write_stdin(session.id, "hello\n") @@ -164,6 +168,25 @@ def test_write_stdin_uses_str_for_windows_pty(monkeypatch, registry): assert isinstance(written[0], str) +@pytest.mark.linux_only +def test_write_stdin_uses_bytes_for_posix_pty(registry): + """The POSIX counterpart: ptyprocess expects bytes, not str.""" + written = [] + + class _FakePty: + def write(self, value): + written.append(value) + + session = _make_session(sid="pty-posix") + session._pty = _FakePty() + registry._running[session.id] = session + + result = registry.write_stdin(session.id, "hello\n") + + assert result == {"status": "ok", "bytes_written": 6} + assert written == [b"hello\n"] + + # ========================================================================= # Get / Poll # ========================================================================= @@ -863,7 +886,6 @@ class TestSpawnRewriteCompoundBackground: fake_thread.daemon = False with patch("tools.process_registry._find_shell", return_value="/bin/bash"), \ - patch("tools.process_registry._IS_WINDOWS", False), \ patch.dict("sys.modules", {"ptyprocess": mock_pty_module}), \ patch("threading.Thread", return_value=fake_thread), \ patch.object(registry, "_write_checkpoint"): @@ -1193,8 +1215,14 @@ class TestTerminateHostPidWindows: target handle only, not the tree. """ + @pytest.mark.windows_only def test_windows_invokes_taskkill_with_tree_and_force_flags(self, monkeypatch): - """The Windows branch must shell out to ``taskkill /PID N /T /F``.""" + """The Windows branch must shell out to ``taskkill /PID N /T /F``. + + Windows-only: ``taskkill.exe`` is the thing under test and only exists + here — with a faked ``_IS_WINDOWS`` the argv was asserted against a + binary that could never have run. + """ from tools import process_registry as pr captured = {} @@ -1204,7 +1232,6 @@ class TestTerminateHostPidWindows: captured["kwargs"] = kwargs return MagicMock(returncode=0, stderr="", stdout="") - monkeypatch.setattr(pr, "_IS_WINDOWS", True) monkeypatch.setattr(pr.subprocess, "run", fake_run) pr.ProcessRegistry._terminate_host_pid(12345) @@ -1242,7 +1269,6 @@ class TestTerminateHostPidPosix: def terminate(self): terminate_order.append(self.pid) - monkeypatch.setattr(pr, "_IS_WINDOWS", False) monkeypatch.setattr(psutil, "Process", _FakeParent) # This test covers only the SIGTERM tree-walk ordering; disable the # SIGKILL-escalation step (which would call psutil.wait_procs on the @@ -1268,7 +1294,6 @@ class TestTerminateHostPidPosix: def fake_kill(pid, sig): kill_calls.append((pid, sig)) - monkeypatch.setattr(pr, "_IS_WINDOWS", False) monkeypatch.setattr(psutil, "Process", boom) monkeypatch.setattr(pr.os, "kill", fake_kill) diff --git a/tests/tools/test_send_message_telegram_proxy.py b/tests/tools/test_send_message_telegram_proxy.py index 45583c932b..468f36e3ea 100644 --- a/tests/tools/test_send_message_telegram_proxy.py +++ b/tests/tools/test_send_message_telegram_proxy.py @@ -134,7 +134,12 @@ class TestSendTelegramStandaloneProxy: monkeypatch.delenv(var, raising=False) monkeypatch.setattr("gateway.run._gateway_runner_ref", lambda: None) # Make sure macOS system-proxy auto-detection (scutil) can't kick in. - monkeypatch.setattr(sys, "platform", "linux") + # Stub the probe itself rather than claiming the host is Linux — that + # keeps this assertion true on the macOS runner too, where a real + # scutil proxy would otherwise be picked up. + monkeypatch.setattr( + "gateway.platforms.base._detect_macos_system_proxy", lambda: None + ) bot = _make_bot() bot_factory = MagicMock(return_value=bot) diff --git a/tests/tools/test_skill_view_path_check.py b/tests/tools/test_skill_view_path_check.py index d4991f6486..c72504ca54 100644 --- a/tests/tools/test_skill_view_path_check.py +++ b/tests/tools/test_skill_view_path_check.py @@ -5,7 +5,6 @@ fails on Windows where Path.resolve() returns backslash-separated paths. Now uses Path.is_relative_to() which handles all platforms correctly. """ -import os import pytest from pathlib import Path @@ -86,7 +85,10 @@ class TestOldCheckWouldFail: and resolved != skill_dir_resolved ) - @pytest.mark.skipif(os.sep == "/", reason="Bug only manifests on Windows") + # ``windows_only`` rather than ``skipif(os.sep == "/")``: the Windows CI + # job greps for the marker to decide which files to import, so a bare + # skipif leaves this running on no host at all. + @pytest.mark.windows_only def test_old_check_false_positive_on_windows(self, tmp_path): """On Windows, the old check incorrectly blocks valid subpaths.""" skill_dir = tmp_path / "skills" / "axolotl" diff --git a/tests/tools/test_telegram_send_message_caption.py b/tests/tools/test_telegram_send_message_caption.py index 6da5590bf0..34240701ab 100644 --- a/tests/tools/test_telegram_send_message_caption.py +++ b/tests/tools/test_telegram_send_message_caption.py @@ -48,7 +48,13 @@ def _no_proxy(monkeypatch: pytest.MonkeyPatch) -> None: ): monkeypatch.delenv(var, raising=False) monkeypatch.setattr("gateway.run._gateway_runner_ref", lambda: None, raising=False) - monkeypatch.setattr(sys, "platform", "linux") + # Neutralize macOS system-proxy auto-detection at its probe rather than by + # claiming the host is Linux: this keeps the test honest on the macOS + # runner (and on a developer's Mac), where a real scutil-configured proxy + # would otherwise leak into the assertion. + monkeypatch.setattr( + "gateway.platforms.base._detect_macos_system_proxy", lambda: None + ) def _tmpfile(suffix: str) -> str: diff --git a/tests/tools/test_tirith_security.py b/tests/tools/test_tirith_security.py index 2de356d1bd..ed5f8be92d 100644 --- a/tests/tools/test_tirith_security.py +++ b/tests/tools/test_tirith_security.py @@ -251,6 +251,12 @@ class TestUnsupportedPlatform: ("Linux", "riscv64", False), ]) def test_is_platform_supported(self, system, machine, expected): + # The patched (system, machine) pairs are table inputs, not a host + # fake: is_platform_supported() is a pure string mapping that touches + # no OS facility beneath the check, so there is nothing for a real + # host to falsify. Two of the rows (Windows/AMD64, Linux/riscv64) + # could never execute honestly anyway — the second has no CI runner + # on any lane. with patch("tools.tirith_security.platform.system", return_value=system), \ patch("tools.tirith_security.platform.machine", return_value=machine): assert _tirith_mod.is_platform_supported() is expected diff --git a/tests/tools/test_tts_streaming.py b/tests/tools/test_tts_streaming.py index 79ee2c8441..fe81de84da 100644 --- a/tests/tools/test_tts_streaming.py +++ b/tests/tools/test_tts_streaming.py @@ -8,6 +8,7 @@ the chunked-streamer playback path, and the universal per-sentence sync fallback import os import queue +import sys import tempfile import threading import time @@ -229,6 +230,16 @@ def test_stream_cap_truncates_runaway_upstream(monkeypatch): # ── Dispatch: chunked streamer path (regression tests) ─────────────────── +# The 12 speaker-path tests below assert on the sounddevice OutputStream +# branch, which stream_tts_to_speaker takes on every host EXCEPT macOS — +# Darwin routes to the tempfile/afplay path by design. They used to fake +# platform.system() == "Linux" (a no-op on the Linux CI lane) purely to +# shield macOS dev machines; an honest exclusion skipif says the same +# thing without lying to the interpreter. +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_streamer_path_handles_misaligned_pcm_chunks(monkeypatch): """Regression: PCM chunks with odd byte counts must not be dropped. @@ -261,8 +272,7 @@ def test_streamer_path_handles_misaligned_pcm_chunks(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_OddChunkProvider({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) # Every chunk must have been written — no drops from misalignment. @@ -280,6 +290,10 @@ def test_streamer_path_handles_misaligned_pcm_chunks(monkeypatch): assert done.is_set() +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_streamer_path_survives_portaudio_write_error(monkeypatch): """Regression: a transient PortAudio error on output_stream.write must not kill the playback thread or hang the pipeline join. @@ -308,14 +322,17 @@ def test_streamer_path_survives_portaudio_write_error(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Fake({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert out.write.called, "expected at least one write attempt" assert done.is_set(), "done event must fire even after PortAudio error" +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_streamer_reinit_after_portaudio_error_plays_remaining_sentences(monkeypatch): """Regression: after a PortAudio error the worker must reinit the stream and continue playing remaining sentences instead of dropping them. @@ -360,8 +377,7 @@ def test_streamer_reinit_after_portaudio_error_plays_remaining_sentences(monkeyp with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Fake({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert broken_out.write.called, "first stream should have received a write" @@ -372,6 +388,10 @@ def test_streamer_reinit_after_portaudio_error_plays_remaining_sentences(monkeyp assert done.is_set(), "done event must fire after recovery" +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_streamer_tempfile_fallback_after_reinit_exhausted(monkeypatch): """Regression: after 3 failed reinits, remaining sentences must play via the temp-file fallback, not be silently dropped. @@ -415,7 +435,6 @@ def test_streamer_tempfile_fallback_after_reinit_exhausted(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Fake({}, {})), \ patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"), \ patch("tools.voice_mode.play_audio_file", side_effect=_fake_play): tts_tool.stream_tts_to_speaker(q, stop, done) @@ -434,6 +453,10 @@ def test_streamer_tempfile_fallback_after_reinit_exhausted(monkeypatch): # ── Dispatch: hybrid batch-prefetch path ────────────────────────────────── +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_first_sentence_streamed_individually(monkeypatch): """The first sentence must get its own stream() call for low TTFA.""" from tools import tts_tool @@ -457,8 +480,7 @@ def test_hybrid_first_sentence_streamed_individually(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Tracking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert len(stream_calls) == 1, ( @@ -467,6 +489,10 @@ def test_hybrid_first_sentence_streamed_individually(monkeypatch): assert done.is_set() +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_subsequent_sentences_prefetched_individually(monkeypatch): """Every sentence should get its own stream() call — per-sentence prefetch fires the HTTP request the moment each sentence completes, @@ -499,8 +525,7 @@ def test_hybrid_subsequent_sentences_prefetched_individually(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Tracking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) # Exactly 4 calls: one per sentence. @@ -516,6 +541,10 @@ def test_hybrid_subsequent_sentences_prefetched_individually(monkeypatch): assert done.is_set() +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_short_sentences_each_get_own_call(monkeypatch): """Short sentences should each get their own stream() call — no batching, no waiting for a threshold or end-of-text.""" @@ -544,8 +573,7 @@ def test_hybrid_short_sentences_each_get_own_call(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Tracking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert len(stream_calls) == 2, ( @@ -557,6 +585,10 @@ def test_hybrid_short_sentences_each_get_own_call(monkeypatch): assert done.is_set() +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_done_event_waits_for_prefetch(monkeypatch): """The done event must not fire until the prefetch thread has finished, otherwise continuous voice mode could overlap turns.""" @@ -592,8 +624,7 @@ def test_hybrid_done_event_waits_for_prefetch(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Blocking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) # done.is_set() is true — but only after the prefetch joined. @@ -605,6 +636,10 @@ def test_hybrid_done_event_waits_for_prefetch(monkeypatch): ) +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_single_sentence_still_works(monkeypatch): """A single-sentence reply should stream immediately with no batch.""" from tools import tts_tool @@ -628,8 +663,7 @@ def test_hybrid_single_sentence_still_works(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Tracking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert len(stream_calls) == 1, ( @@ -638,6 +672,10 @@ def test_hybrid_single_sentence_still_works(monkeypatch): assert done.is_set() +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_playback_serialized_no_overlap(monkeypatch): """Multiple batch flushes must not overlap on the output stream. @@ -684,8 +722,7 @@ def test_hybrid_playback_serialized_no_overlap(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Tracking({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert done.is_set() @@ -694,6 +731,10 @@ def test_hybrid_playback_serialized_no_overlap(monkeypatch): ) +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_hybrid_prefetch_fires_http_immediately(monkeypatch): """The prefetch thread must start consuming the generator (firing the HTTP request) the moment _enqueue_audio is called, NOT when the @@ -742,8 +783,7 @@ def test_hybrid_prefetch_fires_http_immediately(monkeypatch): with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_BlockingFirst({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done) assert done.is_set() @@ -761,6 +801,10 @@ def test_hybrid_prefetch_fires_http_immediately(monkeypatch): ) +@pytest.mark.skipif( + sys.platform == "darwin", + reason="macOS deliberately skips the sounddevice OutputStream path (PR #62601)", +) def test_display_callback_not_called_when_streaming_enabled(monkeypatch): """When streaming is enabled, display_callback must NOT be passed to the TTS consumer — the token stream already renders text. This @@ -789,8 +833,7 @@ def test_display_callback_not_called_when_streaming_enabled(monkeypatch): # display_callback=None simulates the streaming_enabled=True case. with patch("tools.tts_streaming.resolve_streaming_provider", return_value=_Fake({}, {})), \ - patch.object(tts_tool, "_import_sounddevice", return_value=sd), \ - patch("platform.system", return_value="Linux"): + patch.object(tts_tool, "_import_sounddevice", return_value=sd): tts_tool.stream_tts_to_speaker(q, stop, done, display_callback=None) assert done.is_set() diff --git a/tests/tools/test_wake_word.py b/tests/tools/test_wake_word.py index f3aaf7cd01..fc05fa11de 100644 --- a/tests/tools/test_wake_word.py +++ b/tests/tools/test_wake_word.py @@ -239,17 +239,40 @@ def test_bundled_hey_hermes_model_ships_on_disk(): # ── platform-aware backend selection (openWakeWord onnx is broken on macOS ARM64, # upstream dscripka/openWakeWord#336) ──────────────────────────────────────── -def test_default_framework_is_tflite_on_macos_arm64(monkeypatch): - monkeypatch.setattr(ww.sys, "platform", "darwin") - monkeypatch.setattr("platform.machine", lambda: "arm64") +def test_default_framework_tracks_the_macos_arm64_probe(): + """``default_inference_framework()`` is exactly the ``_is_macos_arm64()`` + branch — tflite there, onnx everywhere else. + + Stated as an invariant between the probe and its consumer so it holds on + every host, including the macOS runner (where both sides are real) and an + Intel Mac (where ONNX is fine and both sides say so). + """ + expected = "tflite" if ww._is_macos_arm64() else "onnx" + assert ww.default_inference_framework() == expected + + +@pytest.mark.macos_only +def test_macos_arm64_prefers_tflite_on_this_host(): + """On a real ARM64 Mac the default must be tflite (upstream #336). + + Runs on the macOS CI job, where ``platform.machine()`` and + ``sys.platform`` are the genuine article rather than a patched pair. + """ + if not ww._is_macos_arm64(): + pytest.skip("Intel Mac — ONNX works here, nothing to assert") assert ww.default_inference_framework() == "tflite" + assert ww.resolve_inference_framework({}) == "tflite" + assert ww.resolve_inference_framework({"openwakeword": {"inference_framework": ""}}) == "tflite" + # The one explicit value we override: pinned onnx is provably dead here. + assert ww.resolve_inference_framework( + {"openwakeword": {"inference_framework": "onnx"}} + ) == "tflite" -def test_explicit_framework_kept_off_broken_platform(monkeypatch): +def test_explicit_framework_kept_where_onnx_works(monkeypatch): # An operator who pins a backend keeps it everywhere ONNX actually works. calls = _install_fake_openwakeword(monkeypatch) - monkeypatch.setattr(ww.sys, "platform", "linux") - monkeypatch.setattr("platform.machine", lambda: "x86_64") + monkeypatch.setattr(ww, "_is_macos_arm64", lambda: False) ww._OpenWakeWordEngine( {"provider": "openwakeword", "openwakeword": {"inference_framework": "onnx"}} ) @@ -258,12 +281,17 @@ def test_explicit_framework_kept_off_broken_platform(monkeypatch): def test_empty_framework_falls_back_to_platform_default(monkeypatch): - monkeypatch.setattr(ww.sys, "platform", "darwin") - monkeypatch.setattr("platform.machine", lambda: "arm64") + """Empty/missing config defers to ``default_inference_framework()``. + + The macOS-ARM64 side of the fallback is asserted for real in + ``test_macos_arm64_prefers_tflite_on_this_host``; here we pin the + delegation itself by swapping the platform probe (a seam in our own + module) rather than lying to the interpreter about which OS it is on. + """ + monkeypatch.setattr(ww, "_is_macos_arm64", lambda: True) assert ww.resolve_inference_framework({}) == "tflite" assert ww.resolve_inference_framework({"openwakeword": {"inference_framework": ""}}) == "tflite" - monkeypatch.setattr(ww.sys, "platform", "linux") - monkeypatch.setattr("platform.machine", lambda: "x86_64") + monkeypatch.setattr(ww, "_is_macos_arm64", lambda: False) assert ww.resolve_inference_framework({}) == "onnx" @@ -495,8 +523,8 @@ def test_detector_opens_configured_input_device_and_reports_backend(monkeypatch) det.stop() -def test_windows_silent_hint_names_selected_device(monkeypatch): - monkeypatch.setattr(ww.sys, "platform", "win32") +@pytest.mark.windows_only +def test_windows_silent_hint_names_selected_device(): hint = ww.silent_audio_hint( { "selector": 3, @@ -509,6 +537,26 @@ def test_windows_silent_hint_names_selected_device(monkeypatch): assert "macOS" not in hint +@pytest.mark.macos_only +def test_macos_silent_hint_points_at_privacy_settings(): + """On macOS a silent stream is almost always the TCC mic permission, so the + hint names System Settings rather than the device.""" + hint = ww.silent_audio_hint( + {"selector": 1, "name": "MacBook Pro Microphone", "hostapi": "Core Audio"} + ) + assert "Privacy & Security" in hint + assert "Microphone" in hint + + +@pytest.mark.linux_only +def test_linux_silent_hint_names_selected_device(): + hint = ww.silent_audio_hint( + {"selector": 2, "name": "HD Audio Capture", "hostapi": "ALSA"} + ) + assert "HD Audio Capture (ALSA)" in hint + assert "Privacy & Security" not in hint + + def test_detector_flags_silent_stream_and_recovers(monkeypatch): """A stream of zeros sets audio_silent; audible input clears it.""" monkeypatch.setattr(ww, "_SILENCE_ALERT_SECONDS", 0.001) # trip on the first frame diff --git a/tests/tools/test_windows_native_support.py b/tests/tools/test_windows_native_support.py index 81be4319b5..699e8c42c0 100644 --- a/tests/tools/test_windows_native_support.py +++ b/tests/tools/test_windows_native_support.py @@ -82,13 +82,15 @@ class TestConfigureWindowsStdio: # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestTerminatePidRoutingOnWindows: """``gateway.status.terminate_pid`` must use taskkill /T /F on Windows. - On Linux we can't reload gateway/status with sys.platform=win32 because - the module unconditionally imports ``msvcrt`` in that branch. Instead - we patch the module-level ``_IS_WINDOWS`` flag and ``subprocess.run`` - on the already-loaded module, which exercises the same branching code. + ``windows_only``: this used to patch the module-level ``_IS_WINDOWS`` + flag on Linux, which selected the taskkill branch on a host where + ``taskkill`` does not exist and ``gateway/status`` cannot even import its + ``msvcrt`` branch. On the Windows runner the flag is genuinely True, so + only ``subprocess.run`` is mocked — the dependency, not the host. """ def test_force_uses_taskkill_on_windows(self, monkeypatch): @@ -104,7 +106,6 @@ class TestTerminatePidRoutingOnWindows: result.stdout = "" return result - monkeypatch.setattr(status, "_IS_WINDOWS", True) monkeypatch.setattr(status.subprocess, "run", fake_run) status.terminate_pid(12345, force=True) @@ -124,7 +125,6 @@ class TestTerminatePidRoutingOnWindows: result.stdout = "" return result - monkeypatch.setattr(status, "_IS_WINDOWS", True) monkeypatch.setattr(status.subprocess, "run", fake_run) with pytest.raises(OSError, match="cannot be terminated"): status.terminate_pid(12345, force=True) @@ -163,7 +163,6 @@ class TestTerminatePidRoutingOnWindows: captured["pid"] = pid captured["sig"] = sig - monkeypatch.setattr(status, "_IS_WINDOWS", True) monkeypatch.setattr(status.subprocess, "run", fake_run) monkeypatch.setattr(status.os, "kill", fake_kill) status.terminate_pid(42, force=True) @@ -253,6 +252,7 @@ class TestProcessRegistryOSErrorWidening: assert ProcessRegistry._is_host_pid_alive(os.getpid()) is True +@pytest.mark.linux_only class TestPidExistsOSErrorWidening: """gateway.status._pid_exists itself must widen Windows errors correctly. @@ -260,6 +260,11 @@ class TestPidExistsOSErrorWidening: only path where Python raises ``OSError(WinError 87)`` on Windows for a gone PID instead of ``ProcessLookupError``. The function must catch the wider ``OSError`` to match POSIX semantics. + + ``linux_only``: the subject is the POSIX fallback branch and its + ``os.kill`` error handling, exercised with the errno values Windows + produces. Gating to Linux is what makes ``_IS_WINDOWS`` genuinely False + here instead of forced false by a patch. """ def test_oserror_gone_pid_returns_false(self, monkeypatch): @@ -271,7 +276,6 @@ class TestPidExistsOSErrorWidening: __import__("sys").modules, "psutil", type("P", (), {"pid_exists": staticmethod(lambda pid: (_ for _ in ()).throw(ImportError()))})() ) - monkeypatch.setattr(status, "_IS_WINDOWS", False) def fake_kill(pid, sig): raise OSError(22, "Invalid argument") @@ -287,7 +291,6 @@ class TestPidExistsOSErrorWidening: __import__("sys").modules, "psutil", type("P", (), {"pid_exists": staticmethod(lambda pid: (_ for _ in ()).throw(ImportError()))})() ) - monkeypatch.setattr(status, "_IS_WINDOWS", False) def fake_kill(pid, sig): raise PermissionError(1, "Operation not permitted") @@ -420,9 +423,14 @@ class TestSubprocessCompatHelpers: # name (fallback) — both are acceptable behaviours. - def test_windows_detach_flags_exclude_detached_process(self, monkeypatch): + @pytest.mark.windows_only + def test_windows_detach_flags_exclude_detached_process(self): """DETACHED_PROCESS must stay OUT of every detach bundle. + ``windows_only`` (with ``IS_WINDOWS`` no longer patched): the helpers + return 0 off Windows, so on Linux the old flag patch was the only + thing making the bit assertions reachable at all. + Two reasons (the #54220/#56747 console-flash class): 1. MSDN: CREATE_NO_WINDOW is IGNORED when combined with DETACHED_PROCESS — the hide bit would be dead. @@ -434,7 +442,6 @@ class TestSubprocessCompatHelpers: the desktop backend fix, commit aa2ae36c3f). """ from hermes_cli import _subprocess_compat as sc - monkeypatch.setattr(sc, "IS_WINDOWS", True) assert not sc.windows_detach_flags() & 0x00000008, ( "DETACHED_PROCESS must not be in windows_detach_flags(): it makes " "CREATE_NO_WINDOW a no-op and re-creates the per-descendant " @@ -444,7 +451,8 @@ class TestSubprocessCompatHelpers: "DETACHED_PROCESS must not be in the no-breakaway fallback either." ) - def test_windows_detach_flags_includes_breakaway_from_job(self, monkeypatch): + @pytest.mark.windows_only + def test_windows_detach_flags_includes_breakaway_from_job(self): """CREATE_BREAKAWAY_FROM_JOB is load-bearing for the GUI-driven update path. Without it, the gateway-respawn watcher spawned by ``hermes update`` @@ -459,16 +467,14 @@ class TestSubprocessCompatHelpers: stay in the default bundle going forward. """ from hermes_cli import _subprocess_compat as sc - monkeypatch.setattr(sc, "IS_WINDOWS", True) assert sc.windows_detach_flags() & 0x01000000, ( "CREATE_BREAKAWAY_FROM_JOB (0x01000000) must remain in the " "default detach flag bundle so the Desktop GUI update flow " "can respawn the gateway after Electron exits." ) - def test_windows_detach_flags_without_breakaway_drops_only_that_bit( - self, monkeypatch - ): + @pytest.mark.windows_only + def test_windows_detach_flags_without_breakaway_drops_only_that_bit(self): """Fallback retry payload for restrictive job objects. Some Windows Terminal / container / kiosk configurations refuse @@ -479,7 +485,6 @@ class TestSubprocessCompatHelpers: are still required for the child to survive the parent's exit. """ from hermes_cli import _subprocess_compat as sc - monkeypatch.setattr(sc, "IS_WINDOWS", True) full = sc.windows_detach_flags() fallback = sc.windows_detach_flags_without_breakaway() # Fallback equals full minus the breakaway bit, nothing else changed. @@ -716,11 +721,14 @@ class TestLocalEnvironmentWindowsTempDir: class TestLocalEnvironmentPathInjectionGated: """Sane PATH completion must stay POSIX-only.""" - def test_windows_path_is_left_unchanged(self, monkeypatch): - from tools.environments import local as local_mod + @pytest.mark.windows_only + def test_windows_path_is_left_unchanged(self): + """``windows_only``: the assertion is that a real Windows ``PATH`` + (``;``-separated, drive-lettered) comes back untouched. On Linux the + old ``_IS_WINDOWS`` patch made the function return early without ever + meeting a genuine Windows PATH.""" from tools.environments.local import _append_missing_sane_path_entries - monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) path = r"C:\Windows\System32;C:\Program Files\Git\bin" assert _append_missing_sane_path_entries(path) == path @@ -744,10 +752,14 @@ class TestGitBashPathNormalization: assert _normalize_git_bash_path(None) is None - def test_windows_translation(self, monkeypatch): - """Simulate Windows and verify /c/Users/... becomes C:\\Users\\...""" + @pytest.mark.windows_only + def test_windows_translation(self): + """On native Windows, /c/Users/... becomes C:\\Users\\... + + ``windows_only``: the function's whole job is producing native + Windows paths, which is only meaningful where ``os.sep`` is ``\\``. + """ import cli as cli_mod - monkeypatch.setattr(cli_mod.sys, "platform", "win32") assert cli_mod._normalize_git_bash_path("/c/Users/foo") == r"C:\Users\foo" assert cli_mod._normalize_git_bash_path("/C/Users/foo") == r"C:\Users\foo" assert cli_mod._normalize_git_bash_path("/cygdrive/d/data") == r"D:\data" @@ -920,8 +932,7 @@ class TestWindowlessGatewayRestartSpec: import hermes_cli.gateway_windows as gw argv = ["/path/venv/bin/python", "-m", "hermes_cli.main", "gateway", "run"] - with mock.patch.object(gw.sys, "platform", "linux"): - new_argv, cwd, env = gw.windowless_gateway_restart_spec(list(argv)) + new_argv, cwd, env = gw.windowless_gateway_restart_spec(list(argv)) assert new_argv == argv assert cwd == "" assert env == {} @@ -934,20 +945,21 @@ class TestWindowlessGatewayRestartSpec: assert cwd == "" assert env == {} + @pytest.mark.windows_only def test_windows_keeps_console_python_and_preserves_tail(self): """On Windows the console interpreter is kept (hidden-console launch, NOT a pythonw swap — #54220/#56747) while every subsequent argument - is preserved verbatim.""" - import hermes_cli.gateway_windows as gw + is preserved verbatim. - # Pre-import on the (Linux) host so the function's lazy - # ``from hermes_cli.gateway import PROJECT_ROOT`` resolves from - # sys.modules instead of re-importing under the win32 platform - # patch below — a fresh import would run gateway/status.py's - # ``if sys.platform == "win32": import msvcrt`` branch and crash on - # Linux CI with ModuleNotFoundError. - import hermes_cli.config # noqa: F401 - import hermes_cli.gateway # noqa: F401 + ``windows_only``: faking this on Linux needed two more fakes to hold + it up — a pre-import so the lazy ``hermes_cli.gateway`` import didn't + re-run ``gateway/status``'s ``import msvcrt`` branch, and a mock of + ``get_hermes_home`` because the real one's ``Path.resolve()`` consults + sysconfig and blew up under the platform patch. Both workarounds were + symptoms of testing Windows on a host that isn't Windows; on the + Windows runner neither is needed. + """ + import hermes_cli.gateway_windows as gw argv = [ "C:/venv/Scripts/python.exe", @@ -960,10 +972,9 @@ class TestWindowlessGatewayRestartSpec: "--replace", ] - # Mock get_hermes_home too: the real one calls Path.resolve(), which - # consults sysconfig and raises ModuleNotFoundError under the win32 - # platform patch on a Linux host. - with mock.patch.object(gw.sys, "platform", "win32"), mock.patch.object( + # Only the environment-dependent lookups are stubbed — the host is + # genuinely Windows here. + with mock.patch.object( gw, "_stable_gateway_working_dir", return_value="C:/hermes" ), mock.patch( "hermes_cli.config.get_hermes_home", return_value="C:/hermes" @@ -986,6 +997,7 @@ class TestWindowlessGatewayRestartSpec: # --------------------------------------------------------------------------- +@pytest.mark.windows_only class TestGatewayRunRestartWatcherOuterPopenFallback: """The Windows ``/restart`` watcher in ``gateway.run`` spawns an outer detached ``python -c `` process with @@ -997,10 +1009,13 @@ class TestGatewayRunRestartWatcherOuterPopenFallback: if the retry also fails. Behavioral: drives the real coroutine with a mocked ``subprocess.Popen`` - rather than asserting on source text. Runs on Linux CI via a - ``sys.platform`` patch; the breakaway-bit assertions are gated on the - real host being Windows because ``_subprocess_compat`` caches - ``IS_WINDOWS`` at import time. + rather than asserting on source text. + + ``windows_only``: this used to run on Linux behind a ``sys.platform`` + patch, and the breakaway-bit assertions had to be skipped there anyway + (``_subprocess_compat`` caches ``IS_WINDOWS`` at import, so the flags + were all 0) — i.e. the most important assertions in the class never + executed. On the Windows runner they do. """ @staticmethod @@ -1021,12 +1036,10 @@ class TestGatewayRunRestartWatcherOuterPopenFallback: def test_outer_watcher_retries_without_breakaway_on_oserror(self, monkeypatch): import gateway.run as gr from hermes_cli._subprocess_compat import ( - IS_WINDOWS, windows_detach_flags_without_breakaway, windows_detach_popen_kwargs, ) - monkeypatch.setattr(gr.sys, "platform", "win32") monkeypatch.setattr(gr, "_resolve_hermes_bin", lambda: ["hermes"]) calls = [] @@ -1064,30 +1077,28 @@ class TestGatewayRunRestartWatcherOuterPopenFallback: assert kw2["stderr"] is subprocess.DEVNULL # Primary spreads the full detach helper. Assert every returned helper - # kwarg is present on the call — meaningful on Linux CI too, where the - # helper returns {"start_new_session": True} (no creationflags entry): - # dropping the spread entirely would fail here, not just on Windows. - # The fallback uses the explicit no-breakaway creationflags. + # kwarg is present on the call. The fallback uses the explicit + # no-breakaway creationflags. expected_primary = windows_detach_popen_kwargs() for key, value in expected_primary.items(): assert kw1[key] == value assert kw2["creationflags"] == windows_detach_flags_without_breakaway() assert "start_new_session" not in kw2 - if IS_WINDOWS: - _BREAKAWAY = 0x01000000 - assert kw1["creationflags"] & _BREAKAWAY, ( - "primary spawn must request CREATE_BREAKAWAY_FROM_JOB" - ) - assert not (kw2["creationflags"] & _BREAKAWAY), ( - "fallback spawn must drop CREATE_BREAKAWAY_FROM_JOB" - ) + # The point of the whole fallback: primary asks for breakaway, the + # retry drops exactly that bit. Reachable now that the flags are real. + _BREAKAWAY = 0x01000000 + assert kw1["creationflags"] & _BREAKAWAY, ( + "primary spawn must request CREATE_BREAKAWAY_FROM_JOB" + ) + assert not (kw2["creationflags"] & _BREAKAWAY), ( + "fallback spawn must drop CREATE_BREAKAWAY_FROM_JOB" + ) def test_outer_watcher_happy_path_spawns_once(self, monkeypatch): import gateway.run as gr - monkeypatch.setattr(gr.sys, "platform", "win32") monkeypatch.setattr(gr, "_resolve_hermes_bin", lambda: ["hermes"]) calls = [] @@ -1108,7 +1119,6 @@ class TestGatewayRunRestartWatcherOuterPopenFallback: ): import gateway.run as gr - monkeypatch.setattr(gr.sys, "platform", "win32") monkeypatch.setattr(gr, "_resolve_hermes_bin", lambda: ["hermes"]) calls = []