Skip to content

Testrunner glob parsing seems to be broken #43675

New issue

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

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

Already on GitHub? # to your account

Closed
jtuchel opened this issue Jul 4, 2022 · 2 comments · Fixed by #44241
Closed

Testrunner glob parsing seems to be broken #43675

jtuchel opened this issue Jul 4, 2022 · 2 comments · Fixed by #44241
Labels
cli Issues and PRs related to the Node.js command line interface. test_runner Issues and PRs related to the test runner subsystem.

Comments

@jtuchel
Copy link

jtuchel commented Jul 4, 2022

Version

18.4.0

Platform

Linux pop-os 5.17.15-76051715-generic #202206141358165591911622.04~1db9e34 SMP PREEMPT Wed Jun 22 19 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

  • Create a new project via npm init -y
  • Change the test script to "test": "node --test ./myTestDir/**/*Tests.js"
  • Create the following folder structure
.
├── myTestDir
│   └── aFolder
│       ├── aSubFolder
│       │   └── ignoredTests.js
│       └── pickedUpTests.js
└── package.json

pickedUpTests.js

const assert = require('assert/strict');
const test = require('node:test');

test('picked up by testrunner', async t => {
    await t.test('passes.', async () => {
        assert.ok(true);
    });
});

ignoredTests.js

const assert = require('assert/strict');
const test = require('node:test');

test('ignored by testrunner', async t => {
    await t.test('fails.', async () => {
        assert.ok(false);
    });
});
  • When running npm run test only pickedUpTests run

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

It should run all tests ( failing ones too )

What do you see instead?

It seems the glob parsing is broken so it only runs tests on a specific directory depth

Additional information

A temporary workaround for this would be the glob ./myTestDir/**/**/*Tests.js

@VoltrexKeyva VoltrexKeyva added cli Issues and PRs related to the Node.js command line interface. test_runner Issues and PRs related to the test runner subsystem. labels Jul 4, 2022
@MoLow
Copy link
Member

MoLow commented Jul 4, 2022

Hi @jtuchel,
this is not broken - it was never implemented,
however, it was discussed here #40954 (comment)
@cjihrig were there any conclusions regarding this discussion?

@cjihrig
Copy link
Contributor

cjihrig commented Jul 4, 2022

There was no conclusion that I'm aware of. I think we should implement a CLI flag for this functionality based on regular expressions instead of globs. If/when Node adds glob support, we could switch over to using that (or stick with regular expressions if they are working well).

@MoLow MoLow closed this as completed in 59527de Aug 24, 2022
RafaelGSS pushed a commit that referenced this issue Sep 5, 2022
PR-URL: #44241
Fixes: #44023
Fixes: #43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
PR-URL: nodejs#44241
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 3, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#TBD
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 3, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 3, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 3, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 12, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Oct 12, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node that referenced this issue Nov 23, 2022
PR-URL: nodejs#44241
Backport-PR-URL: nodejs#44873
Fixes: nodejs#44023
Fixes: nodejs#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
richardlau pushed a commit that referenced this issue Dec 7, 2022
PR-URL: #44241
Backport-PR-URL: #44873
Fixes: #44023
Fixes: #43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#44241
Backport-PR-URL: nodejs/node#44873
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#44241
Backport-PR-URL: nodejs/node#44873
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MoLow added a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
PR-URL: nodejs/node#44241
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 59527de13d39327eb3dfa8dedc92241eb40066d5)
MoLow added a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
PR-URL: nodejs/node#44241
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 59527de13d39327eb3dfa8dedc92241eb40066d5)
MoLow added a commit to MoLow/node-core-test that referenced this issue Feb 2, 2023
PR-URL: nodejs/node#44241
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 59527de13d39327eb3dfa8dedc92241eb40066d5)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cli Issues and PRs related to the Node.js command line interface. 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