Commit 3a378ed
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
- control_arena
- export
- tests
- writers
- scorers
- settings/infra
- tools/tests
- utils/tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
64 | 85 | | |
65 | 86 | | |
66 | 87 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
53 | 56 | | |
| 57 | + | |
| 58 | + | |
54 | 59 | | |
55 | 60 | | |
56 | 61 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
72 | 76 | | |
73 | 77 | | |
74 | 78 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| |||
172 | 172 | | |
173 | 173 | | |
174 | 174 | | |
175 | | - | |
176 | 175 | | |
177 | 176 | | |
178 | 177 | | |
| |||
278 | 277 | | |
279 | 278 | | |
280 | 279 | | |
281 | | - | |
| 280 | + | |
282 | 281 | | |
283 | 282 | | |
284 | 283 | | |
| |||
0 commit comments