-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: move ESM fixtures to fixtures dir #19409
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for cleaning these up!
@@ -4,6 +4,7 @@ const { execFileSync } = require('child_process'); | |||
|
|||
const node = process.argv[0]; | |||
|
|||
execFileSync(node, ['--experimental-modules', 'test/es-module/test-esm-ok']); | |||
execFileSync(node, ['--experimental-modules', | |||
'test/fixtures/es-modules/test-esm-ok']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use the common.fixtures
module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks
Also consistently import the `common` module where possible.
8a599b3
to
ecc6965
Compare
Landed in fddcd62 |
Also consistently import the `common` module where possible. PR-URL: #19409 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Also consistently import the
common
module where possible.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes