Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: support using --inspect with --test #44520

Merged
merged 15 commits into from
Sep 10, 2022
5 changes: 5 additions & 0 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
MoLow marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
1 change: 1 addition & 0 deletions lib/internal/cluster/primary.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
MoLow marked this conversation as resolved.
Show resolved Hide resolved
if (ArrayPrototypeSome(execArgv,
(arg) => RegExpPrototypeExec(debugArgRegex, arg) !== null) ||
RegExpPrototypeExec(debugArgRegex, nodeOptions) !== null) {
Expand Down
13 changes: 12 additions & 1 deletion lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@ const {
prepareMainThreadExecution,
markBootstrapComplete
} = require('internal/process/pre_execution');
const { isUsingInspector } = require('internal/util/inspector');
const { run } = require('internal/test_runner/runner');

prepareMainThreadExecution(false);
markBootstrapComplete();

const tapStream = run();
let concurrency = true;
let inspectPort;

if (isUsingInspector()) {
process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +
'Use the inspectPort option to run with concurrency');
concurrency = 1;
inspectPort = process.debugPort;
}

const tapStream = run({ concurrency, inspectPort });
tapStream.pipe(process.stdout);
tapStream.once('test:fail', () => {
process.exitCode = 1;
Expand Down
35 changes: 25 additions & 10 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
'use strict';
const {
ArrayFrom,
ArrayPrototypeConcat,
ArrayPrototypeFilter,
ArrayPrototypeIncludes,
ArrayPrototypeJoin,
ArrayPrototypePush,
ArrayPrototypeSlice,
ArrayPrototypeSort,
ObjectAssign,
Expand All @@ -22,6 +22,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');
Expand Down Expand Up @@ -100,25 +101,39 @@ function filterExecArgv(arg) {
return !ArrayPrototypeIncludes(kFilterArgs, arg);
}

function runTestFile(path, root) {
function getRunArgs({ path, inspectPort }) {
const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
if (isUsingInspector()) {
ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
}
ArrayPrototypePush(argv, path);
return argv;
}

function runTestFile(path, root, inspectPort) {
const subtest = root.createSubtest(Test, path, async (t) => {
const args = ArrayPrototypeConcat(
ArrayPrototypeFilter(process.execArgv, filterExecArgv),
path);
const args = getRunArgs({ path, inspectPort });

const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' });
// TODO(cjihrig): Implement a TAP parser to read the child's stdout
// instead of just displaying it all if the child fails.
let err;
let stderr = '';

child.on('error', (error) => {
err = error;
});

const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([
child.stderr.on('data', (data) => {
if (isInspectorMessage(data)) {
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) {
Expand All @@ -128,7 +143,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,
Expand All @@ -145,7 +160,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');
Expand All @@ -154,7 +169,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;
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down Expand Up @@ -146,7 +144,7 @@ class Test extends AsyncResource {
}

if (parent === null) {
this.concurrency = rootConcurrency;
this.concurrency = 1;
MoLow marked this conversation as resolved.
Show resolved Hide resolved
this.indent = '';
this.indentString = kDefaultIndent;
this.only = testOnlyFlag;
Expand Down Expand Up @@ -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;
Expand Down
42 changes: 42 additions & 0 deletions lib/internal/util/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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; }
Expand Down Expand Up @@ -63,7 +102,10 @@ function wrapConsole(consoleFromNode) {
}

module.exports = {
getInspectPort,
installConsoleExtensions,
isInspectorMessage,
isUsingInspector,
sendInspectorCommand,
wrapConsole,
};
3 changes: 0 additions & 3 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const {
spawnPromisified,
} = common;

const getPort = () => common.PORT;

export {
isMainThread,
isWindows,
Expand Down Expand Up @@ -100,4 +102,5 @@ export {
runWithInvalidFD,
createRequire,
spawnPromisified,
getPort,
};
38 changes: 38 additions & 0 deletions test/fixtures/test-runner/run_inspect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const common = require('../../common');
MoLow marked this conversation as resolved.
Show resolved Hide resolved
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 }) => {
MoLow marked this conversation as resolved.
Show resolved Hide resolved
assert.deepStrictEqual({ name: error.cause.name, code: error.cause.code }, expectedError);
}));
} else {
stream.once('test:fail', ({ error }) => { throw error; });
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}
18 changes: 18 additions & 0 deletions test/fixtures/test-runner/run_inspect_assert.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const assert = require('node:assert');
MoLow marked this conversation as resolved.
Show resolved Hide resolved

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();
MoLow marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 0 additions & 6 deletions test/parallel/test-runner-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
68 changes: 68 additions & 0 deletions test/parallel/test-runner-inspect.js
Original file line number Diff line number Diff line change
@@ -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);
}
Loading