Skip to content

Commit

Permalink
test_runner: allow running test files in single process
Browse files Browse the repository at this point in the history
  • Loading branch information
MoLow committed Feb 27, 2024
1 parent f28ccd3 commit 3ee9d01
Show file tree
Hide file tree
Showing 16 changed files with 164 additions and 39 deletions.
13 changes: 13 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,18 @@ generated as part of the test runner output. If no tests are run, a coverage
report is not generated. See the documentation on
[collecting code coverage from tests][] for more details.

### `--experimental-test-isolation`

<!-- YAML
added:
- REPLACEME
-->

When running tests with the `node:test` module,
each test file is run in its own process.
Running `--experimental-test-isolation=none` will run all
files in the same process.

### `--experimental-vm-modules`

<!-- YAML
Expand Down Expand Up @@ -2505,6 +2517,7 @@ Node.js options that are allowed are:
* `--experimental-policy`
* `--experimental-shadow-realm`
* `--experimental-specifier-resolution`
* `--experimental-test-isolation`
* `--experimental-top-level-await`
* `--experimental-vm-modules`
* `--experimental-wasi-unstable-preview1`
Expand Down
3 changes: 3 additions & 0 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,9 @@ changes:
number. If a nullish value is provided, each process gets its own port,
incremented from the primary's `process.debugPort`.
**Default:** `undefined`.
* `isolation` {string} If `'process'`, each test file is run in
its own process. If `'none'`, all test files run in the same proccess.
**Default:** `'process'`.
* `only`: {boolean} If truthy, the test context will only run tests that
have the `only` option set
* `setup` {Function} A function that accepts the `TestsStream` instance
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
prepareMainThreadExecution(false);
markBootstrapComplete();


const isolation = getOptionValue('--experimental-test-isolation');
let concurrency = getOptionValue('--test-concurrency') || true;
let inspectPort;

Expand Down Expand Up @@ -65,6 +67,7 @@ const timeout = getOptionValue('--test-timeout') || Infinity;
const options = {
concurrency,
inspectPort,
isolation,
watch: getOptionValue('--watch'),
setup: setupTestReporters,
timeout,
Expand Down
22 changes: 22 additions & 0 deletions lib/internal/main/tests_entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
const { getOptionValue } = require('internal/options');
const { getCWDURL } = require('internal/util');
const { pathToFileURL } = require('internal/url');
const { initGlobalRoot } = require('internal/test_runner/harness');

prepareMainThreadExecution(false);
markBootstrapComplete();

const concurrency = getOptionValue('--test-concurrency') || true;
const parentURL = getCWDURL().href;
const files = process.env.NODE_TEST_FILES.split(',');

const { esmLoader } = require('internal/process/esm_loader');
initGlobalRoot({ __proto__: null, concurrency });
for (const path of files) {
esmLoader.import(pathToFileURL(path), parentURL, { __proto__: null });
}
11 changes: 6 additions & 5 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ function setup(root) {

let globalRoot;
let reportersSetup;
function getGlobalRoot() {
function initGlobalRoot(options = kEmptyObject) {
if (!globalRoot) {
globalRoot = createTestTree();
globalRoot = createTestTree(options);
globalRoot.reporter.on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
process.exitCode = kGenericUserError;
Expand All @@ -214,13 +214,13 @@ function getGlobalRoot() {

async function startSubtest(subtest) {
await reportersSetup;
getGlobalRoot().harness.bootstrapComplete = true;
subtest.root.harness.bootstrapComplete = true;
await subtest.start();
}

function runInParentContext(Factory) {
function run(name, options, fn, overrides) {
const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
const parent = testResources.get(executionAsyncId()) || initGlobalRoot();
const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
if (!(parent instanceof Suite)) {
return startSubtest(subtest);
Expand Down Expand Up @@ -252,7 +252,7 @@ function runInParentContext(Factory) {

function hook(hook) {
return (fn, options) => {
const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
const parent = testResources.get(executionAsyncId()) || initGlobalRoot();
parent.createHook(hook, fn, {
__proto__: null,
...options,
Expand All @@ -265,6 +265,7 @@ function hook(hook) {

module.exports = {
createTestTree,
initGlobalRoot,
test: runInParentContext(Test),
describe: runInParentContext(Suite),
it: runInParentContext(Test),
Expand Down
69 changes: 49 additions & 20 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const {
validateBoolean,
validateFunction,
validateObject,
validateOneOf,
validateInteger,
} = require('internal/validators');
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
Expand Down Expand Up @@ -80,8 +81,9 @@ const {
} = internalBinding('errors');

const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination', '--experimental-test-isolation'];
const kDiagnosticsFilterArgs = ['tests', 'suites', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
const kIsolatedProcessName = '<root>';

const kCanceledTests = new SafeSet()
.add(kCancelledByParent).add(kAborted).add(kTestTimeoutFailure);
Expand Down Expand Up @@ -112,7 +114,7 @@ function filterExecArgv(arg, i, arr) {
!ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
}

function getRunArgs(path, { inspectPort, testNamePatterns, only }) {
function getRunArgs(path, { inspectPort, testNamePatterns, only, isolation }) {
const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
if (isUsingInspector()) {
ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
Expand All @@ -123,7 +125,9 @@ function getRunArgs(path, { inspectPort, testNamePatterns, only }) {
if (only === true) {
ArrayPrototypePush(argv, '--test-only');
}
ArrayPrototypePush(argv, path);
if (isolation !== 'none') {
ArrayPrototypePush(argv, path);
}

return argv;
}
Expand All @@ -144,11 +148,12 @@ class FileTest extends Test {

constructor(options) {
super(options);
const file = this.name === kIsolatedProcessName ? resolve('') : resolve(this.name);
this.loc ??= {
__proto__: null,
line: 1,
column: 1,
file: resolve(this.name),
file,
};
}

Expand Down Expand Up @@ -330,6 +335,10 @@ function runTestFile(path, filesWatcher, opts) {
if (opts.root.harness.shouldColorizeTestFiles) {
env.FORCE_COLOR = '1';
}
if (opts.isolation === 'none') {
args.push('--experimental-test-isolation=none');
env.NODE_TEST_FILES = ArrayPrototypeJoin(opts.testFiles, ',');
}

const child = spawn(process.execPath, args, { __proto__: null, signal: t.signal, encoding: 'utf8', env, stdio });
if (watchMode) {
Expand Down Expand Up @@ -397,29 +406,40 @@ function runTestFile(path, filesWatcher, opts) {
return subtest.start();
}

function watchFiles(testFiles, opts) {
function watchFiles(opts) {
const runningProcesses = new SafeMap();
const runningSubtests = new SafeMap();
const watcher = new FilesWatcher({ __proto__: null, debounce: 200, mode: 'filter', signal: opts.signal });
const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests };

async function stopRunning(file) {
const runningProcess = runningProcesses.get(file);
if (runningProcess) {
runningProcess.kill();
await once(runningProcess, 'exit');
}
if (!runningSubtests.size) {
// Reset the topLevel counter
opts.root.harness.counters.topLevel = 0;
}
await runningSubtests.get(file);
runningSubtests.set(file, runTestFile(file, filesWatcher, opts));
}

watcher.on('changed', ({ owners }) => {
if (opts.isolation === 'none') {
PromisePrototypeThen(stopRunning(kIsolatedProcessName), undefined, (error) => {
triggerUncaughtException(error, true /* fromPromise */);
});
return;
}
watcher.unfilterFilesOwnedBy(owners);
PromisePrototypeThen(SafePromiseAllReturnVoid(testFiles, async (file) => {
PromisePrototypeThen(SafePromiseAllReturnVoid(opts.testFiles, async (file) => {
console.log(owners, opts.testFiles, opts.isolation);
if (!owners.has(file)) {
return;
}
const runningProcess = runningProcesses.get(file);
if (runningProcess) {
runningProcess.kill();
await once(runningProcess, 'exit');
}
if (!runningSubtests.size) {
// Reset the topLevel counter
opts.root.harness.counters.topLevel = 0;
}
await runningSubtests.get(file);
runningSubtests.set(file, runTestFile(file, filesWatcher, opts));
await stopRunning(file);
}, undefined, (error) => {
triggerUncaughtException(error, true /* fromPromise */);
}));
Expand All @@ -439,7 +459,7 @@ function watchFiles(testFiles, opts) {
function run(options = kEmptyObject) {
validateObject(options, 'options');

let { testNamePatterns, shard } = options;
let { testNamePatterns, shard, isolation } = options;
const { concurrency, timeout, signal, files, inspectPort, watch, setup, only } = options;

if (files != null) {
Expand Down Expand Up @@ -470,6 +490,10 @@ function run(options = kEmptyObject) {
if (setup != null) {
validateFunction(setup, 'options.setup');
}
isolation ||= 'process';
if (isolation != null) {
validateOneOf(isolation, 'options.isolation', ['process', 'none']);
}
if (testNamePatterns != null) {
if (!ArrayIsArray(testNamePatterns)) {
testNamePatterns = [testNamePatterns];
Expand Down Expand Up @@ -501,13 +525,18 @@ function run(options = kEmptyObject) {

let postRun = () => root.postRun();
let filesWatcher;
const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only };
const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only, isolation, testFiles };
if (watch) {
filesWatcher = watchFiles(testFiles, opts);
filesWatcher = watchFiles(opts);
postRun = undefined;
}
const runFiles = () => {
root.harness.bootstrapComplete = true;
if (isolation === 'none') {
const subtest = runTestFile(kIsolatedProcessName, filesWatcher, opts);
filesWatcher?.runningSubtests.set(kIsolatedProcessName, subtest);
return subtest;
}
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
const subtest = runTestFile(path, filesWatcher, opts);
filesWatcher?.runningSubtests.set(path, subtest);
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ const kHookFailure = 'hookFailed';
const kDefaultTimeout = null;
const noop = FunctionPrototype;
const kShouldAbort = Symbol('kShouldAbort');
const kFilename = process.argv?.[1];
const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
Expand Down Expand Up @@ -349,7 +348,7 @@ class Test extends AsyncResource {
this.diagnostic(warning);
}

if (loc === undefined || kFilename === undefined) {
if (loc === undefined) {
this.loc = undefined;
} else {
this.loc = {
Expand Down Expand Up @@ -826,11 +825,13 @@ class Test extends AsyncResource {
details.type = this.reportedType;
}

const isTopLevel = this.nesting === 0;
const testNumber = isTopLevel ? this.root.harness.counters.topLevel : this.testNumber;
if (this.passed) {
this.reporter.ok(this.nesting, this.loc, this.testNumber, this.name, details, directive);
this.reporter.ok(this.nesting, this.loc, testNumber, this.name, details, directive);
} else {
details.error = this.error;
this.reporter.fail(this.nesting, this.loc, this.testNumber, this.name, details, directive);
this.reporter.fail(this.nesting, this.loc, testNumber, this.name, details, directive);
}

for (let i = 0; i < this.diagnostics.length; i++) {
Expand Down
12 changes: 2 additions & 10 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ const {
const { AsyncResource } = require('async_hooks');
const { relative } = require('path');
const { createWriteStream } = require('fs');
const { pathToFileURL } = require('internal/url');
const { createDeferredPromise } = require('internal/util');
const { createDeferredPromise, getCWDURL } = require('internal/util');
const { getOptionValue } = require('internal/options');
const { green, yellow, red, white, shouldColorize } = require('internal/util/colors');

Expand Down Expand Up @@ -146,14 +145,7 @@ async function getReportersMap(reporters, destinations) {
let reporter = tryBuiltinReporter(name);

if (reporter === undefined) {
let parentURL;

try {
parentURL = pathToFileURL(process.cwd() + '/').href;
} catch {
parentURL = 'file:///';
}

const parentURL = getCWDURL().href;
const { esmLoader } = require('internal/process/esm_loader');
reporter = await esmLoader.import(name, parentURL, { __proto__: null });
}
Expand Down
5 changes: 5 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,11 @@ MaybeLocal<Value> StartExecution(Environment* env, StartExecutionCallback cb) {
return StartExecution(env, "internal/main/check_syntax");
}

if (env->options()->test_isolation == "none" &&
env->env_vars()->Get("NODE_TEST_FILES").IsJust()) {
return StartExecution(env, "internal/main/tests_entry");
}

if (env->options()->test_runner) {
return StartExecution(env, "internal/main/test_runner");
}
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"run test at specific shard",
&EnvironmentOptions::test_shard,
kAllowedInEnvvar);
AddOption("--experimental-test-isolation",
"isolation mode of test runner",
&EnvironmentOptions::test_isolation,
kAllowedInEnvvar);
AddOption("--test-udp-no-try-send", "", // For testing only.
&EnvironmentOptions::test_udp_no_try_send);
AddOption("--throw-deprecation",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class EnvironmentOptions : public Options {
bool test_only = false;
bool test_udp_no_try_send = false;
std::string test_shard;
std::string test_isolation;
bool throw_deprecation = false;
bool trace_atomics_wait = false;
bool trace_deprecation = false;
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/no-isolation/a.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('a', (t, cb) => { globalThis.aCb = cb; });
7 changes: 7 additions & 0 deletions test/fixtures/test-runner/no-isolation/b.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';
const test = require('node:test');

test('b', async () => {
globalThis.aCb();
delete globalThis.aCb;
});
Loading

0 comments on commit 3ee9d01

Please sign in to comment.