The test kit opens whatever database TEST_DATABASE_URL names, the application's included #224

New Issue

2026-10-16T01:52:38Z

cgalo5758 commented

2026-10-16 01:52:38 +00:00

What a contributor runs into

Package tests reach the shared test databases through internal/testkit/dbtest. Open (internal/testkit/dbtest/open.go:59) and Counted (internal/testkit/dbtest/counted.go:27) open the address in TEST_DATABASE_URL. DSN (open.go:46) hands that address to tests that connect on their own, and Scratch (internal/testkit/dbtest/scratch.go:28) creates its scratch databases next to it. OpenE2E (open.go:74) opens E2E_DATABASE_URL for test/e2e/plan-management. The only check, ledgerBehind (open.go:146), fails a database whose core migrations are behind. The application's own database is migrated too, so it passes.

This has happened. A verification run was told to set TEST_DATABASE_URL to the application database's address, MC_DB_DSN in test/.env. The package tests committed their organizations into it. The browser walkthroughs drive the running console on that same database, and one of them then failed on rows it had not made.

test/reset-test-db.sh already refuses to reset the application database, but only by one name: it stops when the database is called member_console (test/reset-test-db.sh:77-80). CONTRIBUTING.md:36-37 tells a contributor to "point TEST_DATABASE_URL at a migrated Postgres", which any application database is.

Why it costs

The tests commit rows and delete none, and some change rows every test shares or drop and recreate an index while they run (#192 lists them). Pointed at an application database, they write into data a person is using, and the damage shows up later and somewhere else, as it did in the walkthrough. Nothing stops the same mistake against any database the address can reach.

Finishing #192 does not cover this. A per-test transaction or a cloned database still starts from whatever database the address names.

Where

internal/testkit/dbtest/open.go: DSN, Open, OpenE2E and opened.
internal/testkit/dbtest/counted.go and internal/testkit/dbtest/scratch.go, which take their address from DSN.
test/reset-test-db.sh:77-80.
CONTRIBUTING.md:36-37.

Done when

dbtest refuses a database whose name does not end in _test or _e2e, the suffixes of the names test/bootstrap-stack.sh writes (member_console_test and member_console_e2e, test/bootstrap-stack.sh:355-356). Open, Counted and OpenE2E check the name current_database() reports before the test runs anything. DSN checks the name in the address and refuses an address that names no database. The refusal fails the test and names the variable and the database.
test/reset-test-db.sh applies the same rule instead of refusing one name.
CONTRIBUTING.md says the database's name must end that way.
browsertest.AppDB (test/e2e/browsertest/session.go:31), which opens the running console's database on purpose for the walkthroughs, stays as it is.

## What a contributor runs into Package tests reach the shared test databases through `internal/testkit/dbtest`. `Open` (`internal/testkit/dbtest/open.go:59`) and `Counted` (`internal/testkit/dbtest/counted.go:27`) open the address in `TEST_DATABASE_URL`. `DSN` (`open.go:46`) hands that address to tests that connect on their own, and `Scratch` (`internal/testkit/dbtest/scratch.go:28`) creates its scratch databases next to it. `OpenE2E` (`open.go:74`) opens `E2E_DATABASE_URL` for `test/e2e/plan-management`. The only check, `ledgerBehind` (`open.go:146`), fails a database whose core migrations are behind. The application's own database is migrated too, so it passes. This has happened. A verification run was told to set `TEST_DATABASE_URL` to the application database's address, `MC_DB_DSN` in `test/.env`. The package tests committed their organizations into it. The browser walkthroughs drive the running console on that same database, and one of them then failed on rows it had not made. `test/reset-test-db.sh` already refuses to reset the application database, but only by one name: it stops when the database is called `member_console` (`test/reset-test-db.sh:77-80`). `CONTRIBUTING.md:36-37` tells a contributor to "point `TEST_DATABASE_URL` at a migrated Postgres", which any application database is. ## Why it costs The tests commit rows and delete none, and some change rows every test shares or drop and recreate an index while they run (#192 lists them). Pointed at an application database, they write into data a person is using, and the damage shows up later and somewhere else, as it did in the walkthrough. Nothing stops the same mistake against any database the address can reach. Finishing #192 does not cover this. A per-test transaction or a cloned database still starts from whatever database the address names. ## Where - `internal/testkit/dbtest/open.go`: `DSN`, `Open`, `OpenE2E` and `opened`. - `internal/testkit/dbtest/counted.go` and `internal/testkit/dbtest/scratch.go`, which take their address from `DSN`. - `test/reset-test-db.sh:77-80`. - `CONTRIBUTING.md:36-37`. ## Done when - `dbtest` refuses a database whose name does not end in `_test` or `_e2e`, the suffixes of the names `test/bootstrap-stack.sh` writes (`member_console_test` and `member_console_e2e`, `test/bootstrap-stack.sh:355-356`). `Open`, `Counted` and `OpenE2E` check the name `current_database()` reports before the test runs anything. `DSN` checks the name in the address and refuses an address that names no database. The refusal fails the test and names the variable and the database. - `test/reset-test-db.sh` applies the same rule instead of refusing one name. - `CONTRIBUTING.md` says the database's name must end that way. - `browsertest.AppDB` (`test/e2e/browsertest/session.go:31`), which opens the running console's database on purpose for the walkthroughs, stays as it is.

cgalo5758 added the

labels 2026-10-16 01:52:38 +00:00

cgalo5758 referenced this issue

2026-10-24 06:51:53 +00:00

Package tests run every query as the cluster's superuser, so a privilege the runtime role lacks shows only in the running console #259

Sign in to join this conversation.