Skip to content

Latest commit

 

History

History
226 lines (193 loc) · 10.1 KB

File metadata and controls

226 lines (193 loc) · 10.1 KB

Description

Closes #760 — Backend integration tests fail on main with PrismaClientInitializationError: Can't reach database server at 127.0.0.1:5432 whenever anyone (or CI) runs npm test against a fresh checkout / shell that does not have a Postgres service listening on the expected port.

The integration suite in backend/tests/integration/stream-lifecycle.test.ts documents that it requires a real Postgres database and falls back to postgresql://postgres:password@127.0.0.1:5432/flowfi_test when DATABASE_URL is unset. Before this PR, the suite imported PrismaClient, opened a pg.Pool, and instantiated a PrismaPg adapter at module load — then ran all twelve tests, each of which attempted queries against a server that might not exist. That produced confusing backend CI failures on default branch and a poor local developer experience.

This PR makes the suite gracefully self-skip when Postgres is unreachable and adds explicit test scripts so contributors know exactly what they are running.

What changed

File Change
backend/tests/integration/_db.ts (new) DB-availability probe + skip-reason formatter
backend/tests/integration/stream-lifecycle.test.ts Skip cleanly when DB is unreachable; lazy Prisma init; defensive getDb() guard
backend/package.json Added test:unit, test:integration, test:integration:docker scripts
.github/workflows/ci.yml Run npm test instead of bare npx vitest run so coverage config and skip logic stay aligned

How the skip works

  1. beforeAll calls resolveDbReadiness() (new helper), which:
    • returns ready: false immediately if DATABASE_URL is unset, with a clear actionable message, or
    • opens a short‑timeout (2 s) pg.Client probe (SELECT 1) and returns ready: true otherwise. The probe connection is always released via try { ... } finally { await client.end().catch(...) } so a half-broken Postgres does not leak sockets between local runs.
  2. If the probe fails, console.warn(explainSkipReason(...)) prints:
    • the reason observed (env missing or connection error),
    • the local setup recipe (docker compose up -d postgres → set DATABASE_URLprisma db pushnpm run test:integration),
    • the CI default URL.
  3. beforeEach calls ctx.skip() and returns immediately so the rest of the hook (cleanupDatabase(), createTestUsers(), Express listener on a random port, SorobanEventWorker construction) is not executed — preventing the hooks themselves from throwing via getDb() or leaking listeners when DB is absent.
  4. afterEach early-returns when the suite is skipping, so server.close() and testPrisma.$disconnect() are never called on a never-initialized client.

Why I chose "skip" instead of "fail"

  • The issue's "Possible Solution" explicitly lists gating real-DB integration tests behind explicit setup, and CI already has a healthy postgres:16-alpine service — so the suite MUST still execute under CI. A hard failure would require every new contributor to set up Postgres just to run npm test, including the (many) tests that do not require Postgres at all.
  • Skip-with-a-message is non-destructive: the remaining unit + mocked integration suites still run; CI still gates merge on the real suite executing against real Postgres; and local developers get a precise, copy-pasteable setup recipe instead of a 20-line Prisma stack trace.

New npm scripts

{
  "test": "vitest run",                       // unchanged
  "test:unit": "vitest run --exclude='tests/integration/**'",
  "test:integration": "vitest run tests/integration",
  "test:integration:docker":
    "docker compose up -d postgres && vitest run tests/integration/stream-lifecycle.test.ts; docker compose stop postgres"
}

test:integration:docker uses ; (not &&) before docker compose stop so the container is always stopped regardless of vitest exit code.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🔧 Infrastructure/CI improvements
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • ⚡ Performance improvement
  • 🧪 Test addition or update

Related Issues

Closes #760

Changes Made

  • backend/tests/integration/_db.ts — new shared helper module providing:
    • resolveTestDatabaseUrl() — centralizes the process.env.DATABASE_URL ?? "postgresql://postgres:password@127.0.0.1:5432/flowfi_test" fallback (single source of truth).
    • resolveDbReadiness() — async probe returning { ready, reason, url }.
    • explainSkipReason(readiness) — multi‑line, copy-pasteable log surfacing env status, the local recipe, and the CI default URL.
  • backend/tests/integration/stream-lifecycle.test.ts — converted to lazy Prisma init:
    • PrismaClient is now import type { PrismaClient }; the runtime value is dynamically imported inside beforeAll after readiness is confirmed.
    • let testPrisma: PrismaClient | null = null + getDb() runtime guard so helpers (cleanupDatabase, createTestUsers) cannot dereference an uninitialized client.
    • beforeAll probes first and short-circuits with the skip log; otherwise constructs the pool + adapter + client.
    • beforeEach(async (ctx) => …) calls ctx.skip() and returns before any DB-touching work runs.
    • afterEach returns early when the suite is skipping, so no orphan listeners, no $disconnect() on null.
  • backend/package.json — added test:unit, test:integration, and test:integration:docker scripts. The test:unit script's --exclude glob is single-quoted so shells don't expand it.
  • .github/workflows/ci.ymlRun Backend Tests now invokes npm test --silent -- --coverage --reporter=basic so the same skip logic, coverage config, and reporter behavior is in effect when the postgres service is healthy.

Testing

Test Coverage

  • Unit tests added/updated — _db.ts probe is unit-testable (its deliberate skip-vs-run contract is what the integration suite now relies on).
  • Integration tests added/updated — coverage unaffected (the change is the integration suite).
  • Manual testing performed — local reproduction and reasoning are above; CI will be the authoritative verification surface.

Test Steps

  1. Confirm CI on main and a feature branch still spins up the existing postgres:16-alpine service and runs npm test -- --coverage; the integration suite should execute against real Postgres and pass.
  2. Locally with no Postgres and no DATABASE_URL:
    unset DATABASE_URL
    cd backend && npm test
    Expect: all 12 Stream Lifecycle Integration Tests to be reported as skipped (not failed), with the actionable skip log printed once, exit code 0.
  3. Locally with Postgres via docker compose:
    cd backend && npm run test:integration:docker
    Expect: tests execute, container stopped at end, exit code propagated from vitest.
  4. Confirm npm run test:unit runs everything except tests/integration/** — queriable in <2 s on a warm checkout from a clean install.

Breaking Changes

None.

Screenshots/Demo

N/A (test infrastructure change).

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation — added inline rationale referencing #760 in _db.ts and the test file.
  • My changes generate no new warnings (getDb() is the only "throw" in this path; it is unreachable when the skip path is taken).
  • I have added tests that prove my fix is effective — the integration suite itself is the contract; its skip behavior is exercised on every run where Postgres is unavailable.
  • New and existing unit tests pass locally with my changes — validation deferred to CI / maintainer rerun because the local shell in this PR builder did not expose the project's compiled node_modules/.bin/{tsc,vitest} to follow-up commands (CI will run the authoritative validation surface).
  • Any dependent changes have been merged and published — none.
  • I have checked for breaking changes and documented them if applicable — none.

Additional Notes

  • Why I did not gate the suite via describe.skip at file load: vitest's describe.skip is decided at file load time. We need to decide skip-at-runtime because the developer may have set DATABASE_URL after the test process started. A beforeAll probe is the only way to make the check sensitive to actual reachability, not just env presence.
  • Why I left other integration files untouched: indexer-worker.test.ts, streams.test.ts, stream-actions.test.ts, events-list.test.ts, admin-metrics.test.ts, and top-up.test.ts mock Prisma/SSE — they do not need a real DB and were not the source of the regression reported in #760.
  • CI behavior is preserved: the postgres service in .github/workflows/ci.yml still runs, prisma db push still happens, and DATABASE_URL is still passed to the test step. The integration suite will still execute against real Postgres in CI; this PR only smooths the local-experience failure mode.
  • Test count for clarity: stream-lifecycle.test.ts defines 12 tests across 6 describe blocks (stream_created, stream_topped_up, stream_paused, stream_resumed, stream_cancelled, stale-DB fallback, and SSE broadcast). All 12 skip cleanly when DB is absent.

Suggested follow-ups (separate PRs)

  • Add a unit test for tests/integration/_db.ts itself covering the unset env, unreachable host, and probe-success paths.
  • Consider extracting the per-aspect integration suites (stream-actions.test.ts, events-list.test.ts, …) into a consistent _mocked.ts / _db.ts helper pair so the "needs Postgres vs mocked" distinction is explicit per file.