Tests: the browser suite's conftest turns Django's async-safety guard off for every unit test #1160

New issue

Closed

opened 2026-10-31 03:04:25 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-31 03:04:25 +00:00

Owner

What exists today

tests/e2e/conftest.py:33 runs os.environ.setdefault("DJANGO_ALLOW_ASYNC_UNSAFE", "true") at import, before pytest.importorskip("playwright") at :35. The comment at :27-32 scopes the reasoning to the browser tests (Playwright's sync API runs an event loop in the test thread).

The unit run imports that conftest. pyproject.toml:197-203 sets testpaths = ["tests"] and deselects with -m 'not e2e and not release and not step and not heavy'; nothing ignores tests/e2e (no collect_ignore in tests/conftest.py, no --ignore in ci.yml:311-325 or every-python.yml:150-164). Deselection by marker happens after collection, so the directory is collected and its conftest imported. Checked with uv run pytest --co -q -p no:cacheprovider -p no:xdist: 20842/27656 tests collected (6814 deselected) in 49.82s, the deselected set including the browser tests. Each xdist worker collects too, so every worker process runs with the guard off.

Correction to the candidate: today nothing in src/postulo is async apart from the ASGI entry point (config/asgi.py, ASGI_APPLICATION in settings/base.py:40), and no unit test uses AsyncClient or an async test runner, so no test currently depends on the guard. The finding is a trap for the first async view or async test, not a present failure.

What would change

Set the variable only when browser tests run. Either:

in tests/e2e/conftest.py, move it into a session-scoped autouse fixture (with pytest.MonkeyPatch.context() so it is undone), which runs only when a test in that directory is selected; or
in tests/conftest.py, collect_ignore_glob = ["e2e/*"] unless the -m expression selects e2e (read from config.getoption("markexpr") in pytest_ignore_collect), which also spares the unit run collecting the browser tests and importing Playwright in every worker (see C1).

The first is the smaller change and keeps collection as it is; the second also saves collection time. Either keeps ci.yml:159's -m e2e run working.

What it saves or protects

small (reliability): the unit suite keeps the guard that would catch a synchronous ORM call made from async code; no such code exists today, so the risk is latent

Accessibility

none

Decisions

Fixture, or skip collecting tests/e2e in unit runs? Recommended: the fixture now; the collection change belongs with C1's --extra chromium work if that lands.

Found by the efficiency and reliability audit of 2026-10-30 (finding X2; index postulo/postulo#1108). Checked against main at daee30b15 (2026-10-30).

## What exists today `tests/e2e/conftest.py:33` runs `os.environ.setdefault("DJANGO_ALLOW_ASYNC_UNSAFE", "true")` at import, before `pytest.importorskip("playwright")` at `:35`. The comment at `:27-32` scopes the reasoning to the browser tests (Playwright's sync API runs an event loop in the test thread). The unit run imports that conftest. `pyproject.toml:197-203` sets `testpaths = ["tests"]` and deselects with `-m 'not e2e and not release and not step and not heavy'`; nothing ignores `tests/e2e` (no `collect_ignore` in `tests/conftest.py`, no `--ignore` in `ci.yml:311-325` or `every-python.yml:150-164`). Deselection by marker happens after collection, so the directory is collected and its conftest imported. Checked with `uv run pytest --co -q -p no:cacheprovider -p no:xdist`: `20842/27656 tests collected (6814 deselected) in 49.82s`, the deselected set including the browser tests. Each xdist worker collects too, so every worker process runs with the guard off. Correction to the candidate: today nothing in `src/postulo` is async apart from the ASGI entry point (`config/asgi.py`, `ASGI_APPLICATION` in `settings/base.py:40`), and no unit test uses `AsyncClient` or an async test runner, so no test currently depends on the guard. The finding is a trap for the first async view or async test, not a present failure. ## What would change Set the variable only when browser tests run. Either: - in `tests/e2e/conftest.py`, move it into a session-scoped autouse fixture (with `pytest.MonkeyPatch.context()` so it is undone), which runs only when a test in that directory is selected; or - in `tests/conftest.py`, `collect_ignore_glob = ["e2e/*"]` unless the `-m` expression selects `e2e` (read from `config.getoption("markexpr")` in `pytest_ignore_collect`), which also spares the unit run collecting the browser tests and importing Playwright in every worker (see C1). The first is the smaller change and keeps collection as it is; the second also saves collection time. Either keeps `ci.yml:159`'s `-m e2e` run working. ## What it saves or protects small (reliability): the unit suite keeps the guard that would catch a synchronous ORM call made from async code; no such code exists today, so the risk is latent ## Accessibility none ## Decisions - **Fixture, or skip collecting `tests/e2e` in unit runs?** Recommended: the fixture now; the collection change belongs with C1's `--extra chromium` work if that lands. --- Found by the efficiency and reliability audit of 2026-10-30 (finding X2; index postulo/postulo#1108). Checked against `main` at `daee30b15` (2026-10-30).

tiagoagueda added this to the 0.6.0 milestone

2026-10-31 03:04:25 +00:00

tiagoagueda added the

performance

bug

labels

2026-10-31 03:04:25 +00:00

tiagoagueda referenced this issue

2026-10-31 03:04:41 +00:00

Efficiency and reliability audit of 2026-10-30: index #1108

tiagoagueda referenced this issue from a pull request that will close it,

2026-11-10 10:31:54 +00:00