Tests share one database and global settings, so packages must run one at a time #192

New Issue

2026-10-06T21:25:47Z

cgalo5758 commented

2026-10-06 21:25:47 +00:00

make test runs packages one at a time because tests commit rows others read. Tests assert on subsets, filter out others' rows, and change the personal org type's default ladder and restore it. About twenty test files set viper, because production reads it directly. #33 is one symptom.

Done when: each test runs in its own transaction, or in a database cloned from a migrated template when it must commit; settings are passed in; and make test runs packages in parallel.

`make test` runs packages one at a time because tests commit rows others read. Tests assert on subsets, filter out others' rows, and change the personal org type's default ladder and restore it. About twenty test files set viper, because production reads it directly. #33 is one symptom. Done when: each test runs in its own transaction, or in a database cloned from a migrated template when it must commit; settings are passed in; and `make test` runs packages in parallel.

cgalo5758 added the

kind

debt

area/testing labels 2026-10-06 21:25:47 +00:00

cgalo5758 referenced this issue

2026-10-06 21:26:11 +00:00

The forum mapping tests pass only when another package has run first #33

cgalo5758 commented

2026-10-09 03:46:03 +00:00

Every test that uses the shared test databases now reaches them through internal/testkit/dbtest: Open (or OpenE2E for the plan-management test), and DSN where a test needs the address itself. Open refuses a database whose core ledger is behind, and no test migrates the shared databases, so test/reset-test-db.sh is their only migrator. A cloned database or a per-test transaction would change what Open and DSN return, not the tests.

Two tests open a second connection from DSN, and each needs it to reach the database Open returned:

TestInvoiceIssuedPaymentPaidBackfill, whose subject is migration 00022, writes its account row through Open. Then, over a second, simple-protocol connection (DSN with Connect) and inside a transaction it rolls back, it drops issued_at and paid_at and replays 00022's Up section on the shared database. That transaction holds ACCESS EXCLUSIVE on core.invoices and core.payments until the rollback.
TestRunValidateConfig_DB seeds a config override row through Open and passes DSN to validate-config, whose own code opens a pool from it and must read that row.

So Open and DSN have to return the same clone, and a per-test transaction would hide the seeded rows from each test's second connection; that option has to handle both tests.

One more test changes the shared schema while it runs. TestPoolAssignments_DedupPreflightDemotesDuplicate, whose subject is migration 00010, drops the unique index core.uq_pool_assignments_one_primary_per_workspace outside any transaction, runs its copy of 00010's pre-flight statement, and recreates the index (its cleanup recreates it again if it is missing). It uses only Open, so nothing changes for DSN. But a package running in parallel on the same database would see the index gone, and under a per-test transaction its DROP INDEX would hold ACCESS EXCLUSIVE on core.pool_assignments until the rollback. The other tests that apply a migration do so on their own scratch databases.

One sentence of the test-database-isolation spec is stale as a result. Its reset requirement says: "Migrating as part of the reset is load-bearing: it leaves every parallel test package's own migration call a no-op, so concurrent packages do not race to create the same objects." No package migrates the shared test databases any more. The next change to that capability rewrites it.

Every test that uses the shared test databases now reaches them through `internal/testkit/dbtest`: `Open` (or `OpenE2E` for the plan-management test), and `DSN` where a test needs the address itself. `Open` refuses a database whose core ledger is behind, and no test migrates the shared databases, so `test/reset-test-db.sh` is their only migrator. A cloned database or a per-test transaction would change what `Open` and `DSN` return, not the tests. Two tests open a second connection from `DSN`, and each needs it to reach the database `Open` returned: - `TestInvoiceIssuedPaymentPaidBackfill`, whose subject is migration 00022, writes its account row through `Open`. Then, over a second, simple-protocol connection (`DSN` with `Connect`) and inside a transaction it rolls back, it drops `issued_at` and `paid_at` and replays 00022's Up section on the shared database. That transaction holds ACCESS EXCLUSIVE on `core.invoices` and `core.payments` until the rollback. - `TestRunValidateConfig_DB` seeds a config override row through `Open` and passes `DSN` to validate-config, whose own code opens a pool from it and must read that row. So `Open` and `DSN` have to return the same clone, and a per-test transaction would hide the seeded rows from each test's second connection; that option has to handle both tests. One more test changes the shared schema while it runs. `TestPoolAssignments_DedupPreflightDemotesDuplicate`, whose subject is migration 00010, drops the unique index `core.uq_pool_assignments_one_primary_per_workspace` outside any transaction, runs its copy of 00010's pre-flight statement, and recreates the index (its cleanup recreates it again if it is missing). It uses only `Open`, so nothing changes for `DSN`. But a package running in parallel on the same database would see the index gone, and under a per-test transaction its `DROP INDEX` would hold ACCESS EXCLUSIVE on `core.pool_assignments` until the rollback. The other tests that apply a migration do so on their own scratch databases. One sentence of the test-database-isolation spec is stale as a result. Its reset requirement says: "Migrating as part of the reset is load-bearing: it leaves every parallel test package's own migration call a no-op, so concurrent packages do not race to create the same objects." No package migrates the shared test databases any more. The next change to that capability rewrites it.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: wiki-cafe/member-console#192