Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
test_runner: support using --inspect with --test
PR-URL: #44520
Backport-PR-URL: #44813
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
MoLow authored and danielleadams committed Oct 4, 2022
1 parent 36227ed commit 3c1e9d4
Show file tree
Hide file tree
Showing 13 changed files with 391 additions and 23 deletions.
5 changes: 5 additions & 0 deletions doc/api/test.md
Expand Up @@ -338,6 +338,11 @@ added: v18.9.0
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
Expand Down
1 change: 1 addition & 0 deletions lib/internal/cluster/primary.js
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/inspector
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
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
59 changes: 49 additions & 10 deletions lib/internal/test_runner/runner.js
@@ -1,18 +1,22 @@
'use strict';
const {
ArrayFrom,
ArrayPrototypeConcat,
ArrayPrototypeFilter,
ArrayPrototypeIncludes,
ArrayPrototypeJoin,
ArrayPrototypePop,
ArrayPrototypePush,
ArrayPrototypeSlice,
ArrayPrototypeSort,
ObjectAssign,
PromisePrototypeThen,
RegExpPrototypeSymbolSplit,
SafePromiseAll,
SafeSet,
StringPrototypeEndsWith,
} = primordials;

const { Buffer } = require('buffer');
const { spawn } = require('child_process');
const { readdirSync, statSync } = require('fs');
const console = require('internal/console/global');
Expand All @@ -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');
Expand Down Expand Up @@ -100,25 +105,59 @@ function filterExecArgv(arg) {
return !ArrayPrototypeIncludes(kFilterArgs, arg);
}

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

function makeStderrCallback(callback) {
if (!isUsingInspector()) {
return callback;
}
let buffer = Buffer.alloc(0);
return (data) => {
callback(data);
const newData = Buffer.concat([buffer, data]);
const str = newData.toString('utf8');
let lines = str;
if (StringPrototypeEndsWith(lines, '\n')) {
buffer = Buffer.alloc(0);
} else {
lines = RegExpPrototypeSymbolSplit(/\r?\n/, str);
buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8');
lines = ArrayPrototypeJoin(lines, '\n');
}
if (isInspectorMessage(lines)) {
process.stderr.write(lines);
}
};
}

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

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

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

const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([
child.stderr.on('data', makeStderrCallback((data) => {
stderr += data;
}));

const { 0: { 0: code, 1: signal }, 1: stdout } = await SafePromiseAll([
once(child, 'exit', { signal: t.signal }),
child.stdout.toArray({ signal: t.signal }),
child.stderr.toArray({ signal: t.signal }),
]);

if (code !== 0 || signal !== null) {
Expand All @@ -128,7 +167,7 @@ function runTestFile(path, root) {
exitCode: code,
signal: signal,
stdout: ArrayPrototypeJoin(stdout, ''),
stderr: ArrayPrototypeJoin(stderr, ''),
stderr,
// The stack will not be useful since the failures came from tests
// in a child process.
stack: undefined,
Expand All @@ -145,7 +184,7 @@ function run(options) {
if (options === null || typeof options !== 'object') {
options = kEmptyObject;
}
const { concurrency, timeout, signal, files } = options;
const { concurrency, timeout, signal, files, inspectPort } = options;

if (files != null) {
validateArray(files, 'options.files');
Expand All @@ -154,7 +193,7 @@ function run(options) {
const root = createTestTree({ concurrency, timeout, signal });
const testFiles = files ?? createTestFileList();

PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root)),
PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)),
() => root.postRun());

return root.reporter;
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/test_runner/test.js
Expand Up @@ -58,8 +58,6 @@ const kDefaultTimeout = null;
const noop = FunctionPrototype;
const isTestRunner = getOptionValue('--test');
const testOnlyFlag = !isTestRunner && getOptionValue('--test-only');
// TODO(cjihrig): Use uv_available_parallelism() once it lands.
const rootConcurrency = isTestRunner ? MathMax(cpus().length - 1, 1) : 1;
const kShouldAbort = Symbol('kShouldAbort');
const kRunHook = Symbol('kRunHook');
const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
Expand Down Expand Up @@ -150,7 +148,7 @@ class Test extends AsyncResource {
}

if (parent === null) {
this.concurrency = rootConcurrency;
this.concurrency = 1;
this.indent = '';
this.indentString = kDefaultIndent;
this.only = testOnlyFlag;
Expand Down Expand Up @@ -180,6 +178,7 @@ class Test extends AsyncResource {

case 'boolean':
if (concurrency) {
// TODO(cjihrig): Use uv_available_parallelism() once it lands.
this.concurrency = parent === null ? MathMax(cpus().length - 1, 1) : Infinity;
} else {
this.concurrency = 1;
Expand Down
42 changes: 42 additions & 0 deletions lib/internal/util/inspector.js
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
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
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,
};
40 changes: 40 additions & 0 deletions test/fixtures/test-runner/run_inspect.js
@@ -0,0 +1,40 @@
'use strict';

const common = require('../../common');
const fixtures = require('../../common/fixtures');
const { run } = require('node:test');
const assert = require('node:assert');

const badPortError = { name: 'RangeError', code: 'ERR_SOCKET_BAD_PORT' };
let inspectPort = 'inspectPort' in process.env ? Number(process.env.inspectPort) : undefined;
let expectedError;

if (process.env.inspectPort === 'addTwo') {
inspectPort = common.mustCall(() => { return process.debugPort += 2; });
} else if (process.env.inspectPort === 'string') {
inspectPort = 'string';
expectedError = badPortError;
} else if (process.env.inspectPort === 'null') {
inspectPort = null;
} else if (process.env.inspectPort === 'bignumber') {
inspectPort = 1293812;
expectedError = badPortError;
} else if (process.env.inspectPort === 'negativenumber') {
inspectPort = -9776;
expectedError = badPortError;
} else if (process.env.inspectPort === 'bignumberfunc') {
inspectPort = common.mustCall(() => 123121);
expectedError = badPortError;
} else if (process.env.inspectPort === 'strfunc') {
inspectPort = common.mustCall(() => 'invalidPort');
expectedError = badPortError;
}

const stream = run({ files: [fixtures.path('test-runner/run_inspect_assert.js')], inspectPort });
if (expectedError) {
stream.on('test:fail', common.mustCall(({ error }) => {
assert.deepStrictEqual({ name: error.cause.name, code: error.cause.code }, expectedError);
}));
} else {
stream.on('test:fail', common.mustNotCall());
}
19 changes: 19 additions & 0 deletions test/fixtures/test-runner/run_inspect_assert.js
@@ -0,0 +1,19 @@
'use strict';

const assert = require('node:assert');

const { expectedPort, expectedInitialPort, expectedHost } = process.env;
const debugOptions =
require('internal/options').getOptionValue('--inspect-port');

if ('expectedPort' in process.env) {
assert.strictEqual(process.debugPort, +expectedPort);
}

if ('expectedInitialPort' in process.env) {
assert.strictEqual(debugOptions.port, +expectedInitialPort);
}

if ('expectedHost' in process.env) {
assert.strictEqual(debugOptions.host, expectedHost);
}
6 changes: 0 additions & 6 deletions test/parallel/test-runner-cli.js
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

0 comments on commit 3c1e9d4

Please sign in to comment.