Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -244,6 +254,7 @@ jobs:
needs:
- detect
- tests
- tests-os
- lint
- js-tests
- installer-tests
Expand Down
152 changes: 152 additions & 0 deletions .github/workflows/tests-os.yml
Original file line number Diff line number Diff line change
@@ -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: ""
38 changes: 38 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <marker>`. 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
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Expand Down
115 changes: 115 additions & 0 deletions scripts/ci/list_os_marked_tests.py
Original file line number Diff line number Diff line change
@@ -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))
Loading
Loading