Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
test_runner: remove stdout and stderr from error#45592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred.
MoLow commented Nov 23, 2022
Now, arbitrary output on stderr is swallowed, I think we should at least pipe it to the parent process stderr. |
cjihrig commented Nov 23, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
From my testing stderr shows up as TAP comments because the child process stderr is (incorrectly) piped into the TAP parser. I also have a follow up PR that stops stderr from being piped to the parser and instead just interprets the lines of stderr as unknown TAP tokens (diff below) - I just have to write a test for it. diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 3f7134c6cd..eb73a46e55 100644 --- a/lib/internal/test_runner/runner.js+++ b/lib/internal/test_runner/runner.js@@ -246,16 +246,25 @@ function runTestFile(path, root, inspectPort, filesWatcher){rl.on('line', (line) =>{if (isInspectorMessage(line)){process.stderr.write(line + '\n'); + return; } ++ // stderr cannot be treated as TAP, per the spec. However, we want to+ // surface stderr lines as TAP diagnostics to improve the DX. Inject+ // each line into the test output as an unknown token as if it came+ // from the TAP parser.+ const node ={+ kind: TokenKind.UNKNOWN,+ node:{+ value: line,+ },+ };++ subtest.addToReport(node); })} const parser = new TapParser(); - child.stderr.pipe(parser).on('data', (ast) =>{- if (ast.lexeme && isInspectorMessage(ast.lexeme)){- process.stderr.write(ast.lexeme + '\n');- }- }); child.stdout.pipe(parser).on('data', (ast) =>{subtest.addToReport(ast); diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 038479b173..c7f18ffdb6 100644 --- a/lib/internal/util/inspector.js+++ b/lib/internal/util/inspector.js@@ -16,7 +16,7 @@ const{validatePort } = require('internal/validators'); const kMinPort = 1024; const kMaxPort = 65535; const kInspectArgRegex = /--inspect(?:-brk|-port)?|--debug-port/; -const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|Debugger attached|Waiting for the debugger to disconnect\.\.\./;+const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|For help, see: https:\/\/nodejs\.org\/en\/docs\/inspector|Debugger attached|Waiting for the debugger to disconnect\.\.\./; const _isUsingInspector = new SafeWeakMap(); function isUsingInspector(execArgv = process.execArgv){ |
nodejs-github-bot commented Nov 23, 2022
nodejs-github-bot commented Nov 23, 2022
nodejs-github-bot commented Nov 25, 2022
Landed in fec0fbc |
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: nodejs#45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: #45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: nodejs/node#45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> (cherry picked from commit fec0fbc333c58e6ebbd2322f5120830fda880eb0)
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: nodejs/node#45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> (cherry picked from commit fec0fbc333c58e6ebbd2322f5120830fda880eb0)
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred. PR-URL: nodejs/node#45592 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> (cherry picked from commit fec0fbc333c58e6ebbd2322f5120830fda880eb0)
The CLI test runner parses the TAP output from child processes now and displays it. This commit removes a previous workaround for displaying child process stdout and stderr when tests failures occurred.