Skip to content

Commit 1a0096f

Browse files
committed
test: size the distributed test budget for a contended runner
The CockroachDB job timed out on two tests of distributedDatabaseTest.ts, both at the 60s ceiling. Nothing was wrong with them: the whole job ran about three times slower than the run fifteen minutes earlier on the same branch, down to the test that only builds a schema and asserts its flags (9.2s, then 28.7s). CI brings up a three-node cluster in docker alongside the suite on a shared two-core runner, so that spread is the runner's, and a budget picked from the fast run reports timeouts in place of the failures the compatibility jobs exist to catch. Distributed backends get 120s, which covers the slow run at the observed spread. The number lived in two files as a literal 60000, and the block override in distributedDatabaseTest.ts caps the global rather than deferring to it, so raising one alone would have changed nothing. Both now read test/timeouts.ts, which holds the per-backend budget and the reasoning behind each value.
1 parent eff26e6 commit 1a0096f

3 files changed

Lines changed: 31 additions & 18 deletions

File tree

test/distributedDatabaseTest.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect, it } from 'vitest'
22
import * as helper from './testHelper.ts'
33
import { ctx } from './hooks.ts'
4+
import { isDistributedBackend, distributedTimeout } from './timeouts.ts'
45

56
// This file holds ONLY the invariants the general suite structurally cannot express. General
67
// behavioral coverage (fetch/complete/fail/retry/policies/flows/dead-letter/...) already runs in
@@ -17,17 +18,15 @@ import { ctx } from './hooks.ts'
1718
// noCoveringIndexes / noAdvisoryLocks) — the ONLY Postgres-side coverage of that DDL, since the
1819
// `DISTRIBUTED=true` job sets noSkipLocked + noMultiMutationCte but NOT the schema no* flags.
1920
//
20-
// Every test here calls helper.start(), which on CockroachDB pays slow per-test DDL (~8-9s observed
21-
// in CI), leaving little headroom under the 10s global timeout. Raise the default for the whole
22-
// block so startup jitter can't push a test over the edge (the concurrency tests keep their explicit
23-
// per-test overrides).
21+
// Every test here calls helper.start(), which on CockroachDB pays slow per-test DDL, leaving little
22+
// headroom under the 10s Postgres global. Raise the default for the whole block so startup jitter
23+
// can't push a test over the edge (the concurrency tests keep their explicit per-test overrides).
2424
//
25-
// The override must only LIFT the Postgres budget — never cap a distributed backend. vitest.config.ts
26-
// already gives cockroachdb/yugabytedb a 60s global; a flat 20s here would lower it for exactly the
27-
// heaviest tests (the withTransaction composition tests do slow DDL + an extra connection), which is
28-
// how they timed out at 20s on the CockroachDB run. So match that 60s on distributed backends.
29-
const isDistributedBackend = process.env.DB_TYPE === 'cockroachdb' || process.env.DB_TYPE === 'yugabytedb'
30-
const blockTimeout = isDistributedBackend ? 60000 : 20000
25+
// The override must only LIFT the Postgres budget, never cap a distributed backend: a per-test or
26+
// per-block value replaces the global in both directions, so a flat 20s here would lower it for
27+
// exactly the heaviest tests (the withTransaction composition tests do slow DDL plus an extra
28+
// connection). A distributed backend keeps the budget test/timeouts.ts sets for it.
29+
const blockTimeout = isDistributedBackend ? distributedTimeout : 20000
3130

3231
// The concurrency tests need more than the block default on Postgres, so they carry their own
3332
// override - which has to follow the same rule, since a per-test value replaces the block value in

test/timeouts.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// The per-backend test budget, in one place: vitest.config.ts reads it for the global default, and
2+
// so does every suite that raises the default for a block of its own. Both sides have to agree,
3+
// because a per-block value replaces the global in either direction and a block that names its own
4+
// number caps the backend that needs the most room.
5+
6+
export const isDistributedBackend = process.env.DB_TYPE === 'cockroachdb' || process.env.DB_TYPE === 'yugabytedb'
7+
8+
// PostgreSQL finishes a test's schema setup in well under a second, so 10s is generous and keeps a
9+
// hung test cheap to find.
10+
export const postgresTimeout = 10000
11+
12+
// CockroachDB and YugabyteDB pay heavy online-DDL/schema-rebuild costs per test, so they get a
13+
// budget sized for a contended runner rather than a good one: CI brings up a three-node CockroachDB
14+
// cluster in docker alongside the suite on a shared two-core runner, where the same tests that each
15+
// take ~9s on one run take ~30s on the next. The budget has to cover the slow run, or the
16+
// compatibility jobs report timeouts in place of the failures they exist to catch.
17+
export const distributedTimeout = 120000

vitest.config.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
import { defineConfig } from 'vitest/config'
2+
import { isDistributedBackend, distributedTimeout, postgresTimeout } from './test/timeouts.ts'
23

3-
// CockroachDB and YugabyteDB pay heavy online-DDL/schema-rebuild costs per test, which blow the
4-
// PostgreSQL-tuned 10s budget. Give the whole suite more headroom when running against a distributed
5-
// backend so the compatibility runs report real failures instead of timeouts.
6-
const isDistributedBackend = process.env.DB_TYPE === 'cockroachdb' || process.env.DB_TYPE === 'yugabytedb'
7-
const testTimeout = isDistributedBackend ? 60000 : 10000
8-
const hookTimeout = isDistributedBackend ? 60000 : 10000
4+
// See test/timeouts.ts for why a distributed backend gets a budget of its own.
5+
const timeout = isDistributedBackend ? distributedTimeout : postgresTimeout
96

107
export default defineConfig({
118
test: {
12-
testTimeout,
13-
hookTimeout,
9+
testTimeout: timeout,
10+
hookTimeout: timeout,
1411
include: ['test/**/*Test.ts'],
1512
globalSetup: ['./test/checkDuplicateTestNames.ts'],
1613
setupFiles: ['./test/hooks.ts'],

0 commit comments

Comments
 (0)