Discussion: allow extension to function when node:test is wrapped by a convenience function

[skabbes]

Context
My company has a 10 year old node.js codebase, and we have recently converted it to use node:test. However, in order to successfully migrate, and for future-proofing the testing - we have wrapped the test function up in our own function.

// test/utils.ts
import { test as nodeTest } from 'node:test';

export class TestContext = // a class with a bunch of test helpers on it

type TestFn = (t: TestContext) => Promise<any>;

export function test(name: string, testFn: TestFn) {
  return nodeTest(name, async (t) => {
    const context = new TestContext(t);
    return testFn(context);
  });
}

We have started to work on making the codebase accessible to very junior developers, and wanted to have "click to run tests" functionality, and stumbled upon this extension. It is honestly great, but changing all of our tests to use node:test directly is a bit too much to ask.

Request up for discussion
I'd like to add a setting / configuration that allows for specifying a "test function override" - where you can specify where your test wrapper function is (instead of import { test } from 'node:test'; it might be import { test } from './utils'; for example ).

This would allow us to use this extension for our testing with minimal changes to the codebase, and provide a single place to update if we ever switched to vitest (for example) or jest or whatever new testing framework comes around in 10 more years.

Other examples
We hit a similar problem with typescript-eslint and I thought their solution to it was elegant, where they allow specifying a callable function with a TypeOrValueSpecifier .

      "@typescript-eslint/no-floating-promises": [
        "error",
        {
          // The recommended usage of top level tests in node:test is to leave those promises floating
          allowForKnownSafeCalls: [
            {
              from: "package",
              name: ["test", "it", "describe", "suite"],
              package: "node:test",
            },
            {
              from: "file",
              name: "test",
              path: "test/utils.ts",
            },
          ],
        },
      ],

Proposal
I think nodejs-testing could adopt a setting that accepted a TypeOrValueSpecifier. The default value of this setting would be:

{
  from: "package", 
  name: ["test", "it"],
  package: "node:test",
}

But anyone who wanted to wrap the test function would replace it with the TypeOfValueSpecifier that matched their wrapped test function.

{
  from: "file", 
  name: ["test"],
  path: "test/utils.ts",
}

The code I attached to this was a very very hacky proof of concept I did to ensure it would work in theory, it is by no means a suggestion to merge this code. I just wanted to have a discussion before doing the "real" implementaiton.

[skabbes]

Instead of this hard-coded value here, it would read the setting and determine if the import matches the configuration.

[connor4312]

Sure, I'm good for this. As you said it should be configurable. I would probably just allow a regex expression to specify the import, file sounds nice but it gets complicated when people have special loaders and setups (or even just TypeScript with sourceRoot). So something like:

const defaultValue = {
  from: "^node:test$",
};

const yours = {
  from: "test/utils\\.ts$",
}

We actually don't look at the function names currently, so this doesn't need to be specified.

[skabbes]

OK that sounds good - I think allowing an array of regex is probably a bit better, as it allows slow adoption into node:test or into a "test wrapper function" without having to switch over their whole codebase at once.

We actually don't look at the function names currently

Hmm, I think I'd need to add that right? At least for me, my test/utils.ts file has more than 1 function exported from it. Only one of them actually defines a test. I think if I understand the code, it is something like this: "a function imported from node:test that has a string as its first argument". I guess if you don't want to add "name matching" that's fine, I can work with that.

[connor4312]

I'm okay if we want to add an array of names to serve as an allowlist. I just didn't need it before.

[skabbes]

OK, I think I'll get started on this. I'm new to developing extensions, if you have any must-read tutorials or pro-tips please send them over.

[connor4312]

You should be able to clone this repo, run npm run watch in a terminal, and hit F5 to debug the extension. A new window will open where the dev version of the extension is running. Breakpoints etc. will work.

[skabbes]

OK, I took a first stab at this here:
#64

[connor4312]

Closing in favor of that PR