From ae799bdf1a1c2576bcd1f17430bc8f5f1997a631 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 5 Sep 2022 16:51:06 +0300 Subject: [PATCH 01/15] test_runner: support using `--inspect` with `--test` --- doc/api/test.md | 4 +++ lib/internal/main/test_runner.js | 14 +++++++-- lib/internal/test_runner/runner.js | 46 ++++++++++++++++++++++++------ lib/internal/test_runner/test.js | 5 ++-- src/node_options.cc | 3 -- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index 3636adaee524f9..6d2a4c49ed423e 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -338,6 +338,10 @@ 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. By default each process gets its own port, incremented from the + primary's `process.debugPort`. * Returns: {TapStream} ```js diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 744a58271138b9..d36947c635e03c 100644 --- a/lib/internal/main/test_runner.js +++ b/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; diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index a4afbcb0a7710f..d4789ed9a31189 100644 --- a/lib/internal/test_runner/runner.js +++ b/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,9 @@ const { const { basename, join, resolve } = require('path'); const { once } = require('events'); +const kMinPort = 1024; +const kMaxPort = 65535; +const kInspectArgRegex = /--inspect(?:-brk|-port)?/; const kFilterArgs = ['--test']; // TODO(cjihrig): Replace this with recursive readdir once it lands. @@ -96,15 +101,38 @@ 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 }) { + const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); + if (isUsingInspector()) { + if (typeof inspectPort === 'function') { + inspectPort = inspectPort(); + } else if (typeof inspectPort !== 'number') { + 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, 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 @@ -145,7 +173,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,10 +182,10 @@ 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; } -module.exports = { run }; +module.exports = { run, isUsingInspector }; diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 544c09206f3f0b..ffe22495f993f4 100644 --- a/lib/internal/test_runner/test.js +++ b/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; diff --git a/src/node_options.cc b/src/node_options.cc index 3ab9ab6e6da09b..d25cc36feaa469 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -161,9 +161,6 @@ void EnvironmentOptions::CheckOptions(std::vector* 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 From 9414d245733d1323b100db70d999bd845a19dd28 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 6 Sep 2022 12:32:18 +0300 Subject: [PATCH 02/15] CR & tests --- doc/api/test.md | 5 +- lib/internal/test_runner/runner.js | 27 ++- test/parallel/test-runner-cli.js | 6 - test/parallel/test-runner-inspect.js | 68 +++++++ test/sequential/test-runner-run-inspect.js | 214 +++++++++++++++++++++ 5 files changed, 303 insertions(+), 17 deletions(-) create mode 100644 test/parallel/test-runner-inspect.js create mode 100644 test/sequential/test-runner-run-inspect.js diff --git a/doc/api/test.md b/doc/api/test.md index 6d2a4c49ed423e..be789dab46d8cb 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -340,8 +340,9 @@ added: REPLACEME **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. By default each process gets its own port, incremented from the - primary's `process.debugPort`. + 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/test_runner/runner.js b/lib/internal/test_runner/runner.js index d4789ed9a31189..93fe897906122f 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -37,6 +37,7 @@ 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. @@ -111,12 +112,12 @@ function filterExecArgv(arg) { } let debugPortOffset = 1; -function getRunArgs({ path, inspectPort }) { +function getRunArgs({ path, inspectPort, usingInspector }) { const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); - if (isUsingInspector()) { + if (usingInspector) { if (typeof inspectPort === 'function') { inspectPort = inspectPort(); - } else if (typeof inspectPort !== 'number') { + } else if (inspectPort == null) { inspectPort = process.debugPort + debugPortOffset; if (inspectPort > kMaxPort) inspectPort = inspectPort - kMaxPort + kMinPort - 1; @@ -130,23 +131,30 @@ function getRunArgs({ path, inspectPort }) { return argv; } -function runTestFile(path, root, inspectPort) { +function runTestFile(path, root, usingInspector, inspectPort) { const subtest = root.createSubtest(Test, path, async (t) => { - const args = getRunArgs({ path, inspectPort }); + 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) { @@ -156,7 +164,7 @@ function runTestFile(path, root, inspectPort) { 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, @@ -181,8 +189,9 @@ function run(options) { const root = createTestTree({ concurrency, timeout, signal }); const testFiles = files ?? createTestFileList(); + const usingInspector = isUsingInspector(); - PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)), + PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, usingInspector, inspectPort)), () => root.postRun()); return root.reporter; diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index 552d64d7c40ba2..8c1f6b3b0bee69 100644 --- a/test/parallel/test-runner-cli.js +++ b/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); diff --git a/test/parallel/test-runner-inspect.js b/test/parallel/test-runner-inspect.js new file mode 100644 index 00000000000000..78ffbc3d9e156d --- /dev/null +++ b/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); +} diff --git a/test/sequential/test-runner-run-inspect.js b/test/sequential/test-runner-run-inspect.js new file mode 100644 index 00000000000000..acea9136f57470 --- /dev/null +++ b/test/sequential/test-runner-run-inspect.js @@ -0,0 +1,214 @@ +'use strict'; + +const common = require('../common'); +const { parseArgs } = require('util'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); + +const debuggerPort = common.PORT; +const childProcess = require('child_process'); + +const { values: { 'top-level': isTopLevel } } = + parseArgs({ options: { topLevel: { type: 'boolean' } }, strict: false }); + +let offset = 0; + +async function testRunnerMain() { + await 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 + }); + + port = debuggerPort + offset++ * 5; + + await spawnRunner({ + execArgv: [`--inspect=127.0.0.1:${port}`], + expectedPort: port + 1 + }); + + if (common.hasIPv6) { + port = debuggerPort + offset++ * 5; + + await spawnRunner({ + execArgv: [`--inspect=[::]:${port}`], + expectedPort: port + 1 + }); + + port = debuggerPort + offset++ * 5; + + await spawnRunner({ + execArgv: [`--inspect=[::1]:${port}`], + expectedPort: port + 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; + + spawnRunner({ + execArgv: [`--inspect=${port}`], + inspectPort: 'strfunc', + }); +} + +function runTest() { + const { run } = require('node:test'); + 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 = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + } else if (process.env.inspectPort === 'null') { + inspectPort = null; + } else if (process.env.inspectPort === 'bignumber') { + inspectPort = 1293812; + expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + } else if (process.env.inspectPort === 'negativenumber') { + inspectPort = -9776; + expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + } else if (process.env.inspectPort === 'bignumberfunc') { + inspectPort = common.mustCall(() => 123121); + expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + } else if (process.env.inspectPort === 'strfunc') { + inspectPort = common.mustCall(() => 'invalidPort'); + expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + } + + const stream = run({ files: [__filename], 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; }); + } +} + + +function assertParams() { + const { expectedPort } = process.env; + if ('expectedPort' in process.env) { + assert.strictEqual(process.debugPort, +expectedPort); + } + process.exit(); +} + +function spawnRunner({ execArgv, expectedPort, inspectPort }) { + return new Promise((resolve) => { + childProcess.spawn(process.execPath, ['--no-warnings', ...execArgv, __filename, '--top-level'], { + env: { ...process.env, + expectedPort: JSON.stringify(expectedPort), + inspectPort, + testProcess: true }, + stdio: 'inherit', + }).on('exit', common.mustCall((code, signal) => { + checkExitCode(code, signal); + resolve(); + })); + }); +} + +function checkExitCode(code, signal) { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + +if (!process.env.testProcess && !isTopLevel) { + testRunnerMain().then(common.mustCall()); +} else if (isTopLevel) { + runTest(); +} else { + assertParams(); +} From 2e344c3989d3a579b26cea61d2aa8f014750655c Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 6 Sep 2022 14:19:36 +0300 Subject: [PATCH 03/15] CR --- test/sequential/test-runner-run-inspect.js | 88 ++++++++++++++-------- 1 file changed, 57 insertions(+), 31 deletions(-) diff --git a/test/sequential/test-runner-run-inspect.js b/test/sequential/test-runner-run-inspect.js index acea9136f57470..86bd4350928500 100644 --- a/test/sequential/test-runner-run-inspect.js +++ b/test/sequential/test-runner-run-inspect.js @@ -13,10 +13,27 @@ const childProcess = require('child_process'); const { values: { 'top-level': isTopLevel } } = parseArgs({ options: { topLevel: { type: 'boolean' } }, strict: false }); +async function spawnRunner({ execArgv, expectedPort, expectedHost, expectedInitialPort, inspectPort }) { + const { code, signal } = await new Promise((resolve) => { + childProcess.spawn(process.execPath, + ['--expose-internals', '--no-warnings', ...execArgv, __filename, '--top-level'], { + env: { ...process.env, + expectedPort: JSON.stringify(expectedPort), + inspectPort, + expectedHost, + expectedInitialPort, + testProcess: true }, + stdio: 'inherit', + }).on('exit', (code, signal) => resolve({ code, signal })); + }); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +} + let offset = 0; async function testRunnerMain() { - await spawnRunner({ + const defaultPortCase = spawnRunner({ execArgv: ['--inspect'], expectedPort: 9230, }); @@ -51,14 +68,14 @@ async function testRunnerMain() { await spawnRunner({ execArgv: [`--inspect=0.0.0.0:${port}`], - expectedPort: port + 1 + expectedPort: port + 1, expectedHost: '0.0.0.0', }); port = debuggerPort + offset++ * 5; await spawnRunner({ execArgv: [`--inspect=127.0.0.1:${port}`], - expectedPort: port + 1 + expectedPort: port + 1, expectedHost: '127.0.0.1' }); if (common.hasIPv6) { @@ -66,14 +83,14 @@ async function testRunnerMain() { await spawnRunner({ execArgv: [`--inspect=[::]:${port}`], - expectedPort: port + 1 + expectedPort: port + 1, expectedHost: '::' }); port = debuggerPort + offset++ * 5; await spawnRunner({ execArgv: [`--inspect=[::1]:${port}`], - expectedPort: port + 1 + expectedPort: port + 1, expectedHost: '::1' }); } @@ -134,12 +151,30 @@ async function testRunnerMain() { port = debuggerPort + offset++ * 5; - spawnRunner({ + 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, + }); } +const badPortError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; function runTest() { const { run } = require('node:test'); let inspectPort = 'inspectPort' in process.env ? Number(process.env.inspectPort) : undefined; @@ -149,21 +184,21 @@ function runTest() { inspectPort = common.mustCall(() => { return process.debugPort += 2; }); } else if (process.env.inspectPort === 'string') { inspectPort = 'string'; - expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + expectedError = badPortError; } else if (process.env.inspectPort === 'null') { inspectPort = null; } else if (process.env.inspectPort === 'bignumber') { inspectPort = 1293812; - expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + expectedError = badPortError; } else if (process.env.inspectPort === 'negativenumber') { inspectPort = -9776; - expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + expectedError = badPortError; } else if (process.env.inspectPort === 'bignumberfunc') { inspectPort = common.mustCall(() => 123121); - expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + expectedError = badPortError; } else if (process.env.inspectPort === 'strfunc') { inspectPort = common.mustCall(() => 'invalidPort'); - expectedError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; + expectedError = badPortError; } const stream = run({ files: [__filename], inspectPort }); @@ -178,31 +213,22 @@ function runTest() { function assertParams() { - const { expectedPort } = process.env; + const { expectedPort, expectedInitialPort, expectedHost } = process.env; + const debugOptions = + require('internal/options').getOptionValue('--inspect-port'); + if ('expectedPort' in process.env) { assert.strictEqual(process.debugPort, +expectedPort); } - process.exit(); -} -function spawnRunner({ execArgv, expectedPort, inspectPort }) { - return new Promise((resolve) => { - childProcess.spawn(process.execPath, ['--no-warnings', ...execArgv, __filename, '--top-level'], { - env: { ...process.env, - expectedPort: JSON.stringify(expectedPort), - inspectPort, - testProcess: true }, - stdio: 'inherit', - }).on('exit', common.mustCall((code, signal) => { - checkExitCode(code, signal); - resolve(); - })); - }); -} + if ('expectedInitialPort' in process.env) { + assert.strictEqual(debugOptions.port, +expectedInitialPort); + } -function checkExitCode(code, signal) { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); + if ('expectedHost' in process.env) { + assert.strictEqual(debugOptions.host, expectedHost); + } + process.exit(); } if (!process.env.testProcess && !isTopLevel) { From d1fdf1c98b974f5c0c445696f9c30e1536f1d45d Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 09:42:40 +0300 Subject: [PATCH 04/15] CR --- lib/internal/cluster/primary.js | 1 + lib/internal/main/test_runner.js | 3 +- lib/internal/test_runner/runner.js | 42 +-- lib/internal/util/inspector.js | 42 +++ test/common/index.mjs | 3 + test/fixtures/test-runner/run_inspect.js | 38 +++ .../test-runner/run_inspect_assert.js | 18 ++ test/sequential/test-runner-run-inspect.js | 240 ------------------ test/sequential/test-runner-run-inspect.mjs | 164 ++++++++++++ 9 files changed, 278 insertions(+), 273 deletions(-) create mode 100644 test/fixtures/test-runner/run_inspect.js create mode 100644 test/fixtures/test-runner/run_inspect_assert.js delete mode 100644 test/sequential/test-runner-run-inspect.js create mode 100644 test/sequential/test-runner-run-inspect.mjs diff --git a/lib/internal/cluster/primary.js b/lib/internal/cluster/primary.js index e71561d724eaa5..a9a42ad6120c9f 100644 --- 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 index d36947c635e03c..63a93d9dc0f4f0 100644 --- 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 index 93fe897906122f..44967dd33ca32e 100644 --- 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 index d78467136c4918..e24c4e0ad35124 100644 --- 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 index 2b30f499343cc4..e77b1b298cbc30 100644 --- 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 new file mode 100644 index 00000000000000..a562e29fedd8cc --- /dev/null +++ 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; }); +} \ No newline at end of file diff --git a/test/fixtures/test-runner/run_inspect_assert.js b/test/fixtures/test-runner/run_inspect_assert.js new file mode 100644 index 00000000000000..5559e22ea2fdf5 --- /dev/null +++ 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(); \ No newline at end of file diff --git a/test/sequential/test-runner-run-inspect.js b/test/sequential/test-runner-run-inspect.js deleted file mode 100644 index 86bd4350928500..00000000000000 --- a/test/sequential/test-runner-run-inspect.js +++ /dev/null @@ -1,240 +0,0 @@ -'use strict'; - -const common = require('../common'); -const { parseArgs } = require('util'); - -common.skipIfInspectorDisabled(); - -const assert = require('assert'); - -const debuggerPort = common.PORT; -const childProcess = require('child_process'); - -const { values: { 'top-level': isTopLevel } } = - parseArgs({ options: { topLevel: { type: 'boolean' } }, strict: false }); - -async function spawnRunner({ execArgv, expectedPort, expectedHost, expectedInitialPort, inspectPort }) { - const { code, signal } = await new Promise((resolve) => { - childProcess.spawn(process.execPath, - ['--expose-internals', '--no-warnings', ...execArgv, __filename, '--top-level'], { - env: { ...process.env, - expectedPort: JSON.stringify(expectedPort), - inspectPort, - expectedHost, - expectedInitialPort, - testProcess: true }, - stdio: 'inherit', - }).on('exit', (code, signal) => resolve({ code, signal })); - }); - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); -} - -let offset = 0; - -async function testRunnerMain() { - 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, - }); -} - -const badPortError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' }; -function runTest() { - const { run } = require('node:test'); - 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: [__filename], 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; }); - } -} - - -function assertParams() { - 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(); -} - -if (!process.env.testProcess && !isTopLevel) { - testRunnerMain().then(common.mustCall()); -} else if (isTopLevel) { - runTest(); -} else { - assertParams(); -} diff --git a/test/sequential/test-runner-run-inspect.mjs b/test/sequential/test-runner-run-inspect.mjs new file mode 100644 index 00000000000000..7f2351cc87c2d6 --- /dev/null +++ b/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'), '--top-level'], { + env: { ...process.env, + expectedPort: JSON.stringify(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, +}); From 572ad0db4cab668cbdc1bf255c38a06a7faf4407 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 09:49:38 +0300 Subject: [PATCH 05/15] fixes --- test/fixtures/test-runner/run_inspect.js | 2 +- test/fixtures/test-runner/run_inspect_assert.js | 2 +- test/sequential/test-runner-run-inspect.mjs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/fixtures/test-runner/run_inspect.js b/test/fixtures/test-runner/run_inspect.js index a562e29fedd8cc..dc19a35c2aa2e3 100644 --- a/test/fixtures/test-runner/run_inspect.js +++ b/test/fixtures/test-runner/run_inspect.js @@ -35,4 +35,4 @@ if (expectedError) { })); } else { stream.once('test:fail', ({ error }) => { throw error; }); -} \ No newline at end of file +} diff --git a/test/fixtures/test-runner/run_inspect_assert.js b/test/fixtures/test-runner/run_inspect_assert.js index 5559e22ea2fdf5..af3a8b915dcca7 100644 --- a/test/fixtures/test-runner/run_inspect_assert.js +++ b/test/fixtures/test-runner/run_inspect_assert.js @@ -15,4 +15,4 @@ if ('expectedInitialPort' in process.env) { if ('expectedHost' in process.env) { assert.strictEqual(debugOptions.host, expectedHost); } -process.exit(); \ No newline at end of file +process.exit(); diff --git a/test/sequential/test-runner-run-inspect.mjs b/test/sequential/test-runner-run-inspect.mjs index 7f2351cc87c2d6..35c380c8d4e33e 100644 --- a/test/sequential/test-runner-run-inspect.mjs +++ b/test/sequential/test-runner-run-inspect.mjs @@ -10,9 +10,9 @@ 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'), '--top-level'], { + ['--expose-internals', '--no-warnings', ...execArgv, fixtures.path('test-runner/run_inspect.js')], { env: { ...process.env, - expectedPort: JSON.stringify(expectedPort), + expectedPort, inspectPort, expectedHost, expectedInitialPort } From dea4784c8545f8661b12f6f28fd5527914103638 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 11:21:47 +0300 Subject: [PATCH 06/15] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/fixtures/test-runner/run_inspect.js | 6 ++++-- test/fixtures/test-runner/run_inspect_assert.js | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/fixtures/test-runner/run_inspect.js b/test/fixtures/test-runner/run_inspect.js index dc19a35c2aa2e3..1586b6aaccf082 100644 --- a/test/fixtures/test-runner/run_inspect.js +++ b/test/fixtures/test-runner/run_inspect.js @@ -1,3 +1,5 @@ +'use strict'; + const common = require('../../common'); const fixtures = require('../../common/fixtures'); const { run } = require('node:test'); @@ -30,9 +32,9 @@ if (process.env.inspectPort === 'addTwo') { const stream = run({ files: [fixtures.path('test-runner/run_inspect_assert.js')], inspectPort }); if (expectedError) { - stream.once('test:fail', common.mustCall(({ error }) => { + stream.on('test:fail', common.mustCall(({ error }) => { assert.deepStrictEqual({ name: error.cause.name, code: error.cause.code }, expectedError); })); } else { - stream.once('test:fail', ({ error }) => { throw error; }); + stream.on('test:fail', common.mustNotCall()); } diff --git a/test/fixtures/test-runner/run_inspect_assert.js b/test/fixtures/test-runner/run_inspect_assert.js index af3a8b915dcca7..daea9b3b6656c1 100644 --- a/test/fixtures/test-runner/run_inspect_assert.js +++ b/test/fixtures/test-runner/run_inspect_assert.js @@ -1,3 +1,5 @@ +'use strict'; + const assert = require('node:assert'); const { expectedPort, expectedInitialPort, expectedHost } = process.env; @@ -15,4 +17,3 @@ if ('expectedInitialPort' in process.env) { if ('expectedHost' in process.env) { assert.strictEqual(debugOptions.host, expectedHost); } -process.exit(); From 85b5716c99fb36e3bb44a82604f99ce759a5e032 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 11:46:46 +0300 Subject: [PATCH 07/15] Update lib/internal/cluster/primary.js Co-authored-by: Antoine du Hamel --- lib/internal/cluster/primary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/cluster/primary.js b/lib/internal/cluster/primary.js index a9a42ad6120c9f..9eeb2735857bec 100644 --- a/lib/internal/cluster/primary.js +++ b/lib/internal/cluster/primary.js @@ -120,7 +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 + // TODO(MoLow): Use getInspectPort from internal/util/inspector if (ArrayPrototypeSome(execArgv, (arg) => RegExpPrototypeExec(debugArgRegex, arg) !== null) || RegExpPrototypeExec(debugArgRegex, nodeOptions) !== null) { From a762de779fb688efc68ce1857930f44d1d09cffe Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 14:15:52 +0300 Subject: [PATCH 08/15] attampt fixing windows build --- test/{parallel => sequential}/test-runner-inspect.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{parallel => sequential}/test-runner-inspect.js (100%) diff --git a/test/parallel/test-runner-inspect.js b/test/sequential/test-runner-inspect.js similarity index 100% rename from test/parallel/test-runner-inspect.js rename to test/sequential/test-runner-inspect.js From 1da1d5ed1e0d5db12893946351246c4964fc61c9 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 17:05:39 +0300 Subject: [PATCH 09/15] try fully sequential --- ...ner-inspect.js => test-runner-inspect.mjs} | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) rename test/sequential/{test-runner-inspect.js => test-runner-inspect.mjs} (50%) diff --git a/test/sequential/test-runner-inspect.js b/test/sequential/test-runner-inspect.mjs similarity index 50% rename from test/sequential/test-runner-inspect.js rename to test/sequential/test-runner-inspect.mjs index 78ffbc3d9e156d..8dcf323873fa08 100644 --- a/test/sequential/test-runner-inspect.js +++ b/test/sequential/test-runner-inspect.mjs @@ -1,18 +1,16 @@ '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'); +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(); -async function debugTest() { - const child = new NodeInstance(['--test', '--inspect-brk=0'], undefined, join(testFixtures, 'index.test.js')); +{ + const child = new NodeInstance(['--test', '--inspect-brk=0'], undefined, fixtures.path('test-runner/index.test.js')); let stdout = ''; let stderr = ''; @@ -23,46 +21,35 @@ async function debugTest() { 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); + const args = ['--test', '--inspect=0', fixtures.path('test-runner/index.js')]; + const { stderr, stdout, code, signal } = await common.spawnPromisified(process.execPath, args); - assert.match(child.stderr.toString(), + 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 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); + 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 child = spawnSync(process.execPath, args); + const { stderr, stdout, code, signal } = await common.spawnPromisified(process.execPath, args); - const stderr = child.stderr.toString(); - assert.strictEqual(child.stdout.toString(), ''); + 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(child.status, 1); - assert.strictEqual(child.signal, null); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); } From c1a956c330170332d46a62bfd5cb0a85bc24781a Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 17:17:07 +0300 Subject: [PATCH 10/15] lint --- test/sequential/test-runner-inspect.mjs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/sequential/test-runner-inspect.mjs b/test/sequential/test-runner-inspect.mjs index 8dcf323873fa08..67095291e2acd3 100644 --- a/test/sequential/test-runner-inspect.mjs +++ b/test/sequential/test-runner-inspect.mjs @@ -1,4 +1,3 @@ -'use strict'; import * as common from '../common/index.mjs'; import * as tmpdir from '../common/tmpdir.js'; import * as fixtures from '../common/fixtures.mjs'; From e1375278e2de9a5f05a8f95624d798b9794935b4 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 19:17:20 +0300 Subject: [PATCH 11/15] try debugging this --- lib/internal/test_runner/runner.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 44967dd33ca32e..530f3e66c2fa7f 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -103,9 +103,11 @@ function filterExecArgv(arg) { function getRunArgs({ path, inspectPort }) { const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); + console.log(`Running test ${path}...`, { inspectPort, isUsingInspector: isUsingInspector() }); if (isUsingInspector()) { ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`); } + console.log(`Running test ${path}...`, { inspectPort, argv }); ArrayPrototypePush(argv, path); return argv; } From 3346b00606b8c4195da2f675facd763640c140be Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 8 Sep 2022 23:23:40 +0300 Subject: [PATCH 12/15] try --- lib/internal/test_runner/runner.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 530f3e66c2fa7f..6de6047cd3a6e6 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -127,7 +127,8 @@ function runTestFile(path, root, inspectPort) { }); child.stderr.on('data', (data) => { - if (isInspectorMessage(data)) { + console.log('[stderr]', { data: data.toString() }); + if (isInspectorMessage(data.toString())) { process.stderr.write(data); } stderr += data; From 92ea9bbc9b382f0d1f6c3763accac969248db965 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 9 Sep 2022 08:28:08 +0300 Subject: [PATCH 13/15] fix --- lib/internal/test_runner/runner.js | 32 ++++++++++++++----- .../test-runner-inspect.mjs | 0 2 files changed, 24 insertions(+), 8 deletions(-) rename test/{sequential => parallel}/test-runner-inspect.mjs (100%) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 6de6047cd3a6e6..59991c8a1f171a 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -13,6 +13,7 @@ const { SafeSet, } = primordials; +const { Buffer } = require('buffer'); const { spawn } = require('child_process'); const { readdirSync, statSync } = require('fs'); const console = require('internal/console/global'); @@ -103,15 +104,34 @@ function filterExecArgv(arg) { function getRunArgs({ path, inspectPort }) { const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); - console.log(`Running test ${path}...`, { inspectPort, isUsingInspector: isUsingInspector() }); if (isUsingInspector()) { ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`); } - console.log(`Running test ${path}...`, { inspectPort, argv }); 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.split(/\r?\n/); + if (str.endsWith('\n')) + buffer = Buffer.alloc(0); + else + buffer = Buffer.from(lines.pop(), 'utf8'); + lines = lines.join('\n'); + if (isInspectorMessage(lines)) { + process.stderr.write(lines); + } + }; +} + function runTestFile(path, root, inspectPort) { const subtest = root.createSubtest(Test, path, async (t) => { const args = getRunArgs({ path, inspectPort }); @@ -126,13 +146,9 @@ function runTestFile(path, root, inspectPort) { err = error; }); - child.stderr.on('data', (data) => { - console.log('[stderr]', { data: data.toString() }); - if (isInspectorMessage(data.toString())) { - process.stderr.write(data); - } + child.stderr.on('data', makeStderrCallback((data) => { stderr += data; - }); + })); const { 0: { 0: code, 1: signal }, 1: stdout } = await SafePromiseAll([ once(child, 'exit', { signal: t.signal }), diff --git a/test/sequential/test-runner-inspect.mjs b/test/parallel/test-runner-inspect.mjs similarity index 100% rename from test/sequential/test-runner-inspect.mjs rename to test/parallel/test-runner-inspect.mjs From b07d5742a8d082cefafa5677c218264d4ee03eb9 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 9 Sep 2022 08:37:38 +0300 Subject: [PATCH 14/15] primordials --- lib/internal/test_runner/runner.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 59991c8a1f171a..62bba7d3ce7dfd 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -4,13 +4,16 @@ const { ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeJoin, + ArrayPrototypePop, ArrayPrototypePush, ArrayPrototypeSlice, ArrayPrototypeSort, ObjectAssign, PromisePrototypeThen, + RegExpPrototypeSymbolSplit, SafePromiseAll, SafeSet, + StringPrototypeEndsWith, } = primordials; const { Buffer } = require('buffer'); @@ -120,12 +123,12 @@ function makeStderrCallback(callback) { callback(data); const newData = Buffer.concat([buffer, data]); const str = newData.toString('utf8'); - let lines = str.split(/\r?\n/); - if (str.endsWith('\n')) + let lines = RegExpPrototypeSymbolSplit(/\r?\n/, str); + if (StringPrototypeEndsWith(str, '\n')) buffer = Buffer.alloc(0); else - buffer = Buffer.from(lines.pop(), 'utf8'); - lines = lines.join('\n'); + buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8'); + lines = ArrayPrototypeJoin(lines, '\n'); if (isInspectorMessage(lines)) { process.stderr.write(lines); } From 2e1c14a05dd7db986414b6c165f2a25ff46e5b87 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 9 Sep 2022 08:43:26 +0300 Subject: [PATCH 15/15] optimize --- lib/internal/test_runner/runner.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 62bba7d3ce7dfd..911a700d68d58d 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -123,12 +123,14 @@ function makeStderrCallback(callback) { callback(data); const newData = Buffer.concat([buffer, data]); const str = newData.toString('utf8'); - let lines = RegExpPrototypeSymbolSplit(/\r?\n/, str); - if (StringPrototypeEndsWith(str, '\n')) + let lines = str; + if (StringPrototypeEndsWith(lines, '\n')) { buffer = Buffer.alloc(0); - else + } else { + lines = RegExpPrototypeSymbolSplit(/\r?\n/, str); buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8'); - lines = ArrayPrototypeJoin(lines, '\n'); + lines = ArrayPrototypeJoin(lines, '\n'); + } if (isInspectorMessage(lines)) { process.stderr.write(lines); }