Skip to content

Commit ed9246f

Browse files
cjihrigdanielleadams
authored andcommitted
test_runner: don't parse TAP from stderr
This commit stops the test runner CLI from parsing child process stderr as TAP. Per the TAP spec, TAP can only come from stdout. To avoid losing stderr data, those logs are injected into the parser as unknown tokens so that they are output as comments. PR-URL: #45618 Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent dc53400 commit ed9246f

File tree

4 files changed

+67
-14
lines changed

4 files changed

+67
-14
lines changed

lib/internal/test_runner/runner.js

+20-13
Original file line numberDiff line numberDiff line change
@@ -240,22 +240,29 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
240240
err = error;
241241
});
242242

243-
if (isUsingInspector()) {
244-
const rl = createInterface({ input: child.stderr });
245-
rl.on('line', (line) => {
246-
if (isInspectorMessage(line)) {
247-
process.stderr.write(line + '\n');
248-
}
249-
});
250-
}
251-
252-
const parser = new TapParser();
253-
child.stderr.pipe(parser).on('data', (ast) => {
254-
if (ast.lexeme && isInspectorMessage(ast.lexeme)) {
255-
process.stderr.write(ast.lexeme + '\n');
243+
const rl = createInterface({ input: child.stderr });
244+
rl.on('line', (line) => {
245+
if (isInspectorMessage(line)) {
246+
process.stderr.write(line + '\n');
247+
return;
256248
}
249+
250+
// stderr cannot be treated as TAP, per the spec. However, we want to
251+
// surface stderr lines as TAP diagnostics to improve the DX. Inject
252+
// each line into the test output as an unknown token as if it came
253+
// from the TAP parser.
254+
const node = {
255+
kind: TokenKind.UNKNOWN,
256+
node: {
257+
value: line,
258+
},
259+
};
260+
261+
subtest.addToReport(node);
257262
});
258263

264+
const parser = new TapParser();
265+
259266
child.stdout.pipe(parser).on('data', (ast) => {
260267
subtest.addToReport(ast);
261268
});

lib/internal/util/inspector.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const { validatePort } = require('internal/validators');
1616
const kMinPort = 1024;
1717
const kMaxPort = 65535;
1818
const kInspectArgRegex = /--inspect(?:-brk|-port)?|--debug-port/;
19-
const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|Debugger attached|Waiting for the debugger to disconnect\.\.\./;
19+
const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|For help, see: https:\/\/nodejs\.org\/en\/docs\/inspector|Debugger attached|Waiting for the debugger to disconnect\.\.\./;
2020

2121
const _isUsingInspector = new SafeWeakMap();
2222
function isUsingInspector(execArgv = process.execArgv) {
+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
const test = require('node:test');
3+
4+
console.error('stderr', 1);
5+
6+
test('a test', async () => {
7+
console.error('stderr', 2);
8+
await new Promise((resolve) => {
9+
console.log('stdout', 3);
10+
setTimeout(() => {
11+
// This should not be sent to the TAP parser.
12+
console.error('not ok 1 - fake test');
13+
resolve();
14+
console.log('stdout', 4);
15+
}, 2);
16+
});
17+
console.error('stderr', 5);
18+
});
19+
20+
console.error('stderr', 6);

test/parallel/test-runner-cli.js

+26
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,29 @@ const testFixtures = fixtures.path('test-runner');
168168
assert.match(stdout, /# pass 2/);
169169
assert.match(stdout, /# fail 1/);
170170
}
171+
172+
{
173+
// Test user logging in tests.
174+
const args = [
175+
'--test',
176+
'test/fixtures/test-runner/user-logs.js',
177+
];
178+
const child = spawnSync(process.execPath, args);
179+
180+
assert.strictEqual(child.status, 0);
181+
assert.strictEqual(child.signal, null);
182+
assert.strictEqual(child.stderr.toString(), '');
183+
const stdout = child.stdout.toString();
184+
assert.match(stdout, /# Subtest: .+user-logs\.js/);
185+
assert.match(stdout, / {4}# stderr 1/);
186+
assert.match(stdout, / {4}# stderr 2/);
187+
assert.match(stdout, / {4}# stdout 3/);
188+
assert.match(stdout, / {4}# stderr 6/);
189+
assert.match(stdout, / {4}# not ok 1 - fake test/);
190+
assert.match(stdout, / {4}# stderr 5/);
191+
assert.match(stdout, / {4}# stdout 4/);
192+
assert.match(stdout, / {4}# Subtest: a test/);
193+
assert.match(stdout, / {4}ok 1 - a test/);
194+
assert.match(stdout, /# tests 1/);
195+
assert.match(stdout, /# pass 1/);
196+
}

0 commit comments

Comments
 (0)