test_runner: make `--test-name-pattern` recursive #48382

MoLow · 2023-06-07T19:06:09Z

Refs: #46728 (comment)

nodejs-github-bot · 2023-06-07T19:06:14Z

Review requested:

@nodejs/test_runner

MoLow · 2023-06-07T19:08:45Z

I am not sure if this should be smver-major

benjamingr · 2023-06-08T21:22:40Z

Arguably this is a bug fix so II don't think it needs semver-major

cjihrig

As this is currently written, I do believe it's a breaking change. I'm more concerned that I don't think this is the right change based on what is being requested in #46728.

test/fixtures/test-runner/output/name_pattern.js

cjihrig · 2023-06-08T21:19:58Z

test/fixtures/test-runner/output/name_pattern_with_only.js

@@ -5,7 +5,7 @@ const { test } = require('node:test');

 test('enabled and only', { only: true }, common.mustCall(async (t) => {
  await t.test('enabled', common.mustCall());
-  await t.test('disabled', common.mustNotCall());
+  await t.test('disabled but parent not', common.mustCall());


Why is this running? Is it because the parent is enabled? If so, I don't think that's what #46728 is about.

see #48382 (comment)

FWIW, this change corresponds to my expectations.
A more realistic use case:

describe('add', () => { it('should return 2', () => assert.strictEqual(add(1, 1), 2)); it('should return 10', () => assert.strictEqual(add(8, 2), 10)); it('should throw', () => assert.throws(() => add(), TypeError)); }); describe('subtract', () => { // ... });

node --test-name-pattern=add should run all children of describe('add')

I'm fine with this PR now that it's clear this is not fixing #46728.

Can you please approve? :)

MoLow · 2023-06-09T03:37:52Z

I'm more concerned that I don't think this is the right change based on what is being requested in #46728.

I remove the link between this PR and that issue.
based on the fact this is the behavior of all other test runners I do think it is a required change.

nodejs-github-bot · 2023-06-09T13:01:22Z

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

nodejs-github-bot · 2023-06-11T14:54:28Z

Commit Queue failed

- Loading data for nodejs/node/pull/48382 ✔ Done loading data for nodejs/node/pull/48382 ----------------------------------- PR info ------------------------------------ Title test_runner: make `--test-name-pattern` recursive (#48382) Author Moshe Atlow (@MoLow) Branch MoLow:filter-by-name-recursive -> nodejs:main Labels author ready, needs-ci, test_runner Commits 2 - test_runner: make `--test-name-pattern` recursive - CR Committers 1 - Moshe Atlow PR-URL: https://github.com/nodejs/node/pull/48382 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48382 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 07 Jun 2023 19:06:09 GMT ✔ Approvals: 2 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/48382#pullrequestreview-1469507668 ✔ - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/48382#pullrequestreview-1473725491 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-06-09T13:01:22Z: https://ci.nodejs.org/job/node-test-pull-request/52180/ - Querying data for job/node-test-pull-request/52180/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 48382 From https://github.com/nodejs/node * branch refs/pull/48382/merge -> FETCH_HEAD ✔ Fetched commits as 4ee8ef269ba4..fb4215b79bc6 -------------------------------------------------------------------------------- [main beee13d595] test_runner: make `--test-name-pattern` recursive Author: Moshe Atlow Date: Wed Jun 7 22:04:02 2023 +0300 5 files changed, 124 insertions(+), 31 deletions(-) [main 233e2f5161] CR Author: Moshe Atlow Date: Fri Jun 9 06:45:43 2023 +0300 2 files changed, 13 insertions(+), 13 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test_runner: make --test-name-pattern recursive

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

[detached HEAD ca883b695b] test_runner: make --test-name-pattern recursive
Author: Moshe Atlow moshe@atlow.co.il
Date: Wed Jun 7 22:04:02 2023 +0300
5 files changed, 124 insertions(+), 31 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
CR

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

[detached HEAD fbfa977150] CR
Author: Moshe Atlow moshe@atlow.co.il
Date: Fri Jun 9 06:45:43 2023 +0300
2 files changed, 13 insertions(+), 13 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

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

nodejs-github-bot · 2023-06-11T15:02:20Z

Landed in c21fe3a

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: nodejs#48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

test_runner: make --test-name-pattern recursive

4a53104

MoLow requested a review from cjihrig June 7, 2023 19:06

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner labels Jun 7, 2023

benjamingr approved these changes Jun 8, 2023

View reviewed changes

cjihrig reviewed Jun 8, 2023

View reviewed changes

MoLow requested a review from cjihrig June 9, 2023 03:44

CR

fb4215b

MoLow mentioned this pull request Jun 9, 2023

test_runner: make --test-name-pattern recursive #48401

Closed

MoLow added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 9, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2023

MoLow requested a review from targos June 11, 2023 11:58

cjihrig approved these changes Jun 11, 2023

View reviewed changes

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 11, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 11, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 11, 2023

nodejs-github-bot merged commit c21fe3a into nodejs:main Jun 11, 2023
67 checks passed

MoLow deleted the filter-by-name-recursive branch June 12, 2023 03:16

RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023

test_runner: make --test-name-pattern recursive

5ebe3a4

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

RafaelGSS mentioned this pull request Jul 3, 2023

v20.4.0 proposal #48643

Merged

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

test_runner: make --test-name-pattern recursive

8858e1e

PR-URL: nodejs#48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

test_runner: make --test-name-pattern recursive

ca17509

PR-URL: nodejs#48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

ruyadorno pushed a commit that referenced this pull request Sep 7, 2023

test_runner: make --test-name-pattern recursive

7051caf

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

ruyadorno mentioned this pull request Sep 8, 2023

v18.18.0 proposal #49220

Merged

DmitryMakhnev mentioned this pull request Oct 16, 2023

Test runner should allow specific subtest filtering, via --test-name-pattern or otherwise #46728

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test_runner: make `--test-name-pattern` recursive #48382

test_runner: make `--test-name-pattern` recursive #48382

MoLow commented Jun 7, 2023 •

edited

nodejs-github-bot commented Jun 7, 2023

MoLow commented Jun 7, 2023

benjamingr commented Jun 8, 2023

cjihrig left a comment

cjihrig Jun 8, 2023

MoLow Jun 9, 2023

targos Jun 9, 2023 •

edited

cjihrig Jun 9, 2023

MoLow Jun 9, 2023

MoLow commented Jun 9, 2023 •

edited

nodejs-github-bot commented Jun 9, 2023

nodejs-github-bot commented Jun 11, 2023

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

Successfully rebased and updated refs/heads/main.

nodejs-github-bot commented Jun 11, 2023

test_runner: make --test-name-pattern recursive #48382

test_runner: make --test-name-pattern recursive #48382

Conversation

MoLow commented Jun 7, 2023 • edited

nodejs-github-bot commented Jun 7, 2023

MoLow commented Jun 7, 2023

benjamingr commented Jun 8, 2023

cjihrig left a comment

Choose a reason for hiding this comment

cjihrig Jun 8, 2023

Choose a reason for hiding this comment

MoLow Jun 9, 2023

Choose a reason for hiding this comment

targos Jun 9, 2023 • edited

Choose a reason for hiding this comment

cjihrig Jun 9, 2023

Choose a reason for hiding this comment

MoLow Jun 9, 2023

Choose a reason for hiding this comment

MoLow commented Jun 9, 2023 • edited

nodejs-github-bot commented Jun 9, 2023

nodejs-github-bot commented Jun 11, 2023

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com

PR-URL: #48382 Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Colin Ihrig cjihrig@gmail.com

Successfully rebased and updated refs/heads/main.

nodejs-github-bot commented Jun 11, 2023

test_runner: make `--test-name-pattern` recursive #48382

test_runner: make `--test-name-pattern` recursive #48382

MoLow commented Jun 7, 2023 •

edited

targos Jun 9, 2023 •

edited

MoLow commented Jun 9, 2023 •

edited

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

PR-URL: #48382
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com