Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit dfe529b

Browse filesBrowse files
cjihrigtargos
cjihrig
authored and
targos
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). PR-URL: #46720 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 93e91f3 commit dfe529b
Copy full SHA for dfe529b

File tree

Expand file treeCollapse file tree

4 files changed

+34
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+34
-7
lines changed
Open diff view settings
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+2-2Lines 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

Collapse file

‎lib/internal/test_runner/harness.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/harness.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const { exitCodes: { kGenericUserError } } = internalBinding('errors');
1818
const { kEmptyObject } = require('internal/util');
1919
const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
2020
const {
21+
kAsyncBootstrapFailure,
2122
parseCommandLine,
2223
setupTestReporters,
2324
} = require('internal/test_runner/utils');
@@ -32,6 +33,13 @@ function createTestTree(options = kEmptyObject) {
3233

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

Collapse file

‎lib/internal/test_runner/utils.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/utils.js
+12-5Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const {
77
RegExp,
88
RegExpPrototypeExec,
99
SafeMap,
10+
Symbol,
1011
} = primordials;
1112
const { basename } = require('path');
1213
const { createWriteStream } = require('fs');
@@ -23,6 +24,7 @@ const {
2324
} = require('internal/errors');
2425
const { compose } = require('stream');
2526

27+
const kAsyncBootstrapFailure = Symbol('asyncBootstrapFailure');
2628
const kMultipleCallbackInvocations = 'multipleCallbackInvocations';
2729
const kRegExpPattern = /^\/(.*)\/([a-z]*)$/;
2830
const kSupportedFileExtensions = /\.[cm]?js$/;
@@ -149,11 +151,15 @@ async function getReportersMap(reporters, destinations) {
149151

150152

151153
async function setupTestReporters(testsStream) {
152-
const { reporters, destinations } = parseCommandLine();
153-
const reportersMap = await getReportersMap(reporters, destinations);
154-
for (let i = 0; i < reportersMap.length; i++) {
155-
const { reporter, destination } = reportersMap[i];
156-
compose(testsStream, reporter).pipe(destination);
154+
try {
155+
const { reporters, destinations } = parseCommandLine();
156+
const reportersMap = await getReportersMap(reporters, destinations);
157+
for (let i = 0; i < reportersMap.length; i++) {
158+
const { reporter, destination } = reportersMap[i];
159+
compose(testsStream, reporter).pipe(destination);
160+
}
161+
} catch (err) {
162+
throw new ERR_TEST_FAILURE(err, kAsyncBootstrapFailure);
157163
}
158164
}
159165

@@ -219,6 +225,7 @@ module.exports = {
219225
doesPathMatchFilter,
220226
isSupportedFileType,
221227
isTestFailureError,
228+
kAsyncBootstrapFailure,
222229
parseCommandLine,
223230
setupTestReporters,
224231
};
Collapse file

‎test/parallel/test-runner-reporters.js‎

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.