Revert "test_runner: change ts default glob"

[theoludwig]

TypeScript files glob for the test runner was added in #55081, and then changed in #57359. With this PR, I would like that we revert #57359, or at least open the discussion. Great default, and a runtime which just "work out of the box" is important, that's why changing the defaults matter.

Why should TypeScript files have different globs than JavaScript files?
It was already suggested that for v24.0.0, we remove this restriction of the test folder: #57359 (comment).

As we are approaching the stabilization of the TypeScript strip types feature, it is now or "never" (or only able to change that in semver major versions) to change the defaults, since it is still experimental for now, there is still time to do it.

Today, I wanted to try Node.js v24.0.0, and since v24 obviously include everything from v23 and v23 includes #57359, I was surprised that my tests were ignored (upgrading from v22).

#57359 (comment)

I think the issue is that people keep their test.ts in the src folder and transpile them, so they will be executed twice. Once node v20 goes EOL we can be more lax and include other patterns

Why should people transpile test files, if it's possible to run them without transpiling thanks to that strip types feature? (I indeed keep my TypeScript files under a src folder).

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[theoludwig]

What do you think? @JakobJingleheimer, @marco-ippolito

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.12%. Comparing base (e38ce27) to head (95d88b8).
Report is 101 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/utils.js	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58202      +/-   ##
==========================================
- Coverage   90.14%   90.12%   -0.02%     
==========================================
  Files         629      629              
  Lines      186634   186634              
  Branches    36616    36621       +5     
==========================================
- Hits       168236   168201      -35     
- Misses      11204    11215      +11     
- Partials     7194     7218      +24     

Files with missing lines	Coverage Δ
lib/internal/test_runner/runner.js	89.03% <100.00%> (ø)
lib/internal/test_runner/utils.js	60.46% <66.66%> (ø)

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[marco-ippolito]

Currently all the release line supporting typescript follow the same behavior.
Since v24 will be the first LTS with strip types enable by default I would like to be cautious and leave the current behavior, which minimizes the risk of breaking change.

[theoludwig]

Currently all the release line supporting typescript follow the same behavior.

That's not the case, all release lines do not follow the same behavior. With the following structure:

├── src
│   ├── dates.ts
│   ├── strings.ts
│   ├── tests
│   │   ├── dates.test.ts
│   │   ├── strings.test.ts
├── package.json
├── tsconfig.json

And with package.json:

{
  "scripts": {
    "test": "node --experimental-strip-types --test"
  }
}

Running node --run test in Node.js v22 results in:

ℹ tests 53
ℹ suites 20
ℹ pass 53
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 175.004645

Running node --run test in Node.js v24 results in:

ℹ tests 0
ℹ suites 0
ℹ pass 0
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 15.541625

[theoludwig]

Since v24 will be the first LTS with strip types enable by default I would like to be cautious and leave the current behavior, which minimizes the risk of breaking change.

That's the opportunity, to make the good defaults. Weird, that you consider this breaking, you should have considered #56350 breaking then.

It will be even more painful, if we do it later. And again, I don't understand why someone would transpile their TypeScript test files, if they can execute them directly. And even if they do, they will at least be executed, and the only "bad thing", the tests will be included twice, whereas, I assume most people, would want to execute their test files, it will be ignored which is worst IMO, than executing twice (of course executing twice is also not the correct behavior), at least the tests runs.

[marco-ippolito]

Apparently it has not been backported to v22 yet. I'm really not sure what to do 🤷 either way some use cases are gonna break

pinging @nodejs/tsc

[marco-ippolito]

I thought about it and I'm ok with reverting. It caused more trouble than it solved.
Can you create a revert commit rather than a new one?

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @marco-ippolito.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[marco-ippolito]

Added notable change because it was requested that we highlight this change since its a breaking change for people using experimental strip types

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 264cad7