feat: support using --inspect with --test

PR-URL: nodejs/node#44520 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> (cherry picked from commit a165193c5c8e4bcfbd12b2c3f6e55a81a251c258)
MoLow · Feb 2, 2023 · 34d933a · 34d933a
1 parent 9f92794
commit 34d933a
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -346,6 +346,12 @@ 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

diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
@@ -1,14 +1,25 @@
-// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/main/test_runner.js
+// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/main/test_runner.js
 'use strict'
 const {
   prepareMainThreadExecution
 } = 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

diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js
@@ -10,10 +10,12 @@ exports.ArrayPrototypeForEach = (arr, fn, thisArg) => arr.forEach(fn, thisArg)
 exports.ArrayPrototypeIncludes = (arr, el, fromIndex) => arr.includes(el, fromIndex)
 exports.ArrayPrototypeJoin = (arr, str) => arr.join(str)
 exports.ArrayPrototypeMap = (arr, mapFn) => arr.map(mapFn)
+exports.ArrayPrototypePop = arr => arr.pop()
 exports.ArrayPrototypePush = (arr, ...el) => arr.push(...el)
 exports.ArrayPrototypeReduce = (arr, fn, originalVal) => arr.reduce(fn, originalVal)
 exports.ArrayPrototypeShift = arr => arr.shift()
 exports.ArrayPrototypeSlice = (arr, offset) => arr.slice(offset)
+exports.ArrayPrototypeSome = (arr, fn) => arr.some(fn)
 exports.ArrayPrototypeSort = (arr, fn) => arr.sort(fn)
 exports.ArrayPrototypeUnshift = (arr, ...el) => arr.unshift(...el)
 exports.Error = Error
@@ -38,13 +40,15 @@ exports.PromiseAll = iterator => Promise.all(iterator)
 exports.PromisePrototypeThen = (promise, thenFn, catchFn) => promise.then(thenFn, catchFn)
 exports.PromiseResolve = val => Promise.resolve(val)
 exports.PromiseRace = val => Promise.race(val)
+exports.RegExpPrototypeSymbolSplit = (reg, str) => reg[Symbol.split](str)
 exports.SafeArrayIterator = class ArrayIterator {constructor (array) { this.array = array }[Symbol.iterator] () { return this.array.values() }}
 exports.SafeMap = Map
 exports.SafePromiseAll = (array, mapFn) => Promise.all(mapFn ? array.map(mapFn) : array)
 exports.SafePromiseRace = (array, mapFn) => Promise.race(mapFn ? array.map(mapFn) : array)
 exports.SafeSet = Set
 exports.SafeWeakMap = WeakMap
 exports.SafeWeakSet = WeakSet
+exports.StringPrototypeEndsWith = (haystack, needle, index) => haystack.endsWith(needle, index)
 exports.StringPrototypeIncludes = (str, needle) => str.includes(needle)
 exports.StringPrototypeMatch = (str, reg) => str.match(reg)
 exports.StringPrototypeReplace = (str, search, replacement) =>

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -1,19 +1,23 @@
-// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/runner.js
+// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/runner.js
 'use strict'
 const {
   ArrayFrom,
-  ArrayPrototypeConcat,
   ArrayPrototypeFilter,
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
+  ArrayPrototypePop,
+  ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,
   ObjectAssign,
   PromisePrototypeThen,
+  RegExpPrototypeSymbolSplit,
   SafePromiseAll,
-  SafeSet
+  SafeSet,
+  StringPrototypeEndsWith
 } = require('#internal/per_context/primordials')
 
+const { Buffer } = require('buffer')
 const { spawn } = require('child_process')
 const { readdirSync, statSync } = require('fs')
 const {
@@ -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')
@@ -101,25 +106,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 })
+      child.stdout.toArray({ signal: t.signal })
     ])
 
     if (code !== 0 || signal !== null) {
@@ -129,7 +168,7 @@ function runTestFile (path, root) {
           exitCode: code,
           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
@@ -146,7 +185,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')
@@ -155,7 +194,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

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/6ee1f3444f8c1cf005153f936ffc74221d55658b/lib/internal/test_runner/test.js
+// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/test.js
 
 'use strict'
 
@@ -60,8 +60,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'])
@@ -148,7 +146,7 @@ class Test extends AsyncResource {
     }
 
     if (parent === null) {
-      this.concurrency = rootConcurrency
+      this.concurrency = 1
       this.indent = ''
       this.indentString = kDefaultIndent
       this.only = testOnlyFlag

diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js
@@ -0,0 +1,47 @@
+// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/util/inspector.js
+const {
+  ArrayPrototypeSome,
+  RegExpPrototypeExec
+} = require('#internal/per_context/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
+}
+
+function isInspectorMessage (string) {
+  return isUsingInspector() && RegExpPrototypeExec(kInspectMsgRegex, string) !== null
+}
+
+module.exports = {
+  isUsingInspector,
+  getInspectPort,
+  isInspectorMessage
+}
diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/1aab13cad9c800f4121c1d35b554b78c1b17bdbd/test/parallel/test-runner-cli.js
+// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/test/parallel/test-runner-cli.js
 'use strict'
 require('../common')
 const assert = require('assert')
@@ -106,13 +106,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)