Repository navigation
v23 test runner no longer matches .ts causing silent success #57734
Description
Activity
Unfortunately that glob was causing a lof of problems in the ecosystem and typescript support being experimental allows such changes. We are sorry for the inconvenience.
Reacted by Pietro MarchiniHm – it's not that I disagree that you're able to make the change because of the
experimentalflag, but I want to underscore that this is a silently breaking change for people relying on the previous behavior. So it's probably worth making sure that this is messaged in the release notes (or backported to v22?)Yes it will be backported to v22 in a future release, might as well highlight it as a notable change
This also caught me out. I was surprised my tests were still passing after noticing a bug and it turns out that no tests were being run after updating to Node 23.
It is very surprising that renaming a foo.test.js file to foo.test.ts starts to ignore it.
I understand the breaking change of matching .ts but that is what TypeScript support means.
With the current behavior all new projects need to supply a glob and only the projects that broke get a nice default. That seems the wrong way around.
Reacted by Daniel Ehrenberg and Chris DickinsonNot sure what's the path forward.
Until Node v20 goes EOL I think we should keep the currect behavior and then revert it.Is that because Node 20's test runner doesn't have the glob pattern support?
Is that because Node 20's test runner doesn't have the glob pattern support?
Yes exactly, you cannot use the same command in v20, v22 and v23. Also consider that if we unflag typescript in v22 it reduces the chance of breaking users that have '.ts' files in their test folder.
It a weird situation but Id rather be conservativeDefaults impact communities.
This default is telling developers that it is better to have their tests in a separate test folder rather than next to their implementation.
I'd rather the test runner didn't match any .ts files by default than give a preference to one particular style. And for it to start to warn that
.test.tsfiles will be matched in the future.Maybe the test runner should also fail if no tests were run but there were potential
.tsmatchesReacted by Daniel Ehrenberg, Alexander “weej” Jones, Chris Dickinson and cobalt@marco-ippolito Could you share a reference for the ecosystem breakage?
#56546
There are also other issues I cannot find right now I'll include them laterThis default is telling developers that it is better to have their tests in a separate test folder rather than next to their implementation.
Yeah, there's a tricky balance to strike here. The default follows in the footsteps of the node-tap/tape testing ecosystem, where tests are typically located in a
test/directory without additional suffixes. I think requiring a.test.tsis (probably!) fine combined with your suggestion that the test runner should fail if it matches no tests – and assuming that this matches the behavior for.js-suffixed files.(Given that the node test runner takes inspiration from node-tap, it might even be true that it's better to locate tests in a dedicated directory, as it was with node-tap. But I think @cjihrig or @isaacs could speak to that better than I could.)
I'd rather the test runner didn't match any .ts files by default than give a preference to one particular style. And for it to start to warn that
.test.tsfiles will be matched in the future.I'd find this surprising given how type-stripping seems to support
.tselsewhere!I think I've said this elsewhere, but the best solution to this (by far IMO) is to backport globbing to v20. That would allow people to more easily specify exactly what they want to run and it would remove over a year of remaining inconsistency between v20 and v22. I'd say that it's well tested at this point and is less risky than some of the other things that have been backported recently. The risk of someone being broken by having a file with an asterisk in the name seems more than acceptable to me.
I also think that it would be good for the test runner to fail when no tests execute.
Reacted by Chris Dickinson, Pietro Marchini, Michaël Zasso and Marco IppolitoThis issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 240 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
Version
v22.14.0
Platform
Subsystem
test runner
What steps will reproduce the bug?
Our test runner relies on the
*.tsglobbing behavior, and as of #57359, our test suite quietly starts passing because no tests are matched.This seems like breaking behavior!
How often does it reproduce? Is there a required condition?
Migrating from node v22 to v23.
What is the expected behavior? Why is that the expected behavior?
Ideally we do not change globbing behavior between minor versions, but failing that adding a big warning on the release notes would suffice.
What do you see instead?
Silently passing test suite without running any other tests.
Additional information
No response