test(update): autostash suite keeps the launchd restart scope off the host (#111866)
The autouse fixture neutralised gateway discovery and the systemd branch but not the launchd one. On a macOS host `_restart_macos_launchd_gateways` derives its labels from the profile layout, so a default profile alone hands it `ai.hermes.gateway`, the label never "comes back", and nine unrelated update tests exit 1 with "Update incomplete". No OS is faked: the seam is stubbed the same way test_update_fleet_restart_pending does.
This commit is contained in:
teknium1
@@ -0,0 +1,135 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Fail when a test file fakes macOS without carrying ``@pytest.mark.macos_only``.
|
||||
|
||||
The OS lanes are marker-driven: ``.github/workflows/tests-os.yml`` selects the
|
||||
files the macOS job imports via ``scripts/ci/list_os_marked_tests.py macos_only``
|
||||
and then runs ``-m macos_only``. A file whose tests only pass because they make
|
||||
the interpreter believe it is on macOS (``is_macos`` patched to ``True``,
|
||||
``sys.platform`` set to ``"darwin"``) but that carries no marker is invisible to
|
||||
that lane: it is green on Linux over a faked branch and never imported on the
|
||||
host it exists for (#111866). Root ``AGENTS.md`` § "Don't fake the host OS" is
|
||||
the rule; this check makes a violation a red job instead of a review catch.
|
||||
|
||||
Flags, per ``tests/**/test_*.py`` without a whole-word ``macos_only``:
|
||||
|
||||
monkeypatch.setattr(mod, "is_macos", lambda: True) / patch(..., return_value=True)
|
||||
monkeypatch.setattr(sys, "platform", "darwin") / patch("sys.platform", "darwin")
|
||||
platform.system patched to return "Darwin"
|
||||
|
||||
Opt out of one line with ``# os-marker: ok — <why>`` on that line (a pure
|
||||
function taking the platform as data is host-independent and stays unmarked).
|
||||
``_BASELINE`` lists the files that already faked macOS when this check landed;
|
||||
they are a burn-down list, not a policy — split the macOS arm out, mark it,
|
||||
and drop the entry (a stale entry fails the check).
|
||||
|
||||
Run: python scripts/ci/check_os_marker_fakes.py [tests_root]
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
MARKER = "macos_only"
|
||||
OPT_OUT = "os-marker: ok"
|
||||
|
||||
_TRUE = r"(?:lambda[^:]*:\s*True|return_value\s*=\s*True|,\s*True\b)"
|
||||
_FAKES = (
|
||||
re.compile(rf"\bis_macos\b.*{_TRUE}"),
|
||||
re.compile(r"\bis_macos\.return_value\s*=\s*True\b"),
|
||||
# setattr(sys, "platform", "darwin") / patch("sys.platform", "darwin") / x.platform = "darwin";
|
||||
# a host-honest READ (`if sys.platform == "darwin":`) is not a fake and does not match.
|
||||
re.compile(r"""\bplatform["']?\s*,\s*["']darwin["']"""),
|
||||
re.compile(r"""\.platform\s*=\s*["']darwin["']"""),
|
||||
re.compile(r"""\bplatform\.system\b.*(?:lambda[^:]*:|return_value\s*=)\s*["']Darwin["']"""),
|
||||
)
|
||||
|
||||
# Files that faked macOS before this check existed (#111866). Burn down, never extend.
|
||||
_BASELINE = frozenset(
|
||||
{
|
||||
"tests/hermes_cli/test_doctor.py",
|
||||
"tests/hermes_cli/test_gateway.py",
|
||||
"tests/hermes_cli/test_gateway_proc_fallback.py",
|
||||
"tests/hermes_cli/test_linux_sandbox_fixup.py",
|
||||
"tests/hermes_cli/test_macos_fda_guidance.py",
|
||||
"tests/hermes_cli/test_orphan_desktop_serve_reap.py",
|
||||
"tests/hermes_cli/test_update_launchd_restart_verification.py",
|
||||
"tests/hermes_cli/test_update_launchd_unloaded_gateway.py",
|
||||
"tests/hermes_cli/test_urllib_security.py",
|
||||
"tests/hermes_state/test_state_synchronous_pragma.py",
|
||||
"tests/test_hermes_constants.py",
|
||||
"tests/tools/test_macos_protected_search.py",
|
||||
"tests/tools/test_skills_tool.py",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _code_lines(text: str) -> list[tuple[int, str, str]]:
|
||||
"""Yield ``(lineno, code, raw)`` with the ``#`` comment stripped from *code*.
|
||||
|
||||
A ``#`` inside a string literal is rare in these patterns and only ever
|
||||
hides a hit (never invents one), so a plain split is the honest trade
|
||||
against a full tokenizer.
|
||||
"""
|
||||
out = []
|
||||
for i, raw in enumerate(text.splitlines(), 1):
|
||||
out.append((i, raw.split("#", 1)[0], raw))
|
||||
return out
|
||||
|
||||
|
||||
def find_unmarked_fakes(root: Path, repo_root: Path) -> dict[str, list[tuple[int, str]]]:
|
||||
"""Map repo-relative test path -> ``[(lineno, line)]`` of un-opted-out macOS fakes."""
|
||||
marker_pat = re.compile(rf"\b{MARKER}\b")
|
||||
hits: dict[str, list[tuple[int, str]]] = {}
|
||||
for dirpath, _dirnames, filenames in os.walk(root):
|
||||
for fname in filenames:
|
||||
if not (fname.startswith("test_") and fname.endswith(".py")):
|
||||
continue
|
||||
path = Path(dirpath) / fname
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
continue
|
||||
if marker_pat.search(text):
|
||||
continue
|
||||
lines = [
|
||||
(n, raw.strip())
|
||||
for n, code, raw in _code_lines(text)
|
||||
if OPT_OUT not in raw and any(p.search(code) for p in _FAKES)
|
||||
]
|
||||
if lines:
|
||||
resolved = path.resolve()
|
||||
base = repo_root if resolved.is_relative_to(repo_root) else root.resolve()
|
||||
hits[resolved.relative_to(base).as_posix()] = lines
|
||||
return hits
|
||||
|
||||
|
||||
def main(argv: list[str]) -> int:
|
||||
repo_root = Path(__file__).resolve().parents[2]
|
||||
root = Path(argv[1]) if len(argv) > 1 else repo_root / "tests"
|
||||
if not root.is_dir():
|
||||
print(f"error: no such directory: {root}", file=sys.stderr)
|
||||
return 2
|
||||
hits = find_unmarked_fakes(root, repo_root)
|
||||
new = {rel: lines for rel, lines in hits.items() if rel not in _BASELINE}
|
||||
stale = sorted(_BASELINE - set(hits))
|
||||
for rel, lines in sorted(new.items()):
|
||||
for n, line in lines:
|
||||
print(f"{rel}:{n}: fakes macOS without @pytest.mark.{MARKER}: {line}")
|
||||
if new:
|
||||
print(
|
||||
f"\n{len(new)} test file(s) make the interpreter believe it is on macOS but carry no "
|
||||
f"`{MARKER}` marker, so the macOS lane never imports them (AGENTS.md § Don't fake the "
|
||||
f"host OS). Split the macOS arm into its own `@pytest.mark.{MARKER}` test that runs the "
|
||||
f"real branch, or mark `# {OPT_OUT} — <why>` on a host-independent line.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
for rel in stale:
|
||||
print(f"{rel}: listed in _BASELINE but no longer fakes macOS — remove the entry")
|
||||
return 1 if new or stale else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main(sys.argv))
|
||||
@@ -0,0 +1,39 @@
|
||||
"""The unmarked-macOS-fake guard (#111866): flags fakes, ignores honest reads and marked files."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "scripts" / "ci"))
|
||||
|
||||
from check_os_marker_fakes import find_unmarked_fakes # noqa: E402
|
||||
|
||||
|
||||
def _write(root: Path, name: str, body: str) -> None:
|
||||
(root / name).write_text(body, encoding="utf-8")
|
||||
|
||||
|
||||
def test_unmarked_fake_is_flagged_marked_and_opted_out_are_not(tmp_path):
|
||||
_write(tmp_path, "test_fake.py", "def test_x(monkeypatch):\n"
|
||||
" monkeypatch.setattr(gw, 'is_macos', lambda: True)\n"
|
||||
" monkeypatch.setattr(sys, 'platform', 'darwin')\n")
|
||||
_write(tmp_path, "test_marked.py", "import pytest\npytestmark = pytest.mark.macos_only\n"
|
||||
"def test_x(monkeypatch):\n monkeypatch.setattr(gw, 'is_macos', lambda: True)\n")
|
||||
_write(tmp_path, "test_opted.py", "def test_x(monkeypatch):\n"
|
||||
" patch('m.is_macos', return_value=True) # os-marker: ok — pure data mapping\n")
|
||||
|
||||
hits = find_unmarked_fakes(tmp_path, tmp_path)
|
||||
|
||||
assert set(hits) == {"test_fake.py"}
|
||||
assert [n for n, _ in hits["test_fake.py"]] == [2, 3]
|
||||
|
||||
|
||||
def test_host_honest_platform_read_is_not_a_fake(tmp_path):
|
||||
_write(tmp_path, "test_read.py", "import sys\n"
|
||||
"def test_x():\n expected = sys.platform == 'darwin'\n"
|
||||
" if sys.platform == 'darwin':\n pass\n"
|
||||
" payload = {'platform': 'darwin'}\n"
|
||||
" # monkeypatch.setattr(gw, 'is_macos', lambda: True) in a comment\n")
|
||||
|
||||
assert find_unmarked_fakes(tmp_path, tmp_path) == {}
|
||||
@@ -57,9 +57,14 @@ def _patch_gateway_discovery():
|
||||
phase's fresh ``from hermes_cli.gateway import ...`` then loads an
|
||||
UNPATCHED copy of the module — silently discarding every mock here and
|
||||
letting real gateway discovery (and real ``os.kill``) run on the dev box.
|
||||
|
||||
The launchd scope is neutralised too: on a macOS host the restart phase
|
||||
derives labels from the profile layout, so a default profile alone hands
|
||||
it ``ai.hermes.gateway`` and the verify step exits 1 (#111866, #110701).
|
||||
"""
|
||||
with patch("hermes_cli.gateway.find_gateway_pids", return_value=[]), \
|
||||
patch("hermes_cli.gateway.supports_systemd_services", return_value=False), \
|
||||
patch("hermes_cli.update_cmd_fleet._restart_macos_launchd_gateways", lambda *a, **k: None), \
|
||||
patch("hermes_cli.gateway.find_profile_gateway_processes", return_value=[]), \
|
||||
patch("hermes_cli.update_inventory.collect_runtime_inventory", return_value=None), \
|
||||
patch("hermes_cli.update_inventory.report_unaccounted_runtimes", return_value=False), \
|
||||
|
||||
Reference in New Issue
Block a user