Turning CI on surfaced `atomic_write` as intermittently failing under load —
self-inflicted. Its fault injection is `CREATE TRIGGER ... ON outbox`, which takes
an ACCESS EXCLUSIVE lock on a table every other suite is concurrently inserting
into. The header even claimed it "cannot affect any test running in parallel";
that was wrong — scoping the trigger by app_id bounds which ROWS it rejects, not
the table lock installing it takes. Shipping that alongside "run the DB tests in
CI" would have poisoned the signal.
The fix is the one already used for the e2e suites: a private database per test.
That logic (build a migrated template once, clone it per test via
`CREATE DATABASE ... TEMPLATE`) was duplicated in `picloud/tests/common`, and this
would have been a third copy — so it moves into a shared `picloud-test-support`
dev-dependency crate, and `picloud/tests/common` now re-exports it. The invariant
it encodes: manager-core's claim loops are global by design, so a test must not
share a database with anything that runs them OR with anything that does DDL.
Moved onto it:
- `atomic_write` — the DDL fault injection above; its per-test cleanup is now
deleted (the database is thrown away).
- `outbox_reclaim` — asserts `reclaim_stale_claims` returns exactly 0, a count
over the WHOLE outbox table. On the shared DB any stale row from any other suite
(or a killed picloud that died holding a claim) would fail it permanently, until
someone cleared the table by hand.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`dispatcher_e2e` failed about one run in three, on a different test each time,
and never under `--test-threads=1`. Root cause, not timing:
Each test calls `build_app`, which spawns a REAL dispatcher, and they all shared
one database — so they shared one `outbox`. `OutboxRepo::claim_due` is
deliberately not app-scoped (in production one dispatcher serves the whole
instance, so claiming any due row is correct). Test A's dispatcher would
therefore claim test B's row, test A would finish, its server would be dropped
mid-dispatch, and the claim was stranded. Test B polled for its handler's side
effect until it timed out.
The harness was modelling something production never does: N independent
instances sharing one database. So the fix belongs in the harness. Weakening the
dispatcher (app-scoping `claim_due`) would be wrong, and shortening
`PICLOUD_OUTBOX_CLAIM_TIMEOUT_SEC` to fit a 10s test window would make production
abandon the live claims of scripts that may legitimately run 300s.
`tests/common` now hands each test its own database. Replaying 73 migrations per
test is too slow, so the first caller builds a migrated TEMPLATE database once
and every test clones it (`CREATE DATABASE ... TEMPLATE` is a file copy), guarded
by a cluster-wide advisory lock because Postgres will not clone a template that
has an open connection. Names are derived from (suite, test), so a test reclaims
its own database on entry: a crashed run leaves nothing to clean up.
Only the five local `pool_or_skip` wrappers change — all 33 call sites are
untouched, and the skip-when-`DATABASE_URL`-is-unset contract is preserved.
(`#[sqlx::test]` already does this and is what `api.rs`/`authz.rs` use, but it
requires `DATABASE_URL` and would force `#[ignore]`, silently dropping these
tests from the default `cargo test --workspace` gate.)
This also stops the e2e tests leaving stranded claims behind in the shared dev
database, which is the likely source of the occasional lone journey failure.
Before: dispatcher_e2e red ~1 run in 3. After: workspace green twice over,
journeys 157/157.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
