Repository navigation
epic(testing): code review — testing #710
Description
Summary
The suite is fast and broad (837 tests, 91 % line and branch coverage), but it has four structural weaknesses:
- It isn't hermetic. Proxy and OpenTelemetry environment variables make "offline" tests reach the network or stall.
- The conformance kit can pass an agent that never calls the model.
- Six of the eight framework adapters are never tested against their real library.
- Nothing measures coverage or detects order-dependence.
Test run summary (HEAD 11b806b)
| Command | Environment | Result |
|---|---|---|
pytest -q (defaults exclude local_gateway, sandbox, benchmark) |
maintainer venv, Python 3.12, most framework extras | 837 passed, 4 skipped, 13 deselected (22 s) |
pytest -q -m local_gateway |
same | 9 passed, 1 skipped |
pytest -q -m benchmark |
same | 1 passed: 18 µs per call against a 1 ms budget |
CONTRIBUTING setup pip install -e ".[dev,llm,cli]", then pytest -q |
fresh Python 3.10 | 748 passed, 49 skipped |
CI install .[dev,llm,cli,local,otel], then pytest -q |
fresh Python 3.10 (resolves openai 3.x) | 830 passed, 11 skipped |
mypy, ruff check ., lint-imports |
fresh Python 3.10 | all pass |
mypy |
venv with framework extras (numpy via crewai) | fails: numpy stubs don't parse under python_version = "3.10" |
ruff format --check . |
— | 51 files would be reformatted (not enforced) |
pytest -q on Python 3.13 and 3.14 |
fresh venvs | 830 passed, 11 skipped (neither version is in CI) |
pytest -q at the declared floors pytest==8.0.0, pytest-asyncio==0.23.0 |
fresh Python 3.10 | INTERNALERROR during collection |
pytest -q in an agent sandbox that sets HTTPS_PROXY |
maintainer venv | stalled for more than 10 min; a real outbound request went through the proxy |
HTTPS_PROXY=http://127.0.0.1:9 pytest on the simulate and conformance tests |
maintainer venv | 24 failed, 17 passed |
pytest -q with OTEL_EXPORTER_OTLP_ENDPOINT exported |
maintainer venv | 7 real span exports to that collector |
| Full suite in reverse order | maintainer venv | 1 failed (test_transport.py::test_correlation_and_attribution_headers_injected) |
Coverage by package (line and branch; default run plus local_gateway)
| Package | Statements | Coverage | Weakest modules |
|---|---|---|---|
conformance |
472 | 73 % | plugin ~30 % and report 34 % (they run only in pytester subprocesses, so they're unmeasured); gateway 56 % without local_gateway |
llm |
57 | 83 % | client 78 % (owned sync client and list_models untested) |
top level (donkey.py, governance.py) |
257 | 87 % | governance 64 % |
provisioning |
416 | 90 % | planner 67 %, lint 76 %, applier 78 % |
core |
1,571 | 94 % | cache 79 %, doctor 86 %, telemetry 90 %, transport 92 % |
simulator |
462 | 94 % | server 57 % without local_gateway |
registry and tools |
252 | 97–98 % | — |
integrations |
257 | 99 % | inflated: the native framework classes are test stubs |
| Total | 3,744 | 91 % |
Proposed conventions / architecture
- Hermetic tests by default. The root conftest owns environment, network and module-state isolation, and tests that need the real environment opt in with a marker. Use
--strict-markers --strict-config -raandxfail_strict = true. - No passes by default. Every conformance scenario, smoke test and offline test must assert that something positive happened: at least one call through the Donkey client, the native client built, or the expected fixture served. A do-nothing agent must fail.
- One adapter table drives CI. Generate the PR and nightly matrices from
ADAPTERS. Deep adapters block PRs using the real framework; shallow adapters run nightly with the real framework and a strict smoke test. - Coverage only goes up. Enforce
fail_underat the current baseline, and give every new module a test file that imports it directly. - Streaming paths are tested with unread streams, not
MockTransportbodies that are pre-read. - Property-based tests for parsers (Hypothesis):
classify(), the budget and rate-limit headers,Retry-After,parse_headersand config precedence. - Live checks run on a schedule, in a protected environment, against the captured fixtures.
Steps
- test: make the suite hermetic — clear proxy/OTEL/DONKEY env, block real sockets, cap test time #747 (p1, 0.1.2)
- test(ci): run framework-dependent tests against real framework installs #748 (p1, 0.1.2)
- test(conformance): implement or retire the internal 20-scenario conformance matrix #749 (p2, 0.1.2)
- test: reset module-level state between tests and randomise test order in CI #750 (p2, 0.1.2)
- test(ci): measure coverage in CI and enforce a floor that only goes up #751 (p2, 0.1.2)
- test(fixtures): extend the integrity lock to every captured fixture #752 (p3, 0.1.2)
- test: scheduled live-contract check against the fixtures, plus benchmark trending #753 (p3, 0.1.2)
Related existing issues
- conformance: wire the Phase 2–2.8 scenario bodies as their features land #444 conformance: wire the Phase 2–2.8 scenario bodies.
- perf: a latency and overhead budget enforced in CI #234 perf: a latency and overhead budget enforced in CI.
- ops: air-gapped and private-cloud operation, proved by an egress-denied CI job #320 ops: air-gapped operation, proved by an egress-denied CI job.
- Verify the eight framework constructor signatures against installed packages (§8) #34 framework constructor verification.
Filed from the code review of 2026-09-30 (commit 11b806b).
Activity
tommasobolis commentedon Oct 6, 2026
All seven sub-issues (#747–#753) are merged. The follow-up #991 (restoring the retired conformance scenarios) is blocked on verification of the MCP/Exchange surfaces, so it was detached and moved to Phase 1.3.