Repository navigation
Portable resolver test suite #49448
Description
Activity
-
This would be amazingly useful.
-
Perhaps a function that takes a user-provided "resolve" function, and runs a bunch of assertions or tape tests with it?
Reacted by Derek Lewis-
Yes please, that would be awesome 👍
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.
Reacted by Corey Farrell, Charles Samborski and RenéeIf 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.
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.
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.
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 [ // RESOLUTIONSOne 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.
Reacted by Derek LewisI 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
filesarray of objects can have variousrelativePathsmapping to the same Module Record instance, [...]I think there's a misunderstanding about what
filesis, maybe. It's strictly meant as a "test environment". TherelativePathssays where the file is. It's a path, not a specifier. So the quote of the spec about specifier normalization shouldn't apply.relativePathswould 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.jsonwould contain the same content as thefilesentry above.Reacted by Derek LewisThe test suite should be based on
resolve(specifier, context) -> Promise<resolved>being tested, and possibly also agetFormat- 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.Reacted by Derek Lewis and Jordan HarbandReacted by Derek LewisI would strongly prefer "literally anything but yaml". TOML, even.
Why does escaping make it untenable? The JSON doesn't need to be hand-edited.
Only to a human reading the raw JSON file - is that a valuable audience?
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.
Reacted by Derek Lewis and Jan Olaf MartinThis issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
This was brought up in nodejs/modules#451 (comment) 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 -