FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: better handle async bootstrap errors · nodejs/node@1ef3c53 · GitHub

/ node Public

Commit 1ef3c53

Browse files
cjihrig
authored and
juanarbol
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 Backport-PR-URL: #46839 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 0a690ef commit 1ef3c53

4 files changed

Lines changed: 34 additions & 7 deletions

File tree

‎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: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const {
1616
const { kEmptyObject } = require('internal/util');
1717
const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
1818
const {
19+
kAsyncBootstrapFailure,
1920
parseCommandLine,
2021
setupTestReporters,
2122
} = require('internal/test_runner/utils');
@@ -30,6 +31,13 @@ function createTestTree(options = kEmptyObject) {
3031

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

‎lib/internal/test_runner/utils.js‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
RegExp,
99
RegExpPrototypeExec,
1010
SafeMap,
11+
Symbol,
1112
} = primordials;
1213
const { basename } = require('path');
1314
const { createWriteStream } = require('fs');
@@ -24,6 +25,7 @@ const {
2425
} = require('internal/errors');
2526
const { compose } = require('stream');
2627

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

151153

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

@@ -220,6 +226,7 @@ module.exports = {
220226
doesPathMatchFilter,
221227
isSupportedFileType,
222228
isTestFailureError,
229+
kAsyncBootstrapFailure,
223230
parseCommandLine,
224231
setupTestReporters,
225232
};

‎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.fileURL('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)
Back | FazBrowse Home | New Git URL