test(db): resolve the integration DSN at run time instead of storing an IP (#161)
ci / lint-test (push) Successful in 1m5s
ci / lint-test (push) Successful in 1m5s
setup-test-db.sh wrote .env.test once with the Postgres container's bridge IP baked in, and Docker reassigns those on recreation. The file rotted silently: its stored 172.22.0.18 belongs to **gitea** as of today, and an earlier drift had the suite pointing its TRUNCATE at **postgres-pantry** — which it would have wiped had the credentials happened to match. Root-cause fix: stop persisting a host at all. - scripts/resolve-test-db.sh re-derives the DSN from the live container and rewrites .env.test. Resolution order is TEST_DATABASE_URL, then the container's published host port, then the bridge IP. `docker port postgres-personal` reports 127.0.0.1:5432 since the 2026-08-17 PG17 migration, so loopback is what gets used here — and a loopback address cannot be reassigned to another container, which is the whole point. (Note this supersedes the smarthome CLAUDE.md's "postgres-personal publishes no host port"; that is now stale.) The bridge-IP fallback is kept for a host where the port is not published, and the script never prints the password. - Wired as `pretest:integration`, so npm runs it before every integration run. .env.test becomes a generated artifact rather than stored configuration. - setup-test-db.sh now only creates the database and copies the schema, then delegates to the resolver. - helpers.ts::assertTestDatabase stays and its message now points at the resolver rather than at `docker inspect`. It still earns its place: running vitest directly bypasses the pretest hook, and a stale .env.test would then still be read. Docs: CLAUDE.md gains a Testing section carrying this, the prisma-generate trap for a fresh checkout or worktree, and the frozen-clock rule for tests whose subject is defined relative to "now". AGENTS.md updated for the same, and for lint now being blocking.
This commit is contained in:
@@ -13,10 +13,11 @@ Keep data flow consistent: API routes call query functions in `src/lib/queries.t
|
|||||||
- `npm ci` installs the locked dependency set (Node 22 is used in CI).
|
- `npm ci` installs the locked dependency set (Node 22 is used in CI).
|
||||||
- `npm run dev` starts the local Next.js development server.
|
- `npm run dev` starts the local Next.js development server.
|
||||||
- `npm run build` creates a production build; `npm start` serves it.
|
- `npm run build` creates a production build; `npm start` serves it.
|
||||||
- `npm run lint` runs the Next.js ESLint configuration. Existing lint debt makes CI lint advisory, but new code should pass.
|
- `npm run lint` runs the Next.js ESLint configuration. **Blocking in CI since #79** — the pre-existing debt is cleared, so it must stay at zero errors (warnings are still allowed).
|
||||||
|
- `npx tsc --noEmit` typechecks, including the test files. Also blocking in CI.
|
||||||
- `npm test` runs fast unit tests.
|
- `npm test` runs fast unit tests.
|
||||||
- `npm run test:setup` prepares the PostgreSQL test database using `.env.test`.
|
- `npm run test:setup` creates/refreshes the `personal_test` database (run it once, and after any migration).
|
||||||
- `npm run test:integration` runs database-backed tests.
|
- `npm run test:integration` runs database-backed tests. It re-resolves the DSN from the live container first (`scripts/resolve-test-db.sh`), so `.env.test` must never carry a hand-written host — see `CLAUDE.md` → Testing.
|
||||||
- `npm run test:all` runs both test suites.
|
- `npm run test:all` runs both test suites.
|
||||||
|
|
||||||
## Coding Style & Naming Conventions
|
## Coding Style & Naming Conventions
|
||||||
@@ -33,4 +34,4 @@ Add schema changes as the next numbered `prisma/migrations/NNNN_description/migr
|
|||||||
|
|
||||||
## Commits & Pull Requests
|
## Commits & Pull Requests
|
||||||
|
|
||||||
History follows concise Conventional Commit-style subjects such as `feat(rules): preview rule changes`, `fix(trips): ...`, and `docs: ...`. Keep commits focused. Pull requests should explain behavior and data-model impact, link related issues, list validation commands, and include screenshots for UI changes. Ensure unit tests and the production build pass; call out any known lint warnings or migration steps.
|
History follows concise Conventional Commit-style subjects such as `feat(rules): preview rule changes`, `fix(trips): ...`, and `docs: ...`. Keep commits focused. Pull requests should explain behavior and data-model impact, link related issues, list validation commands, and include screenshots for UI changes. Ensure lint, typecheck, unit tests and the production build pass; call out any migration steps.
|
||||||
|
|||||||
@@ -59,6 +59,49 @@ docker exec postgres-personal psql -U personal -d personal
|
|||||||
docker logs finance -f
|
docker logs finance -f
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
```bash
|
||||||
|
npm test # unit — pure logic, no database
|
||||||
|
npm run test:integration # database-backed; resolves the DSN itself (see below)
|
||||||
|
npm run test:all # both
|
||||||
|
npm run test:setup # once, and after any migration: creates/refreshes personal_test
|
||||||
|
npm run lint # blocking in CI since #79 — 0 errors, warnings allowed
|
||||||
|
npx tsc --noEmit # blocking in CI; nothing else compiles the test files
|
||||||
|
```
|
||||||
|
|
||||||
|
**The integration DSN is resolved at run time — never hand-write a host into
|
||||||
|
`.env.test`.** `scripts/resolve-test-db.sh` runs automatically as
|
||||||
|
`pretest:integration` and rewrites that file from the live container before
|
||||||
|
every run. It prefers the container's **published host port** (`docker port
|
||||||
|
postgres-personal` → `127.0.0.1:5432`, published since the 2026-08-17 PG17
|
||||||
|
migration) and falls back to the bridge IP only where that is absent. Set
|
||||||
|
`TEST_DATABASE_URL` to override it entirely.
|
||||||
|
|
||||||
|
This replaces the old "refresh the container IP" instruction, which described
|
||||||
|
the defect rather than a workaround (#161). `setup-test-db.sh` used to bake the
|
||||||
|
bridge IP into `.env.test` once, and Docker reassigns those on recreation: by
|
||||||
|
2026-08-21 the stored `172.22.0.18` belonged to **gitea**, and an earlier drift
|
||||||
|
had pointed the suite at **postgres-pantry** — whose database it would have
|
||||||
|
TRUNCATEd had the credentials happened to match. A loopback address cannot be
|
||||||
|
reassigned to another container, which is why the published port wins.
|
||||||
|
|
||||||
|
**Two guards, and both matter.** `helpers.ts::assertTestDatabase` refuses to
|
||||||
|
truncate anything whose `current_database()` is not `personal_test`, and the
|
||||||
|
suite runs `fileParallelism: false` because every file shares that one database
|
||||||
|
and `resetDB()` TRUNCATEs it.
|
||||||
|
|
||||||
|
**A fresh checkout or worktree needs `npx prisma generate` first.** The client is
|
||||||
|
generated into the gitignored `src/generated/prisma`, so without it every test
|
||||||
|
that reaches `src/lib/db.ts` fails on a missing package — which reads as a code
|
||||||
|
error rather than a setup one. CI does this explicitly for the same reason.
|
||||||
|
|
||||||
|
**Freeze the clock in any test whose subject is defined relative to "now".**
|
||||||
|
`analytics-priority0.test.ts` uses `vi.useFakeTimers({ toFake: ["Date"] })` —
|
||||||
|
only `Date`, because `pg` needs real timers for connection handling. Without it
|
||||||
|
the fixtures drift out of the routes' rolling windows over time and the
|
||||||
|
assertions quietly stop testing anything.
|
||||||
|
|
||||||
## Architecture
|
## Architecture
|
||||||
|
|
||||||
### Key Files
|
### Key Files
|
||||||
|
|||||||
@@ -10,6 +10,7 @@
|
|||||||
"test": "vitest run --config vitest.config.ts",
|
"test": "vitest run --config vitest.config.ts",
|
||||||
"test:watch": "vitest --config vitest.config.ts",
|
"test:watch": "vitest --config vitest.config.ts",
|
||||||
"test:setup": "bash scripts/setup-test-db.sh",
|
"test:setup": "bash scripts/setup-test-db.sh",
|
||||||
|
"pretest:integration": "bash scripts/resolve-test-db.sh",
|
||||||
"test:integration": "vitest run --config vitest.integration.config.ts",
|
"test:integration": "vitest run --config vitest.integration.config.ts",
|
||||||
"test:all": "npm test && npm run test:integration"
|
"test:all": "npm test && npm run test:integration"
|
||||||
},
|
},
|
||||||
|
|||||||
Executable
+66
@@ -0,0 +1,66 @@
|
|||||||
|
#!/bin/bash
|
||||||
|
# Resolve the integration-test DSN at run time and (re)write .env.test.
|
||||||
|
#
|
||||||
|
# Why this exists (#161): .env.test used to be written ONCE by
|
||||||
|
# setup-test-db.sh with the Postgres container's bridge IP baked in. Container
|
||||||
|
# IPs are reassigned on recreation, so the file rotted silently — by
|
||||||
|
# 2026-08-21 its 172.22.0.18 belonged to *gitea*, and an earlier drift had the
|
||||||
|
# suite pointing its TRUNCATE at postgres-pantry. It only failed safe because
|
||||||
|
# the credentials happened not to match.
|
||||||
|
#
|
||||||
|
# The fix is to stop persisting a host at all: this runs as `pretest:integration`,
|
||||||
|
# so the DSN is re-derived from the live container immediately before every run.
|
||||||
|
#
|
||||||
|
# Resolution order:
|
||||||
|
# 1. An explicit TEST_DATABASE_URL in the environment wins (CI, remote DB).
|
||||||
|
# 2. The container's published host port (`docker port`) — postgres-personal
|
||||||
|
# publishes 127.0.0.1:5432 since the 2026-08-17 PG17 migration. Preferred
|
||||||
|
# because a loopback address cannot be reassigned to another container.
|
||||||
|
# 3. The container's bridge IP, for a host where the port is not published.
|
||||||
|
#
|
||||||
|
# helpers.ts::assertTestDatabase is the backstop: it refuses to TRUNCATE
|
||||||
|
# anything whose current_database() is not `personal_test`.
|
||||||
|
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
PG_CONTAINER=${PG_CONTAINER:-postgres-personal}
|
||||||
|
PG_USER=${PG_USER:-personal}
|
||||||
|
PG_PASS=${PG_PASS:-personalpassword123}
|
||||||
|
TEST_DB=${TEST_DB:-personal_test}
|
||||||
|
|
||||||
|
ENV_FILE="$(cd "$(dirname "$0")/.." && pwd)/.env.test"
|
||||||
|
|
||||||
|
if [ -n "${TEST_DATABASE_URL:-}" ]; then
|
||||||
|
DSN="$TEST_DATABASE_URL"
|
||||||
|
SOURCE="TEST_DATABASE_URL"
|
||||||
|
else
|
||||||
|
if ! docker inspect "$PG_CONTAINER" >/dev/null 2>&1; then
|
||||||
|
echo "ERROR: container '$PG_CONTAINER' not found. Start it, or set TEST_DATABASE_URL." >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# `docker port` prints e.g. "127.0.0.1:5432" (possibly several lines).
|
||||||
|
PUBLISHED=$(docker port "$PG_CONTAINER" 5432/tcp 2>/dev/null | head -1 || true)
|
||||||
|
|
||||||
|
if [ -n "$PUBLISHED" ]; then
|
||||||
|
HOSTPORT="$PUBLISHED"
|
||||||
|
SOURCE="published port"
|
||||||
|
else
|
||||||
|
IP=$(docker inspect "$PG_CONTAINER" \
|
||||||
|
--format '{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}' 2>/dev/null \
|
||||||
|
| tr ' ' '\n' | grep -v '^$' | head -1)
|
||||||
|
if [ -z "$IP" ]; then
|
||||||
|
echo "ERROR: '$PG_CONTAINER' has neither a published 5432 nor a bridge IP." >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
HOSTPORT="$IP:5432"
|
||||||
|
SOURCE="bridge IP"
|
||||||
|
fi
|
||||||
|
|
||||||
|
DSN="postgresql://$PG_USER:$PG_PASS@$HOSTPORT/$TEST_DB"
|
||||||
|
fi
|
||||||
|
|
||||||
|
printf 'DATABASE_URL=%s\n' "$DSN" > "$ENV_FILE"
|
||||||
|
|
||||||
|
# Never print the password.
|
||||||
|
echo "resolve-test-db: ${DSN%%://*}://***@${DSN#*@} (via $SOURCE)"
|
||||||
+14
-18
@@ -1,25 +1,24 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
# Creates the personal_test database and writes .env.test
|
# Creates the personal_test database and copies the production schema into it.
|
||||||
# Run once before integration tests: npm run test:setup
|
# Run once (and after any migration) before integration tests: npm run test:setup
|
||||||
|
#
|
||||||
|
# It no longer writes a host into .env.test. That was the #161 defect: the file
|
||||||
|
# was written once with the container's bridge IP and then rotted as the IP was
|
||||||
|
# reassigned. scripts/resolve-test-db.sh now re-derives the DSN before every
|
||||||
|
# integration run (wired as `pretest:integration`), and this script just calls it
|
||||||
|
# at the end so a fresh setup leaves a usable .env.test behind.
|
||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
PG_CONTAINER=postgres-personal
|
PG_CONTAINER=${PG_CONTAINER:-postgres-personal}
|
||||||
PG_USER=personal
|
PG_USER=${PG_USER:-personal}
|
||||||
PG_PASS=personalpassword123
|
TEST_DB=${TEST_DB:-personal_test}
|
||||||
TEST_DB=personal_test
|
|
||||||
|
|
||||||
# Discover the container's bridge IP (accessible from the host on Linux)
|
if ! docker inspect "$PG_CONTAINER" >/dev/null 2>&1; then
|
||||||
PG_HOST=$(docker inspect "$PG_CONTAINER" \
|
|
||||||
--format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' 2>/dev/null | head -1)
|
|
||||||
|
|
||||||
if [ -z "$PG_HOST" ]; then
|
|
||||||
echo "ERROR: Could not find container '$PG_CONTAINER'"
|
echo "ERROR: Could not find container '$PG_CONTAINER'"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "Postgres container at $PG_HOST:5432"
|
|
||||||
|
|
||||||
# Create test database (ignore error if it already exists)
|
# Create test database (ignore error if it already exists)
|
||||||
docker exec "$PG_CONTAINER" psql -U "$PG_USER" -d postgres \
|
docker exec "$PG_CONTAINER" psql -U "$PG_USER" -d postgres \
|
||||||
-c "CREATE DATABASE $TEST_DB;" 2>/dev/null || true
|
-c "CREATE DATABASE $TEST_DB;" 2>/dev/null || true
|
||||||
@@ -31,9 +30,6 @@ docker exec "$PG_CONTAINER" pg_dump -U "$PG_USER" --schema-only "$PG_USER" \
|
|||||||
|
|
||||||
echo "Schema ready."
|
echo "Schema ready."
|
||||||
|
|
||||||
# Write .env.test
|
bash "$(dirname "$0")/resolve-test-db.sh"
|
||||||
cat > "$(dirname "$0")/../.env.test" << EOF
|
|
||||||
DATABASE_URL=postgresql://$PG_USER:$PG_PASS@$PG_HOST:5432/$TEST_DB
|
|
||||||
EOF
|
|
||||||
|
|
||||||
echo ".env.test written — integration tests can now run with: npm run test:integration"
|
echo "Integration tests can now run with: npm run test:integration"
|
||||||
|
|||||||
@@ -30,12 +30,17 @@ export function mockDbWithPool(p: Pool) {
|
|||||||
/**
|
/**
|
||||||
* Refuse to truncate anything that is not the test database.
|
* Refuse to truncate anything that is not the test database.
|
||||||
*
|
*
|
||||||
* `DATABASE_URL` in `.env.test` names the Postgres container by IP, and
|
* `.env.test` used to name the Postgres container by bridge IP, and Docker
|
||||||
* container IPs move on recreation: 172.22.0.47 stopped being
|
* reassigns those on recreation: 172.22.0.47 stopped being `postgres-personal`
|
||||||
* `postgres-personal` and became `postgres-pantry`, so the suite spent a while
|
* and became `postgres-pantry`, so the suite spent a while pointing its
|
||||||
* pointing its TRUNCATE at another app's database. It only failed safe because
|
* TRUNCATE at another app's database. It only failed safe because the
|
||||||
* the credentials happened not to match — had they matched, this would have
|
* credentials happened not to match — had they matched, this would have wiped
|
||||||
* wiped pantry-app.
|
* pantry-app.
|
||||||
|
*
|
||||||
|
* `scripts/resolve-test-db.sh` (wired as `pretest:integration`) now re-derives
|
||||||
|
* the DSN from the live container before every run, preferring the published
|
||||||
|
* loopback port. This check stays anyway: running vitest directly bypasses the
|
||||||
|
* pretest hook, and a stale `.env.test` would still be read.
|
||||||
*
|
*
|
||||||
* Checked once per process, before the first truncate.
|
* Checked once per process, before the first truncate.
|
||||||
*/
|
*/
|
||||||
@@ -47,8 +52,9 @@ async function assertTestDatabase(pool: Pool) {
|
|||||||
if (db !== "personal_test") {
|
if (db !== "personal_test") {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Refusing to truncate: connected to "${db}", expected "personal_test". ` +
|
`Refusing to truncate: connected to "${db}", expected "personal_test". ` +
|
||||||
`Check DATABASE_URL in .env.test — the Postgres container IP may have changed ` +
|
`Re-resolve the DSN with \`bash scripts/resolve-test-db.sh\` (or just run ` +
|
||||||
`(docker inspect postgres-personal --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}').`
|
`\`npm run test:integration\`, which does it for you), and run ` +
|
||||||
|
`\`npm run test:setup\` if personal_test does not exist yet.`
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
targetVerified = true;
|
targetVerified = true;
|
||||||
|
|||||||
Reference in New Issue
Block a user