Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: add describe.only and it.only shorthands #46604

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,15 @@ Shorthand for skipping a suite, same as [`describe([name], { skip: true }[, fn])
Shorthand for marking a suite as `TODO`, same as
[`describe([name], { todo: true }[, fn])`][describe options].

## `describe.only([name][, options][, fn])`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do this, we should also support test.only

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test does not currently support test.skip() or test.todo(), and I would prefer to keep it that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is definitely value in having shorthands for the test API - especially skip and only. However, for the scope of this commit, it's only about adding the extra shorthand to the describe/it API. This keeps the options consistent.

@cjihrig are there any issues in particular you see with adding a shorthand API for the test interface in a future PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no technical reason - just a desire to keep the API surface area smaller. The Test class in particular is important because everything else builds on top of it. And, it already supports skip, todo, and only functionality, so I would prefer to not implement another way of doing the same thing.


richiemccoll marked this conversation as resolved.
Show resolved Hide resolved
<!-- YAML
added: REPLACEME
-->

Shorthand for marking a suite as `only`, same as
[`describe([name], { only: true }[, fn])`][describe options].

## `it([name][, options][, fn])`

* `name` {string} The name of the test, which is displayed when reporting test
Expand All @@ -832,6 +841,15 @@ same as [`it([name], { skip: true }[, fn])`][it options].
Shorthand for marking a test as `TODO`,
same as [`it([name], { todo: true }[, fn])`][it options].

## `it.only([name][, options][, fn])`

richiemccoll marked this conversation as resolved.
Show resolved Hide resolved
<!-- YAML
added: REPLACEME
-->

Shorthand for marking a test as `only`,
same as [`it([name], { only: true }[, fn])`][it options].

## `before([fn][, options])`

<!-- YAML
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 @@ -191,7 +191,7 @@ function runInParentContext(Factory) {
run(name, options, fn);
};

ArrayPrototypeForEach(['skip', 'todo'], (keyword) => {
ArrayPrototypeForEach(['skip', 'todo', 'only'], (keyword) => {
cb[keyword] = (name, options, fn) => {
run(name, options, fn, { [keyword]: true });
};
Expand Down
36 changes: 35 additions & 1 deletion test/message/test_runner_only_tests.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Flags: --no-warnings --test-only
'use strict';
require('../common');
const test = require('node:test');
const { test, describe, it } = require('node:test');

// These tests should be skipped based on the 'only' option.
test('only = undefined');
Expand Down Expand Up @@ -46,3 +46,37 @@ test('only = true, with subtests', { only: true }, async (t) => {
await t.test('skipped subtest 3', { only: false });
await t.test('skipped subtest 4', { skip: true });
});

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

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

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

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

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

it('`it` subtest 2 only=false', { only: false }, () => {
throw new Error('This should not run');
});
richiemccoll marked this conversation as resolved.
Show resolved Hide resolved

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

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

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

test('async subtest should run', async () => {});

test('subtest should be skipped', { only: false }, () => {});
});
79 changes: 76 additions & 3 deletions test/message/test_runner_only_tests.out
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,82 @@ ok 11 - only = true, with subtests
---
duration_ms: *
...
1..11
# tests 11
# pass 1
# Subtest: describe only = true, with subtests
# Subtest: `it` subtest 1 should run
ok 1 - `it` subtest 1 should run
---
duration_ms: *
...
# Subtest: `it` subtest 2 should run
ok 2 - `it` subtest 2 should run
---
duration_ms: *
...
1..2
ok 12 - describe only = true, with subtests
---
duration_ms: *
...
# Subtest: describe only = true, with a mixture of subtests
# Subtest: `it` subtest 1
ok 1 - `it` subtest 1
---
duration_ms: *
...
# Subtest: `it` async subtest 1
ok 2 - `it` async subtest 1
---
duration_ms: *
...
# Subtest: `it` subtest 2 only=true
ok 3 - `it` subtest 2 only=true
---
duration_ms: *
...
# Subtest: `it` subtest 2 only=false
ok 4 - `it` subtest 2 only=false # SKIP 'only' option not set
---
duration_ms: *
...
# Subtest: `it` subtest 3 skip
ok 5 - `it` subtest 3 skip # SKIP
---
duration_ms: *
...
# Subtest: `it` subtest 4 todo
ok 6 - `it` subtest 4 todo # SKIP 'only' option not set
---
duration_ms: *
...
1..6
ok 13 - describe only = true, with a mixture of subtests
---
duration_ms: *
...
# Subtest: describe only = true, with subtests
# Subtest: subtest should run
ok 1 - subtest should run
---
duration_ms: *
...
# Subtest: async subtest should run
ok 2 - async subtest should run
---
duration_ms: *
...
# Subtest: subtest should be skipped
ok 3 - subtest should be skipped # SKIP 'only' option not set
---
duration_ms: *
...
1..3
ok 14 - describe only = true, with subtests
---
duration_ms: *
...
1..14
# tests 14
# pass 4
# fail 0
# cancelled 0
# skipped 10
Expand Down