Portable resolver test suite

[guybedford]

This was brought up in https://github.com/nodejs/modules/issues/451#issuecomment-575282242 for resolver stability, that having tests available for tools and userland resolvers would be a great help in ensuring that they are compliant with the Node.js ES module resolver.

If we could move the resolution tests into something resembling an independent test suite that could easily be run against any async resolver function that could assist the ecosystem in this transition process.

So -

1. Do we agree this would be a useful thing to provide? There were no objections when this was discussed previously in the meeting I believe.
2. If so, how would we offer such a portable test suite in a useful way? Suggestions re useful patterns here would be welcome.

[ljharb]

1. This would be amazingly useful.

2. Perhaps a function that takes a user-provided "resolve" function, and runs a bunch of assertions or tape tests with it?

[SimenB]

Yes please, that would be awesome 👍

[jkrems]

I'm a big fan of data driven conformance tests for this kind of thing. If somebody has a Rust/Java/C++ implementation, they can use it without having to worry about JS bindings. A test suite that takes a JS resolution function could be implemented on top of it but I could imagine that some projects would prefer to pull the conformance repo and integrate into their own test suite and runner instead.

[ljharb]

If you provided a JS function (that could be integrated into an existing JS test suite) and then also provided a hook for someone to provide a CLI tool, then that would enable both patterns.

[jkrems]

Possible straw API:

import RESOLUTIONS from '@nodejs/resolver-conformance-tests';

describe('node resolver-conformance', () => {
  for (const resolutionTest of RESOLUTIONS) {
    // Don't want to support non-import resolution.
    if (!resolutionTest.environments.includes('import')) continue;

    it(resolutionTest.id, async () => {
      for (const file of resolutionTest.files) {
        if (file.type === 'symlink') { /* ... */ }
        testFS.writeFileSync(file.relativePath, file.content);
      }
      assertEquals(
        resolutionTest.resolvedURL,
        await resolve(resolutionTest.specifier, resolutionTest.referrerURL)
      );
    });
  }
});

Some of that is boilerplate but a lot of it is really handy for people who want it to work nicely in their test suite (e.g. rerunning failing tests only). E.g. we could have dedicated support for writing the files that belong to the test case to a given FS implementation.

[jkrems]

The content of RESOLUTIONS is a data structure hinted at in the code consuming it. It’s a collection of conformance test cases with the file layout they require, the resolution they test, and the expected URL after resolution finishes. The test case is writing files to realize the file layout for the active test case because it’s not a given that the resolver under test directly interacts with the file system.

[jkrems]

It seems like this is the data structure. Can you reconfirm?

That looks about right. Just want to point out that this was just a rough pseudo-code example of the kind of data required for a conformance data set. I assume gaps would pop up as one would actually try to implement one.

It sounds like the resolver you have in mind would be interacting with an in-memory file system (at least during testing).

I wanted to make sure that virtualized file systems could use the test suite. Another approach for this would be that the test case references a fixture directory. That way resolvers that use anything but the actual disk could at least copy the directory into whatever data structure they use.

export default [ // RESOLUTIONS

One important note: The test cases would hopefully exist as plain JSON. That way a Java implementation for some IDE could use it without having to execute JavaScript just to get the data.

[jkrems]

I imagine they would be duplicates, so why not make it its own top-level key?

Can you explain what you mean by duplicates? I think it would be upon the maintainers of the conformance tests to ensure that the files aren't duplicated within a test. Using potentially shared fixture directories would make that easier than to put the file contents inside of the data structure though.

The files array of objects can have various relativePaths mapping to the same Module Record instance, [...]

I think there's a misunderstanding about what files is, maybe. It's strictly meant as a "test environment". The relativePaths says where the file is. It's a path, not a specifier. So the quote of the spec about specifier normalization shouldn't apply. relativePaths would always be normalized since it doesn't really make sense to put non-normalized paths into test setup data.

One example for "why have files at all" would be package.json. E.g. the following test case may exist:

{
  "id": "self-reference-from-pkg-root",

  "specifier": "enclosing-pkg-name",
  "resolvedURL": "./pkg-root/lib/pkg.mjs",
  "referrerURL": "./pkg-root/ref.mjs",

  "environments": ["import"],
  "files": [
    {
      "type": "file",
      "relativePath": "./pkg-root/package.json",
      "content": "{\"name\":\"enclosing-pkg-name\",\"exports\":\"./lib/pkg.mjs\"}",
    }
  ]
}

Alternatively, with fixture directories it may be:

{
  "id": "self-reference-from-pkg-root",

  "specifier": "enclosing-pkg-name",
  "resolvedURL": "./pkg-root/lib/pkg.mjs",
  "referrerURL": "./pkg-root/ref.mjs",

  "environments": ["import"],
  "fixturesDir": "self-reference"
}

Where <test suite>/fixtures/self-reference/pkg-root/package.json would contain the same content as the files entry above.

[guybedford]

The test suite should be based on resolve(specifier, context) -> Promise<resolved> being tested, and possibly also a getFormat - effectively just like the loader hooks implementation themselves.

So I would worry less about testing syntax / REPL etc and more about effectively getting "coverage" on the specification (https://nodejs.org/dist/latest-v13.x/docs/api/esm.html#esm_resolver_algorithm).

Then, yes all of these that you have listed are cases that need to be covered:

• FS root: /
• FS relative: ./
• FS updir: ../
• Bare specifiers: resolve
• Package-scoped bare specifiers: @nodejs/resolver-conformance-tests
• Deep import specifiers @nodejs/resolver-conformance-tests/test/fixtures/constants.mjs
• File URLs: file://, node://, data:// (are there others?)
• Are they absolute? file:///
• Do they work on Windows? file://C:/

As well as things like symlinks, exports, self resolution, exports edge cases, errors and conditions, file extension lookups on the CJS legacy paths, path encoding (eg emojis in file names, and avoiding URL injections).

All of these cases are covered with the resolutions done by the tests over all of the test/es-modules. So simply extracting those variations out of the main test suite could be one approach.

[ljharb]

I would strongly prefer "literally anything but yaml". TOML, even.

[ljharb]

Why does escaping make it untenable? The JSON doesn't need to be hand-edited.

[ljharb]

Only to a human reading the raw JSON file - is that a valuable audience?

[ljharb]

It’d probably be better for the json to have a file path pointing to an mjs file, so we can link and evaluate the js more easily.