test: #21 run the PostgreSQL integration tests in CI

[erikhfj]

Closes #21 (the remaining acceptance criterion: the tests run in CI without a manual setup step).

What was done

The database-backed suites (facets, search, sort and the MusicBrainz import) were already written and already correct, but every one of them is wrapped in describe.skipIf(!process.env.TEST_DATABASE_URL). CI never set that variable, so pnpm test reported 61 passed | 27 skipped and stayed green with the entire database layer missing. That is the one part of #21's checklist that was open.

• New integration job in .github/workflows/ci.yml. It creates a PostgreSQL cluster with initdb in $RUNNER_TEMP, listens on a unix socket only (listen_addresses=''), applies the migrations, runs vitest with both the default and JSON reporters, and stops and deletes the cluster in an if: always() step.
• No shared test database and no TEST_DATABASE_URL secret: there is nothing to provision by hand and nothing to drift. The application database is not reachable from the job, so db:seed/db:seed:large cannot be pointed at it by accident.
• deploy now has needs: [quality, integration], so a commit cannot deploy with the database tests untested (same shape as Deploy the server build to the path crate-api expects #44, one layer down).
• server/scripts/assert-postgres-tests.mjs: reads the JSON report and fails when a database-backed file executed zero assertions or left any pending. Without it the failure mode only moves from "suite skipped" to "suite silently empty". The file list is explicit so adding a new database-backed suite is a deliberate edit.
• README and docs/VM-setup.md describe how CI runs them and how to run them locally.

What was tested

The whole job was rehearsed locally against the same commands, in a fresh temporary cluster, from initdb to deleting it:

Migrations applied.
 Test Files  9 passed (9)
      Tests  88 passed (88)
src/db/musicBrainzCatalog.test.ts 4 passed
src/search/facets.postgres.test.ts 6 passed
src/search/search.postgres.test.ts 11 passed
src/search/sort.postgres.test.ts 8 passed
All 4 database-backed test files executed.

The guard was checked against a deliberately broken run as well. Same tree, TEST_DATABASE_URL unset:

::error::src/search/facets.postgres.test.ts did not fully run (passed, 0 passed, 6 skipped) - is TEST_DATABASE_URL set?
::error::src/search/search.postgres.test.ts did not fully run (passed, 0 passed, 11 skipped) - is TEST_DATABASE_URL set?
::error::src/search/sort.postgres.test.ts did not fully run (passed, 0 passed, 8 skipped) - is TEST_DATABASE_URL set?
exit 1

pnpm format:check && pnpm lint && pnpm typecheck && pnpm test passes locally.

Known risk

initdb/pg_ctl must be on the runner's PATH. PostgreSQL 18 is installed on the VM, so this should be /usr/lib/postgresql/18/bin; if the runner user's PATH does not include it the job fails loudly on the first step and it is a one-line fix. Worth watching the first run of this PR.

[rachelks]

Nice work getting the PostgreSQL integration tests running in CI! The integration job is passing, but I found two things worth fixing before merging:

1. Skipped tests: The verification script doesn't catch individual tests marked as skipped. This means some database tests could be skipped while the file still passes verification. Could we make sure the verifier catches unintentionally skipped database tests, while still allowing explicitly planned TODOs?

2. PostgreSQL cleanup: The PostgreSQL binary path is only added to PATH during startup, so the cleanup step might not find pg_ctl. Also, PGDATA is only persisted after setup succeeds, which means cleanup might not work if initialization fails. Could we make both available to the cleanup step and ensure the server is stopped before removing its data directory?

One minor documentation suggestion: the README could clarify that the separate test database needs migrations applied before running tests locally.

Otherwise, the overall approach looks good!

[erikhfj]

Thanks — both were real. Fixed in b9d12eb, b12ae46, 528cebb; CI is green.

1. Skipped tests were invisible

You were right, and the cause is worse than a missing check: the script counted assertions with status pending, but Vitest reports a skipped assertion as skipped. That counter was always zero. The whole-file skip was still caught (a skipped file has zero passing assertions), so the gap was exactly the partial case you describe.

Reproduced against a real report: I flipped one assertion in musicBrainzCatalog.test.ts to skipped and ran both versions.

old: src/db/musicBrainzCatalog.test.ts 3 passed ... All 4 database-backed test files executed.  exit 0
new: ::error::src/db/musicBrainzCatalog.test.ts did not fully run (passed, 3 passed, 1 skipped)   exit 1

skipped, pending and disabled all block now. todo does not, and is counted in the output (1 passed, 1 todo), so a test written down on purpose does not fail the build while an unplanned skip cannot hide.

The check moved into checkReport(), called by the CLI, so it is testable without spawning a process. Eight tests: all-ran, missing file, nothing-ran, one skipped among passing, failed file, allowed todo, and the CLI exit codes for both outcomes. The suite list is imported from the script, so adding a database suite cannot silently weaken the test.

2. Cleanup could not find PGDATA or pg_ctl

Also correct: export PATH inside the start step never reached a later step, and PGDATA was written to GITHUB_ENV at the end of that step, so a failed initdb left cleanup with neither.

There is now a Locate PostgreSQL step before anything can fail. It writes the bin directory to GITHUB_PATH and PGDATA to GITHUB_ENV, so cleanup has both whether or not a cluster was ever created.

Cleanup also no longer deletes under a live server. pg_ctl ... || true hid a failed stop and the next rm -rf removed the directory out from under the postmaster. It now checks that pg_ctl is on PATH, waits for postmaster.pid to disappear, and fails instead of deleting. I ran the step body through all three paths: PGDATA unset exits 0, a directory with a postmaster.pid and no usable pg_ctl exits 1 and keeps the directory, and a real throwaway cluster stops and is removed. The CI log for this commit shows server stopped.

3. Migrations before tests

The snippet now applies migrations to the same URL first, and says the suites expect the schema rather than create it:

export TEST_DATABASE_URL=postgres://t19:password@127.0.0.1:5432/crate_test
DATABASE_URL="$TEST_DATABASE_URL" pnpm --filter @t19/server db:migrate
pnpm --filter @t19/server test

docs/VM-setup.md also notes that cleanup runs after a failure and will not delete a data directory under a running server.

Re-review when you have a moment — this one unblocks #75, which rachelks has approved.