diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 506cd03e2eecc..5c54141e19198 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,6 +74,16 @@ jobs: with: slice_count: 12 + # macOS + Windows lanes. The main `tests` lane above is Linux-only, and + # the OS-marked tests it collects are skipped there by design (see the + # `_OS_MARKS` comment in tests/conftest.py) — this is where they run. + # Same `python` lane gate: if no Python changed, neither runs. + tests-os: + name: OS-specific tests + needs: detect + if: needs.detect.outputs.python == 'true' + uses: ./.github/workflows/tests-os.yml + lint: name: Python lints needs: detect @@ -244,6 +254,7 @@ jobs: needs: - detect - tests + - tests-os - lint - js-tests - installer-tests diff --git a/.github/workflows/tests-os.yml b/.github/workflows/tests-os.yml new file mode 100644 index 0000000000000..a91f65572186f --- /dev/null +++ b/.github/workflows/tests-os.yml @@ -0,0 +1,152 @@ +name: OS-specific tests + +# Runs the tests that can only be trusted on their own host OS. +# +# The main Python suite (.github/workflows/tests.yml) runs on +# ubuntu-latest and covers everything that is either platform-agnostic or +# genuinely Linux-specific. Tests whose subject is macOS- or +# Windows-specific behaviour carry a marker (see the ``_OS_MARKS`` block +# comment in tests/conftest.py) and are SKIPPED on Linux, because faking +# ``sys.platform`` on a Linux runner selects the branch under test without +# reproducing any of the OS behaviour that branch exists for. This workflow +# is where those markers actually execute: +# +# macos → ``-m macos_only`` on macos-latest +# windows → ``-m windows_only`` on windows-latest +# +# Deliberately NOT sliced. The marked set is small (tens of tests, not +# thousands), so one plain ``pytest`` process per OS is both faster and far +# less machinery than the LPT-sliced per-file runner the Linux lane needs. +# If either lane grows past its timeout, that is the signal to reach for +# scripts/run_tests.sh --slice here too. +# +# Each lane FAILS when it selects zero tests (pytest exit code 5). Without +# that guard, a renamed marker or a bad selector would report a green job +# that ran nothing — the exact silent-coverage-loss failure this workflow +# exists to prevent. + +on: + workflow_call: + +permissions: + contents: read + +concurrency: + group: tests-os-${{ github.ref }} + cancel-in-progress: true + +jobs: + os-tests: + name: ${{ matrix.name }} + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - name: macOS-only tests + runner: macos-latest + marker: macos_only + - name: Windows-only tests + runner: windows-latest + marker: windows_only + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Install uv + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # 8.2.0 + with: + # Pinned for the same reason as the Linux lane: unpinned, setup-uv + # resolves "latest" by fetching a manifest on every job and a + # transient fetch failure fails the whole job. + version: "0.9.28" + enable-cache: true + cache-dependency-glob: | + pyproject.toml + uv.lock + + - name: Set up Python 3.11 + run: uv python install 3.11 + + - name: Install dependencies + # Same extras as the Linux test lane so an OS-marked test can import + # anything its Linux siblings can. ``[all]`` is deliberately + # Windows/macOS-installable (see the policy comment on the extra in + # pyproject.toml — matrix/python-olm was removed from it precisely + # because it could not build here). + uses: ./.github/actions/retry + with: + command: uv sync --locked --python 3.11 --extra all --extra dev --extra anthropic --extra mistral --extra fal --extra modal --extra daytona --extra hindsight --extra parallel-web + + - name: Minimize uv cache + run: uv cache prune --ci + + - name: Run ${{ matrix.marker }} tests + # Two-step selection: + # + # 1. scripts/ci/list_os_marked_tests.py narrows WHICH FILES are + # imported. ``-m`` filters after collection, and collection + # imports every module under tests/ — on this host that would + # drag ~900 unrelated test modules through import, where a + # single unrelated ImportError would fail a job whose own + # subject is fine. The helper exits non-zero if the marker + # matches no file at all. + # 2. ``-m`` decides WHICH TESTS run, and stays authoritative. + # Passing it on the command line REPLACES pyproject's + # ``-m 'not integration'`` addopts (same option, last wins) — + # hence repeating ``not integration``, or the integration + # suite would return through the side door. + # + # ``--timeout-method`` needs no override: tests/conftest.py's + # pytest_configure already downgrades the signal-based timer on + # Windows, which has no SIGALRM. + shell: bash + run: | + set -uo pipefail + + LIST="${RUNNER_TEMP:-.}/selected-tests.txt" + + # Process substitution would hide the helper's exit status, so write + # to a file and check it explicitly. + if ! uv run --no-sync python scripts/ci/list_os_marked_tests.py \ + "${{ matrix.marker }}" > "$LIST"; then + echo "::error::could not enumerate ${{ matrix.marker }} test files" + exit 1 + fi + if [ ! -s "$LIST" ]; then + echo "::error::empty ${{ matrix.marker }} file list" + exit 1 + fi + + # Deliberately NOT `mapfile`: that is a bash 4 builtin and the macOS + # runner's /bin/bash is 3.2. Word-splitting is safe here because the + # helper emits repo-relative test paths, which contain no spaces. + # shellcheck disable=SC2046 + set -- $(cat "$LIST") + echo "selected $# file(s) for ${{ matrix.marker }}:" + cat "$LIST" + + # ``shell: bash`` runs this script with ``-e`` injected, which + # ``set -uo pipefail`` above does not clear. A bare pytest call + # would therefore abort the script on any non-zero exit and the + # exit-5 branch below would be unreachable dead code — the job + # would still fail red, but the diagnostic would never print. + status=0 + uv run --no-sync python -m pytest \ + "$@" \ + -m "${{ matrix.marker }} and not integration" \ + -v --tb=short || status=$? + if [ "$status" -eq 5 ]; then + echo "::error::No tests matched -m ${{ matrix.marker }}. Either the" \ + "marker was renamed/dropped or selection is broken — this job" \ + "must never pass without running its OS's tests." + exit 1 + fi + exit "$status" + env: + # Belt-and-suspenders with tests/conftest.py's env blanking: no + # test may reach a real provider API. + OPENROUTER_API_KEY: "" + OPENAI_API_KEY: "" + NOUS_API_KEY: "" diff --git a/AGENTS.md b/AGENTS.md index 95f3985c4bd77..e5391fc870647 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 e76e10246326f..cf4820f535177 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -416,6 +416,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/scripts/ci/list_os_marked_tests.py b/scripts/ci/list_os_marked_tests.py new file mode 100644 index 0000000000000..bfbc0a60c1d64 --- /dev/null +++ b/scripts/ci/list_os_marked_tests.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""List the test files that carry a given OS marker. + +Used by ``.github/workflows/tests-os.yml`` to scope what the macOS and +Windows lanes import. + +Why scope at all, when ``pytest -m macos_only`` already selects correctly? +Because ``-m`` filters AFTER collection, and collection IMPORTS every test +module under ``tests/``. On the Linux lane that is fine (it runs them all +anyway), but on the macOS/Windows lanes it would drag ~900 unrelated modules +through import on a host they were never expected to import on — one +unrelated ImportError would fail a job whose actual subject passed. Narrowing +the paths keeps each lane's failure signal about its own tests. + +``-m`` is still passed by the workflow and remains the authoritative +selector: this script only decides which files get imported, never which +tests run. Over-selecting here is harmless (``-m`` drops the extras); the +failure mode to care about is UNDER-selecting, which is why the workflow +fails the job when zero tests end up selected. + +Usage: + python scripts/ci/list_os_marked_tests.py macos_only [tests_root] + +Prints one path per line (POSIX separators, repo-relative), sorted. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +_VALID_MARKERS = ("linux_only", "macos_only", "windows_only") + + +def find_marked_files(marker: str, root: Path) -> list[Path]: + """Return every ``test_*.py`` under *root* that references *marker*. + + Matches the marker as a whole word so ``macos_only`` doesn't pick up a + hypothetical ``macos_only_extra``. Catches both the decorator form + (``@pytest.mark.macos_only``, on a function or a class) and the + module-level ``pytestmark`` form. + """ + pattern = re.compile(rf"\b{re.escape(marker)}\b") + hits: list[Path] = [] + for path in sorted(root.rglob("test_*.py")): + try: + text = path.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + if pattern.search(text): + hits.append(path) + return hits + + +def main(argv: list[str]) -> int: + if len(argv) < 2: + print(__doc__, file=sys.stderr) + return 2 + marker = argv[1] + if marker not in _VALID_MARKERS: + print( + f"error: unknown marker {marker!r} (expected one of " + f"{', '.join(_VALID_MARKERS)})", + file=sys.stderr, + ) + return 2 + + repo_root = Path(__file__).resolve().parents[2] + root = Path(argv[2]) if len(argv) > 2 else repo_root / "tests" + if not root.exists(): + print(f"error: no such directory: {root}", file=sys.stderr) + return 2 + + files = find_marked_files(marker, root) + if not files: + print( + f"error: no test file references @pytest.mark.{marker} — the marker " + "was probably renamed or dropped. Refusing to emit an empty list, " + "which would let the OS lane pass without running anything.", + file=sys.stderr, + ) + return 1 + + lines: list[str] = [] + for path in files: + # POSIX separators so the output is safe to paste into a bash + # command line on the Windows runner (Git Bash accepts them). + # + # Relative to the repo root when the path is inside it (the CI case — + # pytest is invoked from the repo root). A root outside the repo is a + # test/manual invocation; emit it as-is rather than raising, since + # ``relative_to`` refuses non-descendant paths. + try: + rel = path.resolve().relative_to(repo_root) + except ValueError: + lines.append(path.as_posix()) + else: + lines.append(rel.as_posix()) + + # Write bytes with explicit LF rather than print(), which on Windows + # translates "\n" to "\r\n" in text mode. The consumer reads this list with + # ``$(cat ...)`` in bash, and word splitting uses IFS (space/tab/newline) — + # a CR is NOT a separator, so it stays glued to each path and pytest then + # fails with "file or directory not found: tests/...py" for a path that + # looks correct in the log because the CR is invisible. Emitting bytes makes + # the output identical on every host instead of depending on the platform's + # newline translation. + sys.stdout.buffer.write(b"".join(line.encode("utf-8") + b"\n" for line in lines)) + sys.stdout.buffer.flush() + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 4fd1da44ced73..96bec3c56aa72 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -44,6 +44,7 @@ import argparse import json import os +import re import subprocess import sys import threading @@ -136,6 +137,41 @@ def _split_pathspec(value: str) -> List[str]: i += 1 return [p for p in parts if p.strip()] +# Host-OS gating (see the ``_OS_MARKS`` block in tests/conftest.py): tests +# marked for another host are collected and SKIPPED by the conftest hook — +# this runner never executes them, by construction. The summary calls that +# out explicitly so a local run isn't misread as covering macOS/Windows +# behaviour, and names the CI lane where those tests actually execute. +_OS_MARKERS = { + "linux_only": ("linux", "the main Linux CI lane"), + "macos_only": ("darwin", "the tests-os CI lane (macos-latest)"), + "windows_only": ("win32", "the tests-os CI lane (windows-latest)"), +} + + +def _off_host_marker_files(files: List[Path]) -> dict[str, int]: + """Count discovered files referencing each marker for an OS we are not on. + + Whole-word text match, same approach as scripts/ci/list_os_marked_tests.py: + over-counting a prose mention is harmless here (the note is informational); + what matters is never reporting 0 while gated tests exist. + """ + off_host = { + marker: re.compile(rf"\b{marker}\b") + for marker, (host_prefix, _) in _OS_MARKERS.items() + if not sys.platform.startswith(host_prefix) + } + counts = {marker: 0 for marker in off_host} + for path in files: + try: + text = path.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + for marker, pattern in off_host.items(): + if pattern.search(text): + counts[marker] += 1 + return {marker: n for marker, n in counts.items() if n} + def _approximately_count_tests( files: List[Path], repo_root: Path @@ -421,8 +457,6 @@ def _parse_pytest_summary(output: str) -> dict[str, int]: Returns a dict with keys ``passed``, ``failed``, ``skipped``, ``errors``, ``xfailed``, ``xpassed`` (only keys found in the output are present). """ - import re - result: dict[str, int] = {} # Walk backwards from the end — the summary line is always near the tail. for line in reversed(output.splitlines()): @@ -1005,6 +1039,7 @@ def _is_our_flag(tok: str) -> bool: fail_count = 0 tests_passed = 0 tests_failed = 0 + tests_skipped = 0 # Every collected outcome, not just pass/fail: a legitimately all-skipped # (platform-gated) file reports "2 skipped" and must NOT trip the # nothing-ran guard, whereas a file that died before collection reports @@ -1013,7 +1048,7 @@ def _is_our_flag(tok: str) -> bool: lock = threading.Lock() def _on_done(file: Path, started_at: float, fut: "Future[Tuple[Path, int, str, Dict[str, int], float]]") -> None: - nonlocal files_done, tests_done, pass_count, fail_count, tests_passed, tests_failed + nonlocal files_done, tests_done, pass_count, fail_count, tests_passed, tests_failed, tests_skipped nonlocal tests_collected n_tests = test_counts.get(file, 0) try: @@ -1038,6 +1073,7 @@ def _on_done(file: Path, started_at: float, fut: "Future[Tuple[Path, int, str, D # Accumulate test-level counts from parsed summary. tests_passed += summary.get("passed", 0) tests_failed += summary.get("failed", 0) + tests_skipped += summary.get("skipped", 0) tests_collected += sum( summary.get(k, 0) for k in ("passed", "failed", "skipped", "errors", "xfailed", "xpassed") @@ -1078,7 +1114,22 @@ def _on_done(file: Path, started_at: float, fut: "Future[Tuple[Path, int, str, D elapsed = time.monotonic() - started print() pct = min(100, (tests_done / approx_total_tests * 100)) if approx_total_tests else 0 - print(f"=== Summary: {len(files)} files, {tests_passed} tests passed, {tests_failed} failed ({pct:.0f}% complete) in {elapsed:.1f}s ({args.jobs} workers) ===") + skipped_note = f", {tests_skipped} skipped" if tests_skipped else "" + print(f"=== Summary: {len(files)} files, {tests_passed} tests passed, {tests_failed} failed{skipped_note} ({pct:.0f}% complete) in {elapsed:.1f}s ({args.jobs} workers) ===") + + # Host-OS gating note: tests marked for another OS were skipped by the + # conftest hook, not run. Say so explicitly — a green local run on Linux + # proves nothing about the macos_only/windows_only tests, and the reader + # should know where they DO run rather than misreading skips as coverage. + off_host = _off_host_marker_files(files) + if off_host: + print() + for marker, n in sorted(off_host.items()): + _, lane = _OS_MARKERS[marker] + print( + f" note: {marker} tests (in {n} file{'s' if n != 1 else ''}) were " + f"SKIPPED on this host ({sys.platform}); they run on {lane}." + ) # Zero tests collected across the WHOLE run is NOT a pass. Per-file rc=5 # is deliberately tolerated above (platform-gated files), but if NOTHING diff --git a/tests/agent/lsp/test_install_and_lint_fixes.py b/tests/agent/lsp/test_install_and_lint_fixes.py index bfe28f3527455..b614cee50f3e0 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 fake_run(cmd, **kwargs): +@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 fake_run(cmd, **kwargs): 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 397610ff32298..d54a92c0d161a 100644 --- a/tests/agent/test_anthropic_keychain.py +++ b/tests/agent/test_anthropic_keychain.py @@ -17,22 +17,29 @@ 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 @@ def test_returns_none_on_nonzero_exit_code(self): +@pytest.mark.macos_only class TestReadClaudeCodeCredentialsPriority: """Bug 4: Keychain must be checked before the JSON file.""" @@ -60,8 +68,7 @@ def test_keychain_takes_priority_over_json_file(self, tmp_path, monkeypatch): 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 @@ def test_falls_back_to_json_when_keychain_returns_none(self, tmp_path, monkeypat })) 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 @@ def test_returns_none_when_neither_keychain_nor_json_has_creds(self, tmp_path, m """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 @@ def test_keychain_expired_file_fresh_returns_file(self, tmp_path, monkeypatch): ``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 @@ def test_both_expired_prefers_later_expiry(self, tmp_path, monkeypatch): 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 c545220dcf6d5..d02871db13f7f 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 @@ def test_build_environment_hints_suppresses_host_on_docker_backend(self, monkeyp _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 0a3ea3bb601d4..a1a71e093bef1 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 @@ def test_bom_frontmatter_matches_plain(self): 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/ci/test_list_os_marked_tests.py b/tests/ci/test_list_os_marked_tests.py new file mode 100644 index 0000000000000..9b44a7596c98b --- /dev/null +++ b/tests/ci/test_list_os_marked_tests.py @@ -0,0 +1,136 @@ +"""Tests for ``scripts/ci/list_os_marked_tests.py``. + +The helper decides which files the macOS / Windows CI lanes import. Its +failure modes matter more than its happy path: if it silently returned an +empty list, the OS lane would run zero tests and still report green — the +exact silent-coverage-loss the lanes exist to prevent. So the contracts under +test are "finds real markers", "refuses to emit nothing", and "rejects an +unknown marker". +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / "scripts" / "ci" / "list_os_marked_tests.py" + + +def _run(*args: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(SCRIPT), *args], + capture_output=True, + text=True, + timeout=120, + cwd=REPO_ROOT, + ) + + +def _write(root: Path, relpath: str, body: str) -> Path: + path = root / relpath + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + return path + + +@pytest.mark.parametrize("marker", ["linux_only", "macos_only", "windows_only"]) +def test_finds_decorator_and_pytestmark_forms(tmp_path, marker): + """Both the decorator form and module-level ``pytestmark`` are detected.""" + _write( + tmp_path, + "test_decorated.py", + f"import pytest\n\n\n@pytest.mark.{marker}\ndef test_x():\n pass\n", + ) + _write( + tmp_path, + "nested/test_module_level.py", + f"import pytest\n\npytestmark = pytest.mark.{marker}\n\n\ndef test_y():\n pass\n", + ) + # A file with no marker at all must not be selected. + _write(tmp_path, "test_plain.py", "def test_z():\n pass\n") + + result = _run(marker, str(tmp_path)) + + assert result.returncode == 0, result.stderr + listed = result.stdout.split() + assert any(p.endswith("test_decorated.py") for p in listed) + assert any(p.endswith("test_module_level.py") for p in listed) + assert not any(p.endswith("test_plain.py") for p in listed) + + +def test_marker_matched_as_whole_word(tmp_path): + """``macos_only`` must not match a longer identifier that contains it.""" + _write( + tmp_path, + "test_lookalike.py", + "import pytest\n\n\n@pytest.mark.macos_only_extra\ndef test_x():\n pass\n", + ) + + result = _run("macos_only", str(tmp_path)) + + # No genuine match: the helper must fail rather than emit nothing. + assert result.returncode == 1 + assert "macos_only" in result.stderr + + +def test_exits_nonzero_when_no_file_carries_the_marker(tmp_path): + """The load-bearing guard: an empty result is an error, never a silent pass.""" + _write(tmp_path, "test_plain.py", "def test_z():\n pass\n") + + result = _run("windows_only", str(tmp_path)) + + assert result.returncode == 1 + assert result.stdout.strip() == "" + assert "renamed or dropped" in result.stderr + + +def test_rejects_unknown_marker(tmp_path): + result = _run("bsd_only", str(tmp_path)) + + assert result.returncode == 2 + assert "unknown marker" in result.stderr + + +def test_rejects_missing_root(): + result = _run("macos_only", "/nonexistent/path/for/this/test") + + assert result.returncode == 2 + assert "no such directory" in result.stderr + + +def test_emits_repo_relative_posix_paths(): + """Output feeds a bash command line on the Windows runner, so separators + must be POSIX and paths repo-relative. + + Asserted against the real ``tests/`` tree, which is the only case CI + exercises — an out-of-repo root can't be made repo-relative and is + emitted absolute instead. + """ + result = _run("windows_only") + + assert result.returncode == 0, result.stderr + listed = result.stdout.split() + assert listed + for line in listed: + assert "\\" not in line + assert not Path(line).is_absolute() + + +def test_real_tree_selects_files_for_every_marker(): + """Against the actual ``tests/`` tree each marker resolves to real files. + + This is the invariant the CI lanes depend on — not a snapshot of which + files those are, only that each marker is in use and every listed path + exists. + """ + for marker in ("linux_only", "macos_only", "windows_only"): + result = _run(marker) + assert result.returncode == 0, f"{marker}: {result.stderr}" + listed = result.stdout.split() + assert listed, f"{marker} selected no files" + for rel in listed: + assert (REPO_ROOT / rel).is_file(), f"{marker} listed missing {rel}" diff --git a/tests/cli/test_cli_file_drop.py b/tests/cli/test_cli_file_drop.py index 00c0fd5d5f8c4..e08e37f95f644 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 @@ def test_tilde_prefixed_path(self, tmp_path, monkeypatch): 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 76757c0858c4f..f3fe883f745eb 100644 --- a/tests/cli/test_cli_init.py +++ b/tests/cli/test_cli_init.py @@ -6,6 +6,8 @@ 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 @@ def test_lf_enter_binds_to_submit_handler_posix(self): 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 @@ def submit_handler(event): 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 @@ def submit_handler(event): # 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 @@ def submit_handler(event): # 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 @@ def submit_handler(event): 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 @@ def test_cpr_warning_callback_is_disabled(self): - 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 c1feaf1809660..3d636e00b58ff 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 9da1e531afae2..7249f9fa28826 100644 --- a/tests/cli/test_ctrl_enter_newline.py +++ b/tests/cli/test_ctrl_enter_newline.py @@ -7,73 +7,76 @@ 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 f91bab3950fa1..6563162870f2c 100644 --- a/tests/cli/test_slash_confirm_windows.py +++ b/tests/cli/test_slash_confirm_windows.py @@ -8,15 +8,25 @@ 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 _worker(): 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 @@ def test_daemon_thread_uses_modal_via_app_loop(self, platform): timeout=5, ), cli, - platform=platform, response="once", ) assert outcome["stdin_called"] is False, "must use the modal, not raw input()" @@ -126,8 +134,7 @@ def test_daemon_thread_uses_modal_via_app_loop(self, platform): 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 @@ def test_main_thread_with_app_uses_modal(self): 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 @@ def _raise(_cb): timeout=5, ), cli, - platform="win32", response="once", schedule=_raise, ) @@ -172,10 +177,15 @@ def _raise(_cb): 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. The class carries no OS marker, so the + ``windows_only`` lane deselects it — the deadlock tests above are the + Windows-side regression guard. + """ def _make_interactive_cli(self): cli = _make_cli() @@ -194,9 +204,9 @@ def _make_interactive_cli(self): "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 +215,6 @@ def test_confirm_destructive_slash_uses_modal_on_windows(self, response, expecte "This starts a fresh session.\nThe current conversation history will be discarded.", ), cli, - platform="win32", response=response, ) @@ -213,6 +222,7 @@ def test_confirm_destructive_slash_uses_modal_on_windows(self, response, expecte assert outcome["result"] == expected +@pytest.mark.windows_only class TestNativeWindowsNoRawInputDeadlock: """Anti-regression guard exercising the REAL ``_prompt_text_input``. @@ -227,6 +237,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 +266,7 @@ def _blocking_input(prompt=""): # stands in for "no line ever arrives" 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 +320,7 @@ def _tracking_input(prompt=""): 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 02959bd89bcde..73af0fd9654bb 100644 --- a/tests/computer_use/test_cua_no_overlay.py +++ b/tests/computer_use/test_cua_no_overlay.py @@ -10,9 +10,10 @@ """ import os -import sys from unittest.mock import mock_open, patch +import pytest + from tools.computer_use import cua_backend @@ -28,11 +29,27 @@ def test_explicit_true_overrides(self): 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")): + 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")), \ - patch.object(sys, "platform", "darwin"): + 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 e737eefe980f8..038f052e1ceb8 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 c51fc9987f2cf..932ee107c8873 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 54298d70e3dc2..8acbf05da2c49 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 @@ -1095,13 +1159,53 @@ def pytest_runtest_setup(item): ) +def _reject_multiple_os_marks(items): + """Fail collection when one test carries two host-OS markers. + + Every marker in ``_OS_MARKS`` skips on all but one host, so two of them + on the same item means it is skipped on *every* host — a test that never + runs anywhere, reported as green by both the Linux suite and the + tests-os lanes. That is the exact silent-coverage-loss the markers were + introduced to remove, so it is a hard collection error rather than a + warning nobody reads. + """ + offenders = [] + for item in items: + marks = sorted({m.name for m in item.iter_markers() if m.name in _OS_MARKS}) + if len(marks) > 1: + offenders.append(f" {item.nodeid}: {', '.join(marks)}") + if offenders: + raise pytest.UsageError( + "a test may carry at most one host-OS marker " + f"({', '.join(_OS_MARKS)}); these carry several and would be " + "skipped on every host:\n" + "\n".join(offenders) + ) + + 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. """ + _reject_multiple_os_marks(items) + + 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 dbe6bf9f6b483..32642e0341b60 100644 --- a/tests/cron/test_cron_script.py +++ b/tests/cron/test_cron_script.py @@ -130,7 +130,11 @@ def test_script_subprocess_env_sanitized(self, cron_env, monkeypatch): 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 @@ def fake_run(argv, **kwargs): 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 @@ def fake_run(argv, **kwargs): 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 @@ def fake_run(argv, **kwargs): 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 6f7817abbcf81..969a3b1c69f7c 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 @@ def test_reaps_orphaned_children_sigterm_then_wait(self, monkeypatch): 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 @@ def test_survivors_of_sigterm_get_sigkill(self, monkeypatch): 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 a44644ddde5df..7304f93581c28 100644 --- a/tests/gateway/test_restart_drain.py +++ b/tests/gateway/test_restart_drain.py @@ -220,15 +220,18 @@ async def _decoy(): ) +@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 @@ def fake_popen(cmd, **kwargs): 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 d777dcf69da4f..eea8e05a0a48c 100644 --- a/tests/gateway/test_status.py +++ b/tests/gateway/test_status.py @@ -7,6 +7,8 @@ from pathlib import Path from types import SimpleNamespace +import pytest + from gateway import status @@ -310,9 +312,12 @@ def test_live_process_is_stable_int(self): 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 @@ def fake_run(cmd, capture_output=False, text=False, timeout=None, creationflags= # 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 @@ def fake_run(cmd, capture_output=False, text=False, timeout=None, creationflags= 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 @@ def test_windows_file_lock_uses_high_offset(self, tmp_path, monkeypatch): 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 f43582a4c0188..12ee1e00c22cb 100644 --- a/tests/gateway/test_whatsapp_connect.py +++ b/tests/gateway/test_whatsapp_connect.py @@ -312,7 +312,11 @@ def poll_side_effect(): 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 @@ def run_side_effect(cmd, **kwargs): 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 @@ def run_side_effect(cmd, **kwargs): ) + @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 @@ def test_kills_only_listeners_on_linux(self): 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 @@ async def test_disconnect_uses_taskkill_tree_on_windows(self): 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_nous_provider.py b/tests/hermes_cli/test_auth_nous_provider.py index a3be5053d0b33..a1772fd32671e 100644 --- a/tests/hermes_cli/test_auth_nous_provider.py +++ b/tests/hermes_cli/test_auth_nous_provider.py @@ -3,6 +3,7 @@ import base64 import json import logging +import sys import time from datetime import datetime, timezone from pathlib import Path @@ -19,21 +20,24 @@ class TestResolveVerifyFallback: - """Verify _resolve_verify falls back to True when CA bundle path doesn't exist.""" - - @pytest.fixture(autouse=True) - def _pin_platform_to_linux(self, monkeypatch): - """Pin sys.platform so the macOS certifi fallback doesn't alter the - generic "default trust" return value asserted by these tests.""" - monkeypatch.setattr("sys.platform", "linux") + """Verify _resolve_verify falls back to default trust when the CA bundle + path doesn't exist.""" def test_missing_ca_bundle_in_auth_state_falls_back(self): + import ssl from hermes_cli.auth import _resolve_verify result = _resolve_verify(auth_state={ "tls": {"insecure": False, "ca_bundle": "/nonexistent/ca-bundle.pem"}, }) - assert result is True + # The subject is "falls back to _default_verify()", not the literal + # True. Deriving the expectation from the real host keeps the + # regression covered on the macOS lane too, where _default_verify + # pins certifi's bundle and returns a context instead. + if sys.platform == "darwin": + assert isinstance(result, ssl.SSLContext) + else: + assert result is True def test_valid_ca_bundle_in_auth_state_is_returned(self, tmp_path, monkeypatch): import ssl diff --git a/tests/hermes_cli/test_auth_ssl_macos.py b/tests/hermes_cli/test_auth_ssl_macos.py index 48e5a615c99e0..9d9abc1dbfeb2 100644 --- a/tests/hermes_cli/test_auth_ssl_macos.py +++ b/tests/hermes_cli/test_auth_ssl_macos.py @@ -13,7 +13,6 @@ 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_claw.py b/tests/hermes_cli/test_claw.py index 37d95608c5ed0..d9472cd8c8588 100644 --- a/tests/hermes_cli/test_claw.py +++ b/tests/hermes_cli/test_claw.py @@ -351,31 +351,31 @@ def test_empty_report(self, capsys): class TestDetectOpenclawProcesses: def test_returns_match_when_pgrep_finds_openclaw(self): - with patch.object(claw_mod, "sys") as mock_sys: - mock_sys.platform = "linux" - with patch.object(claw_mod, "subprocess") as mock_subprocess: - # systemd check misses, pgrep finds openclaw - mock_subprocess.run.side_effect = [ - MagicMock(returncode=1, stdout=""), # systemctl - MagicMock(returncode=0, stdout="1234\n"), # pgrep - ] - mock_subprocess.TimeoutExpired = subprocess.TimeoutExpired - result = claw_mod._detect_openclaw_processes() - assert len(result) == 1 - assert "1234" in result[0] - - + with patch.object(claw_mod, "subprocess") as mock_subprocess: + # systemd check misses, pgrep finds openclaw + mock_subprocess.run.side_effect = [ + MagicMock(returncode=1, stdout=""), # systemctl + MagicMock(returncode=0, stdout="1234\n"), # pgrep + ] + mock_subprocess.TimeoutExpired = subprocess.TimeoutExpired + result = claw_mod._detect_openclaw_processes() + assert len(result) == 1 + assert "1234" in result[0] + + + @pytest.mark.windows_only def test_returns_empty_on_windows_when_nothing_found(self): - with patch.object(claw_mod, "sys") as mock_sys: - mock_sys.platform = "win32" - with patch.object(claw_mod, "subprocess") as mock_subprocess: - mock_subprocess.run.side_effect = [ - MagicMock(returncode=0, stdout=""), - MagicMock(returncode=0, stdout=""), - MagicMock(returncode=0, stdout=""), - ] - result = claw_mod._detect_openclaw_processes() - assert result == [] + """Faking win32 picked the tasklist/powershell branch on a host that has + neither; only a real Windows host resolves those executables. + + ``return_value`` rather than a ``side_effect`` list: the branch's call + count is not the assertion, and pinning it breaks whenever the host + shells out once more than the dev box did. + """ + with patch.object(claw_mod, "subprocess") as mock_subprocess: + mock_subprocess.run.return_value = MagicMock(returncode=0, stdout="") + result = claw_mod._detect_openclaw_processes() + assert result == [] class TestWarnIfOpenclawRunning: diff --git a/tests/hermes_cli/test_clipboard_text_write.py b/tests/hermes_cli/test_clipboard_text_write.py index 85d28a8f3618a..379ab5b798eeb 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 fake_run(argv, **kwargs): # 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 e550fdc732dcf..14d1681607392 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 c91fe410dc136..6e9d3dd06e97c 100644 --- a/tests/hermes_cli/test_desktop_exe_integrity.py +++ b/tests/hermes_cli/test_desktop_exe_integrity.py @@ -138,26 +138,36 @@ def _windll(name, *args, **kwargs): 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 798bcde1eca67..ceb9bfb382a86 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 @@ def test_returns_parsed_values_when_both_set(self, monkeypatch): 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 f6dec1b5f11f0..e1c4dabe256cc 100644 --- a/tests/hermes_cli/test_gateway_platform_gating.py +++ b/tests/hermes_cli/test_gateway_platform_gating.py @@ -12,23 +12,42 @@ 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. + + 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" - def test_other_platforms_unaffected_on_windows(self, monkeypatch): + @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 ab5cf1b89850f..7872ec2b464a4 100644 --- a/tests/hermes_cli/test_gateway_proc_fallback.py +++ b/tests/hermes_cli/test_gateway_proc_fallback.py @@ -9,6 +9,8 @@ import os from unittest.mock import MagicMock, patch +import pytest + import hermes_cli.gateway as gateway_mod @@ -51,8 +53,15 @@ def _open(path, mode="r", **kwargs): # --------------------------------------------------------------------------- +@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 @@ def test_detects_gateway_pid_via_proc(self): _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 @@ def _open(path, mode="r", **kwargs): 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 a5d9ad6dee95a..9b5aa9942acc5 100644 --- a/tests/hermes_cli/test_gateway_service.py +++ b/tests/hermes_cli/test_gateway_service.py @@ -235,15 +235,20 @@ def test_launchd_plist_does_not_leak_profile_node_symlink_target(self, tmp_path, 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 @@ def fake_subprocess_run(cmd, **kwargs): + @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 616fbac42afc5..b8c4a5c5b39df 100644 --- a/tests/hermes_cli/test_gateway_windows.py +++ b/tests/hermes_cli/test_gateway_windows.py @@ -26,10 +26,17 @@ def _boom(*args, **kwargs): +@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 fake_install_startup_entry(path: Path) -> Path: +@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 ShellExecuteW(self, hwnd, verb, executable, params, cwd, show): 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 @@ class FakeWindll: 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 bedec3ad6d1b3..6e7ff37932758 100644 --- a/tests/hermes_cli/test_gateway_wsl.py +++ b/tests/hermes_cli/test_gateway_wsl.py @@ -59,9 +59,13 @@ def test_running(self, monkeypatch): 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 @@ def test_wsl_with_systemd(self, monkeypatch): 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 @@ def test_termux_still_excluded(self, monkeypatch): 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 @@ def test_install_wsl_no_systemd(self, monkeypatch, capsys): 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_graphical_browser_detection.py b/tests/hermes_cli/test_graphical_browser_detection.py index ab1819e256e41..a7c461a88ac82 100644 --- a/tests/hermes_cli/test_graphical_browser_detection.py +++ b/tests/hermes_cli/test_graphical_browser_detection.py @@ -36,25 +36,29 @@ def _clean_browser_env(monkeypatch): yield -def _force_platform_linux(monkeypatch): - monkeypatch.setattr("hermes_cli.auth.sys.platform", "linux") - - def _force_resolved_browser(monkeypatch, name: str): monkeypatch.setattr(webbrowser, "get", lambda *_a, **_kw: _FakeController(name)) +@pytest.mark.linux_only def test_headless_linux_no_display_refuses(monkeypatch): - """The reported bug: headless Linux, no display server → don't auto-open.""" - _force_platform_linux(monkeypatch) + """The reported bug: headless Linux, no display server → don't auto-open. + + Gated rather than faked: the display-server requirement is the Linux arm + of the helper, and the autouse fixture already strips DISPLAY / + WAYLAND_DISPLAY so a real Linux host reaches it headless. + """ # Even if a GUI browser somehow resolved, no display means no GUI. _force_resolved_browser(monkeypatch, "google-chrome") assert _can_open_graphical_browser() is False def test_browser_env_pointing_at_console_browser_refuses(monkeypatch): - """$BROWSER=w3m must refuse even with a display server present.""" - _force_platform_linux(monkeypatch) + """$BROWSER=w3m must refuse even with a display server present. + + Host-independent: the $BROWSER console check runs before the helper's + per-platform branch, so this holds on every lane. + """ monkeypatch.setenv("DISPLAY", ":0") monkeypatch.setenv("BROWSER", "/usr/bin/w3m") assert _can_open_graphical_browser() is False diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index cde1ee5b50b1d..96b968c5f96bd 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -13,6 +13,24 @@ 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). + + 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" + ) - 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 + +@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 boom(*a, **kw): # --- 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 boom(_project_root): 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 b768758700215..55d82408a4f2e 100644 --- a/tests/hermes_cli/test_install_cua_driver.py +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -29,7 +29,23 @@ 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 @@ def test_upgrade_on_unsupported_platform_is_silent_noop(self): 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 @@ def test_non_upgrade_on_unsupported_platform_warns(self): 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 @@ def test_upgrade_on_macos_with_binary_runs_installer(self): 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 @@ def test_quiet_refresh_prints_single_contextual_progress_line(self): 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 @@ def test_quiet_refresh_prints_single_contextual_progress_line(self): "→ 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 @@ def test_quiet_refresh_can_suppress_progress_line(self): 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 @@ def test_quiet_refresh_can_suppress_progress_line(self): 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 @@ def test_upgrade_can_suppress_installer_progress(self): 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 @@ def test_upgrade_on_macos_non_writable_applications_skips_refresh(self): 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 @@ def test_fresh_install_on_macos_non_writable_applications_skips_install(self): 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 + + 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("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") as runner, \ @@ -208,11 +240,10 @@ def test_non_upgrade_on_macos_with_binary_skips_install(self): 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 @@ def _install(self, system, check_state, require_confirmed): 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 @@ def test_indeterminate_check_keeps_installed_version(self): ) 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 @@ def test_indeterminate_check_points_at_force_path(self): 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 @@ def fake_run(cmd, **kw): 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 @@ def test_fresh_install_does_not_call_github_api(self): """ 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 @@ def test_upgrade_with_binary_does_not_call_github_api_directly(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), \ + 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 @@ def test_no_lock_is_noop(self, tmp_path): 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 @@ def test_windows_branch_uses_file_lock_probe(self): 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 @@ def test_lock_held_with_file_share_none_is_kept(self, tmp_path): 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 @@ def fake_killpg(pgid, sig): 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 @@ def test_timeout_ceiling_exceeds_upstream_lock_window(self): # 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 @@ def fake_popen(*args, **kwargs): 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 @@ def fake_popen(*args, **kwargs): 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 @@ def test_windows_timeout_kills_descendants_and_parent(self): ("", 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 @@ def test_windows_timeout_kills_descendants_and_parent(self): 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 @@ def test_windows_tree_enumeration_failure_falls_back_to_direct_kill(self): ("", 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 @@ def test_windows_tree_enumeration_failure_falls_back_to_direct_kill(self): 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 @@ def fake_popen(cmd, **kw): 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 @@ def test_download_failure_returns_false_without_exec(self): 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 @@ def fake_popen(cmd, **kw): 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 @@ def test_missing_latest_version_falls_back_unpinned(self): 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 @@ def fake_run(cmd, **kw): 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 @@ def test_no_pin_leaves_env_untouched(self): 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 @@ def fake_run(cmd, **kwargs): 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 @@ def fake_run(cmd, **kwargs): ] 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 @@ def fake_which(name: str): 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 @@ def fake_which(name: str): ] 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 @@ def fake_run(cmd, **kwargs): 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 7de9b4c0b7c72..ce254bf9f5deb 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_linux_desktop_entry.py b/tests/hermes_cli/test_linux_desktop_entry.py index 910bddeff2025..37087e36b3cd6 100644 --- a/tests/hermes_cli/test_linux_desktop_entry.py +++ b/tests/hermes_cli/test_linux_desktop_entry.py @@ -115,9 +115,17 @@ def test_install_without_source_icon_uses_themed_name(tmp_path, xdg_home, monkey assert _parse(entry.read_text(encoding="utf-8"))["Icon"] == "hermes" -@pytest.mark.parametrize("platform", ["darwin", "win32"]) -def test_install_is_a_noop_off_linux(tmp_path, monkeypatch, platform): - monkeypatch.setattr(lde.sys, "platform", platform) +@pytest.mark.macos_only +def test_install_is_a_noop_on_macos(tmp_path): + """Faking darwin only renamed the host — the real macOS runner is the + only place the `sys.platform` guard is exercised against a real host.""" + assert lde.install_desktop_entry(_make_project(tmp_path)) is None + + +@pytest.mark.windows_only +def test_install_is_a_noop_on_windows(tmp_path): + """As above for Windows: a fake left POSIX paths and a POSIX XDG layout + in place, so the no-op was never proven against a real one.""" assert lde.install_desktop_entry(_make_project(tmp_path)) is None diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 1c608661012b6..21d1d3f4fcd4b 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 @@ def fake_install(target): 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 @@ def test_success_usable_as_single_value(self, tmp_path): 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 @@ def test_success_unpacks_as_legacy_two_tuple(self, tmp_path): 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 @@ def test_uvresult_would_break_windows_list2cmdline(self): 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 @@ def test_environment_is_private_and_sanitized(self, tmp_path): 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 @@ def fake_run(argv, **kwargs): 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 @@ def test_failed_candidate_preserves_live_venv(self, tmp_path): 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 @@ def test_safe_runtime_sweeps_old_stale_backups(self, tmp_path): (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 @@ def test_successful_repair_removes_parked_backup(self, tmp_path): "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 @@ def fake_install(uv_bin, *, project_root, current): 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 @@ def test_recovers_when_the_cached_module_predates_the_symbol(self, monkeypatch): # 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 @@ def test_recovery_uses_the_shared_helper_not_a_second_copy(self, monkeypatch): 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 @@ def _no_reload(module): # pragma: no cover - must not run 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 3c94f23f07eb8..575a4a8dcdb48 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 cb5e81d548552..576396b49c4d5 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 @@ def test_creates_sh_on_posix(self, profile_env, monkeypatch): 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 @@ def test_create_wrapper_rejects_absolute_path(self, profile_env, tmp_path): 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 @@ def test_ignores_unrelated_files(self, profile_env, monkeypatch): 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 f58b6dea81b21..ffb611950e459 100644 --- a/tests/hermes_cli/test_relaunch.py +++ b/tests/hermes_cli/test_relaunch.py @@ -114,13 +114,26 @@ def fake_execvp(path, argv): 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 @@ def fake_execvp(*args, **kwargs): 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 @@ def test_windows_rejects_py_argv0_falls_through_to_path(self, monkeypatch, tmp_p 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 @@ def test_windows_rejects_py_argv0_falls_through_to_path(self, monkeypatch, tmp_p # 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 @@ def test_windows_py_argv0_with_no_hermes_on_path_returns_none(self, monkeypatch, 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 ae17efb2b65a7..be92a79a3d363 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 @@ 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 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.""" - pythonw, python = _make_venv(tmp_path, with_console_python=True) + argument after the interpreter preserved verbatim. - # 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 + ``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) 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 e61fe8acbb7d2..547b4f887ac8e 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -196,8 +196,12 @@ def fake_run(args, *a, **kw): 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 @@ def fake_run(args, *a, **kw): 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 51827a854ee51..0404d2bb0e6aa 100644 --- a/tests/hermes_cli/test_verify_core_dependencies.py +++ b/tests/hermes_cli/test_verify_core_dependencies.py @@ -17,6 +17,7 @@ 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 @@ def fake_subprocess_run(cmd, **kwargs): 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 @@ def fake_subprocess_run(cmd, **kwargs): 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 e9a18c82f0dd2..fff0da000d471 100644 --- a/tests/hermes_cli/test_win_pty_bridge.py +++ b/tests/hermes_cli/test_win_pty_bridge.py @@ -25,10 +25,11 @@ # 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 @@ def test_spawn_raises_unavailable_off_windows(self): # --------------------------------------------------------------------------- -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeSpawn: def test_spawn_returns_bridge_with_pid(self): @@ -97,7 +98,7 @@ def test_spawn_raises_on_missing_argv0(self, tmp_path): WinPtyBridge.spawn([bogus]) -@windows_only +@pytest.mark.windows_only class TestWinPtyBridgeIO: def test_write_sends_to_child_stdin(self): @@ -136,7 +137,7 @@ def test_read_returns_none_after_child_exits(self): 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 @@ def test_resize_after_close_is_silent(self): 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 @@ def test_non_numeric_falls_back_to_min(self): 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 @@ def test_close_terminates_long_running_child(self): ) -@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 3a0fa30e13009..c4bb16ec119c5 100644 --- a/tests/run_agent/test_tool_batch_segmentation.py +++ b/tests/run_agent/test_tool_batch_segmentation.py @@ -12,7 +12,6 @@ """ import json -import sys import threading import time import uuid @@ -711,10 +710,10 @@ def test_execution_cwd_used_over_process_cwd(self, tmp_path, monkeypatch): ) - @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 61b04232691a9..b95b50d9ad0eb 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 @@ def test_env_vars_set_on_windows(self, monkeypatch): 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 @@ def test_stdout_reconfigured_to_utf8_on_windows(self): "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 @@ def test_env_var_used_when_no_arg(self): 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 a3164c2b2ed3e..30ef95cc1f142 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -54,13 +54,13 @@ def test_docker_profile_active(self, tmp_path, monkeypatch): 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 @@ def test_no_hermes_home_returns_localappdata_root_on_windows(self, tmp_path, mon 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 @@ def test_env_set_returns_that_path(self, tmp_path, monkeypatch): 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 @@ def test_windows_finds_npm_cmd_before_path(self, tmp_path, monkeypatch): + @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 @@ def test_windows_skips_broken_managed_npm_without_path_fallback(self, tmp_path, 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 400b1cbb89fdb..c0a9b03e9b82f 100644 --- a/tests/test_hermes_logging.py +++ b/tests/test_hermes_logging.py @@ -398,20 +398,27 @@ def _make_logger_and_handler(self, log_path: Path): 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 @@ def test_lock_timeout_routed_to_handle_error_is_suppressed(self, tmp_path, capsy 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 43f68d70c891b..710297a7df7d3 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_os_marker_gating.py b/tests/test_os_marker_gating.py new file mode 100644 index 0000000000000..5d348fd73e980 --- /dev/null +++ b/tests/test_os_marker_gating.py @@ -0,0 +1,60 @@ +"""The collection guard against a test carrying two host-OS markers. + +Every marker in ``_OS_MARKS`` skips on all but one host, so two of them on one +item means it runs on no host at all while both the Linux suite and the +tests-os lanes report green. tests/conftest.py fails collection instead; this +pins that behaviour so the guard can't be dropped silently. +""" + +from __future__ import annotations + +import pytest + +from tests.conftest import _OS_MARKS, _reject_multiple_os_marks + + +class _FakeItem: + """Stands in for a collected item: the guard reads only these two.""" + + def __init__(self, nodeid: str, *marks: str) -> None: + self.nodeid = nodeid + self._marks = [getattr(pytest.mark, name).mark for name in marks] + + def iter_markers(self): + return iter(self._marks) + + +def test_single_os_marker_is_accepted(): + items = [_FakeItem(f"t.py::test_{name}", name) for name in _OS_MARKS] + _reject_multiple_os_marks(items) # must not raise + + +def test_unmarked_and_non_os_markers_are_accepted(): + _reject_multiple_os_marks([ + _FakeItem("t.py::test_plain"), + _FakeItem("t.py::test_other", "slow", "integration"), + ]) + + +def test_two_os_markers_fail_collection(): + items = [ + _FakeItem("t.py::test_ok", "linux_only"), + _FakeItem("t.py::test_bad", "linux_only", "windows_only"), + ] + with pytest.raises(pytest.UsageError) as excinfo: + _reject_multiple_os_marks(items) + + message = str(excinfo.value) + assert "t.py::test_bad" in message + assert "linux_only, windows_only" in message + # The passing item must not be named — the error is a list of offenders. + assert "t.py::test_ok" not in message + + +def test_all_three_markers_are_reported_together(): + item = _FakeItem("t.py::test_worst", *_OS_MARKS) + with pytest.raises(pytest.UsageError) as excinfo: + _reject_multiple_os_marks([item]) + + for name in _OS_MARKS: + assert name in str(excinfo.value) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 73d0f986bb945..35743f6a80325 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -12984,7 +12984,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 e14d3151d916d..55534e8b84e9a 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -7,6 +7,7 @@ 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 5ccc6b74dda1a..a3d67af7dff85 100644 --- a/tests/test_windows_subprocess_no_window_flags.py +++ b/tests/test_windows_subprocess_no_window_flags.py @@ -4,6 +4,8 @@ from pathlib import Path from types import SimpleNamespace +import pytest + _CREATE_NO_WINDOW = 0x08000000 @@ -66,13 +68,20 @@ def kill(self): # pragma: no cover - never reached on the fast path 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 boom(cmd, **kwargs): +@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 fake_run(cmd, **kwargs): 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 fake_run(cmd, **kwargs): 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 fake_run(cmd, **kwargs): +@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_clipboard.py b/tests/tools/test_clipboard.py index 71f938e256bc5..6e08d7f9bb7ef 100644 --- a/tests/tools/test_clipboard.py +++ b/tests/tools/test_clipboard.py @@ -402,24 +402,28 @@ def setup_method(self): import hermes_cli.clipboard as cb cb._wsl_detected = None + @pytest.mark.macos_only def test_macos_dispatch(self): - with patch("hermes_cli.clipboard.sys") as mock_sys: - mock_sys.platform = "darwin" - with patch("hermes_cli.clipboard._macos_has_image", return_value=True) as m: - assert has_clipboard_image() is True - m.assert_called_once() + """Faking darwin selected the branch but left `_macos_has_image`'s real + facility (osascript) absent — only a real macOS host has it.""" + with patch("hermes_cli.clipboard._macos_has_image", return_value=True) as m: + assert has_clipboard_image() is True + m.assert_called_once() + @pytest.mark.linux_only def test_wsl_falls_through_to_wayland_when_windows_path_empty(self): - """WSLg often bridges images to wl-paste even when powershell.exe check fails.""" - with patch("hermes_cli.clipboard.sys") as mock_sys: - mock_sys.platform = "linux" - with patch("hermes_cli.clipboard._is_wsl", return_value=True): - with patch("hermes_cli.clipboard._wsl_has_image", return_value=False) as wsl: - with patch.dict(os.environ, {"WAYLAND_DISPLAY": "wayland-0"}): - with patch("hermes_cli.clipboard._wayland_has_image", return_value=True) as wl: - assert has_clipboard_image() is True - wsl.assert_called_once() - wl.assert_called_once() + """WSLg often bridges images to wl-paste even when powershell.exe check fails. + + WSL is Linux, so the host reaches the fallthrough on its own; only the + WSL/Wayland environment probes below are stubbed. + """ + with patch("hermes_cli.clipboard._is_wsl", return_value=True): + with patch("hermes_cli.clipboard._wsl_has_image", return_value=False) as wsl: + with patch.dict(os.environ, {"WAYLAND_DISPLAY": "wayland-0"}): + with patch("hermes_cli.clipboard._wayland_has_image", return_value=True) as wl: + assert has_clipboard_image() is True + wsl.assert_called_once() + wl.assert_called_once() # ═════════════════════════════════════════════════════════════════════════ diff --git a/tests/tools/test_code_execution_windows_env.py b/tests/tools/test_code_execution_windows_env.py index 2963e24fcf110..45d0058f4bdc0 100644 --- a/tests/tools/test_code_execution_windows_env.py +++ b/tests/tools/test_code_execution_windows_env.py @@ -191,10 +191,11 @@ def test_passthrough_still_works_on_windows(self): 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 @@ def test_stub_source_roundtrips_through_utf8(self): 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 @@ def test_live_child_can_print_non_ascii(self): 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 27ce2d56cd528..f106ea14f081e 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 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 51df2b3ce77a4..37b5b08430bce 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -286,18 +286,19 @@ def test_escape_shell_arg_simple(self, file_ops): 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 74e6ae4dfcf65..d368385142723 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -8,6 +8,8 @@ import logging from unittest.mock import MagicMock, patch +import pytest + from tools.file_tools import ( PATCH_SCHEMA, ) @@ -310,25 +312,28 @@ def test_search_exception_returns_error(self, mock_get): 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 4e29eb99c604a..e84fdef0ce133 100644 --- a/tests/tools/test_find_shell.py +++ b/tests/tools/test_find_shell.py @@ -8,7 +8,6 @@ import os import platform import subprocess -import sys from unittest.mock import patch import pytest @@ -66,13 +65,18 @@ def test_falls_back_to_find_bash_when_shell_empty(self): 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 @@ def test_find_bash_still_prefers_bash(self): 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 @@ def fake_run(argv, **kwargs): 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 @@ def test_aslr_failure_surfaces_targeted_windows_command( 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 @@ def failed_probe(path: str) -> bool: 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 @@ def test_system_bash_swallows_but_zsh_does_not(self, tmp_path): 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 @@ def test_system_bash_swallows_but_zsh_does_not(self, tmp_path): # /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 dd23b16cee57d..774a5855bd043 100644 --- a/tests/tools/test_lazy_deps.py +++ b/tests/tools/test_lazy_deps.py @@ -323,7 +323,19 @@ def test_no_active_features_returns_empty(self, monkeypatch): 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 @@ def test_windows_matrix_refresh_is_skipped_before_pip(self, monkeypatch): 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 3431d63b4fb6a..7a486bb367b61 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -558,11 +558,19 @@ def test_make_run_env_real_launchd_path_gains_homebrew(self): 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 @@ def test_prepend_noop_when_unresolved(self, monkeypatch): 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 0d3217821429e..79a3edff0e7a9 100644 --- a/tests/tools/test_local_env_windows_msys.py +++ b/tests/tools/test_local_env_windows_msys.py @@ -13,14 +13,27 @@ 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 @@ # --------------------------------------------------------------------------- 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 @@ def test_empty_string(self, monkeypatch): # --------------------------------------------------------------------------- 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 @@ def test_does_not_translate_non_drive_path(self, monkeypatch): # _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 @@ def test_quote_bash_path_quotes_mixed_windows_path(self, monkeypatch): # _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 @@ def test_marker_output_msys_path_stored_in_native_form( 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 - - # 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, - } - ) + # 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)) + + env._update_cwd( + { + "output": f"x\n{marker}{msys_new}{marker}\n", + "returncode": 0, + } + ) assert env.cwd == str(new_dir) @@ -171,58 +166,50 @@ def fake_translate(p): # 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 @@ def test_valid_msys_marker_normalized_to_native(self, monkeypatch, tmp_path): # 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 @@ def _fake_isdir(self, existing): 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 @@ def test_derives_dirs_from_portablegit_layout(self, monkeypatch): 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 @@ def test_make_run_env_noop_on_posix(self, monkeypatch): # 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 @@ def test_wrap_command_converts_native_cwd_for_builtin_cd(self, monkeypatch): 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 120123c168533..c204fb15bc5e3 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 @@ def test_can_open_browser_false_in_ssh(self, monkeypatch): 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 82ae7e33686d7..bf0c5117299e4 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 write(self, value): 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 write(self, value): 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 @@ def test_pty_path_uses_rewritten_command(self, registry): 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 @@ def fake_run(args, **kwargs): 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 @@ def children(self, recursive=False): 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 @@ def boom(pid): 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 45583c932b29c..468f36e3ea6ce 100644 --- a/tests/tools/test_send_message_telegram_proxy.py +++ b/tests/tools/test_send_message_telegram_proxy.py @@ -134,7 +134,12 @@ def test_no_proxy_env_uses_plain_bot( 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 d4991f648656e..c72504ca54744 100644 --- a/tests/tools/test_skill_view_path_check.py +++ b/tests/tools/test_skill_view_path_check.py @@ -5,7 +5,6 @@ Now uses Path.is_relative_to() which handles all platforms correctly. """ -import os import pytest from pathlib import Path @@ -86,7 +85,10 @@ def _old_path_escapes(self, resolved: Path, skill_dir_resolved: Path) -> bool: 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 6da5590bf0b82..34240701ab038 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 2de356d1bd59a..ed5f8be92d26f 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_macos_output.py b/tests/tools/test_tts_macos_output.py index 3e6a3ff256d9f..e890d7ad9544e 100644 --- a/tests/tools/test_tts_macos_output.py +++ b/tests/tools/test_tts_macos_output.py @@ -22,14 +22,19 @@ def stream(self, text): return iter([]) -def _run_stream(monkeypatch, system_name): - """Drive stream_tts_to_speaker once with a mock client on *system_name*. +def _run_stream(monkeypatch): + """Drive stream_tts_to_speaker once with a mock client on the real host. Returns True if _import_sounddevice was called during the run. + + No platform parameter: the two callers below are the macOS and non-macOS + arms of the same policy, and each now runs on a host that reaches its arm + by itself. Faking ``platform.system()`` here selected the branch without + reproducing anything underneath it — on Darwin the branch exists because + PortAudio init raises a TCC prompt, which no Linux runner can produce. """ import tools.tts_tool as tts - monkeypatch.setattr("tools.tts_tool.platform.system", lambda: system_name) monkeypatch.setattr("tools.tts_tool.get_env_value", lambda name, default=None: "fake-key" if name == "ELEVENLABS_API_KEY" else default) @@ -52,7 +57,10 @@ def convert(self, *a, **k): def _spy_import_sd(): sd_called["hit"] = True - raise AssertionError("sounddevice must not be imported for output on macOS") + # OSError, not AssertionError: the function's own guard handles it, so + # the off-macOS arm can record the call without the raise aborting the + # run. On macOS the call must never happen at all. + raise OSError("no audio device in test") monkeypatch.setattr("tools.tts_tool._import_sounddevice", _spy_import_sd) @@ -66,53 +74,13 @@ def _spy_import_sd(): return sd_called["hit"] +@pytest.mark.macos_only def test_streaming_tts_skips_sounddevice_on_macos(monkeypatch): - assert _run_stream(monkeypatch, "Darwin") is False + assert _run_stream(monkeypatch) is False +@pytest.mark.linux_only def test_streaming_tts_uses_sounddevice_off_macos(monkeypatch): # Off macOS the OutputStream setup runs; _import_sounddevice raising here # is caught by the function's own guard, so the call itself is what we assert. - called = _run_stream_offmac(monkeypatch) - assert called is True - - -def _run_stream_offmac(monkeypatch): - """Like _run_stream but tolerant of the sounddevice import being attempted.""" - import tools.tts_tool as tts - - monkeypatch.setattr("tools.tts_tool.platform.system", lambda: "Linux") - monkeypatch.setattr("tools.tts_tool.get_env_value", - lambda name, default=None: "fake-key" - if name == "ELEVENLABS_API_KEY" else default) - monkeypatch.setattr("tools.tts_tool._load_tts_config", lambda: {}) - - class _FakeTTS: - def __init__(self, *a, **k): - self.text_to_speech = self - - def convert(self, *a, **k): - return iter([]) - - monkeypatch.setattr("tools.tts_tool._import_elevenlabs", lambda: _FakeTTS) - monkeypatch.setattr( - "tools.tts_streaming.resolve_streaming_provider", - lambda cfg, preferred=None: _FakeStreamer(), - ) - - sd_called = {"hit": False} - - def _spy_import_sd(): - sd_called["hit"] = True - raise OSError("no audio device in test") # handled by the function's guard - - monkeypatch.setattr("tools.tts_tool._import_sounddevice", _spy_import_sd) - - text_queue: queue.Queue = queue.Queue() - text_queue.put(None) - stop_event = threading.Event() - done_event = threading.Event() - - tts.stream_tts_to_speaker(text_queue, stop_event, done_event) - assert done_event.is_set() - return sd_called["hit"] + assert _run_stream(monkeypatch) is True diff --git a/tests/tools/test_tts_streaming.py b/tests/tools/test_tts_streaming.py index 79ee2c8441609..fe81de84da551 100644 --- a/tests/tools/test_tts_streaming.py +++ b/tests/tools/test_tts_streaming.py @@ -8,6 +8,7 @@ import os import queue +import sys import tempfile import threading import time @@ -229,6 +230,16 @@ def _endless(): # ── 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 _make_stream(*args, **kwargs): 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 _make_stream(*args, **kwargs): 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 _fake_play(path): 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 _fake_play(path): # ── 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): 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 stream(self, text): ) +@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 stream(self, text): 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 stream(self, text): 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 _mock_write(_data): 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 _mock_write(_data): ) +@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 _mock_write(_data): 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 _mock_write(_data): ) +@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 stream(self, text): # 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_voice_mode.py b/tests/tools/test_voice_mode.py index 51d921d613428..be9f24f6de3c2 100644 --- a/tests/tools/test_voice_mode.py +++ b/tests/tools/test_voice_mode.py @@ -563,12 +563,13 @@ def test_real_speech_not_filtered(self): # ============================================================================ class TestPlayAudioFile: + @pytest.mark.linux_only def test_play_wav_via_sounddevice(self, monkeypatch, sample_wav): np = pytest.importorskip("numpy") - # Pin to a non-macOS platform: on macOS WAV output deliberately skips - # sounddevice (see TestMacOSAudioOutputPolicy), so this path is only - # exercised off Darwin. - monkeypatch.setattr("tools.voice_mode.platform.system", lambda: "Linux") + # Linux-gated rather than faking a non-macOS platform: on macOS WAV + # output deliberately skips sounddevice (see + # TestMacOSAudioOutputPolicy), so this path is only exercised off + # Darwin and the host now selects it by itself. mock_sd_obj = MagicMock() # Simulate stream completing immediately (get_stream().active = False) @@ -594,9 +595,13 @@ def _fake_import(): # ============================================================================ class TestMacOSAudioOutputPolicy: + """macOS-gated: the policy exists because PortAudio/CoreAudio init raises + a TCC media-library prompt, which no faked platform on Linux reproduces — + and `afplay` only resolves on a real macOS host.""" + + @pytest.mark.macos_only def test_play_audio_file_skips_sounddevice_on_macos(self, monkeypatch, sample_wav): """On macOS, WAV playback must not import sounddevice; it routes to afplay.""" - monkeypatch.setattr("tools.voice_mode.platform.system", lambda: "Darwin") def _forbidden_import(): raise AssertionError("sounddevice must not be imported for output on macOS") @@ -618,7 +623,8 @@ def _fake_popen(cmd, **kwargs): popen_cmds.append(cmd) return _FakeProc() - monkeypatch.setattr("shutil.which", lambda exe: f"/usr/bin/{exe}") + # Only Popen is stubbed: the host resolves afplay for real, so the + # argv assertion below reflects real player selection. monkeypatch.setattr("subprocess.Popen", _fake_popen) from tools.voice_mode import play_audio_file @@ -629,10 +635,10 @@ def _fake_popen(cmd, **kwargs): assert popen_cmds, "expected a system player to be invoked" assert popen_cmds[0][0] == "afplay" + @pytest.mark.macos_only def test_play_beep_routes_through_afplay_on_macos(self, monkeypatch): """On macOS, beeps synthesize with numpy but play via the tempfile/afplay path.""" pytest.importorskip("numpy") - monkeypatch.setattr("tools.voice_mode.platform.system", lambda: "Darwin") def _forbidden_import(): raise AssertionError("sounddevice must not be imported for beeps on macOS") diff --git a/tests/tools/test_wake_word.py b/tests/tools/test_wake_word.py index f3aaf7cd01970..fc05fa11de451 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 _stream(**kwargs): 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 81be4319b5578..699e8c42c0368 100644 --- a/tests/tools/test_windows_native_support.py +++ b/tests/tools/test_windows_native_support.py @@ -82,13 +82,15 @@ def test_reconfigure_stream_handles_missing_method(self, monkeypatch): # --------------------------------------------------------------------------- +@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 @@ def fake_run(args, **kwargs): 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 @@ def fake_run(args, **kwargs): 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 @@ def fake_kill(pid, sig): 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 @@ def test_alive_pid_returns_true(self, monkeypatch): 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 @@ def test_oserror_gone_pid_returns_false(self, monkeypatch): __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 @@ def test_permission_error_returns_true(self, monkeypatch): __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 @@ def test_resolve_node_command_returns_absolute_on_posix(self): # 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 @@ def test_windows_detach_flags_exclude_detached_process(self, monkeypatch): 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 @@ def test_windows_detach_flags_exclude_detached_process(self, monkeypatch): "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 @@ def test_windows_detach_flags_includes_breakaway_from_job(self, monkeypatch): 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 @@ def test_windows_detach_flags_without_breakaway_drops_only_that_bit( 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 @@ def test_source_has_windows_branch_using_hermes_home(self): 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 @@ def test_posix_noop(self): 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 @@ def test_noop_on_non_windows(self): 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,21 +945,22 @@ def test_empty_argv_is_safe(self): 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.""" + is preserved verbatim. + + ``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 - # 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 - argv = [ "C:/venv/Scripts/python.exe", "-m", @@ -960,10 +972,9 @@ def test_windows_keeps_console_python_and_preserves_tail(self): "--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 @@ def test_windows_keeps_console_python_and_preserves_tail(self): # --------------------------------------------------------------------------- +@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 @@ def _drive(cls, gr): 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 @@ def fake_popen(argv, **kwargs): 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 @@ def test_outer_watcher_dual_failure_warns_without_leaking_secrets( ): import gateway.run as gr - monkeypatch.setattr(gr.sys, "platform", "win32") monkeypatch.setattr(gr, "_resolve_hermes_bin", lambda: ["hermes"]) calls = []