Skip to content

fix(tests): isolate sync test from the module-scoped async fixture - #4

Closed
ClaraVnk wants to merge 1 commit into
mainfrom
fix/redis-loop-teardown
Closed

fix(tests): isolate sync test from the module-scoped async fixture#4
ClaraVnk wants to merge 1 commit into
mainfrom
fix/redis-loop-teardown

Conversation

@ClaraVnk

Copy link
Copy Markdown
Contributor

Contexte

Test (redis) is red on main, independently of any open PR. Baseline run on untouched main: 29729210049110 passed, 1 error.

The error is a fixture teardown, not a test:

ERROR at teardown of test_abs_url_prefers_configured_base_url
RuntimeError: Event loop is closed
tests/test_api.py:26: await app.state.storage.close()
redis/asyncio/connection.py:606: self._writer.close()

The last green run on main was 2026-05-24. The workflow pins python-version: "3.14", which resolved to a different patch release then; it resolves to 3.14.6 today. Nobody broke this — the environment moved.

Changements

CI attributes the finalizer to test_abs_url_prefers_configured_base_url at 37% of the module, meaning the module-scoped client fixture is finalized mid-module instead of at module exit. That test is the only synchronous one in an otherwise fully async module whose client fixture is bound to a module-scoped event loop (see the asyncio_default_fixture_loop_scope = "module" rationale in pyproject.toml, and the earlier fix(tests): module-scope pytest-asyncio loop so Redis fixture survives).

It is a pure unit test of _abs_url — it touches neither the client nor the loop — so it moves to tests/test_urls.py, a module with no async fixtures. test_api.py is now uniformly async.

Tests

Local, CPython 3.14.4, package versions matched to CI (pytest 9.1.1, pytest-asyncio 1.4.0, redis 8.0.1) against redis:7-alpine:

  • sqlite: 110 passed
  • redis: 110 passed
  • ruff check / ruff format --check clean

Risques

This fix is not locally reproducible, and I want that stated plainly. On 3.14.4 the suite is green 5/5 even before the change, and an instrumented finalizer observably runs on a live loop. CI runs 3.14.6, which I could not obtain (uv offers up to 3.14.4). So the premature-finalization diagnosis rests on CI output alone, and CI is the oracle for this change — if Test (redis) goes green here, the diagnosis holds; if not, the root cause is elsewhere and this should be reworked rather than layered on.

Zero production code is touched. Worst case, this is a no-op reorganisation that improves test hygiene.

A more durable follow-up, out of scope here: pin the patch version in the workflow (python-version: "3.14.6") so the toolchain stops drifting silently between runs.

Test (redis) fails in CI with "RuntimeError: Event loop is closed", raised
while the module-scoped `client` fixture closes the Redis connection during
teardown. 110 tests pass; only the finalizer errors.
CI attributes the finalizer to test_abs_url_prefers_configured_base_url, at
37% of the module — the fixture is finalized mid-module rather than at module
exit. That test is the only synchronous one in an otherwise fully async module
whose `client` fixture is bound to a module-scoped event loop.
It is a pure unit test of _abs_url: it uses neither the client nor the loop, so
it moves to its own module with no async fixtures. test_api.py is now uniformly
async.
Not reproducible locally: with CI-identical package versions (pytest 9.1.1,
pytest-asyncio 1.4.0, redis 8.0.1) and redis:7-alpine, the suite is green 5/5
on CPython 3.14.4 and the finalizer observably runs on a live loop. CI runs
3.14.6. The premature finalization is therefore confirmed only from CI output,
and CI is the oracle for this fix.
@ClaraVnk

Copy link
Copy Markdown
ContributorAuthor

Diagnosis falsified — do not merge as-is

CI run on this branch: 29729737295. Test (redis) still fails, 110 passed, 1 error.

The teardown error simply moved to the test now occupying that slot:

ERROR at teardown of test_security_headers_present[asyncio] [ 36% ]
RuntimeError: Event loop is closed

That test is async. The premise of this PR — that a lone synchronous test was triggering premature finalization — is therefore wrong. The module-scoped client fixture finalizes at ~36% of the module regardless of what sits there.

What the new data suggests

The [asyncio] suffix is anyio_backend parametrization: the suite carries @pytest.mark.anyio markers andasyncio_mode = "auto" for pytest-asyncio. Two async plugins are driving the same tests. If anyio runs the coroutines in its own (function-scoped) loop while client is pinned to pytest-asyncio's module-scoped loop, teardown ordering between the two is incoherent — which would fit both the mid-module finalization and the environment sensitivity.

That is a hypothesis, not a conclusion. I have not verified it, and I was already wrong once here.

Status

Leaving this open for the reorganisation itself, which stands on its own (a pure unit test does not belong in a module-scoped-async-fixture module) but fixes nothing. The underlying failure needs a decision about which async plugin owns this suite — a larger change than this PR.

Still unreproducible locally: green 5/5 on CPython 3.14.4 with CI-matched packages and redis:7-alpine. CI runs 3.14.6.

@ClaraVnk

Copy link
Copy Markdown
ContributorAuthor

Closing: the diagnosis behind this PR was falsified by its own CI run (see the comment above — the teardown error simply moved to the async test occupying that slot). The reorganisation is defensible on its own but fixes nothing, and leaving it open would imply a fix is pending here.

The underlying failure needs a decision about which async plugin owns the suite (@pytest.mark.anyio markers coexisting with asyncio_mode = "auto"). That is a larger change and deserves its own issue rather than a stale branch.

Reproducibility groundwork is in #5.

@ClaraVnk
ClaraVnk deleted the fix/redis-loop-teardown branch July 20, 2026 09:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@ClaraVnk