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

Add command-line test filtering to node:test #42984

Closed
connor4312 opened this issue May 6, 2022 · 6 comments
Closed

Add command-line test filtering to node:test #42984

connor4312 opened this issue May 6, 2022 · 6 comments
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@connor4312
Copy link
Contributor

connor4312 commented May 6, 2022

What is the problem this feature will solve?

It's interesting to see node:test in core, and I wanted to investigate building a VS Code extension for it using our testing API so that users could run it from their UI (e.g. vitest).

Currently, running tests can solely be provided by modifying code with only. This is quite problematic, since there are many UI gestures--in any test UI, not just VS Code--which rely on running specific tests. For example, clicking on a single test to debug, or more advanced actions like re-run failed tests. From my understanding of the docs, these cannot be implemented without requiring changes to code itself.

What is the feature you are proposing to solve the problem?

I request some way to narrow tests from the command line. The means of this are negotiable -- mocha has a "-g" option that allows grepping for full test names. However, this is based on having side-effect-free suites that allow tests to be enumerated and then grepped on, which node:test does not have.

Instead, perhaps an argument like --test-only-paths="<json>", where <json> is an array of test paths, such as:

[
  ["top level test", "subtest 1"],
  ["top level test", "subtest 2"]
]

Not very human-friendly, but easy for machines to deal with.

What alternatives have you considered?

We could alter the source code on disk to pass only parameters to test(). But this is pretty dangerous, if the transformation goes wrong or if the UI crashes before the transformation is rolled back. The closest we can get today is narrowing down to a single test file to run rather than the entire project, but this is still not a good experience.

Alternatively, we could use proxyquire or similar to wrap node:test and inject our own "only" running. This is quite complicated, however, and I don't think it would work with ES modules.

@connor4312 connor4312 added the feature request Issues that request new features to be added to Node.js. label May 6, 2022
@targos
Copy link
Member

targos commented May 6, 2022

@nodejs/test_runner

@targos targos moved this to Pending Triage in Node.js feature requests May 6, 2022
@targos targos added the test_runner Issues and PRs related to the test runner subsystem. label May 6, 2022
@ljharb
Copy link
Member

ljharb commented May 6, 2022

Are there any test runners that provide a facility like this?

I’d expect to be able to filter tests on the CLI with file globs, and perhaps with a way to grep against test suite names, but this functionality doesn’t seem to have precedent.

@connor4312
Copy link
Contributor Author

connor4312 commented May 6, 2022

Pretty much all test runners do! For example, just looking at the top 5 runners from the State of JS survey:

  • Mocha, as mentioned, has --grep, among other filtering options
  • Jest has --testNamePattern
  • Vitest has --testNamePattern as well
  • Playwright has -g as well as the ability to run tests declared on specific lines
  • ava has --match
  • nose, just to throw in a non-JavaScript test runner allows specifying tests in the form module:TestCase.test_method

All of these allow filtering to specific test cases to run. The grep and pattern options generally work on a full test or suite name formed from concatenating it with all of its parents. For example, top level test subtest 1.

As mentioned, a notable difference is that these all expect side-effect-free parent tests, so I'm not sure a grep-like option would work for node:test (though maybe I'm wrong.) That's why I suggested selecting by an explicit full path instead.

@ljharb
Copy link
Member

ljharb commented May 6, 2022

Test name patterns are a feature I’d expect; but “path to the test” and framing this with “only” seems a bit confusing.

I think the expectation of no side effects is a good one; a test that doesn’t do setup/teardown properly is broken.

@connor4312
Copy link
Contributor Author

connor4312 commented May 6, 2022

Test name patterns are a feature I’d expect; but “path to the test” and framing this with “only” seems a bit confusing.

I'm not sure how name patterns would work in the current Node API. Most of the tools mentioned have a differentiation between "suites" and "tests". "suites" are expected to be side-effect free when you call them. This allows tools that run with "grep" to evaluate all suites to enumerate each test, and then match and invoke the test functions for the relevant ones. However, node:test does not have this differentiation.

For example say I have some math tests and want to test addition, in Mocha I might write:

suite('math', () => {
  suite('hard', () => {
    test('factorization', () => {
      factorSomeGiantPrimeNumberThatsVerySlowAndTakesAMinute()
    })
  })

  suite('simple', () => {
    test('addition', () => {
      assert.strictEqual(2 + 2, 4)
    })
  })
})

I could call this by running mocha -g "addition". In this case, mocha will call the closure provided to the suites, enumerate the test cases, and then run only the addition test.

In node:test, this would instead be written as:

test('math', async (t) => {
  await t.test('hard', async (t) => {
    await t.test('factorization', () => {
      factorSomeGiantPrimeNumberThatsVerySlowAndTakesAMinute()
    })
  })
  await t.test('simple', async (t) => {
    await t.test('addition', () => {
      assert.strictEqual(2 + 2, 4)
    })
  })
})

Say there was now some hypothetical argument where I could run node --test-grep addition. How would that work? The runner does not differentiate between tests and suites, so it could not enumerate the tests without also running them, since any t.test might have nested subtests. From a user's point of view, if I want to only run my "addition" tests, I do not want my very slow "factorization" test to also run and cause me to wait a minute for results instead of milliseconds.

That's where my "path to tests" proposal originates -- if the runner knows the path to the tests, it can skip calling (i.e. just return Promise.resolve()) for tests which are outside of the requested path.

@koresar
Copy link

koresar commented May 17, 2022

The CLI filtering is the feature all IDEs use to implement "click to run single test" buttons.

Frankly, I never run unit tests from CLI myself. Only via my IDE.

Thus, this feature is the deal beaker for me to use the new node test runner.

cjihrig added a commit to cjihrig/node that referenced this issue Sep 3, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs#42984

TODO: Still needs tests and docs.
cjihrig added a commit to cjihrig/node that referenced this issue Oct 2, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs#42984
cjihrig added a commit to cjihrig/node that referenced this issue Oct 2, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs#42984
@cjihrig cjihrig closed this as completed in 87170c3 Oct 4, 2022
danielleadams pushed a commit that referenced this issue Oct 11, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: #42984
MoLow pushed a commit to MoLow/node that referenced this issue Nov 23, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs#42984
MoLow pushed a commit to MoLow/node that referenced this issue Dec 9, 2022
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs#42984
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs/node#42984
(cherry picked from commit 87170c3f9271da947a7b33d0696ec4cf8aab6eb6)
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs/node#42984
(cherry picked from commit 87170c3f9271da947a7b33d0696ec4cf8aab6eb6)
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs/node#42984
(cherry picked from commit 87170c3f9271da947a7b33d0696ec4cf8aab6eb6)
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
This commit adds support for running tests that match a
regular expression.

Fixes: nodejs/node#42984
(cherry picked from commit 87170c3f9271da947a7b33d0696ec4cf8aab6eb6)
@avivkeller avivkeller moved this from Awaiting Triage to Done in Node.js feature requests Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants