test_runner: improve --test-name-pattern to allow matching single test

[mdrobny]

Try to match a test by name prefixed with all its ancestors to ensure uniqueness of the name

Fixes: #46728

It adds a feature that is present in all major test runners. It's well described in this comment

It allows developers to quickly run/debug a single test e.g. from their editor

---

I am a first time contributor 😉

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[mdrobny]

I fixed too long commit message, it should pass the lint now

[MoLow]

do we have a benchmark test for --test-name-pattern? the changes looks ok but I wonder what are the performance implications

[mdrobny]

@MoLow you are right, there can be negative performance implications when using --test-name-pattern flag since we must do additional regexp matching for all not matching tests.

I can try to write some naive benchmark test. So far, for 1000 test suits, I didn't observe any meaningful difference

[mdrobny]

I experimented with benchmarking test runner with mitata package. It repeats many times the operation and does the measurements

Here is the test suite repeated 1000 times

const {describe, it} = require('node:test');

for (let i = 0; i < 1000; i++) {
  describe('describeTest', () => {
    it('itTest1', () => {});
    it('itTest2', () => {});

    describe('describeTest', () => {
      it('itTest1', () => {});
      it('itTest2', () => {});
    });
  })
}

here is the benchmark code using my locally compiled version of Node with changes from this branch

import { run, bench, group, baseline } from 'mitata';
import { spawnSync } from 'node:child_process'

const pathToLocalNode = '../../node/node';

group('test runner', () => {
  baseline('no pattern', () => {
    spawnSync(pathToLocalNode, ['--test'], { stdio: 'ignore' })
  });

  bench('with name pattern not matching any test', () => {
    spawnSync(pathToLocalNode, ['--test-name-pattern', 'nothing', '--test'], {stdio: 'ignore'})
  })

  bench('with name pattern matching all tests', () => {
    spawnSync(pathToLocalNode, ['--test-name-pattern', 'describeTest', '--test'], {stdio: 'ignore'})
  })

  bench('with name pattern matching single test', () => {
    spawnSync(pathToLocalNode, ['--test-name-pattern', 'describeTest describeTest itTest2', '--test'], { stdio: 'ignore' })
  })
});

await run();

those are the results from running benchmark on my computer (Macbook with M1 Pro)

Looks like there is almost no difference when using --test-name-pattern flag which is.. surprising 🤔 I expected a bit more slowness. Am I missing something in this benchmark?

[mdrobny]

@MoLow what do you think about the benchmark results?

[MoLow]

LGTM. the overhead is specific for the case when using --test-name-pattern

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57177/

[mdrobny]

@MoLow why do you consider it a major (breaking) change? 🤔
it rather won't affect current usages of --test-name-pattern but adds support for matching single test by prefixing with all parent names.
Do you have some example in mind where this will break some current usage?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57440/

[MoLow]

@MoLow why do you consider it a major (breaking) change? 🤔 it rather won't affect current usages of --test-name-pattern but adds support for matching single test by prefixing with all parent names. Do you have some example in mind where this will break some current usage?

using the example from the documentation, added in this PR:

describe('test 1', (t) => {
  it('some test');
});

describe('test 2', (t) => {
  it('some test');
});

Starting Node.js with --test-name-pattern="test 1 some test" would match
only some test in test 1.

prior to this change, using --test-name-pattern="test 1 some test" would run no tests, and now it will run 1 test - I consider that a breaking change

[MoLow]

@mdrobny please rebase this PR

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/51577
✔  Done loading data for nodejs/node/pull/51577
----------------------------------- PR info ------------------------------------
Title      test_runner: improve `--test-name-pattern` to allow matching single test (#51577)
Author     Michał Drobniak  (@mdrobny, first-time contributor)
Branch     mdrobny:feat/test-runner-match-unique-name -> nodejs:main
Labels     semver-major, author ready, needs-ci, test_runner
Commits    1
 - test_runner: improve `--test-name-pattern` to allow matching single test
Committers 1
 - mdrobny 
PR-URL: https://github.com/nodejs/node/pull/51577
Fixes: https://github.com/nodejs/node/issues/46728
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51577
Fixes: https://github.com/nodejs/node/issues/46728
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 27 Jan 2024 09:22:46 GMT
   ✔  Approvals: 2
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/51577#pullrequestreview-1908557778
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/51577#pullrequestreview-1887168630
   ✘  semver-major requires at least 2 TSC approvals
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-02-29T10:49:29Z: https://ci.nodejs.org/job/node-test-pull-request/57498/
- Querying data for job/node-test-pull-request/57498/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8096266840

[nodejs-github-bot]

Landed in 00dc6d9