test_runner: support using --inspect with --test

doc/api/test.md

@@ -338,6 +338,11 @@ added: REPLACEME
     fail after.
     If unspecified, subtests inherit this value from their parent.
     **Default:** `Infinity`.
+  * `inspectPort` {number|Function} Sets inspector port of test child process.
+    This can be a number, or a function that takes no arguments and returns a
+    number. If a nullish value is provided, each process gets its own port,
+    incremented from the primary's `process.debugPort`.
+    **Default:** `undefined`.
 * Returns: {TapStream}
 
 ```js

lib/internal/main/test_runner.js

@@ -3,12 +3,22 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete
 } = require('internal/process/pre_execution');
-const { run } = require('internal/test_runner/runner');
+const { run, isUsingInspector } = require('internal/test_runner/runner');
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();
 
-const tapStream = run();
+let concurrency = true;
+let inspectPort;
+
+if (isUsingInspector()) {
+  process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +
+  'Use the inspectPort option to run with concurrency');
+  concurrency = 1;
+  inspectPort = process.debugPort;
+}
+
+const tapStream = run({ concurrency, inspectPort });
 tapStream.pipe(process.stdout);
 tapStream.once('test:fail', () => {
   process.exitCode = 1;

lib/internal/test_runner/runner.js

@@ -1,14 +1,16 @@
 'use strict';
 const {
   ArrayFrom,
-  ArrayPrototypeConcat,
   ArrayPrototypeFilter,
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
+  ArrayPrototypePush,
   ArrayPrototypeSlice,
+  ArrayPrototypeSome,
   ArrayPrototypeSort,
   ObjectAssign,
   PromisePrototypeThen,
+  RegExpPrototypeExec,
   SafePromiseAll,
   SafeSet,
 } = primordials;
@@ -21,7 +23,7 @@ const {
     ERR_TEST_FAILURE,
   },
 } = require('internal/errors');
-const { validateArray } = require('internal/validators');
+const { validateArray, validatePort } = require('internal/validators');
 const { kEmptyObject } = require('internal/util');
 const { createTestTree } = require('internal/test_runner/harness');
 const { kSubtestsFailed, Test } = require('internal/test_runner/test');
@@ -32,6 +34,10 @@ const {
 const { basename, join, resolve } = require('path');
 const { once } = require('events');
 
+const kMinPort = 1024;
+const kMaxPort = 65535;
+const kInspectArgRegex = /--inspect(?:-brk|-port)?/;
+const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|Debugger attached|Waiting for the debugger to disconnect\.\.\./;
 const kFilterArgs = ['--test'];
 
 // TODO(cjihrig): Replace this with recursive readdir once it lands.
@@ -96,29 +102,59 @@ function createTestFileList() {
   return ArrayPrototypeSort(ArrayFrom(testFiles));
 }
 
+function isUsingInspector() {
+  return ArrayPrototypeSome(process.execArgv, (arg) => RegExpPrototypeExec(kInspectArgRegex, arg) !== null) ||
+    RegExpPrototypeExec(kInspectArgRegex, process.env.NODE_OPTIONS) !== null;
+}
+
 function filterExecArgv(arg) {
   return !ArrayPrototypeIncludes(kFilterArgs, arg);
 }
 
-function runTestFile(path, root) {
+let debugPortOffset = 1;
+function getRunArgs({ path, inspectPort, usingInspector }) {
+  const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
+  if (usingInspector) {
+    if (typeof inspectPort === 'function') {
+      inspectPort = inspectPort();
+    } else if (inspectPort == null) {
+      inspectPort = process.debugPort + debugPortOffset;
+      if (inspectPort > kMaxPort)
+        inspectPort = inspectPort - kMaxPort + kMinPort - 1;
+      debugPortOffset++;
+    }
+    validatePort(inspectPort);
+
+    ArrayPrototypePush(argv, `--inspect-port=${inspectPort}`);
+  }
+  ArrayPrototypePush(argv, path);
+  return argv;
+}
+
+function runTestFile(path, root, usingInspector, inspectPort) {
   const subtest = root.createSubtest(Test, path, async (t) => {
-    const args = ArrayPrototypeConcat(
-      ArrayPrototypeFilter(process.execArgv, filterExecArgv),
-      path);
+    const args = getRunArgs({ path, inspectPort, usingInspector });
 
     const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' });
     // TODO(cjihrig): Implement a TAP parser to read the child's stdout
     // instead of just displaying it all if the child fails.
     let err;
+    let stderr = '';
 
     child.on('error', (error) => {
       err = error;
     });
 
-    const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([
+    child.stderr.on('data', (data) => {
+      if (usingInspector && RegExpPrototypeExec(kInspectMsgRegex, data) !== null) {
+        process.stderr.write(data);
+      }
+      stderr += data;
+    });
+
+    const { 0: { 0: code, 1: signal }, 1: stdout } = await SafePromiseAll([
       once(child, 'exit', { signal: t.signal }),
       child.stdout.toArray({ signal: t.signal }),
-      child.stderr.toArray({ signal: t.signal }),
     ]);
 
     if (code !== 0 || signal !== null) {
@@ -128,7 +164,7 @@ function runTestFile(path, root) {
           exitCode: code,
           signal: signal,
           stdout: ArrayPrototypeJoin(stdout, ''),
-          stderr: ArrayPrototypeJoin(stderr, ''),
+          stderr,
           // The stack will not be useful since the failures came from tests
           // in a child process.
           stack: undefined,
@@ -145,19 +181,20 @@ function run(options) {
   if (options === null || typeof options !== 'object') {
     options = kEmptyObject;
   }
-  const { concurrency, timeout, signal, files } = options;
+  const { concurrency, timeout, signal, files, inspectPort } = options;
 
   if (files != null) {
     validateArray(files, 'options.files');
   }
 
   const root = createTestTree({ concurrency, timeout, signal });
   const testFiles = files ?? createTestFileList();
+  const usingInspector = isUsingInspector();
 
-  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root)),
+  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, usingInspector, inspectPort)),
                        () => root.postRun());
 
   return root.reporter;
 }
 
-module.exports = { run };
+module.exports = { run, isUsingInspector };

lib/internal/test_runner/test.js

@@ -57,8 +57,6 @@ const kDefaultTimeout = null;
 const noop = FunctionPrototype;
 const isTestRunner = getOptionValue('--test');
 const testOnlyFlag = !isTestRunner && getOptionValue('--test-only');
-// TODO(cjihrig): Use uv_available_parallelism() once it lands.
-const rootConcurrency = isTestRunner ? MathMax(cpus().length - 1, 1) : 1;
 const kShouldAbort = Symbol('kShouldAbort');
 const kRunHook = Symbol('kRunHook');
 const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
@@ -146,7 +144,7 @@ class Test extends AsyncResource {
     }
 
     if (parent === null) {
-      this.concurrency = rootConcurrency;
+      this.concurrency = 1;
       this.indent = '';
       this.indentString = kDefaultIndent;
       this.only = testOnlyFlag;
@@ -176,6 +174,7 @@ class Test extends AsyncResource {
 
       case 'boolean':
         if (concurrency) {
+          // TODO(cjihrig): Use uv_available_parallelism() once it lands.
           this.concurrency = parent === null ? MathMax(cpus().length - 1, 1) : Infinity;
         } else {
           this.concurrency = 1;

src/node_options.cc

@@ -161,9 +161,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
       errors->push_back("either --test or --watch can be used, not both");
     }
 
-    if (debug_options_.inspector_enabled) {
-      errors->push_back("the inspector cannot be used with --test");
-    }
 #ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER
     debug_options_.allow_attaching_debugger = false;
 #endif

test/parallel/test-runner-cli.js

@@ -104,12 +104,6 @@ const testFixtures = fixtures.path('test-runner');
     ['--print', 'console.log("should not print")', '--test'],
   ];
 
-  if (process.features.inspector) {
-    flags.push(
-      ['--inspect', '--test'],
-      ['--inspect-brk', '--test'],
-    );
-  }
 
   flags.forEach((args) => {
     const child = spawnSync(process.execPath, args);

test/parallel/test-runner-inspect.js

@@ -0,0 +1,68 @@
+'use strict';
+const common = require('../common');
+const tmpdir = require('../common/tmpdir.js');
+const { NodeInstance } = require('../common/inspector-helper.js');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+const { join } = require('path');
+const fixtures = require('../common/fixtures');
+const testFixtures = fixtures.path('test-runner');
+
+common.skipIfInspectorDisabled();
+tmpdir.refresh();
+
+async function debugTest() {
+  const child = new NodeInstance(['--test', '--inspect-brk=0'], undefined, join(testFixtures, 'index.test.js'));
+
+  let stdout = '';
+  let stderr = '';
+  child.on('stdout', (line) => stdout += line);
+  child.on('stderr', (line) => stderr += line);
+
+  const session = await child.connectInspectorSession();
+
+  await session.send([
+    { method: 'Runtime.enable' },
+    { method: 'NodeRuntime.notifyWhenWaitingForDisconnect',
+      params: { enabled: true } },
+    { method: 'Runtime.runIfWaitingForDebugger' }]);
+
+
+  await session.waitForNotification((notification) => {
+    return notification.method === 'NodeRuntime.waitingForDisconnect';
+  });
+
+  session.disconnect();
+  assert.match(stderr,
+               /Warning: Using the inspector with --test forces running at a concurrency of 1\. Use the inspectPort option to run with concurrency/);
+}
+
+debugTest().then(common.mustCall());
+
+
+{
+  const args = ['--test', '--inspect=0', join(testFixtures, 'index.js')];
+  const child = spawnSync(process.execPath, args);
+
+  assert.match(child.stderr.toString(),
+               /Warning: Using the inspector with --test forces running at a concurrency of 1\. Use the inspectPort option to run with concurrency/);
+  const stdout = child.stdout.toString();
+  assert.match(stdout, /not ok 1 - .+index\.js/);
+  assert.match(stdout, /stderr: \|-\r?\n\s+Debugger listening on/);
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
+}
+
+
+{
+  // File not found.
+  const args = ['--test', '--inspect=0', 'a-random-file-that-does-not-exist.js'];
+  const child = spawnSync(process.execPath, args);
+
+  const stderr = child.stderr.toString();
+  assert.strictEqual(child.stdout.toString(), '');
+  assert.match(stderr, /^Could not find/);
+  assert.doesNotMatch(stderr, /Warning: Using the inspector with --test forces running at a concurrency of 1\. Use the inspectPort option to run with concurrency/);
+  assert.strictEqual(child.status, 1);
+  assert.strictEqual(child.signal, null);
+}