From 009280cb491cea9773aa3b76166837b1ce96c32d Mon Sep 17 00:00:00 2001 From: Langston-2479 Date: Sat, 17 Oct 2026 22:47:38 -0400 Subject: [PATCH] ci: time limits on ci.yml's jobs, and a ten-minute --test-timeout for npm test Two macOS Node 20 Test steps hung on October 17 and 18 (runs 39082421393 and 39080940576). Each printed its last result about five minutes in, then nothing, and with no timeout-minutes the check stayed pending toward GitHub's six-hour default. Both stopped after tune-out-e2e's suite. Node's runner reports files in order, so the file that never finished is tune-out-e2e or the next one, tune-out-gate-composition; the logs can't say which. - npm test passes --test-timeout=600000. Under Node 20 (20.20.2 in CI) the bound applies to each test file as a whole, from the file's own start: a file still running at the bound is cancelled and fails by its path, whether it's stuck in a test or while loading. Under Node 24 (24.21.0 in CI) it applies to each test and hook, and a file stuck outside its tests and hooks, such as while loading, runs on to the job's limit. Ten minutes is loose on purpose: the slowest file, mcpl-runtime-lifecycle, takes about three on its own. - The unforced step's node --test gets the same flag. - timeout-minutes: 20 on Build & Test and 10 on the pack check. In the 59 most recent runs, passing Build & Test jobs took 3.6 to 6.5 minutes and pack checks 16 to 30 seconds. - test/npm-test-timeout pins npm test's flag and the unforced step's, and keeps Build & Test's limit at least a passing job's time above the bound, so that under Node 20 a file that sticks within that time fails by name before GitHub cancels the job. - CONTRIBUTING's npm test line names the flag, with a paragraph on what it bounds under each Node. - mcpl-coalescing's comment that npm test sets no timeout of its own now names the bound. publish.yml runs npm test under Node 20, so its Test step gets the file bound through package.json. Its jobs get no limits of their own. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 14 ++++++-- CONTRIBUTING.md | 7 +++- package.json | 2 +- test/mcpl-coalescing.test.ts | 2 +- test/npm-test-timeout.test.ts | 62 +++++++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 test/npm-test-timeout.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4267a42f4..bde60d0c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,11 @@ jobs: build-and-test: name: Build & Test (${{ matrix.os }}, node ${{ matrix.node }}) runs-on: ${{ matrix.os }} + # Without a limit, a step that hangs leaves the check pending until + # GitHub's six-hour limit. Passing runs take under seven minutes. A + # test file stuck under Node 20, or a test stuck under Node 24, fails + # sooner and by name, through npm test's --test-timeout. + timeout-minutes: 20 strategy: fail-fast: false # Both platforms: a package-lock.json regenerated over an existing @@ -54,9 +59,11 @@ jobs: # npm test runs with --test-force-exit, which hides async work a test # leaves running, such as a late arrival after stop() ("Queue is # closed"). These files run unforced so that class fails here, not in a - # later PR's qualification. + # later PR's qualification. They get npm test's --test-timeout too: + # under Node 20 it fails a file whose leftover work keeps it alive, by + # name, and under Node 24 the job's limit ends such a file. - name: Test unforced (work in flight at stop) - run: node --test dist/test/stop-with-work-in-flight.test.js dist/test/stop-with-host-operations.test.js dist/test/conversation-routing.test.js + run: node --test --test-timeout=600000 dist/test/stop-with-work-in-flight.test.js dist/test/stop-with-host-operations.test.js dist/test/conversation-routing.test.js # The release as npm would pack it, installed on its own and loaded, as # the publish workflow checks before it publishes (see @@ -70,6 +77,9 @@ jobs: pack-check: name: Packed release installs on its own runs-on: ubuntu-latest + # A limit for the same reason as build-and-test's. Passing runs take + # under a minute. + timeout-minutes: 10 steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff93c8237..c235d834b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -201,7 +201,7 @@ never do. ```bash npm install npm run build # tsc; tests run from dist/, so build first -npm test # node --test --test-force-exit dist/test/*.test.js, with test/helpers/force-exit-blocking-stdout loaded first +npm test # node --test --test-force-exit --test-timeout=600000 dist/test/*.test.js, with test/helpers/force-exit-blocking-stdout loaded first npx tsc --noEmit # typecheck ``` @@ -209,6 +209,11 @@ npx tsc --noEmit # typecheck code you no longer have — always build before testing, and after switching branches. The `pretest` hook copies `test/fixtures/*.mjs` into `dist/`. +`--test-timeout=600000` bounds each test file as a whole under Node 20, +and each test under Node 24, at ten minutes; the slowest file, +`mcpl-runtime-lifecycle`, takes about three. Each job in `ci.yml` also has +a `timeout-minutes` limit, which ends a hang the test timeout doesn't reach. + Push-time CI (`ci.yml`) builds, typechecks and tests every push and PR on ubuntu and macos. Lockfiles are deliberately gitignored in this repo, so CI installs with `npm install` rather than `npm ci`, and there is no lock to diff --git a/package.json b/package.json index ef074cb7c..80a3da6dd 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "clean": "rm -rf dist", "typecheck": "tsc --noEmit", "pretest": "mkdir -p dist/test/fixtures && cp test/fixtures/*.mjs dist/test/fixtures/", - "test": "node --import ./dist/test/helpers/force-exit-blocking-stdout.js --test --test-force-exit dist/test/*.test.js", + "test": "node --import ./dist/test/helpers/force-exit-blocking-stdout.js --test --test-force-exit --test-timeout=600000 dist/test/*.test.js", "preversion": "node scripts/release-bump.mjs && node scripts/release-holds.mjs", "version": "node scripts/release-changelog.mjs && git add CHANGELOG.md changelog.d", "prepublishOnly": "node scripts/release-holds.mjs && npm run build" diff --git a/test/mcpl-coalescing.test.ts b/test/mcpl-coalescing.test.ts index c5bac52d1..82b439b22 100644 --- a/test/mcpl-coalescing.test.ts +++ b/test/mcpl-coalescing.test.ts @@ -975,7 +975,7 @@ test('R16b: a producer retry re-wakes a batch left asleep by a failed freeze', a }); // The timeout bounds a drain that never returns, such as a backoff regressed -// to a tight loop; npm test sets no timeout of its own. +// to a tight loop, far sooner than npm test's ten-minute --test-timeout. /** * R17's harness: a coalescing fixture whose store fails while `failing` is * set, its recovery backoff at `baseMs`, and every traced recovery wake with diff --git a/test/npm-test-timeout.test.ts b/test/npm-test-timeout.test.ts new file mode 100644 index 000000000..68b6591c3 --- /dev/null +++ b/test/npm-test-timeout.test.ts @@ -0,0 +1,62 @@ +/** + * `npm test` passes --test-timeout=600000 to Node's runner. Under Node 20 the + * bound applies to each test file as a whole, from the file's own start: a + * file still running at ten minutes is cancelled and fails by its path, + * whether it's stuck in a test or while loading. Under Node 24 it applies to + * each test and each hook: one still running at ten minutes fails, and the + * report names its test or suite. A file stuck outside its tests and hooks, + * such as while loading, runs on until CI's timeout-minutes ends the job. + * + * Ten minutes is loose on purpose: under Node 20 the slowest file, + * mcpl-runtime-lifecycle, takes about three, and a bound near that could fail + * a correct change on a loaded runner. + * + * ci.yml's unforced step passes the same bound. Build & Test's time limit + * stays at least a passing job's time above it, so that under Node 20 a file + * that sticks within that time fails by name before GitHub cancels the job. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; + +const read = (path: string) => readFileSync(new URL(`../../${path}`, import.meta.url), 'utf8'); +const npmTest = (JSON.parse(read('package.json')) as { scripts: { test: string } }).scripts.test; +const npmTimeoutMs = Number(/ --test-timeout=(\d+) /.exec(npmTest)?.[1]); + +// Passing Build & Test jobs took up to 6.5 minutes in 59 runs in October 2026. +const SLOWEST_PASSING_JOB_MS = 7 * 60_000; + +/** A ci.yml job's timeout-minutes, from the lines between its key and the next one at its level. */ +function jobMinutes(ci: string, job: string): number | undefined { + const lines = ci.split('\n'); + const start = lines.indexOf(` ${job}:`); + if (start < 0) return undefined; + for (const line of lines.slice(start + 1)) { + if (/^ {0,2}\S/.test(line)) return undefined; + const m = /^ {4}timeout-minutes: (\d+)\s*(?:#.*)?$/.exec(line); + if (m) return Number(m[1]); + } + return undefined; +} + +test('npm test passes --test-timeout=600000 to the runner, before the test files', () => { + assert.match(npmTest, / --test-timeout=600000 dist\/test\/\*\.test\.js$/); +}); + +test("ci.yml's unforced node --test passes npm test's --test-timeout", () => { + const ci = read('.github/workflows/ci.yml'); + const unforced = ci.split('\n').filter((line) => /^\s*run: node --test /.test(line)); + assert.equal(unforced.length, 1, 'ci.yml has one unforced node --test step'); + assert.equal(Number(/ --test-timeout=(\d+)(?: |$)/.exec(unforced[0])?.[1]), npmTimeoutMs); +}); + +test("Build & Test and the pack check have time limits, and Build & Test's leaves a passing job's time above the test timeout", () => { + const ci = read('.github/workflows/ci.yml'); + const buildAndTest = jobMinutes(ci, 'build-and-test'); + assert.ok(buildAndTest !== undefined, 'build-and-test has a timeout-minutes'); + assert.ok(jobMinutes(ci, 'pack-check') !== undefined, 'pack-check has a timeout-minutes'); + assert.ok( + buildAndTest * 60_000 >= npmTimeoutMs + SLOWEST_PASSING_JOB_MS, + `build-and-test's ${buildAndTest} minutes leave less than a passing job's time above the ${npmTimeoutMs} ms test timeout`, + ); +});