ReviewOS
Reload

also looking at this

stacks/bun-query-builder

test: raise instead of skipping when a provided live service is missing

#1187
Merged glennmichael123 wants to merge test/loud-service-gates into main
4 files +194 -9
packages/bun-query-builder/test/dist.config-singleton.test.tsmodified+8-1
@@ -5,16 +5,23 @@
55 * BUNDLED output — a source-level test can never catch a bundler binding-split.
66 * config is now stored on a globalThis Symbol so every inlined copy shares it;
77 * this guard imports the built dist in a fresh process and asserts setConfig
8 * reaches the model layer. Skipped when dist/ hasn't been built.
8 * reaches the model layer.
9 *
10 * Skipped when dist/ hasn't been built — locally. The CI `test` job builds
11 * before it tests, so there a missing dist means the build step did not run,
12 * and the one guard that can see a bundler binding-split would skip on exactly
13 * the run that needed it. See `gate.ts`.
914 */
1015
1116import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
1217import { tmpdir } from 'node:os'
1318import { join, resolve } from 'node:path'
1419import { describe, expect, it } from 'bun:test'
20import { gateUnavailable } from './gate'
1521
1622const DIST = resolve(import.meta.dir, '../dist/src/index.js')
1723const distBuilt = existsSync(DIST)
24 || gateUnavailable('the bundled dist', `${DIST} does not exist`, 'Run `bun run build`.')
1825
1926describe.skipIf(!distBuilt)('config singleton survives bundling (#1043)', () => {
2027 it('setConfig reaches the model layer in the built dist', () => {
packages/bun-query-builder/test/gate.test.tsadded+89-0
@@ -0,0 +1,89 @@
1/**
2 * The live-service gate raises where the service is provided, skips where it
3 * is optional.
4 *
5 * `probePostgres()` used to answer `false` for every possible failure, so the
6 * 14 files gated on it vanished into "skip" whenever Postgres was unreachable
7 * for any reason — and a run with all of them gone still exits 0. That is how
8 * this repository once shipped a release whose two most severe guards never
9 * executed: the probe built its URL from `process.env.USER`, auth failed on
10 * every runner, and CI was green throughout. Fixing the URL fixed that cause.
11 * This pins the mechanism.
12 *
13 * Both directions are controlled, because only the pair is evidence: a gate
14 * that always raises would pass the first case here while making the suite
15 * unrunnable for anyone without Postgres, which is the behaviour the skip
16 * exists to provide.
17 *
18 * Each case runs the real gate in a child `bun test`, against a port nothing
19 * answers on, and reads the exit status — the thing CI actually keys on.
20 */
21
22import { describe, expect, it } from 'bun:test'
23import process from 'node:process'
24import { resolve } from 'node:path'
25
26// Any file whose only precondition is a live Postgres. Its own assertions are
27// irrelevant here; what matters is whether the gate let the run pass.
28const GATED = 'packages/bun-query-builder/test/client.json-contains.pg.test.ts'
29const REPO = resolve(import.meta.dir, '../../..')
30const UNREACHABLE = 'postgresql://postgres:postgres@127.0.0.1:59999/test_db'
31
32function runGated(env: Record<string, string | undefined>): { code: number | null, output: string } {
33 const child = Bun.spawnSync([process.execPath, 'test', GATED], {
34 cwd: REPO,
35 env: {
36 ...process.env,
37 // Cleared so the parent environment cannot decide the case under test:
38 // a developer machine has no CI, a runner has it set, and this file has
39 // to produce both regardless of where it runs.
40 CI: undefined,
41 QB_REQUIRE_LIVE: undefined,
42 DATABASE_URL: UNREACHABLE,
43 POSTGRES_URL: UNREACHABLE,
44 ...env,
45 } as Record<string, string>,
46 stdout: 'pipe',
47 stderr: 'pipe',
48 })
49
50 return {
51 code: child.exitCode,
52 output: `${child.stdout.toString()}${child.stderr.toString()}`,
53 }
54}
55
56describe('the live-Postgres gate', () => {
57 it('fails the run when a provided Postgres is unreachable', () => {
58 const { code, output } = runGated({ CI: '1' })
59
60 // The assertion that matters is the status, not the text: a skipped suite
61 // exits 0, and 0 is what CI reads as "the guards ran and passed".
62 expect(code).not.toBe(0)
63 // And it has to say what was missing, or the red run sends someone hunting
64 // through 14 files for a failure that is really a dead service.
65 expect(output).toContain('live Postgres')
66 expect(output).toContain('59999')
67 }, 30_000)
68
69 it('skips, and passes, where Postgres is merely optional', () => {
70 const { code, output } = runGated({})
71
72 expect(code).toBe(0)
73 expect(output).toContain('skipping live Postgres tests')
74 }, 30_000)
75
76 it('lets a CI-like environment without services opt out', () => {
77 // A fork running the suite with no service containers: CI is set, but
78 // nothing provides Postgres, so the gate must stay a skip.
79 const { code } = runGated({ CI: '1', QB_REQUIRE_LIVE: '0' })
80
81 expect(code).toBe(0)
82 }, 30_000)
83
84 it('lets a developer opt in to CI strictness', () => {
85 const { code } = runGated({ QB_REQUIRE_LIVE: '1' })
86
87 expect(code).not.toBe(0)
88 }, 30_000)
89})
packages/bun-query-builder/test/gate.tsadded+62-0
@@ -0,0 +1,62 @@
1/**
2 * Whether a precondition a test needs is allowed to be missing.
3 *
4 * A gated test — one that needs a live server, or a built `dist/` — has to do
5 * something when its precondition is absent. Skipping is right on a developer
6 * machine: nobody should have to run Postgres to change a string helper. In CI
7 * the same absence means the opposite, because the workflow PROVIDES the
8 * precondition: it starts Postgres 17 as a service, waits for `pg_isready`,
9 * exports `DATABASE_URL`, and builds before testing. So in CI a missing
10 * precondition is a broken job, and skipping turns it into a green tick over
11 * tests that never ran.
12 *
13 * This repository has been bitten by that twice, and both are written up where
14 * they happened:
15 *
16 * - every live-Postgres gate built its URL from `process.env.USER`, which on a
17 * runner is `runner`, so every probe failed auth and all 14 files skipped —
18 * "CI stayed green while the guards for the two most severe bugs in the
19 * release never ran at all" (see the docblock in `pg.ts`)
20 * - the CI `test` job did not build, so the published-types guard returned
21 * early on every run and checked nothing, which is how an `any` reached
22 * `client.d.ts` (see `public-surface-types.test.ts`)
23 *
24 * Both were fixed by repairing the cause. Neither closed the mechanism, so the
25 * next cause — a rotated service password, a service that fails to start, a
26 * build step someone reorders — lands in the same silence. That is what this
27 * closes: `required()` is true in CI, so the gate raises instead of skipping.
28 *
29 * `QB_REQUIRE_LIVE` overrides it in both directions: `0` for a CI-like
30 * environment that genuinely has no services (a fork running the suite without
31 * them), `1` to get CI's strictness locally.
32 */
33
34import process from 'node:process'
35
36export function requiredInCi(): boolean {
37 const override = process.env.QB_REQUIRE_LIVE
38 if (override !== undefined)
39 return override !== '0' && override !== 'false' && override !== ''
40 return Boolean(process.env.CI)
41}
42
43/**
44 * Report an absent precondition: raise where it must be present, warn and
45 * return `false` where skipping is legitimate.
46 *
47 * Returning the boolean rather than calling `describe.skipIf` here keeps the
48 * decision at the call site, where bun:test needs it at module scope.
49 */
50export function gateUnavailable(what: string, reason: string, remedy: string): false {
51 if (requiredInCi()) {
52 throw new Error(
53 `[gate] ${what} is required here but unavailable: ${reason}. `
54 + `CI provides it, so this is a broken job rather than an absent optional dependency — `
55 + `skipping would report a green run over tests that never executed. ${remedy} `
56 + `Set QB_REQUIRE_LIVE=0 if this environment genuinely has none.`,
57 )
58 }
59
60 console.warn(`[gate] skipping ${what} tests: ${reason}. ${remedy}`)
61 return false
62}
packages/bun-query-builder/test/pg.tsmodified+35-8
@@ -26,6 +26,7 @@
2626
2727import { SQL } from 'bun'
2828import process from 'node:process'
29import { gateUnavailable, requiredInCi } from './gate'
2930
3031export const PG_URL: string
3132 = process.env.DATABASE_URL
@@ -41,15 +42,41 @@ export const PG_URL: string
4142 * trade. Callers await it at the top of their own `.test.ts`, where the rule
4243 * does not apply. The URL above is the part that was wrong and the part worth
4344 * sharing; this is three lines of probe.
45 *
46 * It no longer answers `false` unconditionally. Fixing the URL fixed the cause
47 * of the incident above but not its mechanism: `catch { return false }` reads
48 * every possible failure — a rotated password, a service that never came up,
49 * an exhausted connection limit — as "this machine has no Postgres", and the
50 * 14 files gated on it disappear. Measured on the suite as it stands, a
51 * Postgres that does not answer takes 30 passing live-Postgres assertions down
52 * to 11 and still exits 0. So where the environment is one that supplies
53 * Postgres, an unreachable server raises; see `gate.ts`.
4454 */
4555export async function probePostgres(): Promise<boolean> {
46 try {
47 const probe = new SQL(PG_URL)
48 await probe.unsafe('SELECT 1')
49 await probe.end()
50 return true
51 }
52 catch {
53 return false
56 // Only where a failure is fatal, and only then: locally the connection is
57 // refused immediately and retrying three times per file for a developer who
58 // has no Postgres is pure latency. In CI the workflow already waits for
59 // `pg_isready`, so this is for a blip after that point rather than for boot.
60 const attempts = requiredInCi() ? 3 : 1
61 let lastError: unknown
62
63 for (let attempt = 1; attempt <= attempts; attempt++) {
64 try {
65 const probe = new SQL(PG_URL)
66 await probe.unsafe('SELECT 1')
67 await probe.end()
68 return true
69 }
70 catch (error) {
71 lastError = error
72 if (attempt < attempts)
73 await Bun.sleep(attempt * 250)
74 }
5475 }
76
77 return gateUnavailable(
78 'live Postgres',
79 `${PG_URL} did not answer after ${attempts} attempt${attempts === 1 ? '' : 's'} (${(lastError as Error)?.message ?? lastError})`,
80 'Start Postgres, or point DATABASE_URL at one.',
81 )
5582}