From cd4317b449f93ef34aab83a7dbce5ef6eb14684f Mon Sep 17 00:00:00 2001 From: ethernet Date: Sat, 8 Aug 2026 17:41:23 -0400 Subject: [PATCH] test: convert the last host-OS fakes and guard double markers Six test files still selected an OS branch with a faked host. Each one now carries the marker for the host that owns the branch, or derives the expectation from the real host: - test_clipboard: macos_only on the has_clipboard_image dispatch. The fake picked the branch, but _macos_has_image needs osascript. - test_claw: windows_only on the tasklist/powershell scan, with return_value in place of a side_effect list that pinned the call count. - test_linux_desktop_entry: the parametrize over "darwin"/"win32" becomes one marked test per host. A fake left POSIX paths and a POSIX XDG layout. - test_graphical_browser_detection: linux_only on the display-server arm. The $BROWSER check runs before the platform branch, so its test stays unmarked. - test_auth_nous_provider: the fixture pinned linux so the macOS certifi fallback could not change the result. The assertion now reads the host, so the macOS lane covers the fallback too. - test_tts_macos_output and test_voice_mode: the afplay policy exists because CoreAudio init raises a TCC prompt, which no Linux runner reproduces. tests/conftest.py refuses collection when one test carries two OS markers. Each marker skips on all but one host, so two of them make a test that runs nowhere while every lane reports green. tests/test_os_marker_gating.py pins that behavior. The docstring on TestConfirmDestructiveSlash said the Windows job runs it. The class has no marker, so -m windows_only deselects it. --- tests/cli/test_slash_confirm_windows.py | 5 +- tests/conftest.py | 25 ++++++++ tests/hermes_cli/test_auth_nous_provider.py | 20 +++--- tests/hermes_cli/test_claw.py | 44 ++++++------- .../test_graphical_browser_detection.py | 20 +++--- tests/hermes_cli/test_linux_desktop_entry.py | 14 +++- tests/test_os_marker_gating.py | 60 +++++++++++++++++ tests/tools/test_clipboard.py | 34 +++++----- tests/tools/test_tts_macos_output.py | 64 +++++-------------- tests/tools/test_voice_mode.py | 20 ++++-- 10 files changed, 193 insertions(+), 113 deletions(-) create mode 100644 tests/test_os_marker_gating.py diff --git a/tests/cli/test_slash_confirm_windows.py b/tests/cli/test_slash_confirm_windows.py index 616f7e3617..6563162870 100644 --- a/tests/cli/test_slash_confirm_windows.py +++ b/tests/cli/test_slash_confirm_windows.py @@ -182,8 +182,9 @@ class TestConfirmDestructiveSlash: This is the flow bug #33961 froze on native Windows. The fix made it platform-agnostic (modal via the app loop), so the assertion holds on - whichever host runs it — including the Windows CI job, where it is the - genuine regression guard. + 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): diff --git a/tests/conftest.py b/tests/conftest.py index d7e746d397..8acbf05da2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1159,6 +1159,29 @@ 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 """Apply host-OS gating, then skip ``requires_wal`` where WAL is unusable. @@ -1171,6 +1194,8 @@ def pytest_collection_modifyitems(config, items): # noqa: D401 — pytest hook 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 diff --git a/tests/hermes_cli/test_auth_nous_provider.py b/tests/hermes_cli/test_auth_nous_provider.py index a3be5053d0..a1772fd326 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 @@ from hermes_cli.auth import AuthError, get_provider_auth_state, resolve_nous_run 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_claw.py b/tests/hermes_cli/test_claw.py index 37d95608c5..d9472cd8c8 100644 --- a/tests/hermes_cli/test_claw.py +++ b/tests/hermes_cli/test_claw.py @@ -351,31 +351,31 @@ class TestPrintMigrationReport: 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_graphical_browser_detection.py b/tests/hermes_cli/test_graphical_browser_detection.py index ab1819e256..a7c461a88a 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_linux_desktop_entry.py b/tests/hermes_cli/test_linux_desktop_entry.py index 910bddeff2..37087e36b3 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/test_os_marker_gating.py b/tests/test_os_marker_gating.py new file mode 100644 index 0000000000..5d348fd73e --- /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/tools/test_clipboard.py b/tests/tools/test_clipboard.py index 71f938e256..6e08d7f9bb 100644 --- a/tests/tools/test_clipboard.py +++ b/tests/tools/test_clipboard.py @@ -402,24 +402,28 @@ class TestHasClipboardImage: 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_tts_macos_output.py b/tests/tools/test_tts_macos_output.py index 3e6a3ff256..e890d7ad95 100644 --- a/tests/tools/test_tts_macos_output.py +++ b/tests/tools/test_tts_macos_output.py @@ -22,14 +22,19 @@ class _FakeStreamer: 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 _run_stream(monkeypatch, system_name): 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 _run_stream(monkeypatch, system_name): 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_voice_mode.py b/tests/tools/test_voice_mode.py index 51d921d613..be9f24f6de 100644 --- a/tests/tools/test_voice_mode.py +++ b/tests/tools/test_voice_mode.py @@ -563,12 +563,13 @@ class TestWhisperHallucinationFilter: # ============================================================================ 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 @@ class TestPlayAudioFile: # ============================================================================ 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 @@ class TestMacOSAudioOutputPolicy: 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 @@ class TestMacOSAudioOutputPolicy: 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")