Skip to content

Commit b802e75

Browse files
committed
test(tmpdir): ratchet every node --test script through the wrapper
The 13 lanes wrapped in package.json were a one-time hand sweep with nothing enforcing the pattern going forward — a 14th node --test script added later without scripts/node-test-tmpdir.ts would silently reopen #1595 for that one lane. Add a structural check to scripts/node-test-tmpdir.test.ts (now part of check:tmpdir-leaks:test) that reads package.json and fails if any script invokes `node ... --test` without routing through the wrapper. Dumb string matching over the scripts map, no shell parsing, with an explicit (currently empty) NODE_TEST_WRAPPER_BYPASS_ALLOWLIST for any lane that must legitimately bypass it. Verified it both passes on the current package.json and fails when a synthetic unwrapped `node --test` script is added.
1 parent 1bacb3e commit b802e75

1 file changed

Lines changed: 46 additions & 0 deletions

File tree

‎scripts/node-test-tmpdir.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,25 @@ import { TEST_RUN_TMP_PREFIX, TEST_RUN_TMP_ROOT } from './vitest-tmpdir-global-s
1111
const REPOSITORY_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
1212
const WRAPPER = path.join(REPOSITORY_ROOT, 'scripts', 'node-test-tmpdir.ts');
1313

14+
// package.json scripts that may legitimately invoke `node --test` without
15+
// routing through the wrapper below. Empty on purpose: every current
16+
// node --test lane is wrapped (#1595). Add a script name here only alongside
17+
// a comment explaining why that lane can't be wrapped — the ratchet test
18+
// below fails closed on anything else, so a 14th `node --test` script added
19+
// later without the wrapper fails CI instead of silently leaking again.
20+
const NODE_TEST_WRAPPER_BYPASS_ALLOWLIST = new Set<string>([]);
21+
22+
// Dumb string matching on purpose (no shell parsing, per the scripts map's
23+
// own style: '&&'-chained commands, nothing fancier). True when a `node ...`
24+
// segment enables the built-in test runner via a word-bounded `--test` flag
25+
// and hasn't already been routed through the wrapper.
26+
function isUnwrappedNodeTestSegment(segment: string): boolean {
27+
const trimmed = segment.trim();
28+
if (!/^node(\s|$)/.test(trimmed)) return false;
29+
if (trimmed.includes('node-test-tmpdir.ts')) return false;
30+
return /(^|\s)--test(\s|$)/.test(trimmed);
31+
}
32+
1433
// Date.now() alone collides when this file's top-level tests happen to start
1534
// within the same millisecond (observed in practice), which overwrites one
1635
// probe's source with another's; the random suffix makes each probe path
@@ -107,3 +126,30 @@ test('deliberately failing probe', () => {
107126
fs.rmSync(evidenceRoot, { recursive: true, force: true });
108127
}
109128
});
129+
130+
// The 13 lanes wrapped in package.json when this fix landed were a one-time
131+
// hand sweep; nothing stopped a 14th `node --test` script from being added
132+
// later without the wrapper, silently reopening #1595 for that one lane.
133+
// This turns the sweep into an invariant instead.
134+
test('every node --test package.json script routes through scripts/node-test-tmpdir.ts', () => {
135+
const manifest = JSON.parse(
136+
fs.readFileSync(path.join(REPOSITORY_ROOT, 'package.json'), 'utf8'),
137+
) as {
138+
scripts?: Record<string, string>;
139+
};
140+
const scripts = manifest.scripts ?? {};
141+
142+
const unwrapped = Object.entries(scripts)
143+
.filter(([name]) => !NODE_TEST_WRAPPER_BYPASS_ALLOWLIST.has(name))
144+
.filter(([, command]) => command.split('&&').some(isUnwrappedNodeTestSegment))
145+
.map(([name]) => name);
146+
147+
assert.deepEqual(
148+
unwrapped,
149+
[],
150+
`these package.json scripts invoke \`node --test\` directly instead of through ` +
151+
`scripts/node-test-tmpdir.ts, so a crash/timeout kill during their run leaks a scratch ` +
152+
`directory again: ${unwrapped.join(', ')}. Route them through the wrapper, or add to ` +
153+
`NODE_TEST_WRAPPER_BYPASS_ALLOWLIST above with a reason if one must legitimately bypass it.`,
154+
);
155+
});

0 commit comments

Comments
 (0)