Skip to content

Commit

Permalink
test_runner: propogate only to work without --test-only
Browse files Browse the repository at this point in the history
  • Loading branch information
MoLow committed Jul 26, 2023
1 parent f842b8b commit b03a866
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 124 deletions.
39 changes: 24 additions & 15 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
.add('uncaughtException').add('unhandledRejection');
const { testNamePatterns, testOnlyFlag } = parseCommandLine();
const { testNamePatterns } = parseCommandLine();
let kResistStopPropagation;

function stopTest(timeout, signal) {
Expand Down Expand Up @@ -193,9 +193,7 @@ class Test extends AsyncResource {
if (parent === null) {
this.concurrency = 1;
this.nesting = 0;
this.only = testOnlyFlag;
this.reporter = new TestsStream();
this.runOnlySubtests = this.only;
this.testNumber = 0;
this.timeout = kDefaultTimeout;
this.root = this;
Expand All @@ -212,9 +210,7 @@ class Test extends AsyncResource {

this.concurrency = parent.concurrency;
this.nesting = nesting;
this.only = only ?? !parent.runOnlySubtests;
this.reporter = parent.reporter;
this.runOnlySubtests = !this.only;
this.testNumber = parent.subtests.length + 1;
this.timeout = parent.timeout;
this.root = parent.root;
Expand Down Expand Up @@ -259,10 +255,6 @@ class Test extends AsyncResource {
skip = 'test name does not match pattern';
}

if (testOnlyFlag && !this.only) {
skip = '\'only\' option not set';
}

if (skip) {
fn = noop;
}
Expand Down Expand Up @@ -301,12 +293,20 @@ class Test extends AsyncResource {
this.subtests = [];
this.waitingOn = 0;
this.finished = false;
this.only = only;
this.runOnlySubtests = false;

if (only && parent !== null) {
parent.markOnly();
}
}

if (!testOnlyFlag && (only || this.runOnlySubtests)) {
const warning =
"'only' and 'runOnly' require the --test-only command-line option.";
this.diagnostic(warning);
markOnly() {
if (this.runOnlySubtests) {
return;
}
this.runOnlySubtests = true;
this.parent?.markOnly();
}

matchesTestNamePatterns() {
Expand Down Expand Up @@ -539,9 +539,18 @@ class Test extends AsyncResource {
}
}

get runOnlySibling() {
return this.parent?.runOnlySubtests && !this.only && !this.hasOnlySubtests;
}

async run() {
this.startTime = hrtime();

if (this.runOnlySibling || this.only === false) {
this.fn = noop;
this.skip('\'only\' option not set');
}

if (this[kShouldAbort]()) {
this.postRun();
return;
Expand Down Expand Up @@ -794,7 +803,6 @@ class Suite extends Test {
this.fn = options.fn || this.fn;
this.skipped = false;
}
this.runOnlySubtests = testOnlyFlag;

try {
const { ctx, args } = this.getRunArgs();
Expand All @@ -815,7 +823,7 @@ class Suite extends Test {
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
this.buildPhaseFinished = this.parent !== null;
}
this.fn = () => {};
this.fn = noop;
}

getRunArgs() {
Expand All @@ -825,6 +833,7 @@ class Suite extends Test {

async run() {
const hookArgs = this.getRunArgs();
this.runOnlySubtests ||= this.runOnlySibling;

try {
await this.buildSuite;
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const {
const { relative } = require('path');
const { createWriteStream } = require('fs');
const { pathToFileURL } = require('internal/url');
const { createDeferredPromise } = require('internal/util');
const { createDeferredPromise, deprecate } = require('internal/util');
const { getOptionValue } = require('internal/options');
const { green, yellow, red, white, shouldColorize } = require('internal/util/colors');

Expand Down Expand Up @@ -185,7 +185,6 @@ function parseCommandLine() {
let destinations;
let reporters;
let testNamePatterns;
let testOnlyFlag;

if (isChildProcessV8) {
kBuiltinReporters.set('v8-serializer', 'internal/test_runner/reporter/v8-serializer');
Expand Down Expand Up @@ -214,12 +213,14 @@ function parseCommandLine() {
}
}

if (getOptionValue('--test-only') && !isChildProcess && !isChildProcessV8) {
deprecate(() => {}, 'The --test-only flag is deprecated.', '111')();
}

if (isTestRunner) {
testOnlyFlag = false;
testNamePatterns = null;
} else {
const testNamePatternFlag = getOptionValue('--test-name-pattern');
testOnlyFlag = getOptionValue('--test-only');
testNamePatterns = testNamePatternFlag?.length > 0 ?
ArrayPrototypeMap(
testNamePatternFlag,
Expand All @@ -231,7 +232,6 @@ function parseCommandLine() {
__proto__: null,
isTestRunner,
coverage,
testOnlyFlag,
testNamePatterns,
reporters,
destinations,
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/test-runner/output/name_pattern_with_only.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --no-warnings --test-only --test-name-pattern=enabled
// Flags: --no-warnings --test-name-pattern=enabled
'use strict';
const common = require('../../../common');
const { test } = require('node:test');
Expand Down
120 changes: 58 additions & 62 deletions test/fixtures/test-runner/output/only_tests.js
Original file line number Diff line number Diff line change
@@ -1,100 +1,96 @@
// Flags: --no-warnings --test-only
// Flags: --no-warnings
'use strict';
require('../../../common');
const common = require('../../../common');
const { test, describe, it } = require('node:test');

// These tests should be skipped based on the 'only' option.
test('only = undefined');
test('only = undefined, skip = string', { skip: 'skip message' });
test('only = undefined, skip = true', { skip: true });
test('only = undefined, skip = false', { skip: false });
test('only = false', { only: false });
test('only = false, skip = string', { only: false, skip: 'skip message' });
test('only = false, skip = true', { only: false, skip: true });
test('only = false, skip = false', { only: false, skip: false });
test('only = undefined', common.mustNotCall());
test('only = undefined, skip = string', { skip: 'skip message' }, common.mustNotCall());
test('only = undefined, skip = true', { skip: true }, common.mustNotCall());
test('only = undefined, skip = false', { skip: false }, common.mustNotCall());
test('only = false', { only: false }, common.mustNotCall());
test('only = false, skip = string', { only: false, skip: 'skip message' }, common.mustNotCall());
test('only = false, skip = true', { only: false, skip: true }, common.mustNotCall());
test('only = false, skip = false', { only: false, skip: false }, common.mustNotCall());

// These tests should be skipped based on the 'skip' option.
test('only = true, skip = string', { only: true, skip: 'skip message' });
test('only = true, skip = true', { only: true, skip: true });
test('only = true, skip = string', { only: true, skip: 'skip message' }, common.mustNotCall());
test('only = true, skip = true', { only: true, skip: true }, common.mustNotCall());

// An 'only' test with subtests.
test('only = true, with subtests', { only: true }, async (t) => {
test('only = true, with subtests', { only: true }, common.mustCall(async (t) => {
// These subtests should run.
await t.test('running subtest 1');
await t.test('running subtest 2');
await t.test('running subtest 1', common.mustCall());
await t.test('running subtest 2', common.mustCall());

// Switch the context to only execute 'only' tests.
t.runOnly(true);
await t.test('skipped subtest 1');
await t.test('skipped subtest 2');
await t.test('running subtest 3', { only: true });
await t.test('skipped subtest 1', common.mustNotCall());
await t.test('skipped subtest 2'), common.mustNotCall();
await t.test('running subtest 3', { only: true }, common.mustCall());

// Switch the context back to execute all tests.
t.runOnly(false);
await t.test('running subtest 4', async (t) => {
await t.test('running subtest 4', common.mustCall(async (t) => {
// These subtests should run.
await t.test('running sub-subtest 1');
await t.test('running sub-subtest 2');
await t.test('running sub-subtest 1', common.mustCall());
await t.test('running sub-subtest 2', common.mustCall());

// Switch the context to only execute 'only' tests.
t.runOnly(true);
await t.test('skipped sub-subtest 1');
await t.test('skipped sub-subtest 2');
});
await t.test('skipped sub-subtest 1', common.mustNotCall());
await t.test('skipped sub-subtest 2', common.mustNotCall());
}));

// Explicitly do not run these tests.
await t.test('skipped subtest 3', { only: false });
await t.test('skipped subtest 4', { skip: true });
});
await t.test('skipped subtest 3', { only: false }, common.mustNotCall());
await t.test('skipped subtest 4', { skip: true }, common.mustNotCall());
}));

describe.only('describe only = true, with subtests', () => {
it.only('`it` subtest 1 should run', () => {});
describe.only('describe only = true, with subtests', common.mustCall(() => {
it.only('`it` subtest 1 should run', common.mustCall());

it('`it` subtest 2 should not run', async () => {});
});
it('`it` subtest 2 should not run', common.mustNotCall());
}));

describe.only('describe only = true, with a mixture of subtests', () => {
it.only('`it` subtest 1', () => {});
describe.only('describe only = true, with a mixture of subtests', common.mustCall(() => {
it.only('`it` subtest 1', common.mustCall());

it.only('`it` async subtest 1', async () => {});
it.only('`it` async subtest 1', common.mustCall(async () => {}));

it('`it` subtest 2 only=true', { only: true });
it('`it` subtest 2 only=true', { only: true }, common.mustCall());

it('`it` subtest 2 only=false', { only: false }, () => {
throw new Error('This should not run');
});
it('`it` subtest 2 only=false', { only: false }, common.mustNotCall());

it.skip('`it` subtest 3 skip', () => {
throw new Error('This should not run');
});
it.skip('`it` subtest 3 skip', common.mustNotCall());

it.todo('`it` subtest 4 todo', { only: false }, () => {
throw new Error('This should not run');
});
it.todo('`it` subtest 4 todo', { only: false }, common.mustNotCall());

test.only('`test` subtest 1', () => {});
test.only('`test` subtest 1', common.mustCall());

test.only('`test` async subtest 1', async () => {});
test.only('`test` async subtest 1', common.mustCall(async () => {}));

test('`test` subtest 2 only=true', { only: true });
test('`test` subtest 2 only=true', { only: true }, common.mustCall());

test('`test` subtest 2 only=false', { only: false }, () => {
throw new Error('This should not run');
});
test('`test` subtest 2 only=false', { only: false }, common.mustNotCall());

test.skip('`test` subtest 3 skip', () => {
throw new Error('This should not run');
});
test.skip('`test` subtest 3 skip', common.mustNotCall());

test.todo('`test` subtest 4 todo', { only: false }, () => {
throw new Error('This should not run');
});
});
test.todo('`test` subtest 4 todo', { only: false }, common.mustNotCall());
}));

describe.only('describe only = true, with subtests', () => {
test.only('subtest should run', () => {});
describe.only('describe only = true, with subtests', common.mustCall(() => {
test.only('subtest should run', common.mustCall());

test('async subtest should not run', async () => {});
test('async subtest should not run', common.mustNotCall());

test('subtest should be skipped', { only: false }, () => {});
});
test('subtest should be skipped', { only: false }, common.mustNotCall());
}));

describe('describe only = undefined, with subtests', common.mustCall(() => {
test('async subtest should not run', common.mustNotCall());
}));

describe('describe only = false, with subtests', { only: false }, common.mustCall(() => {
test('async subtest should not run', common.mustNotCall());
}));
32 changes: 28 additions & 4 deletions test/fixtures/test-runner/output/only_tests.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,36 @@ ok 14 - describe only = true, with subtests
duration_ms: *
type: 'suite'
...
1..14
# tests 40
# suites 3
# Subtest: describe only = undefined, with subtests
# Subtest: async subtest should not run
ok 1 - async subtest should not run # SKIP 'only' option not set
---
duration_ms: *
...
1..1
ok 15 - describe only = undefined, with subtests
---
duration_ms: *
type: 'suite'
...
# Subtest: describe only = false, with subtests
# Subtest: async subtest should not run
ok 1 - async subtest should not run # SKIP 'only' option not set
---
duration_ms: *
...
1..1
ok 16 - describe only = false, with subtests
---
duration_ms: *
type: 'suite'
...
1..16
# tests 42
# suites 5
# pass 15
# fail 0
# cancelled 0
# skipped 25
# skipped 27
# todo 0
# duration_ms *
6 changes: 3 additions & 3 deletions test/fixtures/test-runner/output/output.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ test('callback async throw after done', (t, done) => {
done();
});

test('only is set but not in only mode', { only: true }, async (t) => {
// All of these subtests should run.
test('runOnly is set', async (t) => {
// Subtests should run only outside of a runOnly block, unless they have only: true.
await t.test('running subtest 1');
t.runOnly(true);
await t.test('running subtest 2');
await t.test('skipped subtest 2');
await t.test('running subtest 3', { only: true });
t.runOnly(false);
await t.test('running subtest 4');
Expand Down
Loading

0 comments on commit b03a866

Please sign in to comment.