Skip to content

Commit d63041d

Browse files
committed
test_runner: better handle async bootstrap errors
The test runner is bootstrapped synchronously, with the exception of importing custom reporters. To better handle asynchronously throw errors, this commit introduces an internal error type that can be checked for from the test runner's uncaughtException handler. After #46707 and this change land, the other throw statement in the uncaughtException handler can be removed. This will allow the test runner to handle errors thrown from outside of tests (which currently prevents the test runner from reporting results).
1 parent cafc0b2 commit d63041d

File tree

4 files changed

+36
-7
lines changed

4 files changed

+36
-7
lines changed

lib/internal/errors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,8 +1604,8 @@ E('ERR_TAP_VALIDATION_ERROR', function(errorMsg) {
16041604
}, Error);
16051605
E('ERR_TEST_FAILURE', function(error, failureType) {
16061606
hideInternalStackFrames(this);
1607-
assert(typeof failureType === 'string',
1608-
"The 'failureType' argument must be of type string.");
1607+
assert(typeof failureType === 'string' || typeof failureType === 'symbol',
1608+
"The 'failureType' argument must be of type string or symbol.");
16091609

16101610
let msg = error?.message ?? error;
16111611

lib/internal/test_runner/harness.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ const { exitCodes: { kGenericUserError } } = internalBinding('errors');
1818
const { kEmptyObject } = require('internal/util');
1919
const { getOptionValue } = require('internal/options');
2020
const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
21-
const { setupTestReporters } = require('internal/test_runner/utils');
21+
const {
22+
kAsyncBootstrapFailure,
23+
setupTestReporters,
24+
} = require('internal/test_runner/utils');
2225
const { bigint: hrtime } = process.hrtime;
2326

2427
const isTestRunnerCli = getOptionValue('--test');
@@ -31,6 +34,13 @@ function createTestTree(options = kEmptyObject) {
3134

3235
function createProcessEventHandler(eventName, rootTest) {
3336
return (err) => {
37+
if (err?.failureType === kAsyncBootstrapFailure) {
38+
// Something went wrong during the asynchronous portion of bootstrapping
39+
// the test runner. Since the test runner is not setup properly, we can't
40+
// do anything but throw the error.
41+
throw err.cause;
42+
}
43+
3444
// Check if this error is coming from a test. If it is, fail the test.
3545
const test = testResources.get(executionAsyncId());
3646

lib/internal/test_runner/utils.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
RegExp,
77
RegExpPrototypeExec,
88
SafeMap,
9+
Symbol,
910
} = primordials;
1011
const { basename } = require('path');
1112
const { createWriteStream } = require('fs');
@@ -22,6 +23,7 @@ const {
2223
} = require('internal/errors');
2324
const { compose } = require('stream');
2425

26+
const kAsyncBootstrapFailure = Symbol('asyncBootstrapFailure');
2527
const kMultipleCallbackInvocations = 'multipleCallbackInvocations';
2628
const kRegExpPattern = /^\/(.*)\/([a-z]*)$/;
2729
const kSupportedFileExtensions = /\.[cm]?js$/;
@@ -164,10 +166,14 @@ async function setupTestReporters(testsStream) {
164166
'must match the number of specified \'--test-reporter-destination\'');
165167
}
166168

167-
const reportersMap = await getReportersMap(reporters, destinations);
168-
for (let i = 0; i < reportersMap.length; i++) {
169-
const { reporter, destination } = reportersMap[i];
170-
compose(testsStream, reporter).pipe(destination);
169+
try {
170+
const reportersMap = await getReportersMap(reporters, destinations);
171+
for (let i = 0; i < reportersMap.length; i++) {
172+
const { reporter, destination } = reportersMap[i];
173+
compose(testsStream, reporter).pipe(destination);
174+
}
175+
} catch (err) {
176+
throw new ERR_TEST_FAILURE(err, kAsyncBootstrapFailure);
171177
}
172178
}
173179

@@ -177,5 +183,6 @@ module.exports = {
177183
doesPathMatchFilter,
178184
isSupportedFileType,
179185
isTestFailureError,
186+
kAsyncBootstrapFailure,
180187
setupTestReporters,
181188
};

test/parallel/test-runner-reporters.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,16 @@ describe('node:test reporters', { concurrency: true }, () => {
116116
/^package: reporter-esm{"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/,
117117
);
118118
});
119+
120+
it('should throw when reporter setup throws asynchronously', async () => {
121+
const child = spawnSync(
122+
process.execPath,
123+
['--test', '--test-reporter', fixtures.path('empty.js'), 'reporters.js'],
124+
{ cwd: fixtures.path('test-runner') }
125+
);
126+
assert.strictEqual(child.status, 7);
127+
assert.strictEqual(child.signal, null);
128+
assert.strictEqual(child.stdout.toString(), '');
129+
assert.match(child.stderr.toString(), /ERR_INVALID_ARG_TYPE/);
130+
});
119131
});

0 commit comments

Comments
 (0)