diff --git a/scripts/run-tests.ts b/scripts/run-tests.ts index 0f27b84..3ddbce0 100644 --- a/scripts/run-tests.ts +++ b/scripts/run-tests.ts @@ -16,7 +16,7 @@ // // Pass names or substrings to run a subset: `npx tsx scripts/run-tests.ts cliche` import { readdirSync, readFileSync } from 'node:fs'; -import { spawn } from 'node:child_process'; +import { spawn, type ChildProcess } from 'node:child_process'; /** Tests that never terminate on their own and are not batch-runnable. */ const INTERACTIVE = new Set([ @@ -41,7 +41,8 @@ const CHROME_ANYWAY = new Set([ * in screenshot.ts that every browser-driven test reaches for. */ const CHROME_MARKERS = /takeScreenshot|computeLayout|withPage|withIsolatedPage|puppeteer|from '\.\/src\/screenshot\.js'/; -const TIMEOUT_MS = 180_000; +// FRAMESMITH_TEST_TIMEOUT_MS overrides it (test-runner-timeout.ts uses 3s). +const TIMEOUT_MS = Number(process.env.FRAMESMITH_TEST_TIMEOUT_MS) || 180_000; type Tier = 'fast' | 'full'; const tier: Tier = process.argv.includes('--full') ? 'full' : 'fast'; @@ -59,14 +60,28 @@ const selected = all .filter((f) => tier === 'full' || !needsChrome(f)) .filter((f) => filters.length === 0 || filters.some((q) => f.includes(q))); +/** The test running now. Its own process group doesn't get the terminal's + * Ctrl-C, so the runner passes an interrupt on before exiting. */ +let current: ChildProcess | undefined; +for (const signal of ['SIGINT', 'SIGTERM'] as const) { + process.on(signal, () => { + if (current?.pid) { try { process.kill(-current.pid, 'SIGKILL'); } catch { /* already gone */ } } + process.exit(130); + }); +} + function run(file: string): Promise<{ file: string; ok: boolean; why: string; ms: number }> { return new Promise((resolve) => { const started = Date.now(); - const child = spawn('npx', ['tsx', file], { stdio: ['ignore', 'pipe', 'pipe'] }); + // Its own process group, so a timeout kills npx AND the tsx under it. + // Killing npx alone orphaned tsx, which kept the pipes open, and 'close' + // never fired: one hung test stalled the whole run. + const child = spawn('npx', ['tsx', file], { stdio: ['ignore', 'pipe', 'pipe'], detached: true }); + current = child; let out = ''; child.stdout.on('data', (d) => { out += d; }); child.stderr.on('data', (d) => { out += d; }); - const timer = setTimeout(() => child.kill('SIGKILL'), TIMEOUT_MS); + const timer = setTimeout(() => process.kill(-child.pid!, 'SIGKILL'), TIMEOUT_MS); child.on('close', (code, signal) => { clearTimeout(timer); const ms = Date.now() - started; diff --git a/test-runner-timeout.ts b/test-runner-timeout.ts new file mode 100644 index 0000000..227aadb --- /dev/null +++ b/test-runner-timeout.ts @@ -0,0 +1,64 @@ +// scripts/run-tests.ts must survive a hung test. Its timeout used to SIGKILL +// only the `npx` wrapper; the `tsx` process under it was orphaned, kept the +// output pipes open, and the runner waited on them forever (a Chrome hang in +// test-text-props.ts stalled a full-tier run for 25 minutes). This runs the +// real runner in a temp dir holding one hanging test and one passing test, +// with a 3s timeout, and checks it reports the timeout, finishes, and leaves +// no process of the hung test behind. +// +// Usage: npx tsx test-runner-timeout.ts + +import { mkdtempSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { spawn, execFileSync } from 'node:child_process'; + +let allPass = true; +function check(name: string, cond: boolean, extra?: string) { + if (!cond) allPass = false; + console.log(`${cond ? 'PASS' : 'FAIL'} ${name}${extra ? ` — ${extra}` : ''}`); +} + +const dir = mkdtempSync(join(tmpdir(), 'framesmith-runner-timeout-')); +symlinkSync(resolve('node_modules'), join(dir, 'node_modules')); +writeFileSync(join(dir, 'test-hang.ts'), 'setInterval(() => {}, 1_000);\n'); +writeFileSync(join(dir, 'test-quick.ts'), "console.log('PASS quick');\n"); + +/** PIDs of processes still running the hung test. */ +const leftovers = (): number[] => { + try { + return execFileSync('pgrep', ['-f', 'tsx test-hang\\.ts'], { encoding: 'utf-8' }).split('\n').filter(Boolean).map(Number); + } catch { + return []; // pgrep exits 1 when nothing matches + } +}; + +const started = Date.now(); +const runner = spawn(resolve('node_modules/.bin/tsx'), [resolve('scripts/run-tests.ts')], { + cwd: dir, + env: { ...process.env, FRAMESMITH_TEST_TIMEOUT_MS: '3000' }, + stdio: ['ignore', 'pipe', 'pipe'], + detached: true, +}); +let out = ''; +runner.stdout.on('data', (d) => { out += d; }); +runner.stderr.on('data', (d) => { out += d; }); + +// Our own guard: a runner that never finishes must not hang this test too. +const finished = await new Promise((done) => { + const guard = setTimeout(() => done(false), 45_000); + runner.on('close', () => { clearTimeout(guard); done(true); }); +}); +const secs = (Date.now() - started) / 1000; + +check('the runner finishes despite a hung test', finished, `${secs.toFixed(1)}s`); +check('the hung test is reported as timed out', /FAIL {2}test-hang\.ts {2}timed out after 3s/.test(out), out.split('\n').find((l) => l.includes('test-hang'))); +check('the other test still runs and passes', /ok {4}test-quick\.ts/.test(out) && /1\/2 passed/.test(out)); +check('no process of the hung test is left running', leftovers().length === 0, leftovers().join(', ')); + +// Clean up whatever the old behaviour left behind, by exact PID. +if (!finished) { try { process.kill(-runner.pid!, 'SIGKILL'); } catch { /* already gone */ } } +for (const pid of leftovers()) { try { process.kill(pid, 'SIGKILL'); } catch { /* already gone */ } } + +console.log(allPass ? '\nAll runner-timeout checks passed.' : '\nSome runner-timeout checks FAILED.'); +process.exit(allPass ? 0 : 1);