diff --git a/Dockerfile b/Dockerfile index 4b81875..2a53990 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,9 +18,8 @@ COPY . . RUN yarn run lint RUN script/check-censored -# Test phase, same shape and for the same reason: the jest suite (its worker -# cap is in package.json), rerun verbose on failure, then -# script/test-verify-build. +# Test phase, same shape and for the same reason: the jest suite, rerun +# verbose on failure, then script/test-verify-build. # # node:22-slim (22.x LTS), 2026-02-24 FROM node@sha256:5373f1906319b3a1f291da5d102f4ce5c77ccbe29eb637f072b6c7b70443fc36 AS test @@ -33,9 +32,9 @@ RUN script/bootstrap COPY . . -RUN timeout 90 yarn run test || \ +RUN yarn run test || \ { echo "--- Rerunning with --verbose for details ---"; \ - timeout 90 yarn run test:verbose; exit 1; } + yarn run test:verbose; exit 1; } RUN script/test-verify-build # Development environment with the extension built, and the last stage: a diff --git a/README.md b/README.md index 9f38fe9..32146ca 100644 --- a/README.md +++ b/README.md @@ -1761,8 +1761,8 @@ The `Makefile` targets are thin shims over `script/`: `package.json`. - `script/setup`: `script/bootstrap`, then the git pre-commit hook. - `script/test`: build the `test` stage of the `Dockerfile` with `--no-cache`. - The stage runs the jest suite, stopped at 90 seconds and rerun verbose on - failure, then `script/test-verify-build`. + The stage runs the jest suite, rerun verbose on failure, then + `script/test-verify-build`. - `script/lint`: build the `lint` stage with `--no-cache`. The stage runs ESLint, the prettier check and `script/check-censored`. Linting runs only in Docker. @@ -1861,9 +1861,10 @@ have left a complete, loadable debug bundle behind. `make build-debug` keeps its ### Tests -`make test` runs the jest suite in Docker. The suite must finish within 60 -seconds; the test stage stops it at 90. `make check` runs the tests, the lint -stage and the formatting check. +`make test` runs the jest suite in Docker, in one process. The suite must finish +within 60 seconds. A test that leaves a timer or another open handle running +fails the run. `make check` runs the tests, the lint stage and the formatting +check. Some unit tests pin facts stated elsewhere in this file: diff --git a/eslint.config.js b/eslint.config.js index 1c420d5..64122df 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -135,9 +135,14 @@ module.exports = [ }, }, - // Unit tests, and the helpers they require: jest on node. + // Unit tests, the helpers they require, and the test files they hand to + // jest: jest on node. { - files: ["tests/**/*.test.js", "tests/support/**/*.js"], + files: [ + "tests/**/*.test.js", + "tests/support/**/*.js", + "tests/fixtures/**/*.js", + ], languageOptions: { ...commonjs, globals: { ...globals.node, ...globals.jest }, diff --git a/package.json b/package.json index 2080619..f9f62f4 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,8 @@ "license": "GPL-3.0", "private": true, "scripts": { - "test": "node script/lib/jest.js --maxWorkers=3", - "test:verbose": "node script/lib/jest.js --maxWorkers=3 --verbose", + "test": "node script/lib/jest.js --runInBand", + "test:verbose": "node script/lib/jest.js --runInBand --verbose", "build": "node build.js", "lint": "eslint . && prettier --check .", "fmt": "prettier --write .", diff --git a/script/lib/jest.js b/script/lib/jest.js index 96ba5dc..5e4d545 100644 --- a/script/lib/jest.js +++ b/script/lib/jest.js @@ -1,8 +1,8 @@ -// Runs jest with the arguments it is given, and fails the run when jest had to -// force-stop the worker process that ran a test file. Jest itself only prints a -// warning when that happens, and its run still passes. The warning means a test -// left a timer or another open handle running. The test scripts in package.json -// run jest through this file. +// Runs jest with the arguments it is given, and fails the run at once when a +// test left a timer or another open handle running. Such a handle keeps this +// process alive after the last test, so the run would hang. Jest itself only +// prints a warning one second after the run, and then keeps waiting. The test +// scripts in package.json run jest through this file, in this one process. // // Jest runs inside this process and writes to the terminal directly, so its // output looks the same as a plain jest run. This file only checks each write @@ -10,31 +10,23 @@ "use strict"; -// The start of the warning that jest prints on stderr after it force-stops a -// worker process. -const WORKER_WARNING = "A worker process has failed to exit gracefully"; - -let forceStopped = false; +// The start of the warning that jest prints on stderr when the process has not +// exited one second after the test run. +const OPEN_HANDLE_WARNING = + "Jest did not exit one second after the test run has completed."; const writeStderr = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk, ...rest) => { - if (String(chunk).includes(WORKER_WARNING)) { - forceStopped = true; - } - return writeStderr(chunk, ...rest); -}; - -// Runs after jest has finished, as the process exits. -process.on("exit", () => { - if (forceStopped) { - console.error( - "script/lib/jest.js: failing the run, because jest force-stopped " + - "a worker process. A test left a timer or another open " + - "handle running.", + const written = writeStderr(chunk, ...rest); + if (String(chunk).includes(OPEN_HANDLE_WARNING)) { + writeStderr( + "script/lib/jest.js: failing the run, because a test left a " + + "timer or another open handle running.\n", ); - process.exitCode = 1; + process.exit(1); } -}); + return written; +}; // Jest's own command-line program. It reads its arguments from process.argv. require("jest/bin/jest"); diff --git a/tests/fixtures/leavesIntervalRunning.js b/tests/fixtures/leavesIntervalRunning.js new file mode 100644 index 0000000..a0229cb --- /dev/null +++ b/tests/fixtures/leavesIntervalRunning.js @@ -0,0 +1,7 @@ +// A test file that starts an interval and never clears it. +// tests/jestRunner.test.js runs script/lib/jest.js on this file alone. The +// name does not end in .test.js, so the normal run never picks it up. + +test("starts an interval and never clears it", () => { + setInterval(() => {}, 1000); +}); diff --git a/tests/jestRunner.test.js b/tests/jestRunner.test.js new file mode 100644 index 0000000..dbe79ee --- /dev/null +++ b/tests/jestRunner.test.js @@ -0,0 +1,29 @@ +// script/lib/jest.js, which package.json's test scripts run, fails a run in +// which a test left an open handle, instead of hanging on it. + +const { spawnSync } = require("child_process"); +const path = require("path"); + +const root = path.join(__dirname, ".."); + +test("a test that leaves an interval running fails the run", () => { + // The run ends about one second after its last test. Without the check in + // script/lib/jest.js it would never end, and the timeout stops it. + const result = spawnSync( + process.execPath, + [ + "script/lib/jest.js", + "--runInBand", + "--testMatch", + "/tests/fixtures/leavesIntervalRunning.js", + ], + { cwd: root, encoding: "utf8", timeout: 20000 }, + ); + + expect(result.signal).toBeNull(); + expect(result.status).toBe(1); + expect(result.stderr).toContain("1 passed"); + expect(result.stderr).toContain( + "a test left a timer or another open handle running", + ); +});