Add the backend test harness: fixtures, factories, and DB isolation #5

Closed
opened 2026-07-28 04:52:49 +00:00 by claude-bot · 1 comment

Context

pyproject.toml already declares [tool.pytest.ini_options] testpaths = ["tests"]
and dev dependencies on pytest, pytest-asyncio, and httpx — but no tests/
directory exists. There is currently no automated test of any kind in this repo.
This issue lands the harness the other test issues build on.

Scope

Test package layout, database isolation, an API client fixture with authentication
stubbed, and object factories for the core entities.

Implementation notes

  • Create backend/tests/ with conftest.py.
  • Each test gets a clean database. Prefer a file-backed temporary SQLite DB over
    :memory: so WAL mode and PRAGMA foreign_keys=ON behave as they do in production —
    testing against different pragma behaviour than production would defeat the purpose.
  • Build schema via Alembic upgrade head, not Base.metadata.create_all. That way the
    migrations themselves are exercised on every run and cannot silently drift from the models.
  • Override the get_db dependency so the app and the test share one session/transaction.
  • Auth fixture: override get_current_user / require_role to inject a User at a
    chosen UserRole, so authorization can be tested without a live OAuth provider.
  • Factories for Collection, Photo, DateEvidence, DateDecision, ReviewComment,
    BackgroundJob, and User with sensible defaults and keyword overrides.
  • A tmp_path-based storage root fixture so file writes never touch the real ./storage.
  • Fix the clock where tests assert on ordering or timestamps.

Done when

  • pytest runs green from a clean checkout with no manual setup beyond installing dev deps
  • Tests are fully isolated — order does not affect outcomes and no test touches real storage or circa.db
  • Schema comes from Alembic migrations
  • A reviewer-role and an admin-role authenticated client are both available as fixtures

References

  • backend/pyproject.toml (pytest config already present, tests/ missing)
  • backend/app/db/session.py, backend/app/auth/session.py
  • docs/circa-phase1-backlog.md section 10 (QA tickets)

Blocks: every other backend test issue in this milestone.

## Context `pyproject.toml` already declares `[tool.pytest.ini_options] testpaths = ["tests"]` and dev dependencies on `pytest`, `pytest-asyncio`, and `httpx` — but no `tests/` directory exists. There is currently no automated test of any kind in this repo. This issue lands the harness the other test issues build on. ## Scope Test package layout, database isolation, an API client fixture with authentication stubbed, and object factories for the core entities. ## Implementation notes - Create `backend/tests/` with `conftest.py`. - Each test gets a clean database. Prefer a file-backed temporary SQLite DB over `:memory:` so WAL mode and `PRAGMA foreign_keys=ON` behave as they do in production — testing against different pragma behaviour than production would defeat the purpose. - Build schema via Alembic `upgrade head`, not `Base.metadata.create_all`. That way the migrations themselves are exercised on every run and cannot silently drift from the models. - Override the `get_db` dependency so the app and the test share one session/transaction. - Auth fixture: override `get_current_user` / `require_role` to inject a `User` at a chosen `UserRole`, so authorization can be tested without a live OAuth provider. - Factories for `Collection`, `Photo`, `DateEvidence`, `DateDecision`, `ReviewComment`, `BackgroundJob`, and `User` with sensible defaults and keyword overrides. - A `tmp_path`-based storage root fixture so file writes never touch the real `./storage`. - Fix the clock where tests assert on ordering or timestamps. ## Done when - [ ] `pytest` runs green from a clean checkout with no manual setup beyond installing dev deps - [ ] Tests are fully isolated — order does not affect outcomes and no test touches real storage or `circa.db` - [ ] Schema comes from Alembic migrations - [ ] A reviewer-role and an admin-role authenticated client are both available as fixtures ## References - `backend/pyproject.toml` (pytest config already present, `tests/` missing) - `backend/app/db/session.py`, `backend/app/auth/session.py` - `docs/circa-phase1-backlog.md` section 10 (QA tickets) Blocks: every other backend test issue in this milestone.
claude-bot added this to the v0.2.0 milestone 2026-07-28 04:52:49 +00:00
Author

Done in adc6136. CI green.

Done when

  • pytest runs green from a clean checkout with no manual setup beyond installing dev deps
  • Tests are fully isolated — order does not affect outcomes and no test touches real storage or circa.db
  • Schema comes from Alembic migrations
  • A reviewer-role and an admin-role authenticated client are both available as fixtures

The second item was false, and had been for some time. Three suites called ingest_photo() without redirecting storage, and 257 scan files had accumulated in the real backend/storage/ — the directory that on a deployment holds irreplaceable originals. Nothing failed. The tests passed and quietly wrote to disk, which is the only reason it survived this long.

Redirection is now an autouse fixture, so a suite gets isolation whether or not it remembers to ask. That is the same shape as #70's required keyword argument and #87's method-derived write intent: the property holds because forgetting is not expressible, not because everyone remembers.

Verified rather than asserted:

  • a full run now leaves backend/storage empty (it had 257 files; they are deleted);
  • every test file passes on its own, so ordering does not affect outcomes;
  • test_harness_isolation.py asserts the guarantees directly — the configured storage root is outside the repository, writing through the resolved backend leaves the real directory untouched, the schema carries an alembic_version.

Factories. factories.py had one hash helper; twelve files had each grown their own make_photo. It now holds builders for collection, user, album, photo, evidence, decision, comment and job, and the duplicates are gone. Consolidating them surfaced two latent traps rather than just shortening the code:

  • test_authz_matrix used a fixed sha256 per status, which #91's partial unique index would have collided on the day someone made two photos of the same status. It worked only because it made exactly one of each.
  • Most copies ended on refresh(), which on a write-intent session opens a transaction and holds SQLite's write lock for the rest of the test — the exact pattern #87 documents and conftest warns about.

Two design notes worth recording:

make_decision requires an author because the schema does — created_by is NOT NULL on a decision and nullable on evidence. That asymmetry is the schema saying something true: evidence can come from a machine, a conclusion cannot.

make_decision deliberately does not recompute the projection. A factory that quietly did would hide the drift rebuild_projections exists to detect (#79), and a test could then pass against a photo whose stored state no route could produce.

One thing I got wrong on the way: the autouse fixture originally created its storage directory, which broke test_upload_filename_safety — that suite asserts on the exact contents of its tmp_path to prove a hostile filename wrote nothing it should not have. A fixture that leaves a trace when unused is a fixture other tests can see, so it now lets LocalStorage create its own root on first resolution.

Done in adc6136. CI green. **Done when** - [x] `pytest` runs green from a clean checkout with no manual setup beyond installing dev deps - [x] Tests are fully isolated — order does not affect outcomes and no test touches real storage or `circa.db` - [x] Schema comes from Alembic migrations - [x] A reviewer-role and an admin-role authenticated client are both available as fixtures **The second item was false, and had been for some time.** Three suites called `ingest_photo()` without redirecting storage, and **257 scan files had accumulated in the real `backend/storage/`** — the directory that on a deployment holds irreplaceable originals. Nothing failed. The tests passed and quietly wrote to disk, which is the only reason it survived this long. Redirection is now an **autouse** fixture, so a suite gets isolation whether or not it remembers to ask. That is the same shape as #70's required keyword argument and #87's method-derived write intent: the property holds because forgetting is not expressible, not because everyone remembers. Verified rather than asserted: - a full run now leaves `backend/storage` **empty** (it had 257 files; they are deleted); - **every test file passes on its own**, so ordering does not affect outcomes; - `test_harness_isolation.py` asserts the guarantees directly — the configured storage root is outside the repository, writing through the resolved backend leaves the real directory untouched, the schema carries an `alembic_version`. **Factories.** `factories.py` had one hash helper; twelve files had each grown their own `make_photo`. It now holds builders for collection, user, album, photo, evidence, decision, comment and job, and the duplicates are gone. Consolidating them surfaced two latent traps rather than just shortening the code: - `test_authz_matrix` used a **fixed `sha256` per status**, which #91's partial unique index would have collided on the day someone made two photos of the same status. It worked only because it made exactly one of each. - Most copies ended on **`refresh()`**, which on a write-intent session opens a transaction and holds SQLite's write lock for the rest of the test — the exact pattern #87 documents and `conftest` warns about. **Two design notes worth recording:** `make_decision` requires an author because the schema does — `created_by` is NOT NULL on a decision and nullable on evidence. That asymmetry is the schema saying something true: evidence can come from a machine, a conclusion cannot. `make_decision` deliberately does **not** recompute the projection. A factory that quietly did would hide the drift `rebuild_projections` exists to detect (#79), and a test could then pass against a photo whose stored state no route could produce. **One thing I got wrong on the way:** the autouse fixture originally created its storage directory, which broke `test_upload_filename_safety` — that suite asserts on the exact contents of its `tmp_path` to prove a hostile filename wrote nothing it should not have. A fixture that leaves a trace when unused is a fixture other tests can see, so it now lets `LocalStorage` create its own root on first resolution.
Sign in to join this conversation.
No description provided.