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 Issues and PRs related to the test runner subsystem. 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

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

Loading

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

Loading

cjihrig Jun 9, 2023

MoLow Jun 9, 2023

MoLow commented Jun 9, 2023 •

edited

Loading

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 Loading

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 Loading

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 Loading

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

Loading

targos Jun 9, 2023 •

edited

Loading

MoLow commented Jun 9, 2023 •

edited

Loading

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