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

CLI flag for the Node 18 test runner #1853

Open
meyfa opened this issue Jul 28, 2022 · 11 comments
Open

CLI flag for the Node 18 test runner #1853

meyfa opened this issue Jul 28, 2022 · 11 comments
Labels
research Needs design work, investigation, or prototyping. Implementation uncertain.

Comments

@meyfa
Copy link

meyfa commented Jul 28, 2022

Desired Behavior

A native test runner was added in Node 18: https://nodejs.org/dist/latest-v18.x/docs/api/test.html
It can be started by node --test testfile1.js testfile2.js or node --test testdirectory/. Trying to use it with TypeScript is a bit cumbersome, though, since the directory variant doesn't quite work:

node --loader ts-node/esm --test testdirectory/

The above will not find any tests written in TypeScript, since only .js, .cjs, .mjs extensions are considered.
See https://nodejs.org/dist/latest-v18.x/docs/api/test.html#test-runner-execution-model.

Since ts-node otherwise seems to want to mirror node's CLI behavior, my suggestion is to add a --test flag to ts-node that behaves like node would, but including resolution of TS test files. This could be implemented by searching the paths given as positional args to find all files matching .js, .cjs, .mjs, .ts, .cts, .mts, then spawning a node child process like node --loader=ts-node/esm --test FILES where FILES is the explicit list of files found in the previous step.

Is this request related to a problem?

I would really like to get rid of third-party testing frameworks, especially for the myriad of small packages that don't require any advanced functionality, but this is hindered by not being able to run TypeScript tests easily.

Alternatives you've considered

This works fine:

node --loader ts-node/esm --test foo.test.ts bar.test.ts

But we really don't want to specify each test file in the command. Globbing is not supported (at least not on Windows):

node --loader ts-node/esm --test test/*.test.ts

outputs:

Could not find 'C:\Users\meyfa\project\test\*.ts'

One might also start node --test pointing at an index file, that performs the globbing and dynamically imports all actual test files.
I haven't tried this, and it would be very ugly, IMHO. Additionally, it would lose process separation between test files. (node --test foo.js bar.js will execute foo.js and bar.js in separate processes).

Additional context

I understand that the Node test runner is still experimental, so perhaps adding any kind of support for it to ts-node may seem premature.
Additionally, I have opened an issue at Node.js with a request to add globbing and/or a CLI flag for additional test file extensions, see nodejs/node#44023.

Still, I feel like having support for ts-node --test isn't too far fetched. Especially when Node adds globbing/extension support, the implementation will come almost for free.

@cspotcode
Copy link
Collaborator

I understand the desire for this, but it's a lot of added complexity and maintenance. The addition of globbing when node itself doesn't do that makes things dificult for us, because our behavior deviates from node. Also, since this feature is experimental in node, they might make breaking changes. Then we'd get bugs asking us to do more work to align.

Probably best way to prove out this idea is to prototype it as a separate project. That'll help solidify the design into a more concrete proposal. At that point, node's hooks may have improved to the point that it's no longer necessary, since you might be able to specify the --loader flag in a .noderc or a package.json

the implementation will come almost for free.

Does that mean that node exposes the test runner as an API? Can someone write code that does require('node:test').run(['tests/*.ts'])? And node will do the globbing and everything that it does when you run node --test? If not, then implementation won't be so free. Just things that need to be figured out in a prototype.

If you want help using our API in your prototype, happy to offer guidance.

@cspotcode cspotcode added the research Needs design work, investigation, or prototyping. Implementation uncertain. label Jul 28, 2022
@meyfa
Copy link
Author

meyfa commented Jul 29, 2022

I understand the desire for this, but it's a lot of added complexity and maintenance.

That's understandable. I'd like to apologize in case my request sounded demanding - that was not the intention.

The addition of globbing when node itself doesn't do that makes things dificult for us, because our behavior deviates from node.

Ah, please let me clear up this point of confusion. I completely agree with you there. While I would love glob support in Node.js itself, since it doesn't have any, I don't think adding it to ts-node makes sense. What I meant was that glob support inside Node.js could simplify the implementation for ts-node.

Probably best way to prove out this idea is to prototype it as a separate project.

Sounds good! I made a rough first version (successfully tested on some of my projects; additionally it can also test itself): https://github.com/meyfa/ts-node-test

It works by resolving input file names recursively, then calls node --loader ts-node/esm --test FILE1 FILE2 .... This may not be what you had in mind. I tried implementing something similar in a fork of ts-node, but couldn't quite figure out how to get the additional child process tied into everything else.

@cspotcode
Copy link
Collaborator

I'd like to apologize in case my request sounded demanding - that was not the intention.

Don't worry, it did not sound demanding.

It works by resolving input file names recursively, then calls node --loader ts-node/esm --test FILE1 FILE2 .... This may not be what you had in mind. I tried implementing something similar in a fork of ts-node, but couldn't quite figure out how to get the additional child process tied into everything else.

Very cool, I'll check it out. Your approach is what I had in mind, either using --loader ts-node/esm or using ts-node's API. The child process stuff is tricky and we have an open pull request that I really need to merge that changes it a bit.

Would it help if ts-node exposed some sort of a run() or exec() function that was equivalent to spawning ts-node? I imagine your code would look like this:

import {run} from 'ts-node';
await run(['--test', 'FILE1', 'FILE2']);

The benefit of a run() function is ts-node can do all of our other child process tricks to make the user's experience nicer.

We can also add this project as an official recommendation to our documentation, if you want. I imagine it would live in the "Recipes" section: https://typestrong.org/ts-node/docs/

@cspotcode
Copy link
Collaborator

I realized immediately after posting the above, that my run() example actually doesn't make sense. I'll think about it some more.

@cspotcode
Copy link
Collaborator

Maybe this is a better example:

import {run} from 'ts-node';

// Run node with TS support.  ts-node will do all the necessary bootstrapping to make TS work
await run({
  // force spawning child process, since ts-node does not always require a child process, but we require it here to pass node args
  spawn: true,
  // Additional args for node.  We will merge these with the args necessary to bootstrap ts-node, including `--loader` and `--require`
  execArgv: ['--test', 'FILE1', 'FILE2'],
});

I suppose it's not much different from my example above. But explicitly opting-in to spawning is probably good, since ts-node does not always require spawning another process. execArgv makes it clear that those args should be passed to node, not to the entrypoint script. And in fact omitting the entrypoint script entirely makes it more obvious that we intend to launch node but not to execute an entrypoint.

@meyfa
Copy link
Author

meyfa commented Aug 10, 2022

Would it help if ts-node exposed some sort of a run() or exec() function that was equivalent to spawning ts-node?

A run() function would certainly get rid of some amount of boilerplate. Yet, much of the work remains since we still have to walk the file system. Luckily, the discussion in nodejs/node#44023 has picked up quite a bit of speed, so we may get config support and/or glob support directly via Node.js, eliminating the need for hacks such as my package ts-node-test.

We can also add this project as an official recommendation to our documentation, if you want.

Thanks for the offer, though I'd much prefer not having to maintain my workaround unless there's no other way. When Node.js adds config/glob support, we could add a recipe for testing with ts-node directly.

@nickmccurdy
Copy link

nickmccurdy commented Dec 12, 2022

I've noticed that nodejs/node#44023 has been closed by nodejs/node#44241 as of August 24, adding an API to run tests programmatically in Node 19 (to be backported into Node 16). Would it make implementing this simpler?

@ArmorDarks
Copy link

ArmorDarks commented Jun 19, 2023

It's now trivial in Node 18.

Just create run.ts file which will start all tests:

import { stdout } from 'node:process'
import { run } from 'node:test'
import { glob } from 'glob'

run({
  concurrency: true,
  timeout: 10000,
  files: glob.sync('**/*.+(test|spec).ts', { ignore: 'node_modules/**' }),
}).pipe(stdout)

And in package.json run this file with ts-node loader (with ESM if needed):

"test": "node --loader ts-node/esm run.ts"

@meyfa
Copy link
Author

meyfa commented Jun 19, 2023

@ArmorDarks I was not able to verify your claim. Yes, the test runner shows 1 passed test, but that's the run.ts file itself. Try adding more *.test.ts files and adding console.log() statements to them, or even some malformed JavaScript code, and you may notice the output doesn't change. I tried with Node.js 18.16.0 which is the latest 18.x release.

Glob support like you describe is in the works, but as of yet unmerged: nodejs/node#47653

From what I can tell, since it's considered a breaking change, it won't be backported to 18.x. Neither will it be part of 20.x. The earliest release we can expect to have it is 21.x, and the earliest LTS will be 22.x.

@ArmorDarks
Copy link

ArmorDarks commented Jun 19, 2023

@meyfa you are corret, my example not complete. It lacks output to stdout and when running programmatically tests we should not add --test flag. I also wrongly assumed files uses glob under the hood. Seems like for now we need to specify glob manually indeed. Updated my example.

@sgrishchenko
Copy link

Be careful, code above does not produce exit code 1, so even broken tests will be green on CI, here is my corrected version of this snippet:

import process from "node:process"
import {run} from "node:test"
import {spec as SpecReporter} from "node:test/reporters";
import {pipeline} from "node:stream/promises";
import {glob} from "glob";

let fail = false

const source = run({
    concurrency: true,
    files: await glob("test/**/*.test.ts"),
}).once("test:fail", () => {
    fail = true
})

const reporter = new SpecReporter()

const destination = process.stdout

await pipeline(source, reporter, destination)

if (fail) throw new Error("Tests failed")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
research Needs design work, investigation, or prototyping. Implementation uncertain.
Projects
None yet
Development

No branches or pull requests

5 participants