Skip to content

Commit 3a378ed

Browse files
Rogan-Inglisclaude
andauthored
fix: make the trio test suite pass, and say why where it cannot (#853)
`pytest --runtrio` had **102 failures**. After this it has **0** (214 passed). The default suite is unaffected: 1132 passed. ## Where the 102 went | Cause | Count | Ours? | Action | |---|---:|---|---| | `kubernetes-asyncio` is aiohttp-based | 67 | No -- library choice | Skipped in the infra setting, with the reason recorded | | Reading `EvalLog.samples` under trio | 34 | No -- upstream, **now released** | Fixed by an inspect floor bump; the tests run | | `aiofiles` is asyncio-only | 1 (+3 untested paths) | **Yes** | Fixed | ## The upstream fix is released, so nothing is gated any more An earlier version of this PR carried a version-gated skip for the 34: `eval_async()` returns logs whose `samples` is a `LazyList`, and touching it calls inspect's *sync* `read_eval_log()`, which inspect refuses to run under trio. inspect_ai #4676 fixes it, and at the time it was on inspect's main but in no release. It is released now -- 0.3.252, 4 August. Verified directly: ``` 0.3.250 (previous floor) RuntimeError: read_eval_log cannot be called from a trio async context 0.3.252 works 0.3.257 (current) works ``` So `control_arena/conftest.py`, the `inspect_log_trio_bug` marker and its eight `pytestmark` lines are gone, replaced by `inspect-ai>=0.3.252`. Those 34 tests now **run** under trio rather than being skipped, and there is no gate left for anyone to remember to delete. The full suite passes on 0.3.252 and on 0.3.257; the lock takes the latter. One of the eight was ours rather than upstream's. `test_tool_output_e2e` calls the sync `read_eval_log_sample` from async code, which inspect refuses under trio at any version. It uses the public `read_eval_log_async` now -- fixed rather than skipped. ## aiofiles -- the part that was actually ours aiofiles resolves its executor via `asyncio.get_event_loop()` and raises `no running event loop` under trio. Replaced in the two remaining production call sites (`export/writers/directory.py`, `scorers/_run_monitors.py`) and one test helper; the third, in `iac_fast/scorers.py`, disappeared when #852 rewrote that function. `aiofiles` and `types-aiofiles` are dropped from the dependencies. `copy_eval_files_async` did not want `anyio.open_file` at all -- it read a whole file into memory and wrote it back out. It hands `shutil.copyfile` to a worker thread now, which streams. An .eval log can be hundreds of megabytes. ## The tests for that change were mocking it out `test_directory_writer.py` patched `aiofiles.open`, now `anyio.open_file`, in all ten tests. So the trio variants passed without ever opening a file -- they exercised everything except the thing this PR changes. They write to `tmp_path` and assert on file contents now, which is both simpler (no `AsyncMock` `__aenter__` dance, no indexing into `write.call_args_list`) and real coverage of `anyio.open_file` under both backends. `test_file_encoding` asserted that `encoding="utf-8"` was passed. It now writes `café ✅ 日本語` and checks it survives the round trip, including as raw bytes in the file -- it fails if `ensure_ascii=False` is dropped, which the kwarg assertion did not. ## The infra setting -- a real limitation, not a test artefact Every infra task reaches the cluster through `kubernetes-asyncio`, which is built on aiohttp, and aiohttp is asyncio-only: ``` kubernetes_asyncio/client/api_client.py:76 kubernetes_asyncio/client/rest.py:68 aiohttp/connector.py:313: RuntimeError: no running event loop ``` Most of the 67 were that RuntimeError. The rest were assertion failures like `assert None == {'access-key': ...}`, which looked unrelated until traced -- a broad `except Exception` in the task had swallowed the same error and returned `None`. A conftest under `settings/infra/` skips trio variants there with that reason, and the README now records it. **The infra setting genuinely does not work under trio** and will not until its Kubernetes client is replaced. That conftest is a third of its previous size. It uses `pytest_runtest_setup`, which pytest calls only for the conftests covering an item's path, so it needs none of the manual `INFRA_DIR` filtering the collection hook required -- `pytest_collection_modifyitems` is handed *every* collected item, and the first version of this file, which forgot to filter, silently skipped the entire trio suite. It also shares `anyio_backend_of` with the root conftest instead of being a third copy of it. <details> <summary>A dead end worth recording</summary> I started migrating our own direct `aiohttp.ClientSession` use in those tasks to httpx, and got four files in before pulling a full traceback and discovering the failures come from `kubernetes-asyncio`, not from our calls. Four of the six failing task modules do not use aiohttp directly at all. I reverted it: it would have fixed nothing, while changing scorer code that cannot be exercised without a cluster. (It also introduced a bug on the way -- `as client` shadowing the imported `kubernetes_asyncio.client` module, which would have been an `UnboundLocalError` at a line executed earlier in the same function.) Separately worth knowing: `aiohttp` is not a declared dependency of this project. It arrives via `kubernetes-asyncio` and `miniopy-async`, and several task modules import it directly. </details> ## CI runs trio now CI ran `pytest -n 8 -m "not skip_ci and not long_running"` with no `--runtrio`, so **nothing ran these tests**. Going from 102 failures to 0 without a job to hold it there is a snapshot, not a fix -- 102 is what silent drift looks like. There is a `Unit Tests (trio)` job now, alongside a note in CONTRIBUTING that both invocations are needed for full coverage. Also regenerates `THIRD_PARTY_LICENSES.md`, which still listed `types-aiofiles` and was failing the `checks` job. ## Verification - Trio: **214 passed, 0 failed**. Asyncio: **1132 passed**. - `ruff check`, `ruff format`, `pyright` (0 errors), and the licence check all clean. - Up to date with main, including #851 and #860. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent aea8b86 commit 3a378ed

12 files changed

Lines changed: 230 additions & 238 deletions

File tree

‎.github/workflows/checks.yml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,27 @@ jobs:
6161
- name: Pytest
6262
run: uv run pytest -n 8 -m "not skip_ci and not long_running"
6363

64+
trio-tests:
65+
name: Unit Tests (trio)
66+
runs-on: ubuntu-latest
67+
steps:
68+
- name: Checkout repository
69+
uses: actions/checkout@v4
70+
- name: Install uv
71+
uses: astral-sh/setup-uv@v6
72+
with:
73+
enable-cache: true
74+
- name: Set up Python
75+
run: uv python install 3.12
76+
- name: Install the project
77+
run: uv sync --dev
78+
# Trio variants run in their own invocation: the asyncio pass leaves
79+
# global state behind that is not valid under trio. Without this job
80+
# nothing runs them, and the backend the project claims to support
81+
# regresses silently -- which is how it reached 102 failures.
82+
- name: Pytest (trio backend)
83+
run: uv run pytest -n 8 --runtrio -m "not skip_ci and not long_running"
84+
6485
long-running-tests:
6586
name: Long Running Tests
6687
runs-on: ubuntu-latest

‎CONTRIBUTING.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,15 @@ uv run pytest -n 8 -m "not skip_ci"
4747

4848
# Run just the area you are working on, which is usually seconds
4949
uv run pytest -n 8 control_arena/tools control_arena/monitor
50+
51+
# Run the trio variants of async tests -- CI runs this as a separate job
52+
uv run pytest -n 8 --runtrio -m "not skip_ci and not long_running"
5053
```
5154

5255
Pass `-n 8` to run tests in parallel with `pytest-xdist`, as CI does. Without it the suite is several times slower.
5356

57+
Trio variants run in their own invocation because the asyncio pass leaves global state behind that is not valid under trio. `--runtrio` runs *only* them, and the default invocation skips them, so both are needed for full coverage.
58+
5459
#### Test markers
5560

5661
Two markers control where a test runs, and CI has a job for each:

‎README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ trio, set the environment variable before running:
6969
export INSPECT_ASYNC_BACKEND=trio
7070
```
7171

72+
One exception: the **infra** setting is asyncio-only. It talks to Kubernetes
73+
through `kubernetes-asyncio`, which is built on aiohttp, and aiohttp does not
74+
run under trio.
75+
7276
### Example Use
7377

7478
In this example, we evaluate the [defer to

‎THIRD_PARTY_LICENSES.md‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Third-Party Licenses
22

3-
This file is auto-generated. Last updated: 2026-07-28
3+
This file is auto-generated. Last updated: 2026-08-12
44

55
The Control Arena project is licensed under the MIT License - see the LICENSE file in the root directory.
66

@@ -30,7 +30,7 @@ The Control Arena project is licensed under the MIT License - see the LICENSE fi
3030
- cfgv (3.5.0)
3131
- charset-normalizer (3.4.6)
3232
- cohere (5.21.1)
33-
- control_arena (17.1.3.dev9)
33+
- control_arena (19.0.1.dev6)
3434
- debugpy (1.8.20)
3535
- deepdiff (9.0.0)
3636
- docstring_parser (0.17.0)
@@ -46,7 +46,7 @@ The Control Arena project is licensed under the MIT License - see the LICENSE fi
4646
- hatchling (1.29.0)
4747
- identify (2.6.18)
4848
- iniconfig (2.3.0)
49-
- inspect_ai (0.3.250)
49+
- inspect_ai (0.3.257)
5050
- jedi (0.19.2)
5151
- jiter (0.13.0)
5252
- jmespath (1.1.0)
@@ -172,7 +172,6 @@ The Control Arena project is licensed under the MIT License - see the LICENSE fi
172172
- tornado (6.5.5)
173173
- trove-classifiers (2026.1.14.14)
174174
- types-PyYAML (6.0.12.20250915)
175-
- types-aiofiles (25.1.0.20251011)
176175
- types-networkx (3.6.1.20260321)
177176
- types-requests (2.33.0.20260327)
178177
- types-seaborn (0.13.2.20251221)
@@ -278,7 +277,7 @@ The Control Arena project is licensed under the MIT License - see the LICENSE fi
278277

279278
### Other Licenses
280279

281-
- agent-client-protocol (0.11.1): UNKNOWN
280+
- agent-client-protocol (0.12.0): UNKNOWN
282281
- aiohappyeyeballs (2.6.1): Python Software Foundation License
283282
- certifi (2025.11.12): Mozilla Public License 2.0 (MPL 2.0)
284283
- defusedxml (0.7.1): Python Software Foundation License

0 commit comments

Comments
 (0)