Runs different tests than `node --test`

[rotu]

When discovering tests, this extension looks for the string node:test in file contents^1. This is dangerous and confusing since it skips files that node --test explicitly does not:

Test files must be executable by Node.js, but are not required to use the node:test module internally.

Further, even tests that do use the node:test module internally might be excluded, since the parsing uses simple syntactic matching^2:

e.g

// oops! Doesn't like the `-string
const { test } = require(`node:test`)
// oops! Doesn't like the backwards compatibility
const { test } = (process.versions.node < 18) ? require("node-core-test") : require("node:test")

[uwinkelvos]

Well there are different requirements to the general test runner like node --test and this vscode integration e.g. the first does not care about test output the later does very much require TAP output. TAP can obviously be produced by other test frameworks, e.g. node-tap, ava, etc, but definitely not by all. So if you have jest tests in paths that would be picked up, then running and later parsing the results would just result in a variety of errors. Therefore requiring node:test to be used here makes a lot of sense to me. Also regarding your 2nd point:

This extension requires Node.js >=19: node:test is quite new and did not offer features we need in prior versions.

That being said there are problems with the test discovery: https://github.com/connor4312/nodejs-testing/issues/11

[connor4312]

Yes, test discovery is AST-based. Since we don't execute the code, we cannot discover every test that might possibly exist (see: the halting problem). This is a design decision. However we should support backticks on import, I'll fix that.

[rotu]

Since we don't execute the code ... This is a design decision

Yes, and I think that design decision explicitly deserves reconsideration. There are various checks, file system reads, and environment constraints which can affect the tests, so it is desirable to run files if you can't tell whether it uses node:test or not. It's better to process files you don't have to than to have the user see green test status when the tests were, in fact, implicitly skipped.

For my particular use case, I was trying to run test from a different tap-based tool with this extension. I would expect errors or some sort of output, but there's zero output from this extension as to what happened. Which is why I dove into the source code to figure out what was going on and filed this issue.

There are other users mystified by tests not being discovered. If the behavior stays the same I would recommend:

1. Updating the documentation on the Marketplace page which currently omits any mention of filtering by parsing the files. Reading the documentation as it is, I would expect it to run the same tests node --test itself would run.

   Having Problems? Read this!

   The extension looks for files that use the Node.js test runner naming convention. Make sure your files are named correctly!

2. Logging a user-visible warning when test files are skipped. This is especially important when using a bundler, transpiler, etc. which may make the analyzed code differ from what the user typed.

However we should support backticks on import, I'll fix that.

While you're at it, you may also want to support import expressions; i.e. import('node:test').