CR

nodejs · nodejs-github-bot · Sep 10, 2022 · Sep 5, 2022 · Sep 6, 2022 · Sep 6, 2022
commit d1fdf1c98b974f5c0c445696f9c30e1536f1d45d
diff --git a/lib/internal/cluster/primary.js b/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/inspect
   if (ArrayPrototypeSome(execArgv,
                          (arg) => RegExpPrototypeExec(debugArgRegex, arg) !== null) ||
       RegExpPrototypeExec(debugArgRegex, nodeOptions) !== null) {

diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
@@ -3,7 +3,8 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete
 } = require('internal/process/pre_execution');
-const { run, isUsingInspector } = require('internal/test_runner/runner');
+const { isUsingInspector } = require('internal/util/inspector');
+const { run } = require('internal/test_runner/runner');
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -6,11 +6,9 @@ const {
   ArrayPrototypeJoin,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
-  ArrayPrototypeSome,
   ArrayPrototypeSort,
   ObjectAssign,
   PromisePrototypeThen,
-  RegExpPrototypeExec,
   SafePromiseAll,
   SafeSet,
 } = primordials;
@@ -23,7 +21,8 @@ const {
     ERR_TEST_FAILURE,
   },
 } = require('internal/errors');
-const { validateArray, validatePort } = require('internal/validators');
+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');
@@ -34,10 +33,6 @@ 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.
@@ -102,38 +97,22 @@ 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);
 }
 
-let debugPortOffset = 1;
-function getRunArgs({ path, inspectPort, usingInspector }) {
+function getRunArgs({ path, inspectPort }) {
   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}`);
+  if (isUsingInspector()) {
+    ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
   }
   ArrayPrototypePush(argv, path);
   return argv;
 }
 
-function runTestFile(path, root, usingInspector, inspectPort) {
+function runTestFile(path, root, inspectPort) {
   const subtest = root.createSubtest(Test, path, async (t) => {
-    const args = getRunArgs({ path, inspectPort, usingInspector });
+    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
@@ -146,7 +125,7 @@ function runTestFile(path, root, usingInspector, inspectPort) {
     });
 
     child.stderr.on('data', (data) => {
-      if (usingInspector && RegExpPrototypeExec(kInspectMsgRegex, data) !== null) {
+      if (isInspectorMessage(data)) {
         process.stderr.write(data);
       }
       stderr += data;
@@ -189,12 +168,11 @@ function run(options) {
 
   const root = createTestTree({ concurrency, timeout, signal });
   const testFiles = files ?? createTestFileList();
-  const usingInspector = isUsingInspector();
 
-  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, usingInspector, inspectPort)),
+  PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)),
                        () => root.postRun());
 
   return root.reporter;
 }
 
-module.exports = { run, isUsingInspector };
+module.exports = { run };
diff --git a/lib/internal/util/inspector.js b/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; }
@@ -63,7 +102,10 @@ function wrapConsole(consoleFromNode) {
 }
 
 module.exports = {
+  getInspectPort,
   installConsoleExtensions,
+  isInspectorMessage,
+  isUsingInspector,
   sendInspectorCommand,
   wrapConsole,
 };
diff --git a/test/common/index.mjs b/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,
 };
diff --git a/test/fixtures/test-runner/run_inspect.js b/test/fixtures/test-runner/run_inspect.js
@@ -0,0 +1,38 @@
+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.once('test:fail', common.mustCall(({ error }) => {
+    assert.deepStrictEqual({ name: error.cause.name, code: error.cause.code }, expectedError);
+  }));
+} else {
+  stream.once('test:fail', ({ error }) => { throw error; });
+}
diff --git a/test/fixtures/test-runner/run_inspect_assert.js b/test/fixtures/test-runner/run_inspect_assert.js
@@ -0,0 +1,18 @@
+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);
+}
+process.exit();