From 829b9a3d23d29a62e27413b112f91751acfe0826 Mon Sep 17 00:00:00 2001 From: njbrake Date: Thu, 6 Aug 2026 15:16:49 +0000 Subject: [PATCH] fix(ci): make the endpoint-coverage check offline and deterministic --- .github/workflows/ci.yml | 9 +-- AGENTS.md | 21 ++++--- tests/unit/test_endpoint_coverage.py | 86 +++++++++------------------- 3 files changed, 46 insertions(+), 70 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ea3a45..43c63d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,8 +36,9 @@ jobs: # Integration tests skip automatically when no gateway is on PATH. run: uv run pytest - - name: Endpoint-coverage drift gate - # Fetches the canonical gateway OpenAPI spec and fails if it exposes an - # endpoint absent from sdk-endpoints.txt ([covered] or [excluded]). - # Network is available in CI, so this must not skip here. + - name: Endpoint-coverage manifest checks + # sdk-endpoints.txt is pushed here by the gateway's codegen workflow. The + # drift gate that compares it against the OpenAPI spec now runs in the + # gateway, against the spec from the same commit; these checks are the + # offline structural ones. See the docstring of the test module. run: uv run pytest tests/unit/test_endpoint_coverage.py -v diff --git a/AGENTS.md b/AGENTS.md index d1b98c3..8a54546 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,12 +46,17 @@ Resolved in `_base.py` from constructor args, then environment: `Otari-Key` header; `api_base` is **required** in this mode. Error mapping applies in both modes; do not regress one when changing the other. -### Endpoint-coverage drift gate -`tests/unit/test_endpoint_coverage.py` fetches the canonical gateway spec -(`https://raw.githubusercontent.com/mozilla-ai/otari/main/docs/public/openapi.json`) and -asserts every gateway endpoint is accounted for in `sdk-endpoints.txt` (`[covered]` or -`[excluded]` with a reason). A new gateway endpoint with no wrapper and no explicit exclusion -fails CI. When you add or intentionally skip an endpoint, update `sdk-endpoints.txt`. +### Endpoint-coverage manifest +`sdk-endpoints.txt` records which gateway endpoints this SDK surfaces (`[covered]`) and which it +deliberately does not (`[excluded]`, with a reason). **It is a generated artifact.** The gateway's +codegen workflow pushes it here from the canonical copy at `scripts/sdk_codegen/sdk-endpoints.txt` +in `mozilla-ai/otari`, so an edit made in this repo is overwritten on the next regeneration. To +change coverage classification, edit the canonical copy in the gateway. + +`tests/unit/test_endpoint_coverage.py` only checks the manifest's structure, offline. The drift gate that compares it +against the OpenAPI spec runs in the gateway, against the spec from the same commit. It used to run +here over the network, which made the result depend on when CI ran rather than on the commit; see +mozilla-ai/otari#438. ## Setup Commands - Install (dev): `uv sync --extra dev` @@ -60,7 +65,7 @@ fails CI. When you add or intentionally skip an endpoint, update `sdk-endpoints. - Full suite: `uv run pytest` - Unit only: `uv run pytest tests/unit` - Single test: `uv run pytest tests/unit/test_client.py::TestOtariClient::test_completion -v` -- Drift gate (needs network): `uv run pytest tests/unit/test_endpoint_coverage.py -v` +- Manifest checks: `uv run pytest tests/unit/test_endpoint_coverage.py -v` - Integration tests under `tests/integration/` spawn / require a real gateway and are skipped when one is not available. @@ -85,7 +90,7 @@ fails CI. When you add or intentionally skip an endpoint, update `sdk-endpoints. map errors correctly. - Touched streaming → run the streaming tests; verify chat yields `ChatCompletionChunk` and responses/messages yield raw dicts. -- Added/removed an endpoint wrapper → update `sdk-endpoints.txt` and run the drift gate. +- Added/removed an endpoint wrapper → update the canonical `sdk-endpoints.txt` in `mozilla-ai/otari` (`scripts/sdk_codegen/`); the copy here is regenerated. - Always run `uv run ruff check .` and `uv run mypy src/` before opening a PR. ## Writing style diff --git a/tests/unit/test_endpoint_coverage.py b/tests/unit/test_endpoint_coverage.py index 00b7390..c0bd101 100644 --- a/tests/unit/test_endpoint_coverage.py +++ b/tests/unit/test_endpoint_coverage.py @@ -1,29 +1,26 @@ -"""Endpoint-coverage drift gate. - -Fetches the canonical otari gateway OpenAPI spec and asserts that every API -endpoint it exposes is accounted for in ``sdk-endpoints.txt`` -- either wrapped -by this SDK's public surface (``[covered]``) or deliberately deferred -(``[excluded]``). A new gateway endpoint in neither section fails this test, -so a future endpoint (as ``/messages`` once was) cannot silently go unsurfaced. - -The fetch uses :mod:`urllib.request` (stdlib) so the test runs in the normal -suite. It is skipped offline (network error / ``OTARI_SKIP_NETWORK_TESTS=1``) -but runs in CI, where the network is available. +"""Endpoint-coverage manifest checks. + +``sdk-endpoints.txt`` records which gateway endpoints this SDK surfaces +(``[covered]``) and which it deliberately does not (``[excluded]``). The file is +a generated artifact: the gateway's codegen workflow pushes it here alongside +the generated core, from the canonical copy at +``scripts/sdk_codegen/sdk-endpoints.txt`` in ``mozilla-ai/otari``. + +The drift gate itself lives in the gateway, where the manifest is validated +against ``docs/public/openapi.json`` from the same commit. It used to live here +and fetch the spec from ``main`` over the network at test time, which made the +result depend on when the test ran rather than on what the commit contained: an +unchanged commit passed one day and failed the next, and because CI only runs on +push and pull_request, ``main`` sat red unnoticed for over two weeks +(mozilla-ai/otari#438). What remains here is offline and deterministic. """ from __future__ import annotations -import json -import os -import urllib.error -import urllib.request from pathlib import Path -import pytest - -SPEC_URL = "https://raw.githubusercontent.com/mozilla-ai/otari/main/docs/public/openapi.json" MANIFEST = Path(__file__).resolve().parents[2] / "sdk-endpoints.txt" -HTTP_METHODS = {"get", "post", "put", "patch", "delete"} +HTTP_METHODS = frozenset({"GET", "POST", "PUT", "PATCH", "DELETE"}) def parse_manifest(text: str) -> tuple[set[str], set[str]]: @@ -54,50 +51,23 @@ def parse_manifest(text: str) -> tuple[set[str], set[str]]: return covered, excluded -def spec_endpoints(spec: dict) -> set[str]: - """Extract ``METHOD /path`` pairs from an OpenAPI doc, dropping meta routes.""" - eps: set[str] = set() - for path, methods in spec.get("paths", {}).items(): - if path == "/health" or path.startswith("/health/"): - continue - for method in methods: - if method.lower() in HTTP_METHODS: - eps.add(f"{method.upper()} {path}") - return eps - - -def fetch_spec() -> dict: - if os.environ.get("OTARI_SKIP_NETWORK_TESTS") == "1": - pytest.skip("OTARI_SKIP_NETWORK_TESTS=1") - try: - with urllib.request.urlopen(SPEC_URL, timeout=30) as resp: - return json.loads(resp.read()) - except (urllib.error.URLError, TimeoutError) as exc: - pytest.skip(f"could not fetch otari OpenAPI spec from {SPEC_URL}: {exc}") - - -def test_manifest_parses() -> None: +def test_manifest_sections_are_non_empty() -> None: covered, excluded = parse_manifest(MANIFEST.read_text()) assert covered, "manifest [covered] section is empty" - assert not (covered & excluded), f"endpoints in both sections: {sorted(covered & excluded)}" + assert excluded, "manifest [excluded] section is empty" -def test_spec_endpoints_are_accounted_for() -> None: +def test_manifest_sections_are_disjoint() -> None: covered, excluded = parse_manifest(MANIFEST.read_text()) - spec = spec_endpoints(fetch_spec()) - accounted = covered | excluded - unaccounted = sorted(spec - accounted) - assert not unaccounted, ( - "Gateway OpenAPI exposes endpoint(s) the SDK does not account for: " - f"{unaccounted}. Add a public wrapper and list under [covered], or " - "defer it under [excluded] with a reason, in sdk-endpoints.txt." - ) + both = sorted(covered & excluded) + assert not both, f"endpoints in both [covered] and [excluded]: {both}" -def test_manifest_has_no_stale_entries() -> None: - """Warn (not fail) if a manifest entry no longer exists in the spec.""" +def test_manifest_entries_are_well_formed() -> None: covered, excluded = parse_manifest(MANIFEST.read_text()) - spec = spec_endpoints(fetch_spec()) - stale = sorted((covered | excluded) - spec) - if stale: - pytest.skip(f"manifest entries not present in current spec (review): {stale}") + malformed = sorted( + entry + for entry in covered | excluded + if entry.split(" ", 1)[0] not in HTTP_METHODS or not entry.split(" ", 1)[1].startswith("/") + ) + assert not malformed, f'manifest entries are not "METHOD /path": {malformed}'