test_runner: support using --inspect with --test

PR-URL: #44520
Backport-PR-URL: #44873
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

Author: MoLow

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/cluster/primary.js

@@ -120,6 +120,7 @@ function createWorkerProcess(id, env) {
   const debugArgRegex = /--inspect(?:-brk|-port)?|--debug-port/;
   const nodeOptions = process.env.NODE_OPTIONS || '';
 
+  // TODO(MoLow): Use getInspectPort from internal/util/inspector
   if (ArrayPrototypeSome(execArgv,
                          (arg) => RegExpPrototypeExec(debugArgRegex, arg) !== null) ||
       RegExpPrototypeExec(debugArgRegex, nodeOptions) !== null) {

lib/internal/main/test_runner.js

@@ -2,12 +2,23 @@
 const {
   prepareMainThreadExecution,
 } = require('internal/bootstrap/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,18 +1,22 @@
 'use strict';
 const {
   ArrayFrom,
-  ArrayPrototypeConcat,
   ArrayPrototypeFilter,
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
+  ArrayPrototypePop,
+  ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,
   ObjectAssign,
   PromisePrototypeThen,
+  RegExpPrototypeSymbolSplit,
   SafePromiseAll,
   SafeSet,
+  StringPrototypeEndsWith,
 } = primordials;
 
+const { Buffer } = require('buffer');
 const { spawn } = require('child_process');
 const { readdirSync, statSync } = require('fs');
 const console = require('internal/console/global');
@@ -22,6 +26,7 @@ const {
   },
 } = 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 +105,59 @@ 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 makeStderrCallback(callback) {
+  if (!isUsingInspector()) {
+    return callback;
+  }
+  let buffer = Buffer.alloc(0);
+  return (data) => {
+    callback(data);
+    const newData = Buffer.concat([buffer, data]);
+    const str = newData.toString('utf8');
+    let lines = str;
+    if (StringPrototypeEndsWith(lines, '\n')) {
+      buffer = Buffer.alloc(0);
+    } else {
+      lines = RegExpPrototypeSymbolSplit(/\r?\n/, str);
+      buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8');
+      lines = ArrayPrototypeJoin(lines, '\n');
+    }
+    if (isInspectorMessage(lines)) {
+      process.stderr.write(lines);
+    }
+  };
+}
+
+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', makeStderrCallback((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 +167,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 +184,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 +193,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,
 } = 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\.\.\./;
+
+let _isUsingInspector;
+function isUsingInspector() {
+  _isUsingInspector ??=
+    ArrayPrototypeSome(process.execArgv, (arg) => RegExpPrototypeExec(kInspectArgRegex, arg) !== null) ||
+    RegExpPrototypeExec(kInspectArgRegex, process.env.NODE_OPTIONS) !== null;
+  return _isUsingInspector;
+}
+
+let debugPortOffset = 1;
+function getInspectPort(inspectPort) {
+  if (!isUsingInspector()) {
+    return null;
+  }
+  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; }
@@ -65,7 +104,10 @@ function wrapConsole(consoleFromNode, consoleFromVM) {
 // Stores the console from VM, should be set during bootstrap.
 let consoleFromVM;
 module.exports = {
+  getInspectPort,
   installConsoleExtensions,
+  isInspectorMessage,
+  isUsingInspector,
   sendInspectorCommand,
   wrapConsole,
   get consoleFromVM() {

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());
+}

test/fixtures/test-runner/run_inspect_assert.js

@@ -0,0 +1,19 @@
+'use strict';
+
+const assert = require('node:assert');
+
+const { expectedPort, expectedInitialPort, expectedHost } = process.env;
+const debugOptions =
+  require('internal/options').getOptionValue('--inspect-port');
+
+if ('expectedPort' in process.env) {
+  assert.strictEqual(process.debugPort, +expectedPort);
+}
+
+if ('expectedInitialPort' in process.env) {
+  assert.strictEqual(debugOptions.port, +expectedInitialPort);
+}
+
+if ('expectedHost' in process.env) {
+  assert.strictEqual(debugOptions.host, expectedHost);
+}

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.mjs

@@ -0,0 +1,54 @@
+import * as common from '../common/index.mjs';
+import * as tmpdir from '../common/tmpdir.js';
+import * as fixtures from '../common/fixtures.mjs';
+import assert from 'node:assert';
+import { NodeInstance } from '../common/inspector-helper.js';
+
+
+common.skipIfInspectorDisabled();
+tmpdir.refresh();
+
+{
+  const child = new NodeInstance(['--test', '--inspect-brk=0'], undefined, fixtures.path('test-runner/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: 'Runtime.runIfWaitingForDebugger' }]);
+
+  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/);
+}
+
+
+{
+  const args = ['--test', '--inspect=0', fixtures.path('test-runner/index.js')];
+  const { stderr, stdout, code, signal } = await common.spawnPromisified(process.execPath, args);
+
+  assert.match(stderr,
+               /Warning: Using the inspector with --test forces running at a concurrency of 1\. Use the inspectPort option to run with concurrency/);
+  assert.match(stdout, /not ok 1 - .+index\.js/);
+  assert.match(stdout, /stderr: \|-\r?\n\s+Debugger listening on/);
+  assert.strictEqual(code, 1);
+  assert.strictEqual(signal, null);
+}
+
+
+{
+  // File not found.
+  const args = ['--test', '--inspect=0', 'a-random-file-that-does-not-exist.js'];
+  const { stderr, stdout, code, signal } = await common.spawnPromisified(process.execPath, args);
+
+  assert.strictEqual(stdout, '');
+  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(code, 1);
+  assert.strictEqual(signal, null);
+}

test/sequential/test-runner-run-inspect.mjs

@@ -0,0 +1,164 @@
+import * as common from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import assert from 'node:assert';
+
+common.skipIfInspectorDisabled();
+
+
+const debuggerPort = common.getPort();
+
+async function spawnRunner({ execArgv, expectedPort, expectedHost, expectedInitialPort, inspectPort }) {
+  const { code, signal } = await common.spawnPromisified(
+    process.execPath,
+    ['--expose-internals', '--no-warnings', ...execArgv, fixtures.path('test-runner/run_inspect.js')], {
+      env: { ...process.env,
+             expectedPort,
+             inspectPort,
+             expectedHost,
+             expectedInitialPort }
+    });
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+let offset = 0;
+
+const defaultPortCase = spawnRunner({
+  execArgv: ['--inspect'],
+  expectedPort: 9230,
+});
+
+await spawnRunner({
+  execArgv: ['--inspect=65535'],
+  expectedPort: 1024,
+});
+
+let port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  expectedPort: port + 1,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: ['--inspect', `--inspect-port=${port}`],
+  expectedPort: port + 1,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: ['--inspect', `--debug-port=${port}`],
+  expectedPort: port + 1,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=0.0.0.0:${port}`],
+  expectedPort: port + 1, expectedHost: '0.0.0.0',
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=127.0.0.1:${port}`],
+  expectedPort: port + 1, expectedHost: '127.0.0.1'
+});
+
+if (common.hasIPv6) {
+  port = debuggerPort + offset++ * 5;
+
+  await spawnRunner({
+    execArgv: [`--inspect=[::]:${port}`],
+    expectedPort: port + 1, expectedHost: '::'
+  });
+
+  port = debuggerPort + offset++ * 5;
+
+  await spawnRunner({
+    execArgv: [`--inspect=[::1]:${port}`],
+    expectedPort: port + 1, expectedHost: '::1'
+  });
+}
+
+// These tests check that setting inspectPort in run
+// would take effect and override port incrementing behavior
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: port + 2,
+  expectedPort: port + 2,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'addTwo',
+  expectedPort: port + 2,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'string',
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'null',
+  expectedPort: port + 1,
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'bignumber',
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'negativenumber',
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'bignumberfunc'
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 'strfunc',
+});
+
+port = debuggerPort + offset++ * 5;
+
+await spawnRunner({
+  execArgv: [`--inspect=${port}`],
+  inspectPort: 0,
+  expectedInitialPort: 0,
+});
+
+await defaultPortCase;
+
+port = debuggerPort + offset++ * 5;
+await spawnRunner({
+  execArgv: ['--inspect'],
+  inspectPort: port + 2,
+  expectedInitialPort: port + 2,
+});