Skip to content

Commit

Permalink
test_runner: propagate only to test ancestors
Browse files Browse the repository at this point in the history
  • Loading branch information
MoLow committed Aug 3, 2023
1 parent d61c000 commit bb3d171
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 120 deletions.
41 changes: 28 additions & 13 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,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 @@ -241,9 +239,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 @@ -288,10 +284,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 @@ -330,12 +322,26 @@ class Test extends AsyncResource {
this.subtests = [];
this.waitingOn = 0;
this.finished = false;
this.only = testOnlyFlag ? only : undefined;
this.runOnlySubtests = false;

if (!testOnlyFlag && (only || this.runOnlySubtests)) {
const warning =
"'only' and 'runOnly' require the --test-only command-line option.";

if (!testOnlyFlag && only && !parent.runOnlySubtests) {
const warning = "'only' requires the --test-only command-line option.";
this.diagnostic(warning);
}

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

markOnly() {
if (this.runOnlySubtests) {
return;
}
this.runOnlySubtests = true;
this.parent?.markOnly();
}

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

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

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 @@ -843,7 +858,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 @@ -864,7 +878,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 @@ -874,6 +888,7 @@ class Suite extends Test {

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

let stopPromise;
try {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ function parseCommandLine() {
testOnlyFlag = false;
testNamePatterns = null;
} else {
testOnlyFlag = getOptionValue('--test-only') || (!isChildProcess && !isChildProcessV8);
const testNamePatternFlag = getOptionValue('--test-name-pattern');
testOnlyFlag = getOptionValue('--test-only');
testNamePatterns = testNamePatternFlag?.length > 0 ?
ArrayPrototypeMap(
testNamePatternFlag,
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
15 changes: 6 additions & 9 deletions test/fixtures/test-runner/output/output.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -501,35 +501,32 @@ ok 52 - callback async throw after done
---
duration_ms: *
...
# Subtest: only is set but not in only mode
# Subtest: runOnly is set
# Subtest: running subtest 1
ok 1 - running subtest 1
---
duration_ms: *
...
# Subtest: running subtest 2
ok 2 - running subtest 2
# Subtest: skipped subtest 2
ok 2 - skipped subtest 2 # SKIP 'only' option not set
---
duration_ms: *
...
# 'only' and 'runOnly' require the --test-only command-line option.
# Subtest: running subtest 3
ok 3 - running subtest 3
---
duration_ms: *
...
# 'only' and 'runOnly' require the --test-only command-line option.
# Subtest: running subtest 4
ok 4 - running subtest 4
---
duration_ms: *
...
1..4
ok 53 - only is set but not in only mode
ok 53 - runOnly is set
---
duration_ms: *
...
# 'only' and 'runOnly' require the --test-only command-line option.
# Subtest: custom inspect symbol fail
not ok 54 - custom inspect symbol fail
---
Expand Down Expand Up @@ -723,9 +720,9 @@ not ok 66 - invalid subtest fail
# Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
# tests 80
# suites 0
# pass 37
# pass 36
# fail 25
# cancelled 3
# skipped 10
# skipped 11
# todo 5
# duration_ms *
Loading

0 comments on commit bb3d171

Please sign in to comment.