Skip to content

Commit 9e647d2

Browse files
MoLowMoLow
committed
test_runner: recieve and pass AbortSignal
1 parent 660d17d commit 9e647d2

File tree

8 files changed

+549
-80
lines changed

8 files changed

+549
-80
lines changed

‎lib/internal/main/test_runner.js

Lines changed: 36 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,14 @@ const {
66
ArrayPrototypePush,
77
ArrayPrototypeSlice,
88
ArrayPrototypeSort,
9-
Promise,
10-
PromiseAll,
11-
SafeArrayIterator,
9+
SafePromiseAll,
1210
SafeSet,
1311
} = primordials;
1412
const {
1513
prepareMainThreadExecution,
1614
} = require('internal/bootstrap/pre_execution');
1715
const { spawn } = require('child_process');
1816
const { readdirSync, statSync } = require('fs');
19-
const { finished } = require('internal/streams/end-of-stream');
2017
const console = require('internal/console/global');
2118
const {
2219
codes: {
@@ -30,6 +27,7 @@ const {
3027
doesPathMatchFilter,
3128
} = require('internal/test_runner/utils');
3229
const { basename, join, resolve } = require('path');
30+
const { once } = require('events');
3331
const kFilterArgs = ['--test'];
3432

3533
prepareMainThreadExecution(false);
@@ -102,53 +100,41 @@ function filterExecArgv(arg) {
102100
}
103101

104102
function runTestFile(path) {
105-
return test(path, () => {
106-
return new Promise((resolve, reject) => {
107-
const args = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
108-
ArrayPrototypePush(args, path);
109-
110-
const child = spawn(process.execPath, args);
111-
// TODO(cjihrig): Implement a TAP parser to read the child's stdout
112-
// instead of just displaying it all if the child fails.
113-
let stdout = '';
114-
let stderr = '';
115-
let err;
116-
117-
child.on('error', (error) => {
118-
err = error;
119-
});
120-
121-
child.stdout.setEncoding('utf8');
122-
child.stderr.setEncoding('utf8');
123-
124-
child.stdout.on('data', (chunk) => {
125-
stdout += chunk;
126-
});
127-
128-
child.stderr.on('data', (chunk) => {
129-
stderr += chunk;
130-
});
131-
132-
child.once('exit', async (code, signal) => {
133-
if (code !== 0 || signal !== null) {
134-
if (!err) {
135-
await PromiseAll(new SafeArrayIterator([finished(child.stderr), finished(child.stdout)]));
136-
err = new ERR_TEST_FAILURE('test failed', kSubtestsFailed);
137-
err.exitCode = code;
138-
err.signal = signal;
139-
err.stdout = stdout;
140-
err.stderr = stderr;
141-
// The stack will not be useful since the failures came from tests
142-
// in a child process.
143-
err.stack = undefined;
144-
}
145-
146-
return reject(err);
147-
}
148-
149-
resolve();
150-
});
103+
return test(path, async (t) => {
104+
const args = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
105+
ArrayPrototypePush(args, path);
106+
107+
const child = spawn(process.execPath, args, { signal: t.signal });
108+
// TODO(cjihrig): Implement a TAP parser to read the child's stdout
109+
// instead of just displaying it all if the child fails.
110+
let err;
111+
112+
child.on('error', (error) => {
113+
err = error;
151114
});
115+
116+
child.stdout.setEncoding('utf8');
117+
child.stderr.setEncoding('utf8');
118+
const { 0: { code, signal }, 1: stdout, 2: stderr } = await SafePromiseAll([
119+
once(child, 'exit', { signal: t.signal }),
120+
child.stdout.toArray({ signal: t.signal }),
121+
child.stderr.toArray({ signal: t.signal }),
122+
]);
123+
124+
if (code !== 0 || signal !== null) {
125+
if (!err) {
126+
err = new ERR_TEST_FAILURE('test failed', kSubtestsFailed);
127+
err.exitCode = code;
128+
err.signal = signal;
129+
err.stdout = stdout.join('');
130+
err.stderr = stderr.join('');
131+
// The stack will not be useful since the failures came from tests
132+
// in a child process.
133+
err.stack = undefined;
134+
}
135+
136+
throw err;
137+
}
152138
});
153139
}
154140

‎lib/internal/test_runner/test.js

Lines changed: 86 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,18 @@ const {
99
PromiseResolve,
1010
ReflectApply,
1111
SafeMap,
12-
PromiseRace,
12+
SafePromiseRace,
13+
Symbol,
1314
} = primordials;
1415
const { AsyncResource } = require('async_hooks');
16+
const { once } = require('events');
17+
const { AbortController } = require('internal/abort_controller');
1518
const {
1619
codes: {
1720
ERR_TEST_FAILURE,
1821
},
1922
kIsNodeError,
23+
AbortError,
2024
} = require('internal/errors');
2125
const { getOptionValue } = require('internal/options');
2226
const { TapStream } = require('internal/test_runner/tap_stream');
@@ -26,7 +30,7 @@ const {
2630
kEmptyObject,
2731
} = require('internal/util');
2832
const { isPromise } = require('internal/util/types');
29-
const { isUint32 } = require('internal/validators');
33+
const { isUint32, validateAbortSignal } = require('internal/validators');
3034
const { setTimeout } = require('timers/promises');
3135
const { cpus } = require('os');
3236
const { bigint: hrtime } = process.hrtime;
@@ -44,20 +48,19 @@ const testOnlyFlag = !isTestRunner && getOptionValue('--test-only');
4448
// TODO(cjihrig): Use uv_available_parallelism() once it lands.
4549
const rootConcurrency = isTestRunner ? cpus().length : 1;
4650

51+
const kShouldAbort = Symbol('kShouldAbort');
4752

48-
function testTimeout(promise, timeout) {
53+
54+
function stopTest(timeout, signal) {
4955
if (timeout === kDefaultTimeout) {
50-
return promise;
51-
}
52-
return PromiseRace([
53-
promise,
54-
setTimeout(timeout, null, { ref: false }).then(() => {
55-
throw new ERR_TEST_FAILURE(
56-
`test timed out after ${timeout}ms`,
57-
kTestTimeoutFailure
58-
);
59-
}),
60-
]);
56+
return once(signal, 'abort');
57+
}
58+
return setTimeout(timeout, null, { ref: false, signal }).then(() => {
59+
throw new ERR_TEST_FAILURE(
60+
`test timed out after ${timeout}ms`,
61+
kTestTimeoutFailure
62+
);
63+
});
6164
}
6265

6366
class TestContext {
@@ -67,6 +70,10 @@ class TestContext {
6770
this.#test = test;
6871
}
6972

73+
get signal() {
74+
return this.#test.signal;
75+
}
76+
7077
diagnostic(message) {
7178
this.#test.diagnostic(message);
7279
}
@@ -92,11 +99,14 @@ class TestContext {
9299
}
93100

94101
class Test extends AsyncResource {
102+
#abortController;
103+
#outerSignal;
104+
95105
constructor(options) {
96106
super('Test');
97107

98108
let { fn, name, parent, skip } = options;
99-
const { concurrency, only, timeout, todo } = options;
109+
const { concurrency, only, timeout, todo, signal } = options;
100110

101111
if (typeof fn !== 'function') {
102112
fn = noop;
@@ -149,6 +159,14 @@ class Test extends AsyncResource {
149159
fn = noop;
150160
}
151161

162+
this.#abortController = new AbortController();
163+
this.#outerSignal = signal;
164+
this.signal = this.#abortController.signal;
165+
166+
validateAbortSignal(signal, 'options.signal');
167+
this.#outerSignal?.addEventListener('abort', this.#abortHandler);
168+
169+
152170
this.fn = fn;
153171
this.name = name;
154172
this.parent = parent;
@@ -242,7 +260,8 @@ class Test extends AsyncResource {
242260

243261
// If this test has already ended, attach this test to the root test so
244262
// that the error can be properly reported.
245-
if (this.finished) {
263+
const preventAddingSubtests = this.finished || this.buildPhaseFinished;
264+
if (preventAddingSubtests) {
246265
while (parent.parent !== null) {
247266
parent = parent.parent;
248267
}
@@ -254,7 +273,7 @@ class Test extends AsyncResource {
254273
parent.waitingOn = test.testNumber;
255274
}
256275

257-
if (this.finished) {
276+
if (preventAddingSubtests) {
258277
test.startTime = test.startTime || hrtime();
259278
test.fail(
260279
new ERR_TEST_FAILURE(
@@ -268,18 +287,23 @@ class Test extends AsyncResource {
268287
return test;
269288
}
270289

271-
cancel() {
290+
#abortHandler = () => {
291+
this.cancel(this.#outerSignal?.reason || new AbortError('The test was aborted'));
292+
};
293+
294+
cancel(error) {
272295
if (this.endTime !== null) {
273296
return;
274297
}
275298

276-
this.fail(
299+
this.fail(error ||
277300
new ERR_TEST_FAILURE(
278301
'test did not finish before its parent and was cancelled',
279302
kCancelledByParent
280303
)
281304
);
282305
this.cancelled = true;
306+
this.#abortController.abort();
283307
}
284308

285309
fail(err) {
@@ -330,6 +354,16 @@ class Test extends AsyncResource {
330354
return this.run();
331355
}
332356

357+
[kShouldAbort]() {
358+
if (this.signal.aborted) {
359+
return true;
360+
}
361+
if (this.#outerSignal?.aborted) {
362+
this.cancel(this.#outerSignal.reason || new AbortError('The test was aborted'));
363+
return true;
364+
}
365+
}
366+
333367
getRunArgs() {
334368
const ctx = new TestContext(this);
335369
return { ctx, args: [ctx] };
@@ -339,7 +373,13 @@ class Test extends AsyncResource {
339373
this.parent.activeSubtests++;
340374
this.startTime = hrtime();
341375

376+
if (this[kShouldAbort]()) {
377+
this.postRun();
378+
return;
379+
}
380+
342381
try {
382+
const stopPromise = stopTest(this.timeout, this.signal);
343383
const { args, ctx } = this.getRunArgs();
344384
ArrayPrototypeUnshift(args, this.fn, ctx); // Note that if it's not OK to mutate args, we need to first clone it.
345385

@@ -355,13 +395,19 @@ class Test extends AsyncResource {
355395
'passed a callback but also returned a Promise',
356396
kCallbackAndPromisePresent
357397
));
358-
await testTimeout(ret, this.timeout);
398+
await SafePromiseRace([ret, stopPromise]);
359399
} else {
360-
await testTimeout(promise, this.timeout);
400+
await SafePromiseRace([PromiseResolve(promise), stopPromise]);
361401
}
362402
} else {
363403
// This test is synchronous or using Promises.
364-
await testTimeout(ReflectApply(this.runInAsyncScope, this, args), this.timeout);
404+
const promise = ReflectApply(this.runInAsyncScope, this, args);
405+
await SafePromiseRace([PromiseResolve(promise), stopPromise]);
406+
}
407+
408+
if (this[kShouldAbort]()) {
409+
this.postRun();
410+
return;
365411
}
366412

367413
this.pass();
@@ -410,6 +456,8 @@ class Test extends AsyncResource {
410456
this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed));
411457
}
412458

459+
this.#outerSignal?.removeEventListener('abort', this.#abortHandler);
460+
413461
if (this.parent !== null) {
414462
this.parent.activeSubtests--;
415463
this.parent.addReadySubtest(this);
@@ -477,20 +525,21 @@ class Test extends AsyncResource {
477525
class ItTest extends Test {
478526
constructor(opt) { super(opt); } // eslint-disable-line no-useless-constructor
479527
getRunArgs() {
480-
return { ctx: {}, args: [] };
528+
return { ctx: { signal: this.signal }, args: [] };
481529
}
482530
}
483531
class Suite extends Test {
484532
constructor(options) {
485533
super(options);
486534

487535
try {
488-
this.buildSuite = this.runInAsyncScope(this.fn);
536+
const context = { signal: this.signal };
537+
this.buildSuite = this.runInAsyncScope(this.fn, context, [context]);
489538
} catch (err) {
490539
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
491540
}
492541
this.fn = () => {};
493-
this.finished = true; // Forbid adding subtests to this suite
542+
this.buildPhaseFinished = true;
494543
}
495544

496545
start() {
@@ -505,11 +554,21 @@ class Suite extends Test {
505554
}
506555
this.parent.activeSubtests++;
507556
this.startTime = hrtime();
557+
558+
if (this[kShouldAbort]()) {
559+
this.subtests = [];
560+
this.postRun();
561+
return;
562+
}
563+
564+
const stopPromise = stopTest(this.timeout, this.signal);
508565
const subtests = this.skipped || this.error ? [] : this.subtests;
509-
await testTimeout(ArrayPrototypeReduce(subtests, async (prev, subtest) => {
566+
const promise = ArrayPrototypeReduce(subtests, async (prev, subtest) => {
510567
await prev;
511568
await subtest.run();
512-
}, PromiseResolve()), this.timeout);
569+
}, PromiseResolve());
570+
571+
await SafePromiseRace([promise, stopPromise]);
513572
this.pass();
514573
this.postRun();
515574
}

0 commit comments

Comments
 (0)