From 30da5d0a8932ba149210f13090ecf529ce751763 Mon Sep 17 00:00:00 2001 From: ethernet Date: Tue, 4 Aug 2026 16:52:52 -0400 Subject: [PATCH] test: run os-specific tests on their real host, not a faked one many tests patched sys.platform or a module's _IS_WINDOWS flag, then ran on linux ci. the patch selects the branch under test, but the host does not have the behavior the branch exists for. the test proves the patch, not the platform. some gated assertions never ran on any host. this commit adds three markers: linux_only, macos_only, windows_only. a conftest hook skips a marked test on the other hosts, with a clear reason. no test fakes a host now. two documented fakes remain (android/termux, freebsd) because no ci runner exists for them. each fake site got one of four treatments: - gate it: the real host supplies the platform; mocks cover real dependencies only, never host identity - patch the module's own probe when the subject is the probe's consumer - assert against the real host when the fake stood in for any non-x host - delete the patch when it set the value the host already has bare skipif(sys.platform != ...) guards became markers too. the lane model skips these on linux and never imports them on windows, so they ran on no host. platform parametrize tables are now one marked test per os. running on real hosts found real errors: a chrome-sandbox failure in test_gui_command that main hides, and two windows failures fixed here. the agents.md testing section now documents the policy. --- AGENTS.md | 38 +++ pyproject.toml | 3 + .../agent/lsp/test_install_and_lint_fixes.py | 10 +- tests/agent/test_anthropic_keychain.py | 34 +-- tests/agent/test_prompt_builder.py | 11 +- tests/agent/test_skill_utils.py | 20 +- tests/cli/test_cli_file_drop.py | 6 +- tests/cli/test_cli_init.py | 63 +++-- tests/cli/test_cpr_local_leak.py | 6 +- tests/cli/test_ctrl_enter_newline.py | 75 +++--- tests/cli/test_slash_confirm_windows.py | 68 +++-- tests/computer_use/test_cua_no_overlay.py | 27 +- .../test_cua_spawn_env_sanitization.py | 10 +- .../test_permissions_resolution.py | 10 +- tests/conftest.py | 87 ++++++- tests/cron/test_cron_script.py | 10 +- tests/gateway/test_replace_child_reap.py | 3 - tests/gateway/test_restart_drain.py | 13 +- tests/gateway/test_status.py | 15 +- tests/gateway/test_whatsapp_connect.py | 24 +- tests/hermes_cli/test_auth_ssl_macos.py | 10 +- tests/hermes_cli/test_clipboard_text_write.py | 8 +- tests/hermes_cli/test_dep_ensure.py | 19 +- .../hermes_cli/test_desktop_exe_integrity.py | 37 ++- .../test_ensure_hermes_home_uid_34107.py | 6 +- .../test_gateway_platform_gating.py | 29 ++- .../hermes_cli/test_gateway_proc_fallback.py | 13 +- tests/hermes_cli/test_gateway_service.py | 18 +- tests/hermes_cli/test_gateway_windows.py | 28 +- tests/hermes_cli/test_gateway_wsl.py | 47 ++-- tests/hermes_cli/test_gui_command.py | 160 +++++++++--- tests/hermes_cli/test_install_cua_driver.py | 246 ++++++++++++------ tests/hermes_cli/test_kanban_db.py | 14 +- tests/hermes_cli/test_managed_uv.py | 82 +++--- tests/hermes_cli/test_mem_trim.py | 4 +- tests/hermes_cli/test_profiles.py | 18 +- tests/hermes_cli/test_relaunch.py | 30 ++- .../test_update_gateway_launcher_refresh.py | 25 +- .../hermes_cli/test_update_stale_dashboard.py | 27 +- .../test_verify_core_dependencies.py | 36 ++- tests/hermes_cli/test_win_pty_bridge.py | 21 +- .../run_agent/test_tool_batch_segmentation.py | 9 +- tests/test_hermes_bootstrap.py | 49 ++-- tests/test_hermes_constants.py | 10 +- tests/test_hermes_logging.py | 44 ++-- tests/test_hermes_state_compression_locks.py | 9 +- tests/test_tui_gateway_server.py | 5 +- tests/test_web_server.py | 13 +- ...test_windows_subprocess_no_window_flags.py | 41 ++- .../tools/test_code_execution_windows_env.py | 19 +- .../test_computer_use_cua_backend_linux.py | 16 +- tests/tools/test_file_operations.py | 17 +- tests/tools/test_file_tools.py | 19 +- tests/tools/test_find_shell.py | 51 ++-- tests/tools/test_lazy_deps.py | 22 +- tests/tools/test_local_env_blocklist.py | 18 +- tests/tools/test_local_env_windows_msys.py | 184 +++++++------ tests/tools/test_mcp_oauth.py | 5 +- tests/tools/test_process_registry.py | 41 ++- .../tools/test_send_message_telegram_proxy.py | 7 +- tests/tools/test_skill_view_path_check.py | 6 +- .../test_telegram_send_message_caption.py | 8 +- tests/tools/test_tirith_security.py | 6 + tests/tools/test_tts_streaming.py | 89 +++++-- tests/tools/test_wake_word.py | 72 ++++- tests/tools/test_windows_native_support.py | 128 ++++----- 66 files changed, 1530 insertions(+), 769 deletions(-) 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 = []