diff --git a/.changeset/pin-opossum-suite.md b/.changeset/pin-opossum-suite.md new file mode 100644 index 0000000..39faaaf --- /dev/null +++ b/.changeset/pin-opossum-suite.md @@ -0,0 +1,14 @@ +--- +"resilix": patch +--- + +The opossum compatibility suite is now pinned to a commit instead of tracking `main`. + +`.opossum-compat/` is not cached in CI, so every run re-fetched the suite — meaning the README's +"362 of 362" was measured against whatever opossum had merged that morning. A required check whose +expected value can change upstream without us is not a check. Bumping the pin is now a deliberate +act, with the new total. + +A stalled file also reports its exit status and the tail of its output. It previously printed only +the word `STALLED` with the child's output discarded, so when `test.js` produced no TAP summary on +a CI runner while passing locally, there was nothing to diagnose from. diff --git a/.changeset/sql-integration-runnable.md b/.changeset/sql-integration-runnable.md new file mode 100644 index 0000000..ba3c06b --- /dev/null +++ b/.changeset/sql-integration-runnable.md @@ -0,0 +1,16 @@ +--- +"resilix": patch +--- + +`pnpm test:integration` is runnable again, and says what to do when it is not. + +The path fix landed separately; this is what running it revealed. The describe-level gate only +checks whether `RESILIX_TEST_DATABASE_URL` is *set*, and the script always sets it with a +localhost default — so without a database the suite failed with a bare `AggregateError` and two +`node:net` stack frames. It now names the URL it tried, gives the `docker run` line, and points at +`pnpm test` for skipping. + +Verified end to end against a real PostgreSQL 14.15: **13 of 13 pass**, so `classifySql`'s +mappings hold on 14 as well as the 16 they were captured against. One caveat now documented: +`initdb --auth=trust` makes the "bad password" case unfalsifiable, and that test reports +`NO THROW` until `pg_hba.conf` uses `scram-sha-256` for `127.0.0.1`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a288f80..52e8785 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,7 +84,7 @@ pnpm verify # everything CI runs — do this before pushing pnpm test # unit tests pnpm test:coverage # with thresholds pnpm test:compat # opossum's OWN suite against our shim (362/362) -pnpm test:integration # classifySql against a real Postgres; needs RESILIX_TEST_DATABASE_URL +pnpm test:integration # classifySql against a real Postgres (see below) pnpm lint # biome, including the restricted-globals rule pnpm typecheck pnpm build @@ -101,6 +101,23 @@ New to the codebase? [Reading the code](https://resilix.js.org/guide/reading-the maps every source file to the paper or chapter behind it, and gives a dependency-ordered path through the ~3,500 lines. +### Running the SQL integration tests + +They need a reachable Postgres; `pnpm test:integration` supplies a default URL pointing at +port 5459, so it fails rather than skips if nothing is listening. The error message carries this +command, but for reference: + +```bash +docker run --rm -d -p 5459:5432 \ + -e POSTGRES_PASSWORD=resilix -e POSTGRES_DB=resilix_test postgres:16 +``` + +Without Docker, a local Postgres works — note that `initdb --auth=trust` makes the +"bad password" case unfalsifiable, so set a password and use `scram-sha-256` for +`127.0.0.1` in `pg_hba.conf` or that one test fails with `NO THROW`. + +`pnpm test` skips the whole file, so the default workflow needs no database at all. + Specs live in `docs/specs/` and are written **before** the code they describe — the adaptive limiter and the retry/throttling work both gated on theirs. If you are adding a policy, the spec comes first, and it should carry the provenance of every default it proposes. diff --git a/scripts/opossum-compat.mjs b/scripts/opossum-compat.mjs index 0c26b80..96818ad 100644 --- a/scripts/opossum-compat.mjs +++ b/scripts/opossum-compat.mjs @@ -22,7 +22,11 @@ import { fileURLToPath } from "node:url"; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const HARNESS = join(ROOT, ".opossum-compat"); -const REF = process.env.OPOSSUM_REF ?? "main"; +// PINNED, not "main". CI re-fetches on every run because .opossum-compat/ is not cached, so +// tracking a branch meant the README's "362 of 362" was measured against whatever opossum had +// merged that morning — a required check whose expected value upstream can change without us. +// Bump this deliberately, with the new total, and never as a drive-by. +const REF = process.env.OPOSSUM_REF ?? "decbedf63d7815049233e544dcb351590ff0c84e"; /** Their suite, minus the files that unit-test opossum's private modules. */ const TESTS = [ @@ -102,12 +106,12 @@ const run = (file) => child.stderr.on("data", (d) => { out += d; }); - child.on("close", () => { + child.on("close", (code, signal) => { clearTimeout(kill); const pass = Number(out.match(/^# pass\s+(\d+)/m)?.[1] ?? 0); const fail = Number(out.match(/^# fail\s+(\d+)/m)?.[1] ?? 0); const stalled = !/^# pass/m.test(out); - done({ file, pass, fail, stalled, out }); + done({ file, pass, fail, stalled, out, code, signal }); }); }); @@ -131,6 +135,7 @@ const main = async () => { let fail = 0; console.log(`\n${"FILE".padEnd(32)}${"PASS".padStart(6)}${"FAIL".padStart(6)}`); console.log("-".repeat(46)); + const stalls = results.filter((r) => r.stalled); for (const r of results.sort((a, b) => a.file.localeCompare(b.file))) { pass += r.pass; fail += r.fail; @@ -145,6 +150,17 @@ const main = async () => { `${"TOTAL".padEnd(32)}${String(pass).padStart(6)}${String(fail).padStart(6)} ${pct}%\n`, ); + // A STALLED file used to report only the word STALLED, with the child's output discarded — so + // when test.js stalled on a CI runner while passing locally, there was nothing to diagnose from + // and the cause had to be guessed at. Print the exit status and the tail of what it actually + // said. + for (const r of stalls) { + console.error(`\n${r.file} produced no TAP summary (exit=${r.code} signal=${r.signal})`); + const tail = r.out.trimEnd().split("\n").slice(-12); + for (const line of tail) console.error(` ${line}`); + if (r.out.trim() === "") console.error(" (no output at all)"); + } + console.log("Excluded, and why:"); for (const [f, why] of Object.entries(EXCLUDED)) console.log(` ${f.padEnd(20)} ${why}`); diff --git a/src/scenarios/sql-integration.test.ts b/src/scenarios/sql-integration.test.ts index 81a49c1..20519f4 100644 --- a/src/scenarios/sql-integration.test.ts +++ b/src/scenarios/sql-integration.test.ts @@ -33,7 +33,28 @@ suite("classifySql against a real PostgreSQL server", () => { beforeAll(async () => { pg = await import("pg"); client = new pg.Client({ connectionString: URL_ }); - await client.connect(); + // `pnpm test:integration` always sets RESILIX_TEST_DATABASE_URL, defaulting to localhost, so + // the describe-level gate cannot tell "asked for integration tests" from "has a database". + // Without this the failure is a bare AggregateError with two node:net frames and no hint of + // what to do about it. + try { + await client.connect(); + } catch (cause) { + throw new Error( + [ + `No reachable Postgres at ${URL_}`, + "", + " Start one: docker run --rm -d -p 5459:5432 \\", + " -e POSTGRES_PASSWORD=resilix -e POSTGRES_DB=resilix_test postgres:16", + " Or: pnpm test (skips these entirely)", + "", + "These verify classifySql against real pg and Prisma errors, so they cannot run", + "against fixtures — a captured fixture keeps passing after a driver changes shape,", + "which is the whole reason this file exists.", + ].join("\n"), + { cause }, + ); + } await client.query("drop table if exists resilix_probe"); await client.query( "create table resilix_probe (id int primary key, n int not null check (n > 0))",