ReviewOS

also looking at this

stacks/bun-query-builder

test(pg): make the Postgres-gated tests actually run in CI

#1077
Merged glennmichael123 wants to merge test/pg-gated-tests-in-ci into main
4 files +68 -35
10 days ago

The Postgres-gated tests were skipping in CI, and CI was green because of it.

What was happening

Each gated file built its own URL as postgres://${process.env.USER}@localhost:5432/postgres — a developer machine: peer-authenticated server owned by the current user, no password. CI is not that. It starts Postgres 17 as a service with POSTGRES_USER=postgres, POSTGRES_PASSWORD=postgres, POSTGRES_DB=test_db, and exports DATABASE_URL.

On a runner process.env.USER is runner, so every probe failed:

FATAL:  password authentication failed for user "runner"

The gate read that as "no Postgres here" and skipped. The last CI run on main:

CI:     1695 pass / 8 skip / 0 fail
local:  1719 pass / 4 skip / 0 fail

Among those extra skips were the regression guards for the two most severe bugs in this release — the write surviving ROLLBACK (#1065) and the concurrent migrate collision (#1067). A guard that cannot run on the machine gating releases is worse than no guard, because the green tick claims the opposite.

The fix

One shared test/pg.ts: URL prefers DATABASE_URL, then POSTGRES_URL, then the local shape. Three files stop hand-building it, so they can't drift apart again.

integration.postgres.test.ts also built a second copy of the connection inside its subprocess script (username: process.env.USER, password: '') which would have failed even once the gate opened — it uses the probed URL now.

The probe is a function rather than an awaited constant, so the helper needs no top-level await; callers await it in their own .test.ts where the rule doesn't apply. Suppressing the rule in a helper to save three callers one line each seemed a poor trade.

Verified three ways

environmentresult
DATABASE_URL → postgres/postgres (CI shape)4 gated tests run (were skipping)
no DATABASE_URL (local)still run
DATABASE_URL → unreachableskip cleanly, don't fail

Full suite 1719 pass / 4 skip / 0 fail with and without DATABASE_URL. The 4 remaining skips are gated on a built dist/ and embedded PGlite, not Postgres.

Typecheck 0, lint 0 errors.

After this merges, the CI test job should report ~1699 pass / 4 skip rather than 1695 / 8 — worth confirming on the first run, since that number is the whole point of the change.

🤖 Generated with Claude Code

4 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.