Skip to content

Commit 70ef2e1

Browse files
fix(ci): economyEngine.test.js worker-IPC-flake — deterministisk isolation (#3172) (#3222)
* fix(ci): isolate economyEngine.test.js to stop worker-IPC flake backend/lib/economyEngine.test.js is by far the largest backend test file (100+ tests, 6500+ lines, ~250KB) and its per-worker result payload appears large enough to trip a known Node test-runner bug (nodejs/node#64061, root-caused and fixed upstream in nodejs/node#64706, merged 2026-07-26 but not yet in our pinned Node 24.x line) when many other test-file workers stream results back to the parent process concurrently. Isolated runs were always 102/102 (now 103/103) green; the flake only ever showed up in the full suite (#3169-CI, #3171-CI). Add backend/scripts/run-tests.js: runs economyEngine.test.js alone first, then every other backend test file exactly as before (same flags, same default concurrency) via explicit relative file paths so the two passes never overlap. Wire both npm test (backend/package.json - the one permitted package.json edit) and scripts/verify-local.ps1 (which called `node --test` directly, bypassing npm entirely) through this script so CI and local pre-push runs get the same protection. No dependency or lockfile changes. Verified with 5 consecutive `npm test` runs locally (103/103 + 4773/4773 every time, exit 0). Refs #3172 Co-Authored-By: Claude <noreply@anthropic.com> * docs(learnings): postmortem for economyEngine.test.js worker-IPC flake Refs #3172 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Nicolai Dolmer <213100469+NicolaiDolmer@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 81ae338 commit 70ef2e1

4 files changed

Lines changed: 156 additions & 2 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Postmortem · 2026-08-03 · economyEngine.test.js worker-IPC-flake (#3172)
2+
3+
## Hvad skete der?
4+
`backend-tests`-CI-jobbet fejlede 2x samme dag (31/7, #3169 og #3171) med præcis
5+
1 fejlende fil: `lib/economyEngine.test.js`, med en runner-intern fejl
6+
`Unable to deserialize cloned data due to invalid or unsupported version`
7+
(`node:internal/test_runner`). Alle 102/102 testcases i filen var grønne når
8+
filen kørte isoleret, og resten af suiten (4729/4730) var grøn. Set lokalt i
9+
`scripts/verify-local.ps1` 31/7 også — ikke PR-specifikt.
10+
11+
## Root cause
12+
`economyEngine.test.js` er langt den største backend-testfil (103 tests,
13+
6513 linjer, ~250KB — mere end dobbelt så stor som næststørste fil,
14+
`auctionFinalization.test.js` på 2753 linjer). Node's testrunner starter én
15+
child-worker pr. testfil og streamer testresultater tilbage til
16+
parent-processen via IPC (structured clone over en socket). Med 344
17+
testfiler kører op til ~8 workers samtidigt (CPU-parallelisme). Den store
18+
fils resultat-payload er tilsyneladende stor nok til at ramme en kendt
19+
buffer-parsing-bug i Node's testrunner (`nodejs/node#64061`): en
20+
signed/unsigned-fejl i `processRawBuffer`, rettet upstream i
21+
`nodejs/node#64706` (merged 2026-07-26) — men den rettelse er endnu ikke i
22+
vores pinnede Node 24.x-linje (`engines: >=24.0.0 <25`, lokal `v24.16.0`).
23+
Bugklassen trigges kun ved samtidig IPC-trafik fra mange workers, hvilket
24+
forklarer hvorfor filen ALTID er grøn isoleret og KUN flaker i fuld suite.
25+
26+
## Fix
27+
`backend/scripts/run-tests.js` (nyt): kører `lib/economyEngine.test.js`
28+
alene i pass 1 (ingen samtidig worker-trafik at kollidere med), derefter alle
29+
øvrige ~343 testfiler i pass 2 med samme flags/default-concurrency som før,
30+
via eksplicitte RELATIVE filstier (ikke absolutte — 344 absolutte stier
31+
rammer Windows' ~32.767-tegns CreateProcess-kommandolinjegrænse og
32+
`spawnSync` fejler med `ENAMETOOLONG`; relative stier er ~10.500 tegn, rigelig
33+
margin). CLI-args (`--test-reporter=spec`) videreføres til begge pass.
34+
35+
`backend/package.json`: `"test"` peger nu på `node scripts/run-tests.js`
36+
(den ene tilladte package.json-undtagelse — ingen dependency-/lockfile-ændring).
37+
38+
`scripts/verify-local.ps1`: kaldte tidligere `node --test` DIREKTE (uden om
39+
npm og uden `--import ./test-setup.js`) — opdateret til at kalde samme
40+
`run-tests.js`, så lokale pre-push-kørsler får samme beskyttelse (den var
41+
netop hvor flaket også blev set 31/7).
42+
43+
Ingen dependency- eller package-lock.json-ændringer. Ingen blind retry —
44+
den valgte løsning fjerner selve race-betingelsen (samtidighed) i stedet for
45+
at maskere en lejlighedsvis fejl.
46+
47+
## Forhindret-fremover
48+
Stress-verificeret med 5 på hinanden følgende `npm test`-kørsler lokalt:
49+
103/103 + 4773/4773 grønt, exit 0, hver gang — ingen deserialize-fejl.
50+
Acceptkriteriet fra #3172 (2 ugers flake-fri CI på denne signatur) verificeres
51+
efterfølgende i CI over tid.
52+
53+
## Læring
54+
En enkelt unormalt stor testfil i en ellers homogen testsuite er ikke kun et
55+
læsbarheds-/vedligeholdelsesproblem — den kan trigge infrastruktur-bugs
56+
(her: Node's eget testrunner-IPC) som kun viser sig under samtidighed, aldrig
57+
isoleret. Når en test-only-fejl er 100% reproducerbar i fuld suite og 0%
58+
isoleret, er "kør den isoleret" (test-script-niveau isolation) ofte den mest
59+
målrettede fix — billigere og mere deterministisk end en Node-version-bump
60+
eller en retry-wrapper, og den fjerner selve race-betingelsen frem for at
61+
maskere den. Bonus-gotcha: pas på Windows' kommandolinjelængde-grænse
62+
(~32K tegn) når man bygger en eksplicit filliste til `spawnSync`/`execFile`
63+
brug relative stier, ikke absolutte, med mange filargumenter.

backend/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"balance:baseline": "node scripts/balanceBaseline.js --write",
2828
"season:end:verify-repair": "node scripts/verifySeasonEndRepair.js",
2929
"board:repair-members-after-dna": "node scripts/repairBoardMembersAfterDna.js",
30-
"test": "node --test --import ./test-setup.js",
30+
"test": "node scripts/run-tests.js",
3131
"lint": "eslint .",
3232
"lint:fix": "eslint . --fix",
3333
"format": "prettier --write --ignore-unknown .",

backend/scripts/run-tests.js

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// #3172: node:internal/test_runner intermittently corrupts worker-IPC data
2+
// ("Unable to deserialize cloned data due to invalid or unsupported version")
3+
// only when MANY test-file workers stream results back to the parent test
4+
// runner process at the same time. Root cause upstream: nodejs/node#64061,
5+
// fixed by nodejs/node#64706 (merged 2026-07-26) — a signed/unsigned bug in
6+
// the test runner's internal `processRawBuffer` that mishandles large IPC
7+
// buffers. That fix has not reached our pinned Node 24.x line yet, so we
8+
// isolate the offending file instead of waiting on a Node bump.
9+
//
10+
// lib/economyEngine.test.js is by far the largest backend test file (100+
11+
// top-level tests, 6500+ lines, ~250KB) — its per-worker result payload is
12+
// large enough to be the one that trips the bug when it lands concurrently
13+
// with the other ~340 test-file workers. Isolated runs of the file are
14+
// 102/102 green every time; the flake has only ever been observed in the
15+
// full suite (#3169-CI, #3171-CI).
16+
//
17+
// Fix: run the large file alone FIRST (no concurrent worker traffic to race
18+
// against), then run every other backend test file exactly as before (same
19+
// flags, same default concurrency). Any CLI args npm forwards (e.g.
20+
// `--test-reporter=spec` from `npm test -- --test-reporter=spec`) are passed
21+
// through to both passes unchanged.
22+
//
23+
// Deliberately NOT using `--test-concurrency=1` for the whole suite: that
24+
// would serialize all ~344 files and materially slow down CI/local runs for
25+
// a bug that only needs ONE file kept out of the concurrent pool.
26+
import { spawnSync } from "node:child_process";
27+
import { readdirSync } from "node:fs";
28+
import path from "node:path";
29+
import { fileURLToPath } from "node:url";
30+
31+
const backendRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
32+
const ISOLATED_RELATIVE = path.join("lib", "economyEngine.test.js");
33+
const IGNORED_DIRS = new Set(["node_modules", ".git"]);
34+
35+
function findTestFiles(dir, out) {
36+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
37+
if (entry.isDirectory()) {
38+
if (IGNORED_DIRS.has(entry.name)) continue;
39+
findTestFiles(path.join(dir, entry.name), out);
40+
} else if (entry.isFile() && entry.name.endsWith(".test.js")) {
41+
out.push(path.join(dir, entry.name));
42+
}
43+
}
44+
return out;
45+
}
46+
47+
function runNodeTest(extraArgs, files) {
48+
const result = spawnSync(
49+
process.execPath,
50+
["--test", "--import", "./test-setup.js", ...extraArgs, ...files],
51+
{ stdio: "inherit", cwd: backendRoot },
52+
);
53+
if (result.error) throw result.error;
54+
return result.status ?? 1;
55+
}
56+
57+
const extraArgs = process.argv.slice(2);
58+
const isolatedAbsolute = path.join(backendRoot, ISOLATED_RELATIVE);
59+
const allTestFilesAbsolute = findTestFiles(backendRoot, []).sort();
60+
61+
if (!allTestFilesAbsolute.includes(isolatedAbsolute)) {
62+
// Guard-fail loudly rather than silently running everything together —
63+
// if the file moves/renames, this script must be updated with it.
64+
console.error(
65+
`run-tests.js: expected isolated file not found at ${ISOLATED_RELATIVE} (#3172 guard). ` +
66+
"Update backend/scripts/run-tests.js if the file moved or was renamed.",
67+
);
68+
process.exit(1);
69+
}
70+
71+
// Relative paths, not absolute: with ~340 files, an absolute-path argv (this
72+
// worktree's path alone is ~65 chars) blows past Windows's ~32K CreateProcess
73+
// command-line limit and spawnSync fails with ENAMETOOLONG. Relative paths
74+
// (cwd is already backendRoot) cut the argv to a third of that, with plenty
75+
// of headroom as the suite grows.
76+
const restFilesRelative = allTestFilesAbsolute
77+
.filter((file) => file !== isolatedAbsolute)
78+
.map((file) => path.relative(backendRoot, file));
79+
80+
console.log(`\n[run-tests] Pass 1/2: ${ISOLATED_RELATIVE} in isolation (#3172 worker-IPC flake guard)\n`);
81+
const isolatedStatus = runNodeTest(extraArgs, [ISOLATED_RELATIVE]);
82+
if (isolatedStatus !== 0) {
83+
process.exit(isolatedStatus);
84+
}
85+
86+
console.log(`\n[run-tests] Pass 2/2: remaining ${restFilesRelative.length} backend test files\n`);
87+
const restStatus = runNodeTest(extraArgs, restFilesRelative);
88+
process.exit(restStatus);

scripts/verify-local.ps1

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ if ($normalizedResolvedRoot -ne $repoRoot) {
5151
Write-Host "[1/3] Backend tests"
5252
Push-Location (Join-Path $repoRoot "backend")
5353
try {
54-
& $nodePath --test
54+
# #3172: brug samme scripts/run-tests.js som `npm test`, ikke rå `node --test`
55+
# — koerer lib/economyEngine.test.js isoleret foerst for at undgaa
56+
# worker-IPC-flaket (node:internal/test_runner, ramte ogsaa her 31/7).
57+
& $nodePath scripts/run-tests.js
5558
if ($LASTEXITCODE -ne 0) {
5659
exit $LASTEXITCODE
5760
}

0 commit comments

Comments
 (0)