From 5a832fc5c2f26a088097bdc1d257ea7ddfa5c9f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 11:59:53 +0000 Subject: [PATCH 1/3] test(driver-sql): name the dialects a run did not exercise, under the counts A local run with no servers ends on `178 passed | 11 skipped` and a round reads it as coverage. It is not: every live-Postgres and live-MySQL cell is inside the skipped counts, and 56 further files report PASSED with a live cell skipped inside them. Adds a declaration-only reporter, registered after `default` so it lands under the summary. It names which dialects did not run, the env var that would run each, and a measured in-container PostgreSQL recipe. It changes no behaviour, adds no gate and cannot fail a run. Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude --- .../src/live-dialect-coverage.reporter.ts | 221 ++++++++++++++++++ packages/drivers/driver-sql/vitest.config.ts | 15 ++ 2 files changed, 236 insertions(+) create mode 100644 packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts diff --git a/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts new file mode 100644 index 00000000000..6d46f22f7af --- /dev/null +++ b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18200 — say, at the end of the run, which dialects this run did NOT exercise. + * + * ## The failure this exists for + * + * A local `pnpm --filter @objectstack/driver-sql test` with no servers present + * ends on a line a round reads as coverage: + * + * ``` + * Test Files 178 passed | 11 skipped (189) + * Tests 2627 passed | 168 skipped (2795) + * ``` + * + * Every live-Postgres and live-MySQL cell in this package is inside those + * skipped counts, and `178 passed` is SQLite only. Measured cost, three + * recorded instances: #17469 round 1 shipped four red CI jobs after a clean + * local sweep, one of them a live-PG-only fixture nothing local could have run; + * #17231 declared its live cells skipped locally and then went red on the MySQL + * cell nobody had run. The run "was healthy and delivered nothing". + * + * Worse than the 11 is what the 11 hides: on the same run, **56 further files + * report as PASSED while carrying a skipped live cell inside them**. A file + * with a green tick next to it is the least visible skip in the output. + * + * ## What this is, and deliberately is not + * + * DECLARATION ONLY. It reads the run that already happened and writes a block + * to stdout. It never fails, never changes an exit code, never skips or + * un-skips anything, and adds no gate — a suite that refuses to go green on a + * skipped live cell is a new required gate and is not this file's to introduce + * (#18200 carries that question). + * + * Every hook body is wrapped, so a defect in this reporter cannot redden a run + * either: a signal that can break the thing it reports on would be removed, and + * then there would be no signal. + * + * ## Why the two states differ, and where the numbers come from + * + * - WHICH dialects ran is read from {@link DIALECT_CELLS} — this package's own + * single source of truth for the driver axis — never inferred from test + * names. A cell's `env` is the variable that provisions it, so the block can + * name the exact knob rather than warning in the abstract. + * - HOW MANY tests were skipped is read from the run's own results. Those are + * reported as the raw skip counts they are; the block does not claim each + * one is a backend skip (two in this package are not). The causal sentence + * is attached to the per-dialect lines, which are exact. + * + * With a backend provisioned the block says so and the counts fall, so the + * signal is not the same text in both states. + * + * ## The recipe is printed because a recipe nobody looks for is not a signal + * + * #18200: 「a recipe nobody knows to look for is not a signal」. The steps below + * were run in the CI-shaped configuration before being written here — + * PostgreSQL 16.13 from `/usr/lib/postgresql/16/bin`, `initdb` + `pg_ctl` on a + * non-default port, server `timezone=Asia/Shanghai` against process + * `TZ=America/New_York`. Two details that are not cosmetic: + * + * - On Debian/Ubuntu `initdb` and `pg_ctl` are not on `PATH`, and `initdb` + * refuses to run as root — run both from `/usr/lib/postgresql//bin` + * as a non-root user (`su postgres -c '…'` in a container). + * - The three clocks must disagree: server zone, process `TZ`, and UTC. The + * matrix asserts that skew on purpose (`assertThreeWayZoneSkew`), because + * identical answers from a UTC server are answers no timezone could have + * perturbed — a pass that means nothing. + * + * MySQL is deliberately not in the printed recipe: provisioning it means + * installing a server package into a shared container, which is not something + * to put in front of a reader as a casual next step. The env var is named, so a + * reader who has one knows what to set. + * + * ## Wiring notes + * + * - Registered in `vitest.config.ts` AFTER `'default'`, so this block lands + * under the summary counts it qualifies — the place a round actually looks. + * - An explicit `--reporter=` on the command line replaces the configured + * list, this reporter included. That is vitest's own semantics; a round that + * asks for `--reporter=json` is not reading a terminal summary anyway. + */ + +import type { Reporter, TestModule } from 'vitest/node'; +import { DIALECT_CELLS, EXPECT_LIVE_DIALECTS } from './live-dialect-matrix.testkit.js'; + +/** Everything this reporter needs to know about the run, in one pass. */ +interface SkipCensus { + /** Modules vitest collected in this run. */ + files: number; + /** Tests collected across them. */ + tests: number; + /** Tests whose result state is `skipped`. */ + skippedTests: number; + /** Modules where EVERY collected test was skipped — the "N skipped" files. */ + filesFullySkipped: number; + /** Modules that ran, with at least one skipped test inside — the hidden half. */ + filesPartlySkipped: number; +} + +function census(testModules: ReadonlyArray): SkipCensus { + const c: SkipCensus = { + files: testModules.length, + tests: 0, + skippedTests: 0, + filesFullySkipped: 0, + filesPartlySkipped: 0, + }; + for (const mod of testModules) { + let total = 0; + let skipped = 0; + for (const test of mod.children.allTests()) { + total += 1; + if (test.result().state === 'skipped') skipped += 1; + } + c.tests += total; + c.skippedTests += skipped; + if (skipped === 0) continue; + if (skipped === total) c.filesFullySkipped += 1; + else c.filesPartlySkipped += 1; + } + return c; +} + +const PG_RECIPE = [ + " PGBIN=/usr/lib/postgresql/16/bin # Debian/Ubuntu: not on PATH, and initdb refuses root", + ' $PGBIN/initdb -D /tmp/os-pg -U postgres --auth=trust', + " $PGBIN/pg_ctl -D /tmp/os-pg -l /tmp/os-pg/server.log -w start \\", + " -o '-p 54988 -c timezone=Asia/Shanghai'", + ' OS_TEST_POSTGRES_URL=postgres://postgres@127.0.0.1:54988/postgres TZ=America/New_York \\', + ' pnpm --filter @objectstack/driver-sql test', + ' $PGBIN/pg_ctl -D /tmp/os-pg -m fast stop && rm -rf /tmp/os-pg # teardown', +]; + +function report(testModules: ReadonlyArray): string { + const live = DIALECT_CELLS.filter((cell) => cell.live); + const missing = live.filter((cell) => !cell.available); + const ran = DIALECT_CELLS.filter((cell) => cell.available); + const c = census(testModules); + const lines: string[] = ['']; + + if (missing.length === 0) { + lines.push( + ` driver-sql live-dialect coverage: all ${DIALECT_CELLS.length} dialects were exercised ` + + `(${ran.map((cell) => cell.label).join(', ')}).`, + ); + if (c.skippedTests > 0) { + lines.push( + ` ${c.skippedTests} test(s) were still skipped, in ` + + `${c.filesFullySkipped + c.filesPartlySkipped} of ${c.files} files, for reasons other ` + + `than a missing backend.`, + ); + } + lines.push(''); + return lines.join('\n'); + } + + if (EXPECT_LIVE_DIALECTS) { + // The runner declared it provisioned the servers, so the testkit has already + // turned each missing cell into a named FAILURE. Repeating the warning here + // would compete with a red the run is already carrying; name the cause once. + lines.push( + ` driver-sql live-dialect coverage: OS_EXPECT_LIVE_DIALECT_MATRIX=1, but ` + + `${missing.map((cell) => `${cell.label} (${cell.env})`).join(' and ')} ` + + `${missing.length === 1 ? 'was' : 'were'} not provisioned — this run reported that as a ` + + `named failure, not as a skip.`, + ); + lines.push(''); + return lines.join('\n'); + } + + const width = Math.max(...DIALECT_CELLS.map((cell) => cell.label.length)); + lines.push( + ` !! driver-sql live-dialect coverage: this run exercised ${ran.length} of ` + + `${DIALECT_CELLS.length} dialects.`, + '', + ); + for (const cell of DIALECT_CELLS) { + const label = cell.label.padEnd(width); + lines.push( + cell.available + ? ` ${label} RAN` + : ` ${label} NOT RUN -- set ${cell.env} to run it`, + ); + } + lines.push( + '', + ` The counts above this block are NOT coverage of the dialect(s) marked NOT RUN.`, + ` This run skipped ${c.skippedTests} test(s) across ` + + `${c.filesFullySkipped + c.filesPartlySkipped} of its ${c.files} files: ` + + `${c.filesFullySkipped} file(s) vitest reported as skipped, and ${c.filesPartlySkipped}`, + ` more it reported as PASSED with skipped tests inside them. Every ` + + `${missing.map((cell) => cell.label).join(' and ')} cell in this package is in that`, + ' population, and a green above says nothing about any of them.', + '', + ' CI runs those cells in `Temporal Conformance (live PG + MySQL)`, which sets', + ' OS_EXPECT_LIVE_DIALECT_MATRIX=1 -- there an unprovisioned cell is a named failure', + ' rather than a skip. To run the postgres half here (measured: ~1 min to provision):', + '', + ...PG_RECIPE, + '', + ' The server zone, TZ and UTC must all differ: the matrix asserts that skew, because', + ' identical answers from a UTC server are answers no timezone could have perturbed.', + '', + ); + return lines.join('\n'); +} + +/** + * Prints the block described above after the summary. Reads the run; writes + * stdout; returns nothing. + */ +export default class LiveDialectCoverageReporter implements Reporter { + onTestRunEnd(testModules: ReadonlyArray): void { + try { + process.stdout.write(`${report(testModules)}\n`); + } catch { + // Declaration-only means declaration-only: a reporter that can fail a run + // is a reporter someone deletes, and then the blind spot is back. + } + } +} diff --git a/packages/drivers/driver-sql/vitest.config.ts b/packages/drivers/driver-sql/vitest.config.ts index 778459f12ad..cfa7ef78c07 100644 --- a/packages/drivers/driver-sql/vitest.config.ts +++ b/packages/drivers/driver-sql/vitest.config.ts @@ -22,6 +22,21 @@ export default defineConfig({ // `beforeEach`, and the testkit module is cached per WORKER, not per file. // This runs once, before any worker, and is a no-op without a live URL. globalSetup: ['./src/live-dialect-matrix.globalsetup.ts'], + // #18200: name the dialects this run did NOT exercise, underneath the + // summary counts a round would otherwise read as coverage. ORDER IS THE + // POINT — the default reporter prints those counts from its own + // `onTestRunEnd` and reporters are invoked in list order, so this one comes + // second to land below them. What it prints, and how the two states differ: + // `src/live-dialect-coverage.reporter.ts`. + // + // `github-actions` is re-added by hand because vitest appends it only + // `if (!resolved.reporters.length)` — naming ANY reporter here would drop + // the annotations CI gets today. Same condition vitest itself uses, so CI's + // output is unchanged (vitest 4.1.11, `dist/chunks/coverage.*.js`). + reporters: + process.env.GITHUB_ACTIONS === 'true' + ? ['default', 'github-actions', './src/live-dialect-coverage.reporter.ts'] + : ['default', './src/live-dialect-coverage.reporter.ts'], }, resolve: { alias: [ From 91e68c64028bf6409385adc88b6748e61ea4da0b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 12:02:21 +0000 Subject: [PATCH 2/3] test(driver-sql): wrap the block, and offer the recipe only when PG is missing Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude --- .../src/live-dialect-coverage.reporter.ts | 133 ++++++++++++------ 1 file changed, 90 insertions(+), 43 deletions(-) diff --git a/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts index 6d46f22f7af..c3aacb8918e 100644 --- a/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts +++ b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts @@ -44,8 +44,18 @@ * name the exact knob rather than warning in the abstract. * - HOW MANY tests were skipped is read from the run's own results. Those are * reported as the raw skip counts they are; the block does not claim each - * one is a backend skip (two in this package are not). The causal sentence - * is attached to the per-dialect lines, which are exact. + * one is a backend skip. Measured on this package with no servers: 167 of + * the 168 are, and the one that is not is skipped on the SQLite cell + * (`schema-drift.base-type-mismatch.test.ts`, `skipIf(!corrupts)`). The + * causal sentence is attached to the per-dialect lines, which are exact. + * + * Attribution is deliberately NOT attempted per test. The reporter runs in the + * main process, where a skipped task carries no `meta` and no skip note (only + * `ctx.skip(note)` sets one, and a skipped test never reaches its body), so the + * only per-test channel left is the test's own NAME. Nine files in this package + * guard their live cells with a hand-rolled `skipIf` whose names the testkit + * never wrote, so a name matcher would silently report a smaller number than the + * truth — which is this card's own disease. * * With a backend provisioned the block says so and the counts fall, so the * signal is not the same text in both states. @@ -122,15 +132,30 @@ function census(testModules: ReadonlyArray): SkipCensus { } const PG_RECIPE = [ - " PGBIN=/usr/lib/postgresql/16/bin # Debian/Ubuntu: not on PATH, and initdb refuses root", - ' $PGBIN/initdb -D /tmp/os-pg -U postgres --auth=trust', - " $PGBIN/pg_ctl -D /tmp/os-pg -l /tmp/os-pg/server.log -w start \\", - " -o '-p 54988 -c timezone=Asia/Shanghai'", - ' OS_TEST_POSTGRES_URL=postgres://postgres@127.0.0.1:54988/postgres TZ=America/New_York \\', - ' pnpm --filter @objectstack/driver-sql test', - ' $PGBIN/pg_ctl -D /tmp/os-pg -m fast stop && rm -rf /tmp/os-pg # teardown', + ' PGBIN=/usr/lib/postgresql/16/bin # Debian/Ubuntu: off PATH, and initdb refuses root', + ' $PGBIN/initdb -D /tmp/os-pg -U postgres --auth=trust', + ' $PGBIN/pg_ctl -D /tmp/os-pg -l /tmp/os-pg/server.log -w start \\', + " -o '-p 54988 -c timezone=Asia/Shanghai'", + ' OS_TEST_POSTGRES_URL=postgres://postgres@127.0.0.1:54988/postgres TZ=America/New_York \\', + ' pnpm --filter @objectstack/driver-sql test', + ' $PGBIN/pg_ctl -D /tmp/os-pg -m fast stop && rm -rf /tmp/os-pg # teardown', ]; +/** Terminal-friendly wrapping, so a long sentence does not arrive as one ragged line. */ +function wrap(text: string, indent = ' ', width = 92): string[] { + const out: string[] = []; + let line = indent; + for (const word of text.split(/\s+/).filter(Boolean)) { + if (line.length > indent.length && line.length + 1 + word.length > width) { + out.push(line); + line = indent; + } + line += line.length > indent.length ? ` ${word}` : word; + } + if (line.length > indent.length) out.push(line); + return out; +} + function report(testModules: ReadonlyArray): string { const live = DIALECT_CELLS.filter((cell) => cell.live); const missing = live.filter((cell) => !cell.available); @@ -138,19 +163,20 @@ function report(testModules: ReadonlyArray): string { const c = census(testModules); const lines: string[] = ['']; + const filesWithSkips = c.filesFullySkipped + c.filesPartlySkipped; + if (missing.length === 0) { lines.push( - ` driver-sql live-dialect coverage: all ${DIALECT_CELLS.length} dialects were exercised ` + - `(${ran.map((cell) => cell.label).join(', ')}).`, + ...wrap( + `driver-sql live-dialect coverage: all ${DIALECT_CELLS.length} dialects were ` + + `exercised (${ran.map((cell) => cell.label).join(', ')}).` + + (c.skippedTests > 0 + ? ` ${c.skippedTests} test(s) were still skipped, in ${filesWithSkips} of ` + + `${c.files} files, for reasons other than a missing backend.` + : ''), + ), + '', ); - if (c.skippedTests > 0) { - lines.push( - ` ${c.skippedTests} test(s) were still skipped, in ` + - `${c.filesFullySkipped + c.filesPartlySkipped} of ${c.files} files, for reasons other ` + - `than a missing backend.`, - ); - } - lines.push(''); return lines.join('\n'); } @@ -159,12 +185,14 @@ function report(testModules: ReadonlyArray): string { // turned each missing cell into a named FAILURE. Repeating the warning here // would compete with a red the run is already carrying; name the cause once. lines.push( - ` driver-sql live-dialect coverage: OS_EXPECT_LIVE_DIALECT_MATRIX=1, but ` + - `${missing.map((cell) => `${cell.label} (${cell.env})`).join(' and ')} ` + - `${missing.length === 1 ? 'was' : 'were'} not provisioned — this run reported that as a ` + - `named failure, not as a skip.`, + ...wrap( + `driver-sql live-dialect coverage: OS_EXPECT_LIVE_DIALECT_MATRIX=1, but ` + + `${missing.map((cell) => `${cell.label} (${cell.env})`).join(' and ')} ` + + `${missing.length === 1 ? 'was' : 'were'} not provisioned — this run reported that ` + + `as a named failure, not as a skip.`, + ), + '', ); - lines.push(''); return lines.join('\n'); } @@ -177,31 +205,50 @@ function report(testModules: ReadonlyArray): string { for (const cell of DIALECT_CELLS) { const label = cell.label.padEnd(width); lines.push( - cell.available - ? ` ${label} RAN` - : ` ${label} NOT RUN -- set ${cell.env} to run it`, + cell.available ? ` ${label} RAN` : ` ${label} NOT RUN -- set ${cell.env} to run it`, ); } lines.push( '', - ` The counts above this block are NOT coverage of the dialect(s) marked NOT RUN.`, - ` This run skipped ${c.skippedTests} test(s) across ` + - `${c.filesFullySkipped + c.filesPartlySkipped} of its ${c.files} files: ` + - `${c.filesFullySkipped} file(s) vitest reported as skipped, and ${c.filesPartlySkipped}`, - ` more it reported as PASSED with skipped tests inside them. Every ` + - `${missing.map((cell) => cell.label).join(' and ')} cell in this package is in that`, - ' population, and a green above says nothing about any of them.', - '', - ' CI runs those cells in `Temporal Conformance (live PG + MySQL)`, which sets', - ' OS_EXPECT_LIVE_DIALECT_MATRIX=1 -- there an unprovisioned cell is a named failure', - ' rather than a skip. To run the postgres half here (measured: ~1 min to provision):', - '', - ...PG_RECIPE, - '', - ' The server zone, TZ and UTC must all differ: the matrix asserts that skew, because', - ' identical answers from a UTC server are answers no timezone could have perturbed.', + ...wrap( + `The counts above this block are NOT coverage of the dialect(s) marked NOT RUN. This ` + + `run skipped ${c.skippedTests} test(s) across ${filesWithSkips} of its ${c.files} ` + + `files: ${c.filesFullySkipped} vitest reported as skipped, and ${c.filesPartlySkipped} ` + + `more it reported as PASSED with skipped tests inside them — the least visible skip ` + + `in the output. Every ${missing.map((cell) => cell.label).join(' and ')} cell in this ` + + `package is in that population, and a green above says nothing about any of them.`, + ), '', + ...wrap( + 'CI runs those cells in `Temporal Conformance (live PG + MySQL)`, which sets ' + + 'OS_EXPECT_LIVE_DIALECT_MATRIX=1 — there an unprovisioned cell is a named failure ' + + 'rather than a skip.', + ), ); + if (missing.some((cell) => cell.id === 'pg')) { + lines.push( + ...wrap('To run the postgres half here (measured: ~1 min to provision):'), + '', + ...PG_RECIPE, + '', + ...wrap( + 'The server zone, TZ and UTC must all differ: the matrix asserts that skew, because ' + + 'identical answers from a UTC server are answers no timezone could have perturbed.', + ), + ); + } else { + // Only MySQL is left, and provisioning one means installing a server package + // into whatever container this is running in. That is not a step to put in + // front of a reader as a casual next line; naming the variable is enough. + lines.push( + ...wrap( + `Only the live mysql cell is left. Provisioning MySQL means installing a server into ` + + `this environment, so no recipe is offered here — set OS_TEST_MYSQL_URL if you ` + + `already have one, otherwise CI is where that cell runs.`, + ), + ); + } + lines.push(''); return lines.join('\n'); } From 7959bdc42fe54598460cb28823e77ce6dbf71cad Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 12:06:55 +0000 Subject: [PATCH 3/3] test(driver-sql): fold the CI note into the branch that follows it Claude-Session: https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af Co-authored-by: Claude --- .../src/live-dialect-coverage.reporter.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts index c3aacb8918e..e4915168036 100644 --- a/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts +++ b/packages/drivers/driver-sql/src/live-dialect-coverage.reporter.ts @@ -219,15 +219,14 @@ function report(testModules: ReadonlyArray): string { `package is in that population, and a green above says nothing about any of them.`, ), '', - ...wrap( - 'CI runs those cells in `Temporal Conformance (live PG + MySQL)`, which sets ' + - 'OS_EXPECT_LIVE_DIALECT_MATRIX=1 — there an unprovisioned cell is a named failure ' + - 'rather than a skip.', - ), ); + const ciNote = + 'CI runs those cells in `Temporal Conformance (live PG + MySQL)`, which sets ' + + 'OS_EXPECT_LIVE_DIALECT_MATRIX=1 — there an unprovisioned cell is a named failure ' + + 'rather than a skip.'; if (missing.some((cell) => cell.id === 'pg')) { lines.push( - ...wrap('To run the postgres half here (measured: ~1 min to provision):'), + ...wrap(`${ciNote} To run the postgres half here (measured: ~1 min to provision):`), '', ...PG_RECIPE, '', @@ -242,9 +241,9 @@ function report(testModules: ReadonlyArray): string { // front of a reader as a casual next line; naming the variable is enough. lines.push( ...wrap( - `Only the live mysql cell is left. Provisioning MySQL means installing a server into ` + - `this environment, so no recipe is offered here — set OS_TEST_MYSQL_URL if you ` + - `already have one, otherwise CI is where that cell runs.`, + `${ciNote} Provisioning MySQL locally means installing a server into this ` + + `environment, so no recipe is offered here — set OS_TEST_MYSQL_URL if you already ` + + `have one, otherwise CI is where that cell runs.`, ), ); }