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

Testrunner glob parsing seems to be broken #43675

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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