Skip to content

Commit b4abb0a

Browse files
os-muskclaude
andauthored
test(driver-sql): give the live dialect cells a derived per-test budget, at the seam every matrix consumer already goes through (#16578)
* test(driver-sql): give the live dialect cells a derived per-test budget, at the seam every matrix consumer already goes through The package sets no `testTimeout`, so its live PG + MySQL cells ran under vitest's default 5000 ms — the only live-database driver in the repo with no budget. A queue build spent more than that in one live cell and dequeued an unrelated PR. `declareDialectCell` now wraps LIVE cells only in a suite carrying `LIVE_CELL_TIMEOUT_MS`. SQLite cells are untouched and keep the 5 s guard, and there is deliberately no package-wide `testTimeout` — that knob has no cell-level discrimination. The value is derived from the corridor it has to sit in, not copied by analogy: above the driver's own longest legal connection wait (so a connect fault reports the driver's envelope rather than vitest's stopwatch), below the live job's stall guard (so a hung live test is named rather than swallowed). `live-dialect-matrix.budget.test.ts` pins both inequalities against the bounds read off the code they describe, plus the three vitest cascade rules the seam relies on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): tighten two derivation comments in the live-cell budget Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): attribute the 60_000 convention to all four prior repairs, not two Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): explain the two ways the runner-default fence can red Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): re-measure the live-cell budget against live PG AND live MySQL, and cite the convention's own stated limit The earlier table was taken with only a Postgres URL set, so it never touched the cell that actually timed out. Re-measured in one run against live Postgres 16.13 and live MySQL 8.0.46: live-mysql §2 costs 64 ms idle and 121 ms with the loop held, against the >5000 ms the queue build spent in that same body. Also records what the numbers do not license: this file is one of the heavier ones (12998's live cells peak at 248 ms), and #13902's own comment says it sized 60_000 by sibling convention and NOT as a claim that these tests run near it — which is precisely the half this constant adds a derived corridor to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ed5d557 commit b4abb0a

2 files changed

Lines changed: 291 additions & 1 deletion

File tree

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16434] The live-cell test budget, pinned to the two bounds it was DERIVED
5+
* from rather than to the literal it happens to be.
6+
*
7+
* `LIVE_CELL_TIMEOUT_MS`'s docblock states the derivation; this file makes it
8+
* executable, in the shape #13691 used for `MAX_SPAN_MS` — "the derivation is
9+
* pinned by arithmetic ... so the two halves cannot drift apart in silence".
10+
* Three things could move it and none of them would touch this constant:
11+
*
12+
* - the driver's own connection bounds (`withConnectBound`), which the budget
13+
* must stay ABOVE so a connect fault reports the driver's envelope and not
14+
* vitest's stopwatch;
15+
* - the live job's stall guard, which the budget must stay BELOW so a hung
16+
* live test is NAMED instead of being swallowed as an unattributed stall;
17+
* - vitest's own cascade rules, which are what makes one seam-level suite
18+
* option reach 40 files' live cells while leaving their explicit per-`it`
19+
* budgets alone.
20+
*
21+
* ⚠️ Every bound here is read from the thing it describes — a constructed knex
22+
* config, the workflow file — never re-typed. A pin that copies both sides of
23+
* an equality cannot fail. Each read carries a non-vacuity assertion for the
24+
* same reason: a regex that silently matches nothing is a phantom check.
25+
*
26+
* Runs on every runner: it constructs a driver but never connects, so no live
27+
* server is required and no cell of the matrix is involved.
28+
*/
29+
30+
import { describe, expect, it } from 'vitest';
31+
import { existsSync, readFileSync } from 'node:fs';
32+
import { dirname, join } from 'node:path';
33+
import { fileURLToPath } from 'node:url';
34+
import { SqlDriver } from './index.js';
35+
import { LIVE_CELL_TIMEOUT_MS } from './live-dialect-matrix.testkit.js';
36+
37+
/** The workspace root, found the way the testkit finds it — by its marker file. */
38+
function repoRoot(): string {
39+
let dir = dirname(fileURLToPath(import.meta.url));
40+
for (;;) {
41+
if (existsSync(join(dir, 'pnpm-workspace.yaml'))) return dir;
42+
const parent = dirname(dir);
43+
if (parent === dir) throw new Error('no pnpm-workspace.yaml above this file');
44+
dir = parent;
45+
}
46+
}
47+
48+
/**
49+
* The connection bounds the driver ACTUALLY installs, read off a constructed
50+
* pg config rather than copied from `SqlDriver`'s private constants.
51+
*
52+
* Constructing a driver opens no socket — knex builds its pool lazily — so this
53+
* is a pure read of the config the driver would connect with.
54+
*/
55+
function installedConnectBounds(): { poolCreateMs: number; dialectConnectMs: number } {
56+
const driver = new SqlDriver({
57+
client: 'pg',
58+
connection: 'postgres://u:p@127.0.0.1:5432/never_connected',
59+
} as any);
60+
const config = (driver as any).knex.client.config;
61+
return {
62+
poolCreateMs: Number(config?.pool?.createTimeoutMillis),
63+
dialectConnectMs: Number(config?.connection?.connectionTimeoutMillis),
64+
};
65+
}
66+
67+
describe('[#16434] the live-cell budget stays inside the corridor it was derived from', () => {
68+
it('sits ABOVE the longest wait the driver is entitled to for one connection', () => {
69+
const { poolCreateMs, dialectConnectMs } = installedConnectBounds();
70+
71+
// Non-vacuity: if the driver stopped installing these, both reads would be
72+
// NaN and every comparison below would be vacuously false-y rather than red.
73+
expect(
74+
Number.isFinite(poolCreateMs),
75+
'the driver installed no `pool.createTimeoutMillis` — this pin read nothing, so it is ' +
76+
'measuring nothing (see `withConnectBound`)',
77+
).toBe(true);
78+
expect(
79+
Number.isFinite(dialectConnectMs),
80+
'the driver installed no per-dialect connect timeout — this pin read nothing (see ' +
81+
'`DIALECT_CONNECT_TIMEOUT`)',
82+
).toBe(true);
83+
84+
// The floor. At or below the pool's create backstop, vitest kills the test
85+
// while the driver is still inside a wait it declares legal, and the
86+
// accurate connect message never prints.
87+
expect(
88+
LIVE_CELL_TIMEOUT_MS,
89+
`a live cell budget of ${LIVE_CELL_TIMEOUT_MS} ms does not clear the ${poolCreateMs} ms ` +
90+
`pool create backstop the driver installs, so a connect fault would be reported as ` +
91+
`"Test timed out" instead of by the driver's own envelope`,
92+
).toBeGreaterThan(poolCreateMs);
93+
expect(LIVE_CELL_TIMEOUT_MS).toBeGreaterThan(dialectConnectMs);
94+
95+
// ⭐ The status quo this card is about, asserted rather than recounted:
96+
// vitest's own default is below even the dialect connect bound.
97+
const VITEST_DEFAULT_TEST_TIMEOUT_MS = 5_000;
98+
expect(
99+
VITEST_DEFAULT_TEST_TIMEOUT_MS,
100+
'vitest’s default no longer sits below the driver’s connect bound — re-derive the ' +
101+
'floor above, because the reason an unbudgeted live cell could never report a connect ' +
102+
'fault has changed',
103+
).toBeLessThan(dialectConnectMs);
104+
});
105+
106+
it('sits BELOW the stall guard the live job wraps this suite in', () => {
107+
const ci = readFileSync(join(repoRoot(), '.github/workflows/ci.yml'), 'utf8');
108+
const guarded = /run-with-stall-guard\.mjs[^\n]*--stall-minutes\s+(\d+)[\s\S]{0,400}?driver-sql/;
109+
const match = guarded.exec(ci);
110+
111+
// Non-vacuity: no match means the workflow moved and this pin is measuring
112+
// nothing — a louder failure than a green over a regex that matches nothing.
113+
expect(
114+
match,
115+
'no `run-with-stall-guard --stall-minutes N` step wrapping the driver-sql suite was found ' +
116+
'in .github/workflows/ci.yml — the ceiling half of this budget’s derivation now reads ' +
117+
'nothing, so re-derive it against wherever that guard moved to',
118+
).not.toBeNull();
119+
120+
const stallWindowMs = Number(match![1]) * 60_000;
121+
expect(stallWindowMs).toBeGreaterThan(0);
122+
expect(
123+
LIVE_CELL_TIMEOUT_MS,
124+
`a live cell budget of ${LIVE_CELL_TIMEOUT_MS} ms is not comfortably under the ` +
125+
`${stallWindowMs} ms stall window: at that size a hung live test is killed as an ` +
126+
`unattributed stall instead of being named by vitest`,
127+
).toBeLessThan(stallWindowMs / 2);
128+
});
129+
});
130+
131+
describe('[#16434] the seam-level suite option behaves the way the seam assumes', () => {
132+
const SUITE_BUDGET = 4_242;
133+
const OWN_BUDGET = 1_337;
134+
135+
describe('a suite option', { timeout: SUITE_BUDGET }, () => {
136+
it('reaches a test declared directly in that suite', (ctx) => {
137+
expect(ctx.task.timeout).toBe(SUITE_BUDGET);
138+
});
139+
140+
describe('and a describe nested inside it — the shape every matrix consumer writes', () => {
141+
it('reaches a test one level deeper too', (ctx) => {
142+
expect(ctx.task.timeout).toBe(SUITE_BUDGET);
143+
});
144+
145+
it(
146+
'but does NOT override a budget the test declared for itself',
147+
(ctx) => {
148+
// The 62 explicit budgets already in this package (60 x 60_000, one
149+
// 40_000, one 120_000) keep the value their own site chose.
150+
expect(ctx.task.timeout).toBe(OWN_BUDGET);
151+
},
152+
OWN_BUDGET,
153+
);
154+
});
155+
});
156+
157+
it('leaves a test OUTSIDE that suite on the runner default — the SQLite cells', (ctx) => {
158+
expect(
159+
ctx.task.timeout,
160+
'a suite option leaked out of its own suite — the whole "live cells only" claim rests on ' +
161+
'it not doing that',
162+
).not.toBe(SUITE_BUDGET);
163+
164+
// ⛔ The fence, executable: this package must keep inheriting the runner
165+
// default outside a live cell. It reds two ways, and both are the point —
166+
// a package-wide `testTimeout` added to `vitest.config.ts` (which is the
167+
// fix #16434 declined), or a vitest upgrade that moves the default out from
168+
// under the FLOOR argument in `LIVE_CELL_TIMEOUT_MS`'s docblock. Either one
169+
// needs a human to re-derive, not a number bumped here.
170+
expect(
171+
ctx.task.timeout,
172+
'a test outside every live cell no longer runs at vitest’s 5000 ms default — either this ' +
173+
'package grew a package-wide `testTimeout` (the fix #16434 declined, because it has no ' +
174+
'cell-level discrimination) or the runner default moved; re-derive LIVE_CELL_TIMEOUT_MS ' +
175+
'rather than editing this number',
176+
).toBe(5_000);
177+
});
178+
});

‎packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,95 @@ export function declareUnprovisionedCell(cell: DialectCell, matrix: string): voi
382382
});
383383
}
384384

385+
/**
386+
* [#16434] The per-test budget a LIVE cell runs under — the one the matrix was
387+
* missing, and the reason a merge-queue build dequeued an unrelated PR.
388+
*
389+
* ## Why a live cell needs its own budget at all
390+
*
391+
* This package sets no `testTimeout`, so every cell inherited vitest's default
392+
* 5000 ms — the SQLite cell, which does no I/O, and the live cells, which talk
393+
* to a separate server over a socket. Measured against a live Postgres 16.13
394+
* and a live MySQL 8.0.46 in ONE run, on the cell that actually timed out:
395+
* `sql-driver-11224-update-stamp-precision.test.ts` §2 on live mysql (six
396+
* rounds of create → read → update → a server-side cursor comparison, so 24
397+
* live round-trips in one test body):
398+
*
399+
* ```
400+
* §2 idle loop loop held by a re-scheduling 12 ms hog (8 on 4 CPUs)
401+
* sqlite 21 ms 24 ms
402+
* live postgres 50 ms 50 ms
403+
* live mysql 64 ms 121 ms
404+
* ```
405+
*
406+
* ⚠️ Read what that does NOT license, in three directions.
407+
*
408+
* - It does not derive this number, and cannot. The queue build that dequeued
409+
* PR #16430 spent MORE than 5000 ms in that same live-mysql body — 40x to
410+
* 75x the figures above. A budget written as "measured cost times a margin"
411+
* would have landed in the low hundreds of milliseconds and been wrong by
412+
* two orders of magnitude. What the measurement establishes is the opposite:
413+
* the cost of the WORK is not what sets this bound, so the bound is derived
414+
* from what it has to sit BETWEEN instead.
415+
* - It is not the cost of live cells in general. This file is one of the
416+
* heavier ones; `sql-driver-12998-shadow-null-safe-key.test.ts`'s live cells
417+
* were measured on this same container at 248 ms for the slowest of them.
418+
* ⛔ Nothing here claims any live cell is normally near this ceiling.
419+
* - The numbers above are ONE world. Earlier readings taken on this container
420+
* with only `OS_TEST_POSTGRES_URL` set are not comparable with them: the box
421+
* and the cell population both differ. Whole-row comparisons only.
422+
*
423+
* ## The two bounds it sits between, both read off the code it guards
424+
*
425+
* FLOOR — the driver's own longest LEGAL wait for one connection. `SqlDriver`
426+
* bounds every live connection itself: a per-dialect connect timeout of
427+
* 10_000 ms and a deliberately looser `pool.createTimeoutMillis` backstop of
428+
* 15_000 ms ("The two bounds must not be equal. They race, and knex wins a
429+
* tie", `withConnectBound`). Any live round-trip may have to acquire a pooled
430+
* connection, so 15_000 ms is a wait the driver is ENTITLED to inside a test
431+
* body. A budget at or below it pre-empts the driver's own envelope: vitest
432+
* kills the test with `Test timed out in Nms` while the driver was still inside
433+
* a legal wait, and the accurate message the black-hole test pins (`timeout
434+
* expired` from pg, `connect ETIMEDOUT` from mysql2) never prints.
435+
* ⇒ the budget must be strictly ABOVE 15_000 ms.
436+
* ⭐ Note where that leaves the status quo: 5000 ms is below even the 10_000 ms
437+
* dialect connect bound, so an unbudgeted live cell could never report a
438+
* connect fault at all — vitest always won that race.
439+
*
440+
* CEILING — the stall guard the live job wraps this suite in
441+
* (`run-with-stall-guard.mjs --stall-minutes 10`, ci.yml). A per-test budget at
442+
* or above ten minutes of silence never fires first: the guard kills the
443+
* process group and reports an unattributed stall, losing WHICH test hung.
444+
* ⇒ the budget must be well BELOW 600_000 ms.
445+
*
446+
* ## The point inside that corridor, stated as a choice rather than a measurement
447+
*
448+
* Nothing in the corridor (15_000, 600_000) is distinguishable by measurement,
449+
* so the value is fixed by this package's OWN existing answer for live-touching
450+
* sites: 60 explicit `60_000` budgets across 22 files — #13688 and its sweep
451+
* #13902 put them on live test BODIES, #14213 and #14628 on the hooks that pay
452+
* a live connect. Adopting it leaves the live matrix with ONE live budget
453+
* instead of two, so a red at 60_000 ms is unambiguous about which bound it hit.
454+
*
455+
* ⭐ That convention states its own reasoning, and states its own limit —
456+
* `sql-driver-12998-shadow-null-safe-key.test.ts`, on the four budgets #13902
457+
* gave it: "Sized like this package's siblings — 60_000 is 7 of its 9 explicit
458+
* budgets — and NOT an assertion that these tests are normally anywhere near
459+
* that slow." So the precedent picked the value by convention and said so; what
460+
* it never had is a CORRIDOR the value must lie in. That is what this constant
461+
* adds, and it is the half that is derived. ⛔ It is NOT `driver-mongodb`'s 30_000 carried over
462+
* by analogy — that is that package's number, and this one is this package's.
463+
*
464+
* It clears the derived floor by 4x — arithmetically, room for four
465+
* full-length pool creations inside one test body before the budget could
466+
* pre-empt the driver — and sits an order of magnitude under the derived
467+
* ceiling. `live-dialect-matrix.budget.test.ts` pins both inequalities
468+
* against the bound the driver ACTUALLY installs, read off a constructed
469+
* connection rather than copied here, so the two halves cannot drift apart in
470+
* silence.
471+
*/
472+
export const LIVE_CELL_TIMEOUT_MS = 60_000;
473+
385474
/**
386475
* Run a cell EITHER WAY — measured when it is provisioned, declared un-run when
387476
* it is not — with no third outcome available to the caller.
@@ -423,7 +512,30 @@ export function declareDialectCell(
423512
declareUnprovisionedCell(cell, matrix);
424513
return;
425514
}
426-
measure(cell);
515+
// [#16434] LIVE cells only — the budget is applied HERE, at the one seam
516+
// every matrix consumer already goes through, rather than at each `it` in the
517+
// 40 files that call this. Same argument the rest of this module makes: a
518+
// guard copy-pasted per suite is a guard that can weaken in one copy and
519+
// nowhere else, and a new live file would arrive without it.
520+
//
521+
// ⛔ Deliberately NOT a package-wide `testTimeout` in `vitest.config.ts`.
522+
// That is the one knob with no cell-level discrimination, so it would raise
523+
// the ceiling for the SQLite cell too — measured at 21 ms idle / 24 ms hogged
524+
// for the same test body the live-mysql cell spends 64-121 ms on — and this
525+
// package's fast in-memory cells are where a 5 s guard is doing real work.
526+
//
527+
// A suite-level `timeout` cascades to the tests the consumer's own describes
528+
// declare, and an explicit per-`it` third argument still wins over it — both
529+
// asserted in `live-dialect-matrix.budget.test.ts`, so the 62 explicit
530+
// budgets already in this package (60 x 60_000, one 40_000, one 120_000)
531+
// keep the value their own site chose.
532+
if (!cell.live) {
533+
measure(cell);
534+
return;
535+
}
536+
describe(`live cell budget (${LIVE_CELL_TIMEOUT_MS} ms)`, { timeout: LIVE_CELL_TIMEOUT_MS }, () => {
537+
measure(cell);
538+
});
427539
}
428540

429541
/** What a server reports about its own timezone. */

0 commit comments

Comments
 (0)