test_runner: remove stdout and stderr from error

[cjihrig]

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]

Now, arbitrary output on stderr is swallowed, I think we should at least pipe it to the parent process stderr.

[cjihrig]

Now, arbitrary output on stderr is swallowed, I think we should at least pipe it to the parent process stderr.

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]

CI: https://ci.nodejs.org/job/node-test-pull-request/48120/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/48127/

[nodejs-github-bot]

Landed in fec0fbc