[v18.x backport] test_runner: support using --inspect with --test

doc/api/test.md

@@ -338,6 +338,11 @@ added: v18.9.0
     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/cluster/primary.js

@@ -6,10 +6,14 @@ const {
   ArrayPrototypeSome,
   ObjectKeys,
   ObjectValues,
-  RegExpPrototypeExec,
   SafeMap,
   StringPrototypeStartsWith,
 } = primordials;
+const {
+  codes: {
+    ERR_SOCKET_BAD_PORT,
+  }
+} = require('internal/errors');
 
 const assert = require('internal/assert');
 const { fork } = require('child_process');
@@ -18,14 +22,12 @@ const EventEmitter = require('events');
 const RoundRobinHandle = require('internal/cluster/round_robin_handle');
 const SharedHandle = require('internal/cluster/shared_handle');
 const Worker = require('internal/cluster/worker');
+const { getInspectPort, isUsingInspector } = require('internal/util/inspector');
 const { internal, sendHelper } = require('internal/cluster/utils');
 const cluster = new EventEmitter();
 const intercom = new EventEmitter();
 const SCHED_NONE = 1;
 const SCHED_RR = 2;
-const minPort = 1024;
-const maxPort = 65535;
-const { validatePort } = require('internal/validators');
 
 module.exports = cluster;
 
@@ -40,7 +42,6 @@ cluster.SCHED_NONE = SCHED_NONE;  // Leave it to the operating system.
 cluster.SCHED_RR = SCHED_RR;      // Primary distributes connections.
 
 let ids = 0;
-let debugPortOffset = 1;
 let initialized = false;
 
 // XXX(bnoordhuis) Fold cluster.schedulingPolicy into cluster.settings?
@@ -117,28 +118,12 @@ function setupSettingsNT(settings) {
 function createWorkerProcess(id, env) {
   const workerEnv = { ...process.env, ...env, NODE_UNIQUE_ID: `${id}` };
   const execArgv = [...cluster.settings.execArgv];
-  const debugArgRegex = /--inspect(?:-brk|-port)?|--debug-port/;
-  const nodeOptions = process.env.NODE_OPTIONS || '';
-
-  if (ArrayPrototypeSome(execArgv,
-                         (arg) => RegExpPrototypeExec(debugArgRegex, arg) !== null) ||
-      RegExpPrototypeExec(debugArgRegex, nodeOptions) !== null) {
-    let inspectPort;
-    if ('inspectPort' in cluster.settings) {
-      if (typeof cluster.settings.inspectPort === 'function')
-        inspectPort = cluster.settings.inspectPort();
-      else
-        inspectPort = cluster.settings.inspectPort;
-
-      validatePort(inspectPort);
-    } else {
-      inspectPort = process.debugPort + debugPortOffset;
-      if (inspectPort > maxPort)
-        inspectPort = inspectPort - maxPort + minPort - 1;
-      debugPortOffset++;
-    }
 
-    ArrayPrototypePush(execArgv, `--inspect-port=${inspectPort}`);
+  if (cluster.settings.inspectPort === null) {
+    throw new ERR_SOCKET_BAD_PORT('Port', null, true);
+  }
+  if (isUsingInspector(cluster.settings.execArgv)) {
+    ArrayPrototypePush(execArgv, `--inspect-port=${getInspectPort(cluster.settings.inspectPort)}`);
   }
 
   return fork(cluster.settings.exec, cluster.settings.args, {

lib/internal/main/test_runner.js

@@ -3,12 +3,23 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete
 } = require('internal/process/pre_execution');
+const { isUsingInspector } = require('internal/util/inspector');
 const { run } = 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,10 +1,10 @@
 'use strict';
 const {
   ArrayFrom,
-  ArrayPrototypeConcat,
   ArrayPrototypeFilter,
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
+  ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,
   ObjectAssign,
@@ -15,13 +15,16 @@ const {
 
 const { spawn } = require('child_process');
 const { readdirSync, statSync } = require('fs');
+// TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern.
+const { createInterface } = require('readline');
 const console = require('internal/console/global');
 const {
   codes: {
     ERR_TEST_FAILURE,
   },
 } = require('internal/errors');
 const { validateArray } = require('internal/validators');
+const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
 const { kEmptyObject } = require('internal/util');
 const { createTestTree } = require('internal/test_runner/harness');
 const { kSubtestsFailed, Test } = require('internal/test_runner/test');
@@ -100,25 +103,46 @@ function filterExecArgv(arg) {
   return !ArrayPrototypeIncludes(kFilterArgs, arg);
 }
 
-function runTestFile(path, root) {
+function getRunArgs({ path, inspectPort }) {
+  const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
+  if (isUsingInspector()) {
+    ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
+  }
+  ArrayPrototypePush(argv, path);
+  return argv;
+}
+
+
+function runTestFile(path, root, inspectPort) {
   const subtest = root.createSubtest(Test, path, async (t) => {
-    const args = ArrayPrototypeConcat(
-      ArrayPrototypeFilter(process.execArgv, filterExecArgv),
-      path);
+    const args = getRunArgs({ path, inspectPort });
 
     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) => {
+      stderr += data;
+    });
+
+    if (isUsingInspector()) {
+      const rl = createInterface({ input: child.stderr });
+      rl.on('line', (line) => {
+        if (isInspectorMessage(line)) {
+          process.stderr.write(line + '\n');
+        }
+      });
+    }
+
+    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 +152,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,7 +169,7 @@ 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');
@@ -154,7 +178,7 @@ function run(options) {
   const root = createTestTree({ concurrency, timeout, signal });
   const testFiles = files ?? createTestFileList();
 
-  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root)),
+  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)),
                        () => root.postRun());
 
   return root.reporter;

lib/internal/test_runner/test.js

@@ -58,8 +58,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']);
@@ -150,7 +148,7 @@ class Test extends AsyncResource {
     }
 
     if (parent === null) {
-      this.concurrency = rootConcurrency;
+      this.concurrency = 1;
       this.indent = '';
       this.indentString = kDefaultIndent;
       this.only = testOnlyFlag;
@@ -180,6 +178,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;

lib/internal/util/inspector.js

@@ -2,12 +2,47 @@
 
 const {
   ArrayPrototypeConcat,
+  ArrayPrototypeSome,
   FunctionPrototypeBind,
   ObjectDefineProperty,
   ObjectKeys,
   ObjectPrototypeHasOwnProperty,
+  RegExpPrototypeExec,
+  SafeWeakMap,
 } = primordials;
 
+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 _isUsingInspector = new SafeWeakMap();
+function isUsingInspector(execArgv = process.execArgv) {
+  if (!_isUsingInspector.has(execArgv)) {
+    _isUsingInspector.set(execArgv,
+                          ArrayPrototypeSome(execArgv, (arg) => RegExpPrototypeExec(kInspectArgRegex, arg) !== null) ||
+      RegExpPrototypeExec(kInspectArgRegex, process.env.NODE_OPTIONS) !== null);
+  }
+  return _isUsingInspector.get(execArgv);
+}
+
+let debugPortOffset = 1;
+function getInspectPort(inspectPort) {
+  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);
+
+  return inspectPort;
+}
+
 let session;
 function sendInspectorCommand(cb, onError) {
   const { hasInspector } = internalBinding('config');
@@ -22,6 +57,10 @@ function sendInspectorCommand(cb, onError) {
   }
 }
 
+function isInspectorMessage(string) {
+  return isUsingInspector() && RegExpPrototypeExec(kInspectMsgRegex, string) !== null;
+}
+
 // Create a special require function for the inspector command line API
 function installConsoleExtensions(commandLineApi) {
   if (commandLineApi.require) { return; }
@@ -63,7 +102,10 @@ function wrapConsole(consoleFromNode) {
 }
 
 module.exports = {
+  getInspectPort,
   installConsoleExtensions,
+  isInspectorMessage,
+  isUsingInspector,
   sendInspectorCommand,
   wrapConsole,
 };

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/common/index.mjs

@@ -52,6 +52,8 @@ const {
   spawnPromisified,
 } = common;
 
+const getPort = () => common.PORT;
+
 export {
   isMainThread,
   isWindows,
@@ -100,4 +102,5 @@ export {
   runWithInvalidFD,
   createRequire,
   spawnPromisified,
+  getPort,
 };

test/fixtures/test-runner/run_inspect.js

@@ -0,0 +1,40 @@
+'use strict';
+
+const common = require('../../common');
+const fixtures = require('../../common/fixtures');
+const { run } = require('node:test');
+const assert = require('node:assert');
+
+const badPortError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' };
+let inspectPort = 'inspectPort' in process.env ? Number(process.env.inspectPort) : undefined;
+let expectedError;
+
+if (process.env.inspectPort === 'addTwo') {
+  inspectPort = common.mustCall(() => { return process.debugPort += 2; });
+} else if (process.env.inspectPort === 'string') {
+  inspectPort = 'string';
+  expectedError = badPortError;
+} else if (process.env.inspectPort === 'null') {
+  inspectPort = null;
+} else if (process.env.inspectPort === 'bignumber') {
+  inspectPort = 1293812;
+  expectedError = badPortError;
+} else if (process.env.inspectPort === 'negativenumber') {
+  inspectPort = -9776;
+  expectedError = badPortError;
+} else if (process.env.inspectPort === 'bignumberfunc') {
+  inspectPort = common.mustCall(() => 123121);
+  expectedError = badPortError;
+} else if (process.env.inspectPort === 'strfunc') {
+  inspectPort = common.mustCall(() => 'invalidPort');
+  expectedError = badPortError;
+}
+
+const stream = run({ files: [fixtures.path('test-runner/run_inspect_assert.js')], inspectPort });
+if (expectedError) {
+  stream.on('test:fail', common.mustCall(({ error }) => {
+    assert.deepStrictEqual({ name: error.cause.name, code: error.cause.code }, expectedError);
+  }));
+} else {
+  stream.on('test:fail', common.mustNotCall());
+}