From c84d0efb1a9c5fedb48e0aa5dab65bd88fa30c17 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 16:42:22 +0200 Subject: [PATCH] ci(server): hold unmeasured methods and only SQL errors to the probe baseline The Postgres probe only ratcheted the methods it managed to call. A method it could not build arguments for, one that threw before its first statement and one that timed out were counted and logged but held to nothing, so a new repository method with a callback parameter could land SQLite-only SQL unseen, and the part of the code the probe reaches could shrink without anyone noticing. pg-probe-baseline.json gains an `uncovered` list of those methods. A method outside it that goes unmeasured fails the run, and so does an entry whose method is measured now or gone, so the list only shrinks. It is seeded from the same first measurement as `failing`; a baseline without the key reads as unmeasured. The failing count also counted every refusal, including data errors the sample values cause (a "probe" string in a date column, SQLSTATE 22007), so renaming a parameter could fail the gate on a change that never touched the SQL. Only classes 42 and 0A count now; the rest is reported in the log and the step summary without being held. The docs said the probe measures every repository statement. They now say it measures what each method reaches with sample arguments against empty tables, and that the methods it cannot reach are held to the list. --- .github/workflows/test.yml | 11 +- CLAUDE.md | 4 +- server/CLAUDE.md | 4 +- server/scripts/pg-probe-baseline.json | 3 +- server/scripts/pg-probe.ts | 51 ++++++-- server/scripts/pg-probe/arg-samples.ts | 9 +- server/scripts/pg-probe/baseline.ts | 113 +++++++++++++++--- server/scripts/pg-probe/report.ts | 97 ++++++++++++--- .../unit/scripts/pg-probe/baseline.test.ts | 79 +++++++++--- .../unit/scripts/pg-probe/report.test.ts | 93 +++++++++++--- 10 files changed, 373 insertions(+), 91 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba13cea66..51b139e8f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,8 +20,8 @@ permissions: # gates cleared in preflight, they can only fail because tests failed. # client-e2e drives the real app in Chromium and WebKit, client-bundle # builds the client and checks each map engine keeps a chunk of its own, -# and postgres-probe runs the dialect helpers and every repository -# statement against a Postgres service. None of them feeds the scan. +# and postgres-probe runs the dialect helpers and the repository +# statements it reaches against a Postgres service. None of them feeds the scan. # - Scan `needs:` exactly the four coverage jobs. A coverage job failing # must stop the scan — a missing lcov file would make every line of that # package report as uncovered, and the dashboard would show a collapse @@ -529,9 +529,10 @@ jobs: # Postgres it derives a schema from the entities, runs every dialect helper # on SQLite and Postgres (every case must pass), and calls every public # repository method once, each statement in its own rolled-back transaction. - # The statements Postgres refuses are held per method to - # server/scripts/pg-probe-baseline.json, which may only shrink; an unmeasured - # baseline (failing: null) or a run that called no method fails. The measured + # The statements Postgres refuses as SQL are held per method to + # server/scripts/pg-probe-baseline.json, and so is the list of methods it + # could not measure (uncovered); both may only shrink. An unmeasured + # baseline (failing or uncovered null) or a run that called no method fails. The measured # baseline and the full report are uploaded either way, so the first run's # artifact seeds the file and a change that fixes a statement can lower it # without a local Postgres. diff --git a/CLAUDE.md b/CLAUDE.md index b29fb048d..5b7f3de80 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,7 +44,7 @@ cd client && npm run lint:skips # no .only; skipped tests may cd client && npm run lint:i18n-keys # every key the client names exists in en (CI gate) cd server && npm run lint:boundaries # no new import cycles or cross-domain reach-ins (CI gate) cd server && npm run lint:dialect # SQLite-only SQL spellings may only shrink (CI gate; rules in server/CLAUDE.md) -cd server && npm run probe:pg # dialect helpers + every repository statement against Postgres (CI job; needs TREK_PG_PROBE_URL) +cd server && npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job; needs TREK_PG_PROBE_URL) cd client && npm run theme:lint # flag styling that bypasses appearance tokens (local only) npm run typecheck --workspace=server # tsc --noEmit (also client/shared; server also has typecheck:tests) ``` @@ -119,7 +119,7 @@ These principles come out of a full-repo audit and shape all new code: ## Quality gates in CI -`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:boundaries`, the client's `lint:format`, `lint:layers`, `lint:offline`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and every repository statement against a Postgres service, held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. +`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:boundaries`, the client's `lint:format`, `lint:layers`, `lint:offline`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. - **`ci-ok` is the required check.** The branch ruleset requires `ci-ok` (plus `lint` and `check-target` from their own workflows), and `ci-ok` needs every job above except the scan. It fails if any of them failed or was cancelled and passes if they passed or were skipped. A new gate job counts only once it is listed in `ci-ok`'s `needs:`. - **What runs is decided by `changes`, not by `on.paths`**: `scripts/ci/changed-areas.mjs` maps the changed files to the `code`, `image` and `deploy` areas (tests: `npm run test:ci-scripts`). A path filter on the workflow would leave `ci-ok` unreported on a docs-only PR and block it. Add a new top-level file the build reads to the right area there. diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 6c3cfb501..c2fed5065 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -22,7 +22,7 @@ npm run db:call-graph -- --sync --tx [--domains nest/days,…] # sync DB-touchi npm run lint:size # no file in src/ or scripts/ past 1000 lines, longer ones only shrink (CI gate) npm run lint:boundaries # import cycles and cross-domain reach-ins may only shrink (CI gate) npm run lint:dialect # SQLite-only SQL spellings under src/ may only shrink (CI gate) -npm run probe:pg # dialect helpers + every repository statement against Postgres (CI job postgres-probe; needs TREK_PG_PROBE_URL) +npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job postgres-probe; needs TREK_PG_PROBE_URL) npm run contracts:dto-open # open shapes in the createZodDto schemas may only shrink (runs in the unit tests) ``` @@ -38,7 +38,7 @@ Single test: `npx vitest run tests/unit/nest/weather.controller.test.ts`, or `np **`lint:dialect`** (`scripts/dialect-lint.mjs`) reads every string and template literal under `src/` (comments do not count) and every `.insertId` read, and counts the SQLite-only spellings per file and rule: `INSERT OR IGNORE`/`REPLACE`, `datetime()`/`julianday()`/`unixepoch()` (also as the bare name handed to `eb.fn`), `strftime`, `json_extract` and its siblings, `GLOB`, `last_insert_rowid`, `AUTOINCREMENT`, a `PRAGMA` statement outside the `src/db/*.ts` lifecycle files and the migrations, and `||` concatenation in SQL. The counts are held to `scripts/dialect-baseline.json`; a new hit fails, and so does an entry above what its file holds now, so the change that removes one runs `npm run lint:dialect -- --update` (it never raises or adds an entry). The dialect layer `src/db/dialect/` is not scanned, and neither are the migrations up to `FROZEN_MIGRATIONS_THROUGH` in the script; a newer migration is held to the DML rules, while its DDL stays engine specific. -**`probe:pg`** (`scripts/pg-probe.ts`, CI job `postgres-probe`) measures what of the SQL runs on Postgres. Given an empty database (`TREK_PG_PROBE_URL`, or `--url=`; it refuses one holding tables it did not create) it derives a Postgres schema from the entities with MikroORM's schema generator (timestamps stay TEXT, SQLite-only defaults and `COLLATE NOCASE` get their Postgres spelling, no foreign keys), runs every dialect helper on an in-memory SQLite and on Postgres and checks both against the same expectation (`scripts/pg-probe/helper-cases.ts`; every case must pass), then calls every public method of every class in `src/db/repositories/*.ts` once. The arguments come from the parameter types, read with the TypeScript checker (`pg-probe/arg-samples.ts`), and the calls go through a MikroORM Postgres driver that runs each statement alone in a rolled-back transaction with a statement timeout and records whether Postgres accepted it (`pg-probe/recorder.ts`), so every statement meets empty tables and nothing persists. The distinct statements Postgres refuses are held per `Class.method` to `scripts/pg-probe-baseline.json`: a method may fail no more than its entry, and an entry above what it fails now fails too. The job uploads the measured baseline and a JSON report (`pg-probe-baseline` artifact), so the change that fixes a statement copies the lowered baseline from there or runs `npm run probe:pg -- --update` against a local Postgres (`docker run --rm -p 5432:5432 -e POSTGRES_PASSWORD=probe postgres:17`); `--update` never raises or adds an entry. While the baseline's `failing` is `null` it has never been measured, and the run fails (outside `--update`) until the first measurement from the artifact is committed, so the gate cannot land on `dev` holding nothing. A run that called no repository method fails too, so a broken plan can neither pass nor seed an empty baseline. The unit tests under `tests/unit/scripts/pg-probe/` cover the probe's logic, including every helper case on SQLite. +**`probe:pg`** (`scripts/pg-probe.ts`, CI job `postgres-probe`) measures what of the SQL runs on Postgres. Given an empty database (`TREK_PG_PROBE_URL`, or `--url=`; it refuses one holding tables it did not create) it derives a Postgres schema from the entities with MikroORM's schema generator (timestamps stay TEXT, SQLite-only defaults and `COLLATE NOCASE` get their Postgres spelling, no foreign keys), runs every dialect helper on an in-memory SQLite and on Postgres and checks both against the same expectation (`scripts/pg-probe/helper-cases.ts`; every case must pass), then calls every public method of every class in `src/db/repositories/*.ts` once. The arguments come from the parameter types, read with the TypeScript checker (`pg-probe/arg-samples.ts`), and the calls go through a MikroORM Postgres driver that runs each statement alone in a rolled-back transaction with a statement timeout and records whether Postgres accepted it (`pg-probe/recorder.ts`), so every statement meets empty tables and nothing persists. What it measures is the statements each method reaches with those sample arguments against empty tables, not every statement the code can send. The distinct statements Postgres refuses as SQL (SQLSTATE class 42 or 0A: syntax, an unknown function or column, a datatype mismatch, an unsupported feature) are held per `Class.method` to `failing` in `scripts/pg-probe-baseline.json`: a method may fail no more than its entry, and an entry above what it fails now fails too. A refusal over a sample value (class 22, say a `probe` string in a date column) or the empty tables is reported in the log and the step summary but not held, so renaming a parameter cannot turn a sample into dialect debt. The methods whose SQL the probe did not see (it could not build their arguments, they sent no SQL because they threw first or never reach the database, or they timed out) are held to the same file's `uncovered` list: a method outside the list that goes unmeasured fails the run, and so does a listed method that is measured now or gone, so the list only shrinks and a new method the sampler cannot reach does not land unchecked. Fix such a method by giving it parameters the sampler can build or by making it reach its SQL with sample values. The job uploads the measured baseline and a JSON report (`pg-probe-baseline` artifact), so the change that fixes a statement copies the lowered baseline from there or runs `npm run probe:pg -- --update` against a local Postgres (`docker run --rm -p 5432:5432 -e POSTGRES_PASSWORD=probe postgres:17`); `--update` never raises or adds an entry in either part. While the baseline's `failing` or `uncovered` is `null` it has never been measured, and the run fails (outside `--update`) until the first measurement from the artifact is committed, so the gate cannot land on `dev` holding nothing. A run that called no repository method fails too, so a broken plan can neither pass nor seed an empty baseline. The unit tests under `tests/unit/scripts/pg-probe/` cover the probe's logic, including every helper case on SQLite. ## Architecture invariants diff --git a/server/scripts/pg-probe-baseline.json b/server/scripts/pg-probe-baseline.json index 35785d4ee..21d4307ac 100644 --- a/server/scripts/pg-probe-baseline.json +++ b/server/scripts/pg-probe-baseline.json @@ -1,3 +1,4 @@ { - "failing": null + "failing": null, + "uncovered": null } diff --git a/server/scripts/pg-probe.ts b/server/scripts/pg-probe.ts index df26a210c..4d0975670 100644 --- a/server/scripts/pg-probe.ts +++ b/server/scripts/pg-probe.ts @@ -15,9 +15,18 @@ * (pg-probe/arg-samples.ts), through a MikroORM Postgres driver that * runs each statement in its own rolled-back transaction and records * whether Postgres accepted it (pg-probe/recorder.ts); - * 4. holds the statements Postgres refuses, per method, to - * scripts/pg-probe-baseline.json, which may only shrink - * (pg-probe/baseline.ts). + * 4. holds the statements Postgres refuses as SQL (SQLSTATE class 42 or + * 0A), per method, to scripts/pg-probe-baseline.json, which may only + * shrink (pg-probe/baseline.ts); a refusal over a sample value + * (class 22) or the empty tables is reported, not held; + * 5. holds the methods whose SQL it did not see (it could not call them, + * they sent no SQL, or they timed out) to the same file's `uncovered` + * list, which may only shrink too, so a new method the probe cannot + * reach fails the run instead of landing unmeasured. + * + * What it measures is the statements each method reaches with sample + * arguments against empty tables, not every statement the code can send: a + * branch that only runs on data the empty tables do not hold stays unseen. * * It needs a Postgres server, so CI runs it in the `postgres-probe` job * against a service container. Locally: @@ -30,7 +39,7 @@ * --update lower scripts/pg-probe-baseline.json to this run (never raises or adds an entry; * an unmeasured baseline gets its first measurement) * - * A baseline whose `failing` is `null` has never been measured and fails the + * A baseline whose `failing` or `uncovered` is `null` has never been measured and fails the * run (outside `--update`); the baseline the run writes with * `--next-baseline` (the CI artifact) is the first measurement to commit. A * run that called no repository method fails as well. @@ -102,15 +111,18 @@ export function repositoryFiles(dir: string): string[] { /** * 0 when every helper case passed, the probe called at least one repository - * method, the baseline is measured and no method fails more statements than - * its entry allows. An entry above what its method fails now fails the run - * too, and so does an unmeasured baseline, unless this run is the `--update` - * that lowers or seeds it. + * method, the baseline is measured, no method fails more statements than + * its entry allows and every method the probe did not measure is listed as + * uncovered. An entry above what the run saw (a count above what its method + * fails, an uncovered method that is measured now) fails the run too, and so + * does an unmeasured baseline, unless this run is the `--update` that lowers + * or seeds it. */ export function exitCode(input: Pick, update: boolean): number { if (passed(input)) return 0; const helpersOk = input.helpers.every((result) => result.failure === null); - return helpersOk && !probedNothing(input.summary) && input.verdict.grown.length === 0 && update ? 0 : 1; + const held = input.verdict.grown.length === 0 && input.verdict.newlyUncovered.length === 0; + return helpersOk && !probedNothing(input.summary) && held && update ? 0 : 1; } /** GitHub annotations when running in Actions, plain lines otherwise. */ @@ -163,18 +175,23 @@ async function main(): Promise { } const summary = summarize(results); - const verdict = compareWithBaseline(baseline, summary.failingByMethod); + const uncoveredNow = new Set(summary.uncovered.keys()); + const verdict = compareWithBaseline(baseline, summary.failingByMethod, uncoveredNow); const input: ReportInput = { summary, verdict, helpers, schemaFailures, results }; console.log(formatConsole(input)); - const next = lowerBaseline(baseline, summary.failingByMethod); + const next = lowerBaseline(baseline, summary.failingByMethod, uncoveredNow); if (args.nextBaseline) writeFileSync(args.nextBaseline, formatBaseline(next)); if (args.update) { writeFileSync(BASELINE_PATH, formatBaseline(next)); console.log(`Wrote ${path.relative(SERVER_ROOT, BASELINE_PATH)}.`); } if (args.report) { - const counts = { failingByMethod: Object.fromEntries(summary.failingByMethod), failuresByCode: Object.fromEntries(summary.failuresByCode) }; + const counts = { + failingByMethod: Object.fromEntries(summary.failingByMethod), + uncovered: Object.fromEntries(summary.uncovered), + failuresByCode: Object.fromEntries(summary.failuresByCode), + }; const report = { summary: { ...summary, ...counts }, verdict, helpers, schemaFailures, results }; writeFileSync(args.report, `${JSON.stringify(report, null, 2)}\n`); } @@ -193,6 +210,16 @@ async function main(): Promise { 'Lower it (--update, or the pg-probe-baseline artifact).', ); } + for (const method of verdict.newlyUncovered) { + annotate( + 'error', + `pg-probe: ${method} ${summary.uncovered.get(method) ?? 'went unmeasured'}, so its SQL was not checked. ` + + 'Give it arguments the probe can sample, or make it reach its SQL with them.', + ); + } + for (const method of verdict.nowCovered) { + annotate('error', `pg-probe: ${method} is listed as uncovered but is measured now (or gone). Drop it (--update, or the pg-probe-baseline artifact).`); + } if (probedNothing(summary)) annotate('error', `pg-probe: no repository method was called (${summary.methods} planned).`); if (verdict.unseeded) { annotate( diff --git a/server/scripts/pg-probe/arg-samples.ts b/server/scripts/pg-probe/arg-samples.ts index fc92f818d..56b73dda5 100644 --- a/server/scripts/pg-probe/arg-samples.ts +++ b/server/scripts/pg-probe/arg-samples.ts @@ -7,9 +7,12 @@ * date-shaped string when the name says so, an object gets every property * it can sample, an array one element). The values only have to get the * method to send its SQL; what the probe measures is whether Postgres - * accepts that SQL against an empty schema. A parameter the sampler cannot - * build (a callback, an EntityManager, an entity instance) makes the method - * unprobeable, which the report lists instead of guessing. + * accepts that SQL against an empty schema. A sample the column cannot take + * (a name-shaped string in a date column) is a data error, which the + * ratchet does not hold (pg-probe/baseline.ts). A parameter the sampler + * cannot build (a callback, an EntityManager, an entity instance) makes the + * method unprobeable: the report lists it instead of guessing, and the + * baseline's `uncovered` list must name it, so a new one fails the run. */ import path from 'node:path'; diff --git a/server/scripts/pg-probe/baseline.ts b/server/scripts/pg-probe/baseline.ts index 24844a05f..fe51e04c7 100644 --- a/server/scripts/pg-probe/baseline.ts +++ b/server/scripts/pg-probe/baseline.ts @@ -1,6 +1,7 @@ /** * The Postgres probe's ratchet: how many statements of each repository method - * Postgres may still refuse. + * Postgres may still refuse, and which methods the probe may still leave + * unmeasured. * * `scripts/pg-probe-baseline.json` maps `Class.method` to the number of * distinct statements of that method that failed on Postgres when the entry @@ -8,11 +9,27 @@ * without one), and an entry above what its method fails now fails the check * too, so the change that fixes a statement lowers the baseline with it. * `--update` lowers entries and drops the ones that reach zero or whose - * method is gone; it never raises or adds one. The only exception is the - * first run: a baseline whose `failing` is `null` has never been measured, - * so there is nothing to compare and the run fails (scripts/pg-probe.ts) - * until the first measurement, written by `--update` or taken from the CI - * artifact, is committed. + * method is gone; it never raises or adds one. + * + * Only statements Postgres refuses as SQL count here (SQLSTATE classes 42 + * and 0A, see `countsTowardRatchet`). A value the sample arguments got wrong + * (class 22, a sample string in a date column) says nothing about the + * dialect and is reported without being held. + * + * `uncovered` lists the methods whose SQL the probe did not see: the ones it + * could not call, the ones that sent no SQL (they threw first, or never + * reach the database) and the ones that timed out. A method outside that + * list that ends up uncovered (a new method the sampler cannot build + * arguments for, or an old one that starts throwing before its first + * statement) fails the run, and so does an entry whose method is measured + * now or gone, so the list only shrinks and the part of the code the probe + * reaches cannot quietly shrink with it. + * + * The only exception is the first run: a baseline whose `failing` or + * `uncovered` is `null` has never been measured, so there is nothing to + * compare and the run fails (scripts/pg-probe.ts) until the first + * measurement, written by `--update` or taken from the CI artifact, is + * committed. */ import { readFileSync } from 'node:fs'; @@ -20,6 +37,8 @@ import { readFileSync } from 'node:fs'; export interface ProbeBaseline { /** `Class.method` → failing statements allowed; `null` until the first measurement is written. */ failing: Record | null; + /** `Class.method` names the probe may leave unmeasured; `null` until the first measurement is written. */ + uncovered: string[] | null; } export interface BaselineVerdict { @@ -29,8 +48,25 @@ export interface BaselineVerdict { grown: { method: string; allowed: number; now: number }[]; /** Entries above what their method fails now (the method may be gone). */ stale: { method: string; allowed: number; now: number }[]; + /** Methods the probe did not measure that the baseline does not list. */ + newlyUncovered: string[]; + /** Entries in `uncovered` whose method is measured now, or gone. */ + nowCovered: string[]; } +/** + * Whether a refused statement counts toward the ratchet: SQLSTATE class 42 + * (syntax error, unknown function, operator or column, datatype mismatch) + * and class 0A (feature not supported) are what a SQLite-only spelling gets. + * Anything else (a data exception such as 22P02 or 22007, a constraint, a + * timeout) comes from the sample values or the empty tables, not the SQL. + */ +export function countsTowardRatchet(code: string): boolean { + return /^(42|0A)/.test(code); +} + +const METHOD_KEY = /^[A-Za-z_$][\w$]*\.[A-Za-z_$][\w$]*$/; + export function parseBaseline(text: string, source: string): ProbeBaseline { let data: unknown; try { @@ -41,14 +77,20 @@ export function parseBaseline(text: string, source: string): ProbeBaseline { if (data === null || typeof data !== 'object' || Array.isArray(data) || !('failing' in data)) { throw new Error(`${source} must be an object with a "failing" key`); } - const failing = (data as { failing: unknown }).failing; - if (failing === null) return { failing: null }; + return { + failing: parseFailing((data as { failing: unknown }).failing, source), + uncovered: parseUncovered((data as { uncovered?: unknown }).uncovered, source), + }; +} + +function parseFailing(failing: unknown, source: string): Record | null { + if (failing === null) return null; if (typeof failing !== 'object' || Array.isArray(failing)) { throw new Error(`${source}: "failing" must be null or an object of Class.method to counts`); } const entries: Record = {}; for (const [method, count] of Object.entries(failing as Record)) { - if (!/^[A-Za-z_$][\w$]*\.[A-Za-z_$][\w$]*$/.test(method)) { + if (!METHOD_KEY.test(method)) { throw new Error(`${source}: "${method}" is not a Class.method key`); } if (typeof count !== 'number' || !Number.isInteger(count) || count < 1) { @@ -56,16 +98,40 @@ export function parseBaseline(text: string, source: string): ProbeBaseline { } entries[method] = count; } - return { failing: entries }; + return entries; +} + +/** A missing `uncovered` key reads as unmeasured, so a baseline written before the list existed gets it seeded. */ +function parseUncovered(uncovered: unknown, source: string): string[] | null { + if (uncovered === undefined || uncovered === null) return null; + if (!Array.isArray(uncovered)) throw new Error(`${source}: "uncovered" must be null or an array of Class.method names`); + const seen = new Set(); + for (const method of uncovered as unknown[]) { + if (typeof method !== 'string' || !METHOD_KEY.test(method)) { + throw new Error(`${source}: "uncovered" holds ${JSON.stringify(method)}, expected a Class.method name`); + } + if (seen.has(method)) throw new Error(`${source}: "uncovered" lists ${method} twice`); + seen.add(method); + } + return [...seen]; } export function readBaseline(path: string): ProbeBaseline { return parseBaseline(readFileSync(path, 'utf8'), path); } -/** Compares what failed now (`Class.method` → failing statements, zero counts allowed) with the baseline. */ -export function compareWithBaseline(baseline: ProbeBaseline, failingNow: ReadonlyMap): BaselineVerdict { - if (baseline.failing === null) return { unseeded: true, grown: [], stale: [] }; +/** + * Compares what failed now (`Class.method` → failing statements, zero counts + * allowed) and which methods went unmeasured now with the baseline. + */ +export function compareWithBaseline( + baseline: ProbeBaseline, + failingNow: ReadonlyMap, + uncoveredNow: ReadonlySet, +): BaselineVerdict { + if (baseline.failing === null || baseline.uncovered === null) { + return { unseeded: true, grown: [], stale: [], newlyUncovered: [], nowCovered: [] }; + } const allowedFor = baseline.failing; const grown: BaselineVerdict['grown'] = []; const stale: BaselineVerdict['stale'] = []; @@ -78,15 +144,22 @@ export function compareWithBaseline(baseline: ProbeBaseline, failingNow: Readonl const now = failingNow.get(method) ?? 0; if (now < allowed) stale.push({ method, allowed, now }); } - return { unseeded: false, grown, stale }; + const listed = new Set(baseline.uncovered); + const newlyUncovered = [...uncoveredNow].filter((method) => !listed.has(method)).sort((a, b) => a.localeCompare(b)); + const nowCovered = [...listed].filter((method) => !uncoveredNow.has(method)).sort((a, b) => a.localeCompare(b)); + return { unseeded: false, grown, stale, newlyUncovered, nowCovered }; } /** * The baseline `--update` writes: every entry lowered to what its method - * fails now, zeros dropped, nothing added. An unseeded baseline is seeded - * with everything that fails now. + * fails now, zeros dropped, `uncovered` cut to the methods still unmeasured, + * nothing added. An unseeded part is seeded with what the run saw. */ -export function lowerBaseline(baseline: ProbeBaseline, failingNow: ReadonlyMap): ProbeBaseline { +export function lowerBaseline( + baseline: ProbeBaseline, + failingNow: ReadonlyMap, + uncoveredNow: ReadonlySet, +): ProbeBaseline { const next: Record = {}; if (baseline.failing === null) { for (const [method, now] of failingNow) if (now > 0) next[method] = now; @@ -96,7 +169,11 @@ export function lowerBaseline(baseline: ProbeBaseline, failingNow: ReadonlyMap 0) next[method] = now; } } - return { failing: Object.fromEntries(Object.entries(next).sort(([a], [b]) => a.localeCompare(b))) }; + const uncovered = baseline.uncovered === null ? [...uncoveredNow] : baseline.uncovered.filter((method) => uncoveredNow.has(method)); + return { + failing: Object.fromEntries(Object.entries(next).sort(([a], [b]) => a.localeCompare(b))), + uncovered: [...new Set(uncovered)].sort((a, b) => a.localeCompare(b)), + }; } export function formatBaseline(baseline: ProbeBaseline): string { diff --git a/server/scripts/pg-probe/report.ts b/server/scripts/pg-probe/report.ts index 6f1f61bdc..2abfcc4ea 100644 --- a/server/scripts/pg-probe/report.ts +++ b/server/scripts/pg-probe/report.ts @@ -3,7 +3,7 @@ * Markdown the CI job writes to its step summary. */ -import type { BaselineVerdict } from './baseline'; +import { countsTowardRatchet, type BaselineVerdict } from './baseline'; import type { HelperResult } from './helper-cases'; import type { StatementRecord } from './recorder'; @@ -23,6 +23,8 @@ export interface FailedStatement { code: string; message: string; sql: string; + /** A dialect refusal (SQLSTATE class 42 or 0A) the ratchet holds; anything else is reported only. */ + counted: boolean; } export interface ProbeSummary { @@ -34,9 +36,19 @@ export interface ProbeSummary { timedOut: number; /** Distinct statements per method, summed. */ statements: number; + /** Every distinct statement Postgres refused, counted toward the ratchet or not. */ failedStatements: FailedStatement[]; - /** `Class.method` → distinct statements Postgres refused; every called method appears, zeros included. */ + /** + * `Class.method` → distinct statements Postgres refused as SQL (SQLSTATE + * class 42 or 0A); every called method appears, zeros included. + */ failingByMethod: Map; + /** + * `Class.method` → why the probe saw none or not all of its SQL: it could + * not call it, it sent no SQL (it threw first, or never reaches the + * database), or it timed out. Held to the baseline's `uncovered` list. + */ + uncovered: Map; /** SQLSTATE → distinct failing statements. */ failuresByCode: Map; } @@ -51,15 +63,22 @@ export function summarize(results: readonly MethodResult[]): ProbeSummary { statements: 0, failedStatements: [], failingByMethod: new Map(), + uncovered: new Map(), failuresByCode: new Map(), }; for (const result of results) { if (result.unprobeable !== undefined) { summary.unprobeable += 1; + summary.uncovered.set(result.key, `could not be called: ${result.unprobeable}`); continue; } summary.called += 1; - if (result.timedOut) summary.timedOut += 1; + if (result.timedOut) { + summary.timedOut += 1; + summary.uncovered.set(result.key, 'timed out'); + } else if (result.statements.length === 0) { + summary.uncovered.set(result.key, result.threw === undefined ? 'sent no SQL' : `threw before any statement: ${result.threw}`); + } if (result.statements.length === 0) summary.withoutSql += 1; const seen = new Set(); const failed = new Map(); @@ -68,17 +87,28 @@ export function summarize(results: readonly MethodResult[]): ProbeSummary { if (!statement.outcome.ok && !failed.has(statement.sql)) failed.set(statement.sql, statement); } summary.statements += seen.size; - summary.failingByMethod.set(result.key, failed.size); + let counted = 0; for (const statement of failed.values()) { const outcome = statement.outcome; if (!('code' in outcome)) continue; - summary.failedStatements.push({ method: result.key, code: outcome.code, message: outcome.message, sql: statement.sql }); + const dialect = countsTowardRatchet(outcome.code); + if (dialect) counted += 1; + summary.failedStatements.push({ method: result.key, code: outcome.code, message: outcome.message, sql: statement.sql, counted: dialect }); summary.failuresByCode.set(outcome.code, (summary.failuresByCode.get(outcome.code) ?? 0) + 1); } + summary.failingByMethod.set(result.key, counted); } return summary; } +/** Refused statements the ratchet holds (dialect errors) and the ones it only reports. */ +export function splitRefusals(summary: Pick): { counted: FailedStatement[]; reported: FailedStatement[] } { + return { + counted: summary.failedStatements.filter((failed) => failed.counted), + reported: summary.failedStatements.filter((failed) => !failed.counted), + }; +} + /** SQL on one line, cut to `max` characters, for a log or a table cell. */ export function oneLine(sql: string, max = 160): string { const flat = sql.replace(/\s+/g, ' ').trim(); @@ -112,8 +142,10 @@ export function probedNothing(summary: Pick): boolean { /** * True when the run passes: every helper case on both engines, at least one - * repository method called, a measured baseline, and the ratchet held. An - * unmeasured baseline fails, so the gate cannot merge before it holds anything. + * repository method called, a measured baseline, and the ratchet held (no + * method failing more statements than allowed, no new unmeasured method, no + * entry left above what the run saw). An unmeasured baseline fails, so the + * gate cannot merge before it holds anything. */ export function passed(input: Pick): boolean { return ( @@ -121,7 +153,9 @@ export function passed(input: Pick b[1] - a[1])) { - lines.push(` ${code}: ${count}`); + lines.push(` ${code}: ${count}${countsTowardRatchet(code) ? '' : ' (not held)'}`); } for (const failed of summary.failedStatements) { - lines.push(` ${failed.method} ${failed.code} ${failed.message}`); + lines.push(` ${failed.method} ${failed.code} ${failed.message}${failed.counted ? '' : ' (not held)'}`); lines.push(` ${oneLine(failed.sql)}`); } + if (summary.uncovered.size > 0) { + lines.push(`Unmeasured: ${summary.uncovered.size} method(s) whose SQL the probe did not see:`); + for (const [method, why] of [...summary.uncovered.entries()].sort(([a], [b]) => a.localeCompare(b))) lines.push(` ${method}: ${why}`); + } if (probedNothing(summary)) { lines.push(`FAIL No repository method was called (${summary.methods} planned); there is nothing to hold to the baseline.`); } @@ -162,6 +204,15 @@ export function formatConsole(input: ReportInput): string { for (const entry of verdict.stale) { lines.push(`FAIL ${entry.method} is held at ${entry.allowed} failing statement(s) but fails ${entry.now} now; lower the baseline.`); } + for (const method of verdict.newlyUncovered) { + lines.push( + `FAIL ${method} ${summary.uncovered.get(method) ?? 'went unmeasured'}, and the baseline does not list it as uncovered. ` + + 'Give it arguments the probe can sample, or make it reach its SQL with them.', + ); + } + for (const method of verdict.nowCovered) { + lines.push(`FAIL ${method} is listed as uncovered but is measured now (or gone); drop it from the baseline.`); + } lines.push(passed(input) ? 'Postgres probe passed.' : 'Postgres probe failed.'); return lines.join('\n'); } @@ -174,9 +225,12 @@ export function formatMarkdown(input: ReportInput): string { out.push('| | |', '|---|---|'); out.push(`| Dialect helper cases (SQLite and Postgres) | ${input.helpers.length - failures.length} of ${input.helpers.length} pass |`); out.push(`| Schema statements refused | ${input.schemaFailures.length} |`); + const refusals = splitRefusals(summary); out.push(`| Repository methods | ${summary.methods} (${summary.called} called, ${summary.unprobeable} unprobeable) |`); + out.push(`| Methods whose SQL the probe did not see | ${summary.uncovered.size} |`); out.push(`| Distinct statements | ${summary.statements} |`); - out.push(`| Refused by Postgres | ${summary.failedStatements.length} |`); + out.push(`| Refused by Postgres as SQL (held) | ${refusals.counted.length} |`); + out.push(`| Refused over a sample value or the empty tables (reported only) | ${refusals.reported.length} |`); out.push(''); if (probedNothing(summary)) out.push('No repository method was called, so there is nothing to hold to the baseline.', ''); if (verdict.unseeded) { @@ -190,6 +244,12 @@ export function formatMarkdown(input: ReportInput): string { for (const entry of [...verdict.grown, ...verdict.stale]) out.push(`| ${cell(entry.method)} | ${entry.allowed} | ${entry.now} |`); out.push(''); } + if (verdict.newlyUncovered.length > 0 || verdict.nowCovered.length > 0) { + out.push('### Coverage ratchet', '', '| Method | Baseline | Now |', '|---|---|---|'); + for (const method of verdict.newlyUncovered) out.push(`| ${cell(method)} | measured | ${cell(summary.uncovered.get(method) ?? 'unmeasured')} |`); + for (const method of verdict.nowCovered) out.push(`| ${cell(method)} | uncovered | measured or gone |`); + out.push(''); + } if (failures.length > 0) { out.push('### Helper cases that failed', '', '| Engine | Case | Why |', '|---|---|---|'); for (const failure of failures) out.push(`| ${failure.engine} | ${cell(failure.name)} | ${cell(failure.failure ?? '')} |`); @@ -199,14 +259,15 @@ export function formatMarkdown(input: ReportInput): string { out.push('### Refused statements by SQLSTATE', '', '| SQLSTATE | Statements |', '|---|---|'); for (const [code, count] of [...summary.failuresByCode.entries()].sort((a, b) => b[1] - a[1])) out.push(`| ${code} | ${count} |`); out.push(''); - out.push('
Every refused statement', '', '| Method | SQLSTATE | Message |', '|---|---|---|'); - for (const failed of summary.failedStatements) out.push(`| ${cell(failed.method)} | ${failed.code} | ${cell(failed.message)} |`); + out.push('
Every refused statement', '', '| Method | SQLSTATE | Held | Message |', '|---|---|---|---|'); + for (const failed of summary.failedStatements) { + out.push(`| ${cell(failed.method)} | ${failed.code} | ${failed.counted ? 'yes' : 'no'} | ${cell(failed.message)} |`); + } out.push('', '
', ''); } - const unprobeable = input.results.filter((result) => result.unprobeable !== undefined); - if (unprobeable.length > 0) { - out.push('
Methods the probe could not call', '', '| Method | Why |', '|---|---|'); - for (const result of unprobeable) out.push(`| ${cell(result.key)} | ${cell(result.unprobeable ?? '')} |`); + if (summary.uncovered.size > 0) { + out.push('
Methods whose SQL the probe did not see', '', '| Method | Why |', '|---|---|'); + for (const [method, why] of [...summary.uncovered.entries()].sort(([a], [b]) => a.localeCompare(b))) out.push(`| ${cell(method)} | ${cell(why)} |`); out.push('', '
', ''); } return `${out.join('\n')}\n`; diff --git a/server/tests/unit/scripts/pg-probe/baseline.test.ts b/server/tests/unit/scripts/pg-probe/baseline.test.ts index 34598e2a6..892cb119e 100644 --- a/server/tests/unit/scripts/pg-probe/baseline.test.ts +++ b/server/tests/unit/scripts/pg-probe/baseline.test.ts @@ -2,21 +2,29 @@ * The Postgres probe's ratchet (scripts/pg-probe/baseline.ts). The probe * itself only runs in CI against a Postgres service, so the rules that decide * whether that job passes are pinned here: a method may not fail more - * statements than its entry, a stale entry fails until it is lowered, an - * unmeasured baseline has nothing to compare (the run fails on it, see - * report.test.ts), and --update never raises or adds. + * statements than its entry, a stale entry fails until it is lowered, a + * method the probe did not measure must be listed as uncovered and the list + * only shrinks, only SQL errors count, an unmeasured baseline has nothing to + * compare (the run fails on it, see report.test.ts), and --update never + * raises or adds. */ import { readFileSync } from 'node:fs'; import path from 'node:path'; import { describe, expect, it } from 'vitest'; -import { compareWithBaseline, formatBaseline, lowerBaseline, parseBaseline } from '../../../../scripts/pg-probe/baseline'; +import { compareWithBaseline, countsTowardRatchet, formatBaseline, lowerBaseline, parseBaseline } from '../../../../scripts/pg-probe/baseline'; const now = (entries: Record): Map => new Map(Object.entries(entries)); +const none = new Set(); describe('pg-probe baseline', () => { - it('PGPROBE-001: parses a measured and an unmeasured baseline', () => { - expect(parseBaseline('{"failing": null}', 'b.json')).toEqual({ failing: null }); - expect(parseBaseline('{"failing": {"TagsRepository.listByUser": 2}}', 'b.json')).toEqual({ failing: { 'TagsRepository.listByUser': 2 } }); + it('PGPROBE-001: parses a measured and an unmeasured baseline; a missing uncovered list reads as unmeasured', () => { + expect(parseBaseline('{"failing": null, "uncovered": null}', 'b.json')).toEqual({ failing: null, uncovered: null }); + expect(parseBaseline('{"failing": null}', 'b.json')).toEqual({ failing: null, uncovered: null }); + expect(parseBaseline('{"failing": {"TagsRepository.listByUser": 2}, "uncovered": ["A.b"]}', 'b.json')).toEqual({ + failing: { 'TagsRepository.listByUser': 2 }, + uncovered: ['A.b'], + }); + expect(parseBaseline('{"failing": {}}', 'b.json')).toEqual({ failing: {}, uncovered: null }); }); it('PGPROBE-002: refuses a malformed baseline instead of reading it as empty', () => { @@ -27,14 +35,21 @@ describe('pg-probe baseline', () => { expect(() => parseBaseline('{"failing": {"listByUser": 1}}', 'b.json')).toThrow(/not a Class.method key/); expect(() => parseBaseline('{"failing": {"A.b": 0}}', 'b.json')).toThrow(/expected a positive integer/); expect(() => parseBaseline('{"failing": {"A.b": 1.5}}', 'b.json')).toThrow(/expected a positive integer/); + expect(() => parseBaseline('{"failing": {}, "uncovered": {}}', 'b.json')).toThrow(/"uncovered" must be null or an array/); + expect(() => parseBaseline('{"failing": {}, "uncovered": ["nodot"]}', 'b.json')).toThrow(/expected a Class.method name/); + expect(() => parseBaseline('{"failing": {}, "uncovered": [1]}', 'b.json')).toThrow(/expected a Class.method name/); + expect(() => parseBaseline('{"failing": {}, "uncovered": ["A.b", "A.b"]}', 'b.json')).toThrow(/lists A.b twice/); }); - it('PGPROBE-003: an unmeasured baseline is flagged instead of compared', () => { - expect(compareWithBaseline({ failing: null }, now({ 'A.b': 3 }))).toEqual({ unseeded: true, grown: [], stale: [] }); + it('PGPROBE-003: an unmeasured baseline (either part null) is flagged instead of compared', () => { + const flagged = { unseeded: true, grown: [], stale: [], newlyUncovered: [], nowCovered: [] }; + expect(compareWithBaseline({ failing: null, uncovered: null }, now({ 'A.b': 3 }), new Set(['C.d']))).toEqual(flagged); + expect(compareWithBaseline({ failing: { 'A.b': 1 }, uncovered: null }, now({ 'A.b': 3 }), none)).toEqual(flagged); + expect(compareWithBaseline({ failing: null, uncovered: [] }, now({ 'A.b': 3 }), none)).toEqual(flagged); }); it('PGPROBE-004: a method failing more statements than its entry (or any without one) fails', () => { - const verdict = compareWithBaseline({ failing: { 'A.b': 1 } }, now({ 'A.b': 2, 'C.d': 1, 'E.f': 0 })); + const verdict = compareWithBaseline({ failing: { 'A.b': 1 }, uncovered: [] }, now({ 'A.b': 2, 'C.d': 1, 'E.f': 0 }), none); expect(verdict.grown).toEqual([ { method: 'A.b', allowed: 1, now: 2 }, { method: 'C.d', allowed: 0, now: 1 }, @@ -43,7 +58,7 @@ describe('pg-probe baseline', () => { }); it('PGPROBE-005: an entry above what its method fails now is stale, also when the method is gone', () => { - const verdict = compareWithBaseline({ failing: { 'A.b': 2, 'Gone.away': 1 } }, now({ 'A.b': 1 })); + const verdict = compareWithBaseline({ failing: { 'A.b': 2, 'Gone.away': 1 }, uncovered: [] }, now({ 'A.b': 1 }), none); expect(verdict.grown).toEqual([]); expect(verdict.stale).toEqual([ { method: 'A.b', allowed: 2, now: 1 }, @@ -51,13 +66,45 @@ describe('pg-probe baseline', () => { ]); }); - it('PGPROBE-006: --update lowers and drops entries but never raises or adds one', () => { - const lowered = lowerBaseline({ failing: { 'A.b': 2, 'C.d': 1, 'Gone.away': 1 } }, now({ 'A.b': 1, 'C.d': 5, 'New.one': 3 })); - expect(lowered).toEqual({ failing: { 'A.b': 1, 'C.d': 1 } }); + it('PGPROBE-009: a method the probe did not measure must be listed, and a listed method measured now or gone is stale', () => { + const verdict = compareWithBaseline( + { failing: {}, uncovered: ['Kept.out', 'Now.measured', 'Gone.away'] }, + now({ 'Now.measured': 0 }), + new Set(['Kept.out', 'New.callback', 'Old.throws']), + ); + expect(verdict).toEqual({ + unseeded: false, + grown: [], + stale: [], + newlyUncovered: ['New.callback', 'Old.throws'], + nowCovered: ['Gone.away', 'Now.measured'], + }); + expect(compareWithBaseline({ failing: {}, uncovered: ['A.b'] }, now({}), new Set(['A.b']))).toMatchObject({ newlyUncovered: [], nowCovered: [] }); }); - it('PGPROBE-007: the first --update seeds an unmeasured baseline with what fails, sorted', () => { - expect(lowerBaseline({ failing: null }, now({ 'Z.z': 1, 'A.a': 2, 'M.m': 0 }))).toEqual({ failing: { 'A.a': 2, 'Z.z': 1 } }); + it('PGPROBE-006: --update lowers and drops entries but never raises or adds one, in either part', () => { + const lowered = lowerBaseline( + { failing: { 'A.b': 2, 'C.d': 1, 'Gone.away': 1 }, uncovered: ['Kept.out', 'Now.measured'] }, + now({ 'A.b': 1, 'C.d': 5, 'New.one': 3 }), + new Set(['Kept.out', 'New.callback']), + ); + expect(lowered).toEqual({ failing: { 'A.b': 1, 'C.d': 1 }, uncovered: ['Kept.out'] }); + }); + + it('PGPROBE-007: the first --update seeds an unmeasured baseline with what fails and what went unmeasured, sorted', () => { + expect(lowerBaseline({ failing: null, uncovered: null }, now({ 'Z.z': 1, 'A.a': 2, 'M.m': 0 }), new Set(['Y.y', 'B.b']))).toEqual({ + failing: { 'A.a': 2, 'Z.z': 1 }, + uncovered: ['B.b', 'Y.y'], + }); + expect(lowerBaseline({ failing: { 'A.a': 3 }, uncovered: null }, now({ 'A.a': 2, 'N.n': 1 }), new Set(['B.b']))).toEqual({ + failing: { 'A.a': 2 }, + uncovered: ['B.b'], + }); + }); + + it('PGPROBE-010: only syntax and feature errors count toward the ratchet, not data errors from the sample values', () => { + for (const code of ['42601', '42883', '42804', '42P01', '42703', '0A000']) expect(countsTowardRatchet(code)).toBe(true); + for (const code of ['22P02', '22007', '22008', '23505', '57014', 'NOSQLSTATE']) expect(countsTowardRatchet(code)).toBe(false); }); it('PGPROBE-008: the checked-in baseline parses and formats back to itself', () => { diff --git a/server/tests/unit/scripts/pg-probe/report.test.ts b/server/tests/unit/scripts/pg-probe/report.test.ts index 4422092fc..e8ae71e06 100644 --- a/server/tests/unit/scripts/pg-probe/report.test.ts +++ b/server/tests/unit/scripts/pg-probe/report.test.ts @@ -11,7 +11,17 @@ import { describe, expect, it } from 'vitest'; import { exitCode, parseArgs, repositoryFiles } from '../../../../scripts/pg-probe'; import type { BaselineVerdict } from '../../../../scripts/pg-probe/baseline'; import type { HelperResult } from '../../../../scripts/pg-probe/helper-cases'; -import { formatConsole, formatMarkdown, oneLine, passed, probedNothing, summarize, type MethodResult, type ReportInput } from '../../../../scripts/pg-probe/report'; +import { + formatConsole, + formatMarkdown, + oneLine, + passed, + probedNothing, + splitRefusals, + summarize, + type MethodResult, + type ReportInput, +} from '../../../../scripts/pg-probe/report'; const ok = (sql: string) => ({ sql, outcome: { ok: true as const } }); const refused = (sql: string, code = '42883', message = 'function datetime(unknown) does not exist') => ({ @@ -25,9 +35,12 @@ const RESULTS: MethodResult[] = [ { key: 'C.silent', statements: [] }, { key: 'D.hung', statements: [ok('select 4')], timedOut: true }, { key: 'E.skipped', statements: [], unprobeable: 'fn: a function parameter' }, + { key: 'F.thrower', statements: [], threw: 'no trip 1' }, + { key: 'G.sampled', statements: [refused('select $1::date', '22007', 'invalid input syntax for type date: "probe"'), ok('select 5')] }, ]; -const CLEAN: BaselineVerdict = { unseeded: false, grown: [], stale: [] }; +const CLEAN: BaselineVerdict = { unseeded: false, grown: [], stale: [], newlyUncovered: [], nowCovered: [] }; +const UNSEEDED: BaselineVerdict = { ...CLEAN, unseeded: true }; const helper = (failure: string | null): HelperResult => ({ engine: 'postgres', name: 'dateOf', failure }); function input(overrides: Partial = {}): ReportInput { @@ -37,10 +50,34 @@ function input(overrides: Partial = {}): ReportInput { describe('pg-probe report', () => { it('PGPROBE-060: counts methods and distinct statements, and what Postgres refused per method and SQLSTATE', () => { const summary = summarize(RESULTS); - expect(summary).toMatchObject({ methods: 5, called: 4, unprobeable: 1, withoutSql: 1, timedOut: 1, statements: 6 }); - expect(summary.failingByMethod).toEqual(new Map([['A.clean', 0], ['B.broken', 2], ['C.silent', 0], ['D.hung', 0]])); - expect(summary.failuresByCode).toEqual(new Map([['42883', 1], ['42601', 1]])); - expect(summary.failedStatements.map((f) => `${f.method} ${f.code}`)).toEqual(['B.broken 42883', 'B.broken 42601']); + expect(summary).toMatchObject({ methods: 7, called: 6, unprobeable: 1, withoutSql: 2, timedOut: 1, statements: 8 }); + expect(summary.failingByMethod).toEqual( + new Map([['A.clean', 0], ['B.broken', 2], ['C.silent', 0], ['D.hung', 0], ['F.thrower', 0], ['G.sampled', 0]]), + ); + expect(summary.failuresByCode).toEqual(new Map([['42883', 1], ['42601', 1], ['22007', 1]])); + expect(summary.failedStatements.map((f) => `${f.method} ${f.code} ${f.counted}`)).toEqual([ + 'B.broken 42883 true', + 'B.broken 42601 true', + 'G.sampled 22007 false', + ]); + }); + + it('PGPROBE-068: a data error from a sample value is reported but not held to the baseline', () => { + const summary = summarize([{ key: 'G.sampled', statements: [refused('select $1::date', '22P02', 'bad'), refused('select x', '0A000', 'nope')] }]); + expect(summary.failingByMethod.get('G.sampled')).toBe(1); + expect(splitRefusals(summary).counted.map((f) => f.code)).toEqual(['0A000']); + expect(splitRefusals(summary).reported.map((f) => f.code)).toEqual(['22P02']); + }); + + it('PGPROBE-069: names every method whose SQL the probe did not see, and why', () => { + expect(summarize(RESULTS).uncovered).toEqual( + new Map([ + ['C.silent', 'sent no SQL'], + ['D.hung', 'timed out'], + ['E.skipped', 'could not be called: fn: a function parameter'], + ['F.thrower', 'threw before any statement: no trip 1'], + ]), + ); }); it('PGPROBE-061: passes only with every helper case green and the ratchet held', () => { @@ -48,14 +85,16 @@ describe('pg-probe report', () => { expect(passed(input({ helpers: [helper('expected 1, got 2')] }))).toBe(false); expect(passed(input({ verdict: { ...CLEAN, grown: [{ method: 'B.broken', allowed: 1, now: 2 }] } }))).toBe(false); expect(passed(input({ verdict: { ...CLEAN, stale: [{ method: 'B.broken', allowed: 3, now: 2 }] } }))).toBe(false); + expect(passed(input({ verdict: { ...CLEAN, newlyUncovered: ['E.skipped'] } }))).toBe(false); + expect(passed(input({ verdict: { ...CLEAN, nowCovered: ['Z.gone'] } }))).toBe(false); }); it('PGPROBE-066: an unmeasured baseline fails the run, and only --update (which seeds it) passes', () => { - const unseeded = input({ verdict: { unseeded: true, grown: [], stale: [] } }); + const unseeded = input({ verdict: UNSEEDED }); expect(passed(unseeded)).toBe(false); expect(exitCode(unseeded, false)).toBe(1); expect(exitCode(unseeded, true)).toBe(0); - expect(exitCode(input({ verdict: { unseeded: true, grown: [], stale: [] }, helpers: [helper('threw: boom')] }), true)).toBe(1); + expect(exitCode(input({ verdict: UNSEEDED, helpers: [helper('threw: boom')] }), true)).toBe(1); }); it('PGPROBE-067: a run that called no repository method fails, also under --update', () => { @@ -67,7 +106,7 @@ describe('pg-probe report', () => { expect(passed(empty)).toBe(false); expect(exitCode(empty, false)).toBe(1); expect(exitCode(empty, true)).toBe(1); - expect(exitCode(input({ summary, results, verdict: { unseeded: true, grown: [], stale: [] } }), true)).toBe(1); + expect(exitCode(input({ summary, results, verdict: UNSEEDED }), true)).toBe(1); } expect(probedNothing(summarize(RESULTS))).toBe(false); }); @@ -81,22 +120,41 @@ describe('pg-probe report', () => { expect(exitCode(input(), false)).toBe(0); }); + it('PGPROBE-072: --update forgives an uncovered entry that is measured now, never a newly unmeasured method', () => { + const covered = input({ verdict: { ...CLEAN, nowCovered: ['Z.gone'] } }); + expect(exitCode(covered, false)).toBe(1); + expect(exitCode(covered, true)).toBe(0); + const uncovered = input({ verdict: { ...CLEAN, newlyUncovered: ['E.skipped'] } }); + expect(exitCode(uncovered, false)).toBe(1); + expect(exitCode(uncovered, true)).toBe(1); + }); + it('PGPROBE-063: the console log names every refused statement and every ratchet failure', () => { const text = formatConsole( input({ helpers: [helper('expected 1, got 2')], - verdict: { unseeded: false, grown: [{ method: 'B.broken', allowed: 1, now: 2 }], stale: [{ method: 'Z.gone', allowed: 1, now: 0 }] }, + verdict: { + ...CLEAN, + grown: [{ method: 'B.broken', allowed: 1, now: 2 }], + stale: [{ method: 'Z.gone', allowed: 1, now: 0 }], + newlyUncovered: ['F.thrower'], + nowCovered: ['Y.measured'], + }, schemaFailures: [{ statement: 'create table "x" (a text collate "NOCASE")', message: 'collation "NOCASE" does not exist' }], }), ); expect(text).toContain('FAIL [postgres] dateOf: expected 1, got 2'); expect(text).toContain('collation "NOCASE" does not exist'); - expect(text).toContain('Statements: 6 distinct, 2 refused by Postgres.'); + expect(text).toContain('Statements: 8 distinct, 3 refused by Postgres (2 as SQL, held to the baseline; 1 over a sample value'); + expect(text).toContain(' 22007: 1 (not held)'); + expect(text).toContain(' F.thrower: threw before any statement: no trip 1'); + expect(text).toContain('FAIL F.thrower threw before any statement: no trip 1, and the baseline does not list it as uncovered.'); + expect(text).toContain('FAIL Y.measured is listed as uncovered but is measured now (or gone); drop it from the baseline.'); expect(text).toContain('B.broken 42883 function datetime(unknown) does not exist'); expect(text).toContain('FAIL B.broken sends 2 statement(s) Postgres refuses, 1 allowed.'); expect(text).toContain('FAIL Z.gone is held at 1 failing statement(s) but fails 0 now; lower the baseline.'); expect(text.trim().endsWith('Postgres probe failed.')).toBe(true); - const unseeded = formatConsole(input({ verdict: { unseeded: true, grown: [], stale: [] } })); + const unseeded = formatConsole(input({ verdict: UNSEEDED })); expect(unseeded).toContain('FAIL Baseline: not measured yet'); expect(unseeded).toContain('as scripts/pg-probe-baseline.json'); expect(unseeded.trim().endsWith('Postgres probe failed.')).toBe(true); @@ -109,8 +167,15 @@ describe('pg-probe report', () => { expect(md).toContain('Result: **failed**'); expect(md).toContain('| postgres | a\\|b | got \'x\' |'); expect(md).toContain('| 42883 | 1 |'); - expect(md).toContain('| E.skipped | fn: a function parameter |'); - const unseeded = formatMarkdown(input({ verdict: { unseeded: true, grown: [], stale: [] } })); + expect(md).toContain('| E.skipped | could not be called: fn: a function parameter |'); + expect(md).toContain('| Refused by Postgres as SQL (held) | 2 |'); + expect(md).toContain('| Refused over a sample value or the empty tables (reported only) | 1 |'); + expect(md).toContain('| G.sampled | 22007 | no | invalid input syntax for type date: "probe" |'); + const ratchet = formatMarkdown(input({ verdict: { ...CLEAN, newlyUncovered: ['C.silent'], nowCovered: ['Z.gone'] } })); + expect(ratchet).toContain('### Coverage ratchet'); + expect(ratchet).toContain('| C.silent | measured | sent no SQL |'); + expect(ratchet).toContain('| Z.gone | uncovered | measured or gone |'); + const unseeded = formatMarkdown(input({ verdict: UNSEEDED })); expect(unseeded).toContain('Result: **failed**'); expect(unseeded).toContain('has not been measured yet, which fails the run'); expect(formatMarkdown(input({ summary: summarize([]), results: [] }))).toContain('No repository method was called');