Skip to content

test(driver-sql): budget the 35 inline live-DDL it() blocks across 8 files - #14098

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-13902-live-ddl-test-budgets
Sep 1, 2026
Merged

test(driver-sql): budget the 35 inline live-DDL it() blocks across 8 files#14098
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-13902-live-ddl-test-budgets

Conversation

@claude

@claude claude Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #13902

The defect

Thirty-five it(...) blocks nested in a declareDialectCell(...) callback, across eight
files in packages/drivers/driver-sql, each construct a fresh new SqlDriver(...)
against the cell's live MySQL/Postgres server inside their own body. Every one of them
therefore pays a full live connect cycle per test — and, for all but one charset probe,
schema-sync DDL plus an information_schema read-back on top of it — rather than sharing a
driver opened once in a beforeAll.

None carried an explicit timeout, so all inherited vitest's own 5000ms default. That is not
a chosen budget: packages/drivers/driver-sql/vitest.config.ts sets no testTimeout (re-read
on this tree, not taken on faith), so the 5000ms really is vitest's number and nobody in this
repo picked it for live connect/DDL work.

What this is, and what it is not. This is a structural sweep — "does an explicit budget
exist" — not a list of observed CI failures. There is no live MySQL/PG in this environment,
so none of these 35 blocks executed here (proved below). The evidence that the shape is a real
defect is #13688: one measured instance, whose failures were timeouts rather than assertion
failures, with no MySQL error in the logs, on a diff that touched no driver. These 35 are not
35 observations of a timeout
, and this PR does not claim they are.

The fix

Each of the 35 gets an explicit 60_000 budget, matching the value chosen for this exact
shape in commit e4dc299a (PR #13908), plus one rationale comment per file that names why the
budget exists and — copying that precedent's disclaimer — states it is not an assertion
that these tests are normally anywhere near that slow.

60_000 is not re-decided here. Measured on this tree, the package carries 9 explicit numeric
budgets: 60_000 x7, 40_000 x1, 120_000 x1. The 40_000 outlier is sql-driver-connect-bound.test.ts,
whose budget is load-bearing for a duration assertion (elapsed > 8_000 and < 20_000) — a
different purpose entirely.

Scoping: per-it(), not per-file, and not per-package

Deliberately not touched

Five it() blocks that are inside a dialect cell and unbudgeted, but whose live work is
hoisted into a beforeAll — a different cost model:

File Blocks Why excluded
sql-driver-backend-fault-envelope.test.ts 2 (:360, :379) re-verified: beforeAll at :160 / :343 builds the driver; the it()s issue 1-2 queries against the already-open connection
sql-driver-diagnostic-value-probe.test.ts 3 (:366, :385, :431) stronger still — these are synchronous it()s reading a Map populated in beforeAll; they touch no server at all

The second file is also the repo's own answer for this shape: its beforeAll already carries
}, 60_000) at :335. Budgeting the hook, not the it(), is what the reuse shape wants.

A2.1 — re-run the triage sweep on the current tree, with its control

The triage's expression (^\s*\}, ?[0-9_]+\);) still returns 2 against #13688's landed
target file, so a zero elsewhere is a reading rather than a broken regex. Re-run structurally
(TypeScript AST, not grep) at dda969cd71:

  • The card's 9 files / 37 blocks still hold exactly — per-file counts 9/6/5/4/4/3/3/2/1.
  • One file the card missed: sql-driver-diagnostic-value-probe.test.ts, 3 more in-cell
    unbudgeted blocks
    (40 in total, not 37). The card's instrument appears to have pre-filtered
    to files with zero budget lines, and this file has one — on its beforeAll. That file is
    the only such leak in the package.
  • Those 3 fall into the card's own excluded class (A2.2), so the in-scope set is 35 either
    way. The card's scoping was right; only its instrument was slightly lossy.

A2.2 — are all 37 really full connect/DDL cycles? Checked one by one, not on faith

No. Of the 40 in-cell unbudgeted blocks, 35 construct a fresh new SqlDriver(...) in their
own body
and 5 do not (the table above). Every one of the 35 was inspected for its
constructor argument; three of them (sql-driver-11389-date-tz-skew.test.ts:439,
sql-driver-11565-row-byte-budget.test.ts:201, sql-driver-keyed-text-mysql.test.ts:285) do
their DDL through driver.execute('create table ...') or an internal probe rather than
initObjects(...), but all three still open a fresh live connection per test, so they belong
to the same class. sql-driver-11565-row-byte-budget.test.ts:201 is the cheapest of them — a
connect plus one metadata query, no DDL — and is included on the connect cost alone.

A2.3 — same-shaped live tests OUTSIDE declareDialectCell: yes, 4. Reported, not folded in

Discriminated by reading each new SqlDriver(ARG) argument rather than by guessing:

Location Gate Config
live-dialect-matrix.isolation.test.ts:281 it.skipIf(!PG_CELL.available) PG_CELL.config()
live-dialect-matrix.isolation.test.ts:314 it.skipIf(!PG_CELL.available) PG_CELL.config()
sql-driver-autonumber-cold-race.test.ts:159 it.skipIf(!pgCell.available) pgCell.config()
sql-driver-json-binding-without-ddl.test.ts:173 it { ...PG_CELL.config(), schemaMode: 'validate-only' }

The other 42 outside-cell driver-constructing it()s are embedded SQLite
(dialectCell('sqlite').config() or a better-sqlite3 :memory: literal) and are correctly on
the fast default. Per the dispatch order these 4 are reported, not folded into this PR;
filed with the beforeAll gap as a separate finding.

A2.4 — does a 60_000 budget turn a should-be-red test into a slowly-green one?

No — and this was checked with a firing positive control for every detector, so each zero
is a measurement rather than a broken expression.

Detector Positive control (whole package) Hits inside the 35
clock read (Date.now / performance.now) 9 0
duration assertion (elapsed, span compare) 1 (sql-driver-connect-bound.test.ts:196) 0
explicit sleep / timer 4 (sql-driver-server-timing.test.ts:68) 0
unbounded loop 0 in *.test.ts; expression proved on live-dialect-matrix.testkit.ts:190 (for (;;)) 0
lock / deadlock subject 7 (sql-driver-autonumber-tx.test.ts:94) 0
concurrency race (Promise.all) 7 0
retry / poll loop 8 0
prose naming a timeout as the expected red 160 3, all false positives (Shanghai, change, unchanged)

None of the 35 asserts on duration, sleeps, loops unboundedly, or contends for a lock. Each
asserts on information_schema contents, an error envelope, or row values — so the failure
mode a real regression produces here is an assertion mismatch, which a larger budget does
not hide.

The honest cost, stated rather than buried: a future regression that manifests as unbounded
slowness would now take 60s to surface instead of 5s. That is the same cost #13688 accepted
for the same reason, and it is what the per-file disclaimer comment exists to keep visible.

Ablation

Two parts, because the literal one cannot reach the mechanism. No rebuild leg applies and
none was fabricated
: the mutated subject is a test file vitest reads from disk, and these
files import the driver by relative specifier (./sql-driver.js), never through the package
exports map, so nothing here is served from dist/.

Part 1 — literal, on the committed implementation. Predicted before running: no
change
, because the edited blocks sit in unprovisioned cells. Removed all three budgets from
sql-driver-string-maxlength-varchar.test.ts; mutation confirmed on disk by blob hash
(646b0050a5... to 9450642b90...) and by count (3 budgets to 0), never by the editor's exit
code. Both legs read identically: Test Files 1 passed (1), Tests 4 passed | 2 skipped (6),
with the two skips named:

↓ ... matrix (live mysql) > is provisioned — set OS_TEST_MYSQL_URL to run this cell of the D-A3 driver axis
↓ ... matrix (live postgres) > is provisioned — set OS_TEST_POSTGRES_URL to run this cell of the D-A3 driver axis

That is the measurement establishing the vacuity of a local run over these blocks — it is
not evidence the budget does nothing. Restored with git checkout HEAD -- ABSOLUTE_PATH
under a trap ... EXIT INT TERM; restore proven by the blob returning to 646b0050a5... and
git diff HEAD being empty for the path.

(A first attempt at Part 1 exited 1 in both legs with No test files found — a repo-relative
path handed to a vitest whose cwd is the package. That is an invocation error, recorded as
such and re-run correctly; it is not reported as a red leg.)

Part 2 — mechanism probe, the leg that can run here. Predicted before running: without a
third argument the probe reds on 5000ms; with , 60_000 it goes green. A temporary probe
sleeping 6500ms, run under this package's real vitest config:

leg A (no third argument):   × probe: 6500ms of work ... 5007ms
                             → Test timed out in 5000ms.          exit 1
leg B (with , 60_000):       ✓ probe: 6500ms of work ... 6505ms   exit 0

Both legs matched the prediction. This is what Part 1 cannot reach: that the inherited default
really is 5000ms in this package, and that the third argument this PR adds is honoured. The
probe was deleted afterwards and git status --porcelain verified empty.

Tests

All at HEAD 5abe7caa02, via scripts/pm/os-verify-lock.sh (shared-box seconds).

  • Dependency closure built first: pnpm --workspace-concurrency=2 --filter '@objectstack/driver-sql^...' build
    VERDICT command-exit 0.
  • pnpm --filter @objectstack/driver-sql typecheck && pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2
    VERDICT command-exit 0 (one && chain, so the verdict certifies both).
    • vitest: 148 passed | 9 skipped test files (157); 2260 passed | 134 skipped tests (2394).
    • The typecheck really does read the edited files: tsc --noEmit --listFiles names all 8 of
      them (157 *.test.ts in the program), so this is not the "typecheck excludes tests" green
      over source nothing read.
  • The 35 edited blocks did not execute here. OS_TEST_MYSQL_URL, OS_TEST_POSTGRES_URL and
    OS_EXPECT_LIVE_DIALECT_MATRIX are all unset, so declareDialectCell collapses each live
    cell to one named skip (quoted above). They run for real in Temporal Conformance (live PG + MySQL).
  • Gate union, re-derived after the final commit with
    node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (change set read by the
    script itself from merge base dda969cd7: 8 paths, matching the diff): 22 path-matched plus 5
    convention-triggered ("adds or edits a test file"). 23 run, all exit 0, each captured
    before any pipe. Their own verdict lines, not a bare $?:
    • check-cross-package-test-inputs: OK: 24 package(s) read outside themselves, all declared
    • check:where-matcher: 323 matcher(s) discovered ... 0 silently-wrong ... baseline key set verified against dda969c: no files added (ratchet did not move)
    • check-nul-bytes: OK (scanned 7702 text file(s) ... no raw ASCII control bytes)
    • check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt
    • plus check:doc-authoring, check:logger-receiver-detach, check:objectql-double-limit,
      check:page-declaration-shape, check:published-files, check:slot-lookup,
      check:test-source-alias, check:type-source-resolution, check:type-check-coverage,
      check:engine-double-contract, check-ci-filter-parity, check-comment-mask-adoption,
      check-keyed-text-bounds, check-plugin-teardown-shape, check-shard-attestation,
      check-undeclared-dep-imports, docs-audit/check-affected-docs, docs-audit/check-drift-comment.
    • NOT MEASURED (never counted as passes): check:dual-build-cjs-loads and
      check:type-check-debt need the whole workspace built (CI's own Build Core scope, not
      warranted locally for a test-only diff); check:test-completeness exits
      PREREQUISITE NOT MET without a saved turbo run test log, which CI tees and this branch
      cannot.
  • Full repo lint, not a narrowing: node --stack-size=4000 node_modules/eslint/bin/eslint.js . --no-inline-config
    VERDICT command-exit 0, zero output lines, 61s.

No changeset

skip-changeset. The entire diff is eight test files — timeout budgets and explanatory
comments. No production code, no assertion and no control changed; nothing is published from
any package. Same disposition as PR #13908, whose diff was the same kind.


Generated by Claude Code

Thirty-five it() blocks nested in a declareDialectCell(...) callback across
eight files each construct a FRESH new SqlDriver(...) against the cell's live
MySQL/Postgres server inside their own body, so every one pays a full live
connect cycle -- and for all but the charset probe, schema-sync DDL and an
information_schema read-back -- per test rather than once in a beforeAll.
None carried an explicit timeout, so all inherited vitest's own 5000ms
default; the package sets no testTimeout, so that really is vitest's number
and not a chosen one.

Give each an explicit 60_000 budget, matching e4dc299, with one rationale
comment per file naming why and disclaiming that it asserts anything about
how slow these tests normally are.

Deliberately NOT touched: the five in-cell it() blocks whose live work is
hoisted into a beforeAll (sql-driver-backend-fault-envelope.test.ts x2,
sql-driver-diagnostic-value-probe.test.ts x3) -- a different cost model,
and the latter file's beforeAll is already budgeted at 60_000.

No production code, no assertion, and no control changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 1, 2026
@github-actions github-actions Bot added the size/m label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ddea371d344d5c8ecafb41e69f836e5710c32e11packageMentionDocs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-sql: 37 more live-DDL it() blocks across 9 files inherit vitest's 5000ms default (same shape as #13688)

2 participants