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

test: missing fixture test-esm-pkg-over-ext.mjs #15280

Closed
wants to merge 1 commit into from

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Sep 8, 2017

I think I figured out the issue causing the test suite to fail (see #15276) with the following error:

=== release test-esm-pkg-over-ext ===                                          
Path: es-module/test-esm-pkg-over-ext
(node:12584) ExperimentalWarning: The ESM module loader is experimental.
Error: module ../fixtures/module-pkg-over-ext/inner not found
    at module.exports (internal/loader/search.js:16:12)
    at resolveRequestUrl (internal/loader/resolveRequestUrl.js:81:13)
    at Loader.getModuleJob (internal/loader/Loader.js:50:27)
    at ModuleWrap.module.link (internal/loader/ModuleJob.js:30:25)
    at linked (internal/loader/ModuleJob.js:28:21)
    at <anonymous>
  1. there was a missing fixture fixtures/module-pkg-over-ext/inner/package.json.
  2. I think the first import in the test should be ../fixtures/module-pkg-over-ext/inner/package vs. ../fixtures/module-pkg-over-ext/inner, or at least at this time importing a directory does not seem to default to package.json if no index.js is found (is this expected behavior @bmeck?).
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test,es-module

Added package.json fixture for test-esm-pkg-over-ext.mjs.
Modified import in test to load inner/package
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 8, 2017
@bmeck
Copy link
Member

bmeck commented Sep 8, 2017

kind of the inverse, there should be an inner.js

@bmeck
Copy link
Member

bmeck commented Sep 8, 2017

the subject of #14990 is what this fixture is supposed to be about

@bcoe
Copy link
Contributor Author

bcoe commented Sep 8, 2017

@bmeck sounds like there's a bit more work than just adding the fixture, following #14990. Want me to close this, and I can rebase #15276 once tests are passing?

@bmeck
Copy link
Member

bmeck commented Sep 8, 2017

sure, we can wait and rebase

@bcoe bcoe closed this Sep 8, 2017
@bcoe bcoe deleted the fix-module-test branch September 8, 2017 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants