diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index de532648094d..5052ff9e2b74 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -97,12 +97,6 @@ function getNodeParallelTestTimeout(testPath) { if (testPath.includes("test-cluster-")) return 60_000; // cluster IPC + socket-handle passing is process-heavy under runner concurrency if (testPath.includes("-docker-")) return 60_000; if (testPath.includes("test-stdin-pipe-large")) return 60_000; // pipes 1MB stdin->stdout through an extra child process; slow under runner concurrency - // test-fs-read-stream-pos.js exit condition is a pure timing race (writer must append - // between two consecutive ReadStream preads) with a 90s upstream safety timer; solo - // runtimes are ~1s on linux-x64 but 1-40s on Windows since #34834 raised its timer - // resolution, and aarch64 CI retries running alone have exceeded 20s (builds 85866, - // 85400). 120s lets the safety timer fire. - if (testPath.includes("test-fs-read-stream-pos")) return 120_000; if (!isCI) return 60_000; // everything slower in debug mode if (options["step"]?.includes("-asan-")) return 60_000; return 20_000; @@ -641,12 +635,6 @@ async function runTests() { const isParallelSafeTest = testPath => { const p = testPath.replaceAll("\\", "/"); if (!p.includes("js/node/test/parallel/") && !p.includes("js/bun/test/parallel/")) return false; - // test-fs-read-stream-pos.js arms a common.mustCallAtLeast per stream; under - // I/O-heavy neighbours (e.g. test-fs-read-stream-fd-leak) the 1ms append - // interval is starved long enough for a stream to catch cur == EOF and get - // zero 'data' events, failing the mustCallAtLeast check on exit. Run it in - // the serial phase so the writer keeps its 1ms cadence. - if (p.endsWith("test-fs-read-stream-pos.js")) return false; return true; }; console.log("parallel-safe width", parallelSafeWidth); diff --git a/test/js/node/test/parallel/test-fs-read-stream-pos.js b/test/js/node/test/parallel/test-fs-read-stream-pos.js index 8a5812f81ef2..eaa521a964b9 100644 --- a/test/js/node/test/parallel/test-fs-read-stream-pos.js +++ b/test/js/node/test/parallel/test-fs-read-stream-pos.js @@ -15,21 +15,28 @@ fs.writeFileSync(file, ''); let counter = 0; -const writeInterval = setInterval(() => { +const appendLine = () => { counter = counter + 1; - const line = `hello at ${counter}\n`; - fs.writeFileSync(file, line, { flag: 'a' }); -}, 1); + fs.writeFileSync(file, `hello at ${counter}\n`, { flag: 'a' }); +}; + +// Seed a line so the first stream is guaranteed at least one 'data' event and a +// short (< hwm) tail chunk even before the 1ms writer has had a chance to fire. +appendLine(); + +const writeInterval = setInterval(appendLine, 1); const hwm = 10; let bufs = []; let isLow = false; let cur = 0; let stream; +let streamStart = 0; const readInterval = setInterval(common.mustCallAtLeast(() => { if (stream) return; + streamStart = cur; stream = fs.createReadStream(file, { highWaterMark: hwm, start: cur @@ -38,7 +45,16 @@ const readInterval = setInterval(common.mustCallAtLeast(() => { cur += chunk.length; bufs.push(chunk); if (isLow) { - const brokenLines = Buffer.concat(bufs).toString() + const read = Buffer.concat(bufs); + const onDisk = fs.readFileSync(file); + assert.strictEqual(read.length, cur - streamStart); + assert.ok(cur <= onDisk.length, `cur ${cur} exceeds file size ${onDisk.length}`); + // nodejs/node#33940: after a short read the next pread was issued at the + // wrong position, so the bytes delivered here overlapped or skipped what + // earlier chunks had already returned. The concatenated chunks must be + // exactly the file's bytes at [streamStart, cur). + assert.deepStrictEqual(read, onDisk.subarray(streamStart, cur)); + const brokenLines = read.toString() .split('\n') .filter((line) => { const s = 'hello at'.slice(0, line.length); @@ -53,6 +69,13 @@ const readInterval = setInterval(common.mustCallAtLeast(() => { } if (chunk.length !== hwm) { isLow = true; + // Upstream relies on the 1ms writer landing between this short read and + // the stream's next pread, and carries a 90s safety timer for when that + // race never hits (reached routinely on Windows, where the test then + // exits without asserting anything). Appending here, before the next + // _read is scheduled, makes the follow-up chunk deterministic without + // changing the code path under test. + appendLine(); } })); stream.on('end', () => { @@ -62,15 +85,9 @@ const readInterval = setInterval(common.mustCallAtLeast(() => { }); }), 10); -// Time longer than 90 seconds to exit safely -const endTimer = setTimeout(() => { - exitTest(); -}, 90000); - const exitTest = () => { clearInterval(readInterval); clearInterval(writeInterval); - clearTimeout(endTimer); if (stream && !stream.destroyed) { stream.on('close', () => { process.exit();