b7b545f090
## Summary `buzz-db` tests that create their own databases or roles dropped them as the last line of the test body. A failed assertion or a timeout skipped that line and left the database or role on the shared Postgres server. Seven files carried their own copy of `create_scratch_db_through` and its drop helper. - Adds `ScratchDb` and `ScratchRole` guards to `crates/buzz-db/src/test_support.rs`. Their `Drop` runs `DROP DATABASE IF EXISTS ... WITH (FORCE)` or `DROP ROLE IF EXISTS` on a fresh admin connection, on a separate thread with its own runtime. `ScratchSchema` already works this way, so the guards work under both runtime flavours and while a panicking test unwinds. `WITH (FORCE)` ends any sessions still open on the database, so pools need not be closed first. - A failed drop prints a warning instead of panicking, because a panic during unwinding would abort the test binary. - The seven per-file helpers and every end-of-test `DROP DATABASE` in `buzz-db` now use the guard. The replica-fence masked-activity test's least-privilege role uses `ScratchRole`. Admin pools and `close()` calls that only served cleanup are removed. Test behaviour is otherwise unchanged. - `crates/buzz-db/TESTING.md` now points new tests at the guards. - The second commit fixes a comment in `armed_pool_rejects_old_channel_inserts_through_public_api` that still described the old end-of-test drop. **Existing leftovers need dropping by hand.** This change stops new leftovers but does not remove the ones earlier failed runs already left in dev `buzz-postgres` containers. There were 24 on the author's machine, named `stall_r_*`, `buzz_admin_*`, `floor_probe_*`, `fence_*` and `cxl_*`. List them, check the list, then drop each one: ```sql SELECT datname FROM pg_database WHERE datname ~ '^(stall_r|buzz_admin|floor_probe|fence|cxl)_'; DROP DATABASE "<name>" WITH (FORCE); ``` The guards are private to `buzz-db`. `buzz-push-gateway`, `buzz-admin` and `buzz-relay` tests still create and drop scratch databases by hand, which could get a shared copy of the guards in a follow-up. ### Related issue None found. This continues the test-cleanup work in #8727. ### Testing - Ran the 70 affected `buzz-db` tests against the local `buzz-postgres` container: 68 passed. Neither failure comes from this change. `cluster_global_probe_commits_tokens_and_sessions_prove_coverage` hit the known `MaskedActivity { masked: 1 }` flake under load and passed when run alone. Because it panicked while holding a `ScratchDb`, it also showed a panicking test leaves nothing behind. `serving_lease_statement_timeout_cancels_slow_statement` failed with `VersionMismatch(56)`, because the shared dev database already has another branch's migration 56 applied. - After the run, the server's database and role lists matched the snapshot taken before it. - A temporary test panicked while holding an open transaction, an exclusive table lock and a live session as the scratch role. It left no database or role behind under either runtime flavour and printed no drop warnings. That test is not part of this PR. - `cargo fmt --check` and `cargo clippy -p buzz-db --all-targets -- -D warnings` pass. The tests ran before the commits were cherry-picked onto current main. Only clippy was re-run afterwards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
3.9 KiB
PostgreSQL-backed tests in buzz-db
The dedicated PostgreSQL CI lane discovers tests and Cargo packages by structure rather than by exact lists. Follow this checklist so a new database test is run automatically and remains safe under parallel execution.
Adding a test
- Put the test in a module whose name ends in
postgres_tests. - Mark it
#[ignore = "requires Postgres"]so infrastructure-free unit-test jobs stay fast. - Connect through
crate::test_support::database_url(). The CI wrapper sets this helper's environment to a unique database for each test process; never hard-code the shared development database. - Keep tests that need infrastructure beyond PostgreSQL and Redis in an
external_infra*_testsmodule. The PostgreSQL lane excludes those tests. - Run
scripts/test-postgres-test-discovery.shafter adding or moving the test. The same guard runs in CI immediately after changed-path detection.
The wrapper isolates destructive tests by dropping the entire per-test
database after the process exits. It does not DELETE rows or TRUNCATE
shared tables, so tests may run concurrently without coordinating cleanup.
The wrapper drops only that per-test database. A test that needs another
database or role on the server creates it with test_support::ScratchDb or
test_support::ScratchRole, whose Drop removes it even when the test
panics. Do not end a test with a manual DROP DATABASE or DROP ROLE: a
failed assertion skips it and leaves the object behind.
Choose the schema intentionally
Most tests use the committed desired-state schema from schema/schema.sql,
a manifest that includes one schema/tables/public/<table>.sql per table plus
its types, functions, partitions, triggers, and seed rows.
That is the default and is appropriate for data-access behavior.
Tests in migration::postgres_tests receive an empty database and own the
embedded migration lifecycle. A test outside that module that intentionally
depends on migration-created triggers or seed rows must prefix its function
name with migration_schema_; it also receives an empty database with
BUZZ_TEST_SCHEMA_MODE=migration.
Helpers that normally run migrations honor BUZZ_TEST_SCHEMA_MODE=desired in
the default lane. Do not rerun migrations against a desired-state database.
When behavior should match in both schema paths, add explicit desired-state and
migration-applied coverage rather than making the bootstrap implicit.
Tests that inspect cluster-wide PostgreSQL state or open least-privilege
sessions include cluster_global_ in the function name. Migration-backed cases
use migration_schema_cluster_global_. Nextest serializes this small group
because separate databases still share pg_stat_activity and roles.
Run the lane locally
Start native PostgreSQL and Redis, activate Hermit, and run:
. ./bin/activate-hermit
scripts/test-postgres-test-discovery.sh
scripts/postgres-test-run.sh
Set BUZZ_POSTGRES_ADMIN_URL to a PostgreSQL maintenance database owned by a
role that can create and drop databases. Set PGHOST, PGPORT, PGUSER, and
PGPASSWORD for desired-state bootstrap, plus REDIS_URL for Redis-backed
tests. The complete privilege-boundary inventory also needs CREATEROLE and
membership in pg_read_all_stats, or an ephemeral superuser as CI uses.
The runner creates one desired-state source database per invocation and clones it for ordinary tests. Migration-mode tests start empty. Cleanup retries transient disconnect races before reporting a warning.
Invite upgrade tests start from the complete schema through migration 0074, including channel join roles, entity deletion approval, the unlisted channel setting and the user deletion target and fences. They exercise the invitation schema expansion (migration 0075) through both migration and desired-state paths against populated data, retaining a non-default channel join role. Historical repair is validated separately from the application/schema change.