Skip to content

Commit

Permalink
test_runner: remove parseCommandLine() from test.js
Browse files Browse the repository at this point in the history
The lib/internal/test_runner/test.js should not use the
parseCommandLine() function. This commit refactors the code to
avoid doing so.

PR-URL: #54353
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
cjihrig authored and RafaelGSS committed Aug 21, 2024
1 parent 961cbf0 commit 7766c1d
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 26 deletions.
47 changes: 22 additions & 25 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const {
createDeferredCallback,
countCompletedTest,
isTestFailureError,
parseCommandLine,
} = require('internal/test_runner/utils');
const {
createDeferredPromise,
Expand Down Expand Up @@ -79,14 +78,6 @@ const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
.add('uncaughtException').add('unhandledRejection');
const {
forceExit,
sourceMaps,
testNamePatterns,
testSkipPatterns,
only: testOnlyFlag,
updateSnapshots,
} = parseCommandLine();
let kResistStopPropagation;
let assertObj;
let findSourceMap;
Expand Down Expand Up @@ -132,7 +123,7 @@ function lazyAssertObject(harness) {
const { getOptionValue } = require('internal/options');
if (getOptionValue('--experimental-test-snapshots')) {
const { SnapshotManager } = require('internal/test_runner/snapshot');
harness.snapshotManager = new SnapshotManager(updateSnapshots);
harness.snapshotManager = new SnapshotManager(harness.config.updateSnapshots);
assertObj.set('snapshot', harness.snapshotManager.createAssert());
}
}
Expand Down Expand Up @@ -406,16 +397,17 @@ class Test extends AsyncResource {
this.filtered = false;

if (parent === null) {
this.root = this;
this.harness = options.harness;
this.config = this.harness.config;
this.concurrency = 1;
this.nesting = 0;
this.only = testOnlyFlag;
this.only = this.config.only;
this.reporter = new TestsStream();
this.runOnlySubtests = this.only;
this.childNumber = 0;
this.timeout = kDefaultTimeout;
this.entryFile = entryFile;
this.root = this;
this.harness = options.harness;
this.hooks = {
__proto__: null,
before: [],
Expand All @@ -428,6 +420,9 @@ class Test extends AsyncResource {
const nesting = parent.parent === null ? parent.nesting :
parent.nesting + 1;

this.root = parent.root;
this.harness = null;
this.config = this.root.harness.config;
this.concurrency = parent.concurrency;
this.nesting = nesting;
this.only = only ?? (parent.only && !parent.runOnlySubtests);
Expand All @@ -436,8 +431,6 @@ class Test extends AsyncResource {
this.childNumber = parent.subtests.length + 1;
this.timeout = parent.timeout;
this.entryFile = parent.entryFile;
this.root = parent.root;
this.harness = null;
this.hooks = {
__proto__: null,
before: [],
Expand All @@ -452,7 +445,7 @@ class Test extends AsyncResource {
this.parent.filteredSubtestCount++;
}

if (testOnlyFlag && only === false) {
if (this.config.only && only === false) {
fn = noop;
}
}
Expand Down Expand Up @@ -522,7 +515,7 @@ class Test extends AsyncResource {
this.waitingOn = 0;
this.finished = false;

if (!testOnlyFlag && (only || this.parent?.runOnlySubtests)) {
if (!this.config.only && (only || this.parent?.runOnlySubtests)) {
const warning =
"'only' and 'runOnly' require the --test-only command-line option.";
this.diagnostic(warning);
Expand All @@ -538,7 +531,7 @@ class Test extends AsyncResource {
file: loc[2],
};

if (sourceMaps === true) {
if (this.config.sourceMaps === true) {
const map = lazyFindSourceMap(this.loc.file);
const entry = map?.findEntry(this.loc.line - 1, this.loc.column - 1);

Expand All @@ -556,7 +549,9 @@ class Test extends AsyncResource {
}

willBeFiltered() {
if (testOnlyFlag && !this.only) return true;
if (this.config.only && !this.only) return true;

const { testNamePatterns, testSkipPatterns } = this.config;

if (testNamePatterns && !testMatchesPattern(this, testNamePatterns)) {
return true;
Expand Down Expand Up @@ -920,7 +915,7 @@ class Test extends AsyncResource {
// This helps catch any asynchronous activity that occurs after the tests
// have finished executing.
this.postRun();
} else if (forceExit) {
} else if (this.config.forceExit) {
// This is the root test, and all known tests and hooks have finished
// executing. If the user wants to force exit the process regardless of
// any remaining ref'ed handles, then do that now. It is theoretically
Expand Down Expand Up @@ -1163,11 +1158,13 @@ class Suite extends Test {
constructor(options) {
super(options);

if (testNamePatterns !== null && testSkipPatterns !== null && !options.skip) {
if (this.config.testNamePatterns !== null &&
this.config.testSkipPatterns !== null &&
!options.skip) {
this.fn = options.fn || this.fn;
this.skipped = false;
}
this.runOnlySubtests = testOnlyFlag;
this.runOnlySubtests = this.config.only;

try {
const { ctx, args } = this.getRunArgs();
Expand Down Expand Up @@ -1198,9 +1195,9 @@ class Suite extends Test {
this.filtered = false;
this.parent.filteredSubtestCount--;
} else if (
testOnlyFlag &&
testNamePatterns == null &&
testSkipPatterns == null &&
this.config.only &&
this.config.testNamePatterns == null &&
this.config.testSkipPatterns == null &&
this.filteredSubtestCount === this.subtests.length
) {
// If no subtests are marked as "only", run them all
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-runner-v8-deserializer.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ describe('v8 deserializer', () => {
let reported;
beforeEach(() => {
reported = [];
fileTest = new runner.FileTest({ name: 'filetest' });
fileTest = new runner.FileTest({
name: 'filetest',
harness: { config: {} },
});
fileTest.reporter.on('data', (data) => reported.push(data));
assert(fileTest.isClearToSend());
});
Expand Down

0 comments on commit 7766c1d

Please sign in to comment.