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 Jan 27, 2024
1 parent 09da597 commit 3c85f9a
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 17 deletions.
5 changes: 4 additions & 1 deletion lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ 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;

if (isUsingInspector()) {
if (isUsingInspector() && isolation !== 'none') {
process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +
'Use the inspectPort option to run with concurrency');
concurrency = 1;
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
2 changes: 1 addition & 1 deletion lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function getGlobalRoot() {

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

Expand Down
60 changes: 54 additions & 6 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ const {

const { spawn } = require('child_process');
const { finished } = require('internal/streams/end-of-stream');
const { resolve } = require('path');
const { resolve, isAbsolute } = require('path');
const { pathToFileURL } = require('internal/url');
const { DefaultDeserializer, DefaultSerializer } = require('v8');
// TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern.
const { createInterface } = require('readline');
Expand All @@ -50,6 +51,7 @@ const {
validateBoolean,
validateFunction,
validateObject,
validateOneOf,
validateInteger,
} = require('internal/validators');
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
Expand All @@ -64,6 +66,7 @@ const {
kTestCodeFailure,
kTestTimeoutFailure,
Test,
Suite,
} = require('internal/test_runner/test');

const {
Expand Down Expand Up @@ -134,7 +137,34 @@ const v8Header = serializer.releaseBuffer();
const kV8HeaderLength = TypedArrayPrototypeGetLength(v8Header);
const kSerializedSizeHeader = 4 + kV8HeaderLength;

class FileTest extends Test {

class InProcessFileTest extends Suite {
constructor(options) {
super(options);
this.loc ??= {
__proto__: null,
line: 1,
column: 1,
file: resolve(this.name),
};
this.nesting = -1;
this.reportedType = 'test';
}

#reported = false;
reportStarted() {}
report() {
const skipReporting = this.subtests.length > 0;
if (!skipReporting && !this.#reported) {
this.nesting = 0;
this.#reported = true;
super.reportStarted();
super.report();
}
}
}

class SpawnFileTest extends Test {
// This class maintains two buffers:
#reportBuffer = []; // Parsed items waiting for this.isClearToSend()
#rawBuffer = []; // Raw data waiting to be parsed
Expand Down Expand Up @@ -319,7 +349,21 @@ class FileTest extends Test {

function runTestFile(path, filesWatcher, opts) {
const watchMode = filesWatcher != null;
const subtest = opts.root.createSubtest(FileTest, path, { __proto__: null, signal: opts.signal }, async (t) => {
const Factory = opts.isolation === 'none' ? InProcessFileTest : SpawnFileTest;
const subtest = opts.root.createSubtest(Factory, path, { __proto__: null, signal: opts.signal }, async (t) => {
if (opts.isolation === 'none') {
let parentURL;
try {
parentURL = pathToFileURL(process.cwd() + '/').href;
} catch {
parentURL = 'file:///';
}

const { esmLoader } = require('internal/process/esm_loader');

await esmLoader.import(isAbsolute(path) ? path : `./${path}`, parentURL, { __proto__: null });
return;
}
const args = getRunArgs(path, opts);
const stdio = ['pipe', 'pipe', 'pipe'];
const env = { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' };
Expand Down Expand Up @@ -439,7 +483,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 +514,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,7 +549,7 @@ 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 };
if (watch) {
filesWatcher = watchFiles(testFiles, opts);
postRun = undefined;
Expand All @@ -521,6 +569,6 @@ function run(options = kEmptyObject) {
}

module.exports = {
FileTest, // Exported for tests only
SpawnFileTest, // Exported for tests only
run,
};
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
3 changes: 2 additions & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ inline bool Environment::owns_inspector() const {

inline bool Environment::should_create_inspector() const {
return (flags_ & EnvironmentFlags::kNoCreateInspector) == 0 &&
!options_->test_runner && !options_->watch_mode;
(!options_->test_runner || options_->test_isolation == "none") &&
!options_->watch_mode;
}

inline bool Environment::tracks_unmanaged_fds() const {
Expand Down
11 changes: 8 additions & 3 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,10 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
"--watch-path cannot be used in combination with --test");
}

#ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER
debug_options_.allow_attaching_debugger = false;
#endif
if (watch_mode && test_isolation == "none") {
errors->push_back(
"--watch cannot be used with --experimental-test-isolation=none");
}
}

if (watch_mode) {
Expand Down Expand Up @@ -641,6 +642,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
2 changes: 1 addition & 1 deletion test/parallel/test-runner-v8-deserializer.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('v8 deserializer', () => {
let reported;
beforeEach(() => {
reported = [];
fileTest = new runner.FileTest({ name: 'filetest' });
fileTest = new runner.SpawnFileTest({ name: 'filetest' });
fileTest.reporter.on('data', (data) => reported.push(data));
assert(fileTest.isClearToSend());
});
Expand Down

0 comments on commit 3c85f9a

Please sign in to comment.