Skip to content

Commit 27558c4

Browse files
MoLowtargos
authored andcommitted
test_runner: catch reporter errors
PR-URL: #49646 Fixes: #48937 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 142ac3f commit 27558c4

File tree

5 files changed

+49
-4
lines changed

5 files changed

+49
-4
lines changed

lib/internal/test_runner/harness.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@ const { kEmptyObject } = require('internal/util');
2121
const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test');
2222
const {
2323
parseCommandLine,
24+
reporterScope,
2425
setupTestReporters,
2526
} = require('internal/test_runner/utils');
2627
const { bigint: hrtime } = process.hrtime;
2728

2829
const testResources = new SafeMap();
2930

31+
testResources.set(reporterScope.asyncId(), reporterScope);
32+
3033
function createTestTree(options = kEmptyObject) {
3134
return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
3235
}
@@ -40,9 +43,14 @@ function createProcessEventHandler(eventName, rootTest) {
4043
throw err;
4144
}
4245

43-
// Check if this error is coming from a test. If it is, fail the test.
4446
const test = testResources.get(executionAsyncId());
4547

48+
// Check if this error is coming from a reporter. If it is, throw it.
49+
if (test === reporterScope) {
50+
throw err;
51+
}
52+
53+
// Check if this error is coming from a test. If it is, fail the test.
4654
if (!test || test.finished) {
4755
// If the test is already finished or the resource that created the error
4856
// is not mapped to a Test, report this as a top level diagnostic.

lib/internal/test_runner/utils.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const {
2020
StringPrototypeSlice,
2121
} = primordials;
2222

23+
const { AsyncResource } = require('async_hooks');
2324
const { basename, relative } = require('path');
2425
const { createWriteStream } = require('fs');
2526
const { pathToFileURL } = require('internal/url');
@@ -169,15 +170,15 @@ async function getReportersMap(reporters, destinations, rootTest) {
169170
});
170171
}
171172

172-
173-
async function setupTestReporters(rootTest) {
173+
const reporterScope = new AsyncResource('TestReporterScope');
174+
const setupTestReporters = reporterScope.bind(async (rootTest) => {
174175
const { reporters, destinations } = parseCommandLine();
175176
const reportersMap = await getReportersMap(reporters, destinations, rootTest);
176177
for (let i = 0; i < reportersMap.length; i++) {
177178
const { reporter, destination } = reportersMap[i];
178179
compose(rootTest.reporter, reporter).pipe(destination);
179180
}
180-
}
181+
});
181182

182183
let globalTestOptions;
183184

@@ -424,6 +425,7 @@ module.exports = {
424425
isSupportedFileType,
425426
isTestFailureError,
426427
parseCommandLine,
428+
reporterScope,
427429
setupTestReporters,
428430
getCoverageReport,
429431
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict';
2+
3+
module.exports = async function * customReporter() {
4+
yield 'Going to throw an error\n';
5+
setImmediate(() => {
6+
throw new Error('Reporting error');
7+
});
8+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
'use strict';
2+
3+
module.exports = async function * customReporter() {
4+
yield 'Going to throw an error\n';
5+
throw new Error('Reporting error');
6+
};

test/parallel/test-runner-reporters.js

+21
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,25 @@ describe('node:test reporters', { concurrency: true }, () => {
134134
assert.strictEqual(child.stdout.toString(), '');
135135
assert.match(child.stderr.toString(), /ERR_INVALID_ARG_TYPE/);
136136
});
137+
138+
it('should throw when reporter errors', async () => {
139+
const child = spawnSync(process.execPath,
140+
['--test', '--test-reporter', fixtures.fileURL('test-runner/custom_reporters/throwing.js'),
141+
fixtures.path('test-runner/default-behavior/index.test.js')]);
142+
assert.strictEqual(child.status, 7);
143+
assert.strictEqual(child.signal, null);
144+
assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
145+
assert.match(child.stderr.toString(), /Error: Reporting error\r?\n\s+at customReporter/);
146+
});
147+
148+
it('should throw when reporter errors asynchronously', async () => {
149+
const child = spawnSync(process.execPath,
150+
['--test', '--test-reporter',
151+
fixtures.fileURL('test-runner/custom_reporters/throwing-async.js'),
152+
fixtures.path('test-runner/default-behavior/index.test.js')]);
153+
assert.strictEqual(child.status, 7);
154+
assert.strictEqual(child.signal, null);
155+
assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
156+
assert.match(child.stderr.toString(), /Emitted 'error' event on Duplex instance/);
157+
});
137158
});

0 commit comments

Comments
 (0)