retoor/devplacepy
1
0
Fork 1
Code Issues 44 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

Report every test failure in one pass and fix the whole suite

Browse Source 
The suite ran with -x, so a run stopped at the first failure and finding N
failures cost N full runs. Move -rf into the pytest addopts so every run
lists each failure, and add the triage targets test-fast (unit + api, no
browser), test-failed (--last-failed), test-first-failure (the old -x),
test-slowest and test-cache-clean. A stale .pytest_cache holding node ids
from deleted files made --last-failed select everything; make clean and
test-cache-clean drop it.

Fix the fifteen failures this surfaced.

DeepSearch crawling raised AttributeError in its finally block on every run:
async_playwright().__aenter__() returns a Playwright, which has no __aexit__.
Use start()/stop() at both call sites.

Update the tests left behind by changed signatures: VectorStore is async now,
fetch_page takes a browser, _summary_payload takes dom_evidence, and the
messages/notifications page compounds take a user_uid.

Close four real flakes that fail-fast had been hiding, all of them late in
the run. Harvest assertions pinned crop.reward_coins while is_golden pays
five times on about five percent of harvests, so they now assert through
realizable_harvest_coins with the observed golden flag. Market saturation
fixtures assumed a single active farm and landed two tiers milder once the
api and e2e tiers had created farms, so they scale by active_farms(). The
primary-administrator container test raced the one second cross-worker cache
version window and now waits for the server to agree. The isslop tools test
matched the collapsed nav dropdown link instead of the tools grid card.

Stop burning ninety seconds waiting out server-side display caches:
DEVPLACE_RANKING_TTL and DEVPLACE_MARKET_SATURATION_TTL follow the existing
sitemap and home cache precedent and are zero for the suite, taking the
leaderboard test from 60.6s to 4.4s and the saturation test from 30.1s to
under a second.

2881 passed, 1 skipped in 15:13.
This commit is contained in:
retoor
2026-07-26 17:23:00 +02:00
parent 4780016980
commit f996336afb
17 changed files with 149 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/CLAUDE.md
+33 -4
View File
@@ -8,6 +8,26 @@ This file documents detailed testing patterns, fixtures, and pitfalls for devpla
During iteration a single test may be run for a fast feedback loop: `python -m pytest tests/e2e/feed.py::test_name -v --tb=line -x` - but the change is finished only when the complete suite is green.
## Finding failures fast
The suite does NOT stop at the first failure. `-rf` lives in `pyproject.toml` `addopts` (one source of truth, so a bare `python -m pytest ...` gets it too), and every run prints a `FAILED <nodeid>` line per failure at the end - the complete failure list from a single pass instead of one full pass per bug. Triage cheapest tier first:
| Command | Covers | Cost |
|---|---|---|
| `make test-fast` | unit + api, no browser | ~3 min |
| `make test-unit` | unit only | ~50 s |
| `make test-api` | api only | ~2 min |
| `make test-e2e` | Playwright browser tier | the expensive one |
| `make test` | all three tiers | full |
| `make test-failed` | only what failed last run (`--last-failed`) | proportional to the failure count |
| `make test-first-failure` | all three tiers with `-x` | stops at failure 1 |
| `make test-slowest` | all three tiers + the 40 slowest tests | full |
| `make test-cache-clean` | drops `.pytest_cache/` when `--last-failed` misbehaves | instant |
**Server-side display caches are disabled for the suite, never waited out.** A test that changes state the server serves from a TTL cache must not poll until the TTL expires - that burns the TTL in wall-clock on every run (the leaderboard ranking test cost 60s and the Code Farm saturation test 30s exactly this way). Each such cache reads its TTL from an env var with the production value as the default, and `conftest.py` sets it to `0` before the server subprocess starts: `DEVPLACE_SITEMAP_TTL` (`seo.SITEMAP_TTL`), `DEVPLACE_HOME_CACHE_TTL` (`main._home_cache`), `DEVPLACE_RANKING_TTL` (`database/ranking.py` `_authors_cache`), `DEVPLACE_MARKET_SATURATION_TTL` (`services/game/store/market.py` `_saturation_cache`). Add a new display cache to that list rather than sleeping in the test. **Correctness caches are different**: the cross-worker `cache_state` version read (`_cache_version_cache`, 1s) is load-bearing and stays on, so a test that flips the admin set from the test process must wait for the server to agree before asserting (see `tests/e2e/admin/containers/manage.py` `_await_instance_visible`), never assume the change is instant.
The loop: `make test-fast` -> fix -> `make test-failed` until it is empty -> `make test` as the final gate. `make test-first-failure` is only for the rare case where one early failure poisons every test after it (a corrupted session fixture, a stopped service). Pytest's cache lives in `.pytest_cache/` (git-ignored), so `--last-failed` works with no extra setup; do not pass `-p no:cacheprovider` in a run whose failures you intend to re-run. **If `make test-failed` runs the whole suite instead of just the failures, the cache is polluted**: pytest keeps `lastfailed` entries for tests it did not collect, so node ids from deleted files (or from a stray `pytest` invocation that walked `data/`/`var/`) linger forever and make `--last-failed` give up and select everything. `make test-cache-clean` drops `.pytest_cache/` and fixes it; `make clean` does it too.
## Fixture stack
- `app_server` (session-scoped): spawns uvicorn as a subprocess on port 10501 (the `PORT` constant) with a tempfile DB.
@@ -15,12 +35,12 @@ During iteration a single test may be run for a fast feedback loop: `python -m p
- `page` (function-scoped): `clear_cookies()` on the session context, then a fresh page.
- `alice` / `bob`: seeded users `alice_test` / `bob_test` logged in via the login form. `bob` gets its own context for multi-user tests. Returns `(page, user_dict)`.
Tests run on port 10501 with a tempfile SQLite DB and a dedicated `DEVPLACE_DATA_DIR`, and `DEVPLACE_DISABLE_SERVICES=1` so background services don't start. `make test` runs **serially, one test at a time, in a single process**: one isolated DB + data dir + uvicorn subprocess + Chromium, shared across the whole session, so session fixtures (`seeded_db`) and intra-file ordering hold. Serial execution is enforced centrally in `pyproject.toml` (`[tool.pytest.ini_options]` `addopts = "--tb=line -p no:xdist"`); pytest-xdist is no longer a dependency and `-n` is rejected, so the suite can never run concurrently.
Tests run on port 10501 with a tempfile SQLite DB and a dedicated `DEVPLACE_DATA_DIR`, and `DEVPLACE_DISABLE_SERVICES=1` so background services don't start. `make test` runs **serially, one test at a time, in a single process**: one isolated DB + data dir + uvicorn subprocess + Chromium, shared across the whole session, so session fixtures (`seeded_db`) and intra-file ordering hold. Serial execution is enforced centrally in `pyproject.toml` (`[tool.pytest.ini_options]` `addopts = "--tb=line -rf -p no:xdist"`); pytest-xdist is no longer a dependency and `-n` is rejected, so the suite can never run concurrently.
## General
- **Around 1959 tests across `tests/{unit,api,e2e}/`.** Playwright integration + HTTP + unit tests. All must pass before any merge.
- **Tests use `-x` (fail-fast).** The suite stops at the first failure. Fix that test, then re-run.
- **Around 2882 tests across `tests/{unit,api,e2e}/`.** Playwright integration + HTTP + unit tests. All must pass before any merge.
- **The suite runs to completion and reports every failure** (`-rf`), it does not stop at the first one. See "Finding failures fast" above.
- **Validate each touched language manually:** Python (compile + import), JS (parse / bracket matching), CSS (brace matching), HTML (tag matching). Zero tolerance.
- Playwright is used directly, NOT pytest-playwright (uninstall it if present - it conflicts).
@@ -100,9 +120,18 @@ Opening a URL with a `#comment-<uid>` fragment scrolls the comment into view and
## Failure handling
- **Failure screenshots auto-save** to `/tmp/devplace_test_screenshots/`.
- **Tests stop at first failure** (`-x` flag in Makefile). No cascading failures.
- **Every failure is reported in one pass** (`-rf` in the Makefile); re-run just those with `make test-failed`.
- **If the server won't start, kill leftover processes:** `kill -9 $(pgrep -f "uvicorn")`
## Never assert against a formula's base value when the real call has probabilistic or global inputs
Two whole-suite flakes came from tests that pinned an exact number the production formula does not actually guarantee. Both were invisible under `-x` because they sit late in the run.
- **Golden crops.** `economy.is_golden(plot_uid, planted_at)` is a deterministic hash, but its inputs are freshly generated per run, so ~5% of harvests pay `GOLDEN_MULTIPLIER` (5x). `assert result["coins"] == crop.reward_coins` is therefore a 5%-per-harvest flake. Every harvest-reward assertion must go through `economy.realizable_harvest_coins(crop, ..., golden=result["golden"])` - `store.harvest` returns `golden` precisely so the test can.
- **Global market state.** `economy.supply_days` divides by `market_store.active_farms()`, a **count over every farm in the database**, so a saturation fixture computed as `worst_days * 86400 / crop.grow_seconds` is only correct when exactly one farm is active. After the api/e2e tiers have created farms it lands two tiers milder and the assertion fails. Scale the seeded harvest count by `market_store.active_farms()` (see `_worst_tier_harvests` in `tests/unit/services/game/store.py`).
The rule generalizes: if the production function reads a random-ish input or a table-wide aggregate, assert against **that same function's output for the observed state**, never against the bare constant.
## Common pitfalls
| Pitfall | Fix |