From 87040b59b1385480391a314760fb34294baa8dcc Mon Sep 17 00:00:00 2001 From: Trevor Hudson Date: Thu, 30 Jul 2026 16:16:50 -0700 Subject: [PATCH] Run every slow e2e suite in CI, not just one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI ran 3 of the 27 slow tests. The other 24 existed only on a maintainer's laptop, which meant three of the six supported runtimes — cloudflare, dbos-ts, inngest — had no automated proof at all. The fan_out regression fixed in #76 was caught by exactly one of those suites running locally; on CI alone it would have merged. The job's comment said the TS/Docker suites "stay opt-in (they need Node/Docker)". That premise was never checked: ubuntu-latest ships both. The real cost is minutes, and three runtimes' worth of live coverage is worth minutes. Three jobs now, split by toolchain rather than by whether anyone remembered: - **Python e2e** — every slow suite needing no external toolchain. Previously only test_mcp_e2e; now also park-on-auth, the DBOS Python runtime, the OAuth device flow, the MCP-trigger server and the empirical eval path. - **TypeScript e2e** — cloudflare, inngest, the TS agent loop, MCP over the TS SDK, and the cloudflare/inngest park-on-auth suites. - **DBOS-TS e2e (Docker)** — split out because the DBOS TypeScript SDK is Postgres-only, so these two start a throwaway container. npm's download cache is keyed on `_ts_common.py`, since each suite installs into its own emitted directory and there is no lockfile for setup-node to key on. `tests/test_ci_workflow.py` keeps this from rotting: it asks pytest which files carry slow tests and asserts each appears in a CI job, so adding a slow suite without wiring it up fails rather than silently going unrun. It also catches ci.yml referencing a test file that no longer exists. Verified by unwiring a suite and watching it fail with that suite named. Marking a suite slow is a statement about the toolchain it needs, not about whether it should run. 1172 fast pass; ruff, strict mypy, sanity-check clean. The new jobs are proven by this PR's own CI run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SBuSDL7Svm7iDzgmhTpDSW --- .github/workflows/ci.yml | 98 +++++++++++++++++++++++++++++++++++++-- tests/test_ci_workflow.py | 84 +++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 tests/test_ci_workflow.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51d9d3f..27284b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,9 +67,9 @@ jobs: e2e: # Pure-Python slow e2e tests (no Node/Docker toolchain): they spawn a - # real FastMCP server / DBOS SQLite app and run emitted code end to end, - # so the empirical proof is automated on every PR — not just `-m slow` - # locally. The TS/Docker e2e suites stay opt-in (they need Node/Docker). + # real FastMCP server / DBOS SQLite app and run emitted code end to + # end, so the empirical proof is automated on every PR — not just + # `-m slow` locally. name: Python e2e (slow) runs-on: ubuntu-latest steps: @@ -82,5 +82,93 @@ jobs: - name: Install package with dev extras run: python -m pip install -e ".[dev]" - - name: MCP backend e2e (live mock MCP server + DBOS SQLite) - run: pytest tests/test_mcp_e2e.py -m slow -v + # Every slow suite that needs no external toolchain. Previously + # only test_mcp_e2e ran here, which left the DBOS Python runtime, + # park-on-auth, the OAuth flow and the MCP-trigger server proven + # only on a maintainer's laptop. + - name: Python e2e (MCP backend, DBOS, park-on-auth, OAuth, serve) + run: | + pytest -m slow -v \ + tests/test_mcp_e2e.py \ + tests/test_mcp_park_e2e.py \ + tests/test_mcp_oauth_e2e.py \ + tests/test_dbos_e2e.py \ + tests/test_dbos_serve_e2e.py \ + tests/test_eval_empirical_dbos.py \ + tests/test_serve_server.py + + ts-e2e: + # The TypeScript runtimes' live suites. These were opt-in on the + # assumption that a runner lacks the toolchain — ubuntu-latest ships + # Node and npm, so the only real cost is time. Worth it: the emitted + # TS is three of the six supported runtimes, and a fan_out regression + # that only these suites catch would otherwise reach main unnoticed. + name: TypeScript e2e (slow) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: "3.12" + + - uses: actions/setup-node@v4 + with: + node-version: "22" + + - name: Install package with dev extras + run: python -m pip install -e ".[dev]" + + # Each suite npm-installs into its own emitted directory, so there + # is no lockfile for setup-node to key a cache on; cache the npm + # download cache itself instead. + - name: Cache npm downloads + uses: actions/cache@v4 + with: + path: ~/.npm + key: npm-e2e-${{ runner.os }}-${{ hashFiles('src/rote/adapters/_ts_common.py') }} + restore-keys: npm-e2e-${{ runner.os }}- + + - name: Cloudflare / Inngest / agent-loop / MCP e2e + run: | + pytest -m slow -v \ + tests/test_cloudflare_e2e.py \ + tests/test_inngest_e2e.py \ + tests/test_ts_agent_loop_e2e.py \ + tests/test_mcp_ts_e2e.py \ + tests/test_mcp_park_cf_e2e.py \ + tests/test_mcp_park_inngest_e2e.py + + dbos-ts-e2e: + # Split from ts-e2e because these two also need Docker: the DBOS + # TypeScript SDK is Postgres-only (no SQLite parity with DBOS + # Python), and the suite starts a throwaway container when no + # reachable Postgres is configured. + name: DBOS-TS e2e (slow, Docker) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: "3.12" + + - uses: actions/setup-node@v4 + with: + node-version: "22" + + - name: Install package with dev extras + run: python -m pip install -e ".[dev]" + + - name: Cache npm downloads + uses: actions/cache@v4 + with: + path: ~/.npm + key: npm-dbosts-${{ runner.os }}-${{ hashFiles('src/rote/adapters/_ts_common.py') }} + restore-keys: npm-dbosts-${{ runner.os }}- + + - name: DBOS TypeScript e2e (live runtime against Docker Postgres) + run: | + pytest -m slow -v \ + tests/test_dbos_ts_e2e.py \ + tests/test_mcp_park_ts_e2e.py diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py new file mode 100644 index 0000000..d592db1 --- /dev/null +++ b/tests/test_ci_workflow.py @@ -0,0 +1,84 @@ +"""Every slow test suite must actually run somewhere in CI. + +`@pytest.mark.slow` excludes a suite from the default run, which is the +point — they need Node, Docker, or minutes. The failure mode is that +excluding them locally also excluded them from CI: for a long stretch +only ``test_mcp_e2e.py`` ran there, so 24 of 27 slow tests existed +solely on a maintainer's laptop. Three of the six supported runtimes +had no automated proof at all, and a fan_out regression that only the +Cloudflare e2e catches would have reached main unnoticed. + +Marking a suite slow is a statement about the *toolchain it needs*, not +about whether it should run. This test makes adding a slow suite without +wiring it into a CI job a failure rather than an omission. +""" + +from __future__ import annotations + +import re +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +CI_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "ci.yml" + + +def _slow_test_files() -> set[str]: + """Collect the slow suite by asking pytest, not by globbing. + + A file is only slow if pytest actually collects a slow test from it, + so this can't drift from the markers. + """ + proc = subprocess.run( + [ + sys.executable, + "-m", + "pytest", + "tests/", + "-m", + "slow", + "-q", + "--collect-only", + "--no-header", + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + timeout=300, + ) + assert proc.returncode == 0, f"collection failed:\n{proc.stdout}\n{proc.stderr}" + return { + line.split("::")[0].rsplit("/", 1)[-1] + for line in proc.stdout.splitlines() + if line.startswith("tests/") and "::" in line + } + + +def test_every_slow_suite_is_wired_into_a_ci_job() -> None: + collected = _slow_test_files() + # Sanity: if collection returns nothing the assertion below is vacuous. + assert len(collected) >= 10, f"expected a substantial slow suite, got {sorted(collected)}" + + referenced = set(re.findall(r"tests/(test_\w+\.py)", CI_WORKFLOW.read_text(encoding="utf-8"))) + missing = sorted(collected - referenced) + + assert not missing, ( + f"These slow suites run nowhere in CI: {missing}. Add them to a job in " + f"{CI_WORKFLOW.relative_to(REPO_ROOT)} — 'Python e2e' for pure-Python " + f"suites, 'TypeScript e2e' for Node ones, 'DBOS-TS e2e' if they also " + f"need Docker. Marking a suite slow says what toolchain it needs, not " + f"that it should go unrun." + ) + + +def test_ci_does_not_reference_deleted_test_files() -> None: + """A renamed suite must not leave CI silently running nothing. + + `pytest path/that/does/not/exist.py` exits 4, so this would surface + as a red job — but only on the next push that happens to touch CI. + Failing here names the stale path directly. + """ + referenced = set(re.findall(r"tests/(test_\w+\.py)", CI_WORKFLOW.read_text(encoding="utf-8"))) + absent = sorted(name for name in referenced if not (REPO_ROOT / "tests" / name).is_file()) + assert not absent, f"ci.yml references test files that no longer exist: {absent}"