Add an end-to-end reviewer flow test #12
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
Component and API tests each verify one side of the contract. The Phase 1 definition
of done is stated as a user journey, so one test should walk that journey end to end
against a real backend.
Scope
An end-to-end test covering: sign in, browse, open a photo, inspect evidence, comment,
edit notes, submit a decision, and request a rerun.
Implementation notes
Depends on the dev-login bypass issue in this milestone; the e2e suite should use that
path, and the bypass must be impossible to enable in production.
POST /api/ingestrather than writingDB rows directly, so ingest is exercised as part of the flow.
pendingapprovedconflict state and no silent overwrite.
unit and contract suites.
Done when
References
docs/circa-phase1-plan.mdsection 12 (definition of done)10.1.1.14perAGENTS.mdDepends on: the dev-login bypass and the CI pipeline.
Done in
24613aa.e2e/, Playwright, headless Chromium — 6 tests, all passing against a real stack.Nothing is stubbed
A real browser, a real
vite previewof the built bundle, a real FastAPI process, a real SQLite file migrated by real Alembic. Everything else in the repo tests one side of a contract; this is the only thing that can show the pieces are wired to each other.The bundle rather than the dev server, deliberately: what a reviewer loads is the output of
npm run build, and a suite that only ever exercised the dev server would not notice a build that produces something else. That needed one fix —vite.config.tsgained apreview.proxy, becausepreviewdoes not inheritserver, and without it every/apirequest is answered by Vite's own 404 page (an outcome that looks like a backend failure and is not one).The journeys
Reviewer — one test, not nine. The steps are not independent (you cannot decide on a photo you have not opened), and splitting them would mean either nine sign-ins and nine ingests or shared state that makes a failure in step 3 look like a failure in step 7.
Sign in → land on the shell → browse → filter by
pending(and assert the seeded photo disappears, since a filter that returns everything is not a filter) → open it → see filename evidence from #7 → confirm the image actually loads (naturalWidth, so the media route, the storage key and the file on disk all agree) → comment → edit notes and check the revision from #66 → approve with a range and rationale → confirm the projection followed the decision (#79 — a decision recorded but not reaching the photo is the silent-wrong-data failure this milestone is about) → queue an AI rerun → see it on the Jobs page.Plus three smaller ones: a viewer is not shown work they may not do, an unauthenticated visitor gets the sign-in page rather than the shell, and the API refuses a write the UI does not offer — that last one through the API precisely because there is no button to click, so the control's absence is not the only thing stopping the write.
Conflict — the one the plan calls the top risk. Two independent browser contexts, two cookie jars, one photograph. Bob saves, Alice's decision is refused, she is told the current version, and every versioned control is disabled until she reloads. The archive is then read back and asserted to hold Bob's work, not Alice's and not a mixture. Reload restores her page, shows her what she was about to overwrite, and her decision then lands. A second test does the same for notes — the path that used to fail open (#11).
Each layer of that defence is already tested alone (#77, #9, #11). What only this can show is that they are connected.
Choices worth naming
POST /api/ingest, as the issue asks. A seeded row would be a row no ingest path ever produced — exactly the fixture that lets a broken ingest pass an end-to-end test. The fixture signs in as an admin to do it, so it exercises #8's role boundary rather than assuming it away.globalTeardown. A per-row cleanup would leave the stored originals behind, which is the half people forget — and this way a run touches neither a developer's database norbackend/storage/.Running it
CI has a new
e2ejob onmcr.microsoft.com/playwright:v1.56.1-noble(pinned to the version ine2e/package.json— a mismatch fails at run time with "Executable doesn't exist"), with Python added. Locally,cd e2e && npx playwright test; on a machine without a browser or Node,./e2e/run-in-docker.shruns the identical suite in a container, which is how it was verified here.Done when
README updated with how to run it.