Skip to content

Commit 003b9cc

Browse files
MoLowaduh95
authored andcommitted
test_runner: dont buffer unordered events in process isolation mode
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #63432 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 6294719 commit 003b9cc

4 files changed

Lines changed: 83 additions & 0 deletions

File tree

lib/internal/test_runner/runner.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ const kDiagnosticsFilterArgs = ['tests', 'suites', 'pass', 'fail', 'cancelled',
128128
const kCanceledTests = new SafeSet()
129129
.add(kCancelledByParent).add(kAborted).add(kTestTimeoutFailure);
130130

131+
// Execution-ordered events are forwarded immediately, bypassing the
132+
// per-file declaration-order buffer.
133+
const kExecutionOrderedEvents = new SafeSet()
134+
.add('test:enqueue').add('test:dequeue').add('test:complete');
135+
131136
let kResistStopPropagation;
132137

133138
// Worker ID pool management for concurrent test execution
@@ -331,6 +336,10 @@ class FileTest extends Test {
331336
}
332337
}
333338
addToReport(item) {
339+
if (kExecutionOrderedEvents.has(item.type)) {
340+
this.#handleReportItem(item);
341+
return;
342+
}
334343
this.#accumulateReportItem(item);
335344
if (!this.isClearToSend()) {
336345
ArrayPrototypePush(this.#reportBuffer, item);
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { test } from 'node:test';
2+
import assert from 'node:assert';
3+
4+
test('fast-fail', () => {
5+
assert.fail('fast');
6+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { test } from 'node:test';
2+
import { setTimeout as sleep } from 'node:timers/promises';
3+
4+
test('slow', async () => {
5+
// Long enough that fast-fail's process can spawn, run, and round-trip its
6+
// bypassed test:complete to the host on slow CI, but short enough that the
7+
// test does not waste much time when the bypass is working.
8+
await sleep(30_000);
9+
});
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Flags: --no-warnings
2+
3+
import '../common/index.mjs';
4+
import * as fixtures from '../common/fixtures.mjs';
5+
import assert from 'node:assert';
6+
import { test, run } from 'node:test';
7+
8+
const files = [
9+
fixtures.path('test-runner', 'execution-ordered-bypass', 'slow.mjs'),
10+
fixtures.path('test-runner', 'execution-ordered-bypass', 'fast-fail.mjs'),
11+
];
12+
13+
test('execution-ordered events bypass FileTest declaration-order buffer', async () => {
14+
// Concurrency must be a number so the runner does not collapse it to 1 on
15+
// single-core CI runners (where `concurrency: true` resolves to
16+
// `availableParallelism() - 1`). Without two slots the runner spawns the
17+
// files sequentially and fast-fail never starts while slow is sleeping.
18+
const stream = run({
19+
files,
20+
isolation: 'process',
21+
concurrency: 2,
22+
});
23+
24+
const events = [];
25+
26+
stream.on('test:complete', (data) => {
27+
if (data.name === 'slow' || data.name === 'fast-fail') {
28+
events.push(`complete:${data.name}`);
29+
}
30+
});
31+
32+
stream.on('test:fail', (data) => {
33+
if (data.name === 'fast-fail') {
34+
events.push(`fail:${data.name}`);
35+
}
36+
});
37+
38+
// eslint-disable-next-line no-unused-vars
39+
for await (const _ of stream);
40+
41+
const completeFast = events.indexOf('complete:fast-fail');
42+
const completeSlow = events.indexOf('complete:slow');
43+
const failFast = events.indexOf('fail:fast-fail');
44+
45+
assert.notStrictEqual(completeFast, -1);
46+
assert.notStrictEqual(completeSlow, -1);
47+
assert.notStrictEqual(failFast, -1);
48+
49+
assert.ok(
50+
completeFast < completeSlow,
51+
`test:complete for fast-fail should arrive before slow; events=${events.join(', ')}`,
52+
);
53+
54+
// test:fail is declaration-ordered, so the bypass must not affect it.
55+
assert.ok(
56+
failFast > completeSlow,
57+
`test:fail for fast-fail should arrive after test:complete for slow; events=${events.join(', ')}`,
58+
);
59+
});

0 commit comments

Comments
 (0)