-
Notifications
You must be signed in to change notification settings - Fork 163
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
chore: Add support for ESM in jest #2606
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2606 +/- ##
=======================================
Coverage 95.80% 95.80%
=======================================
Files 742 743 +1
Lines 20320 20335 +15
Branches 6859 6923 +64
=======================================
+ Hits 19467 19482 +15
Misses 797 797
Partials 56 56 ☔ View full report in Codecov by Sentry. |
package.json
Outdated
"test:a11y": "gulp test:a11y", | ||
"test:integ": "gulp test:integ", | ||
"test:motion": "gulp test:motion", | ||
"gulp:integ": "NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\" gulp", |
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.
I already added this flag before:
components/build-tools/tasks/unit.js
Lines 19 to 23 in a35b753
runJest('build-tools', 'jest.build-tools.config.js', 'build-tools/**/__tests__/*.test.js', { | |
// stylelint rules require ESM | |
// https://stylelint.io/migration-guide/to-16#jest-preset-stylelint | |
NODE_OPTIONS: '--experimental-vm-modules', | |
}), |
How about doing the same way here, so even when running npx gulp ...
it still sets the expected options?
Also, I am not sure this line in our contributing docs will remain working:
Lines 155 to 156 in a35b753
# Run all input integration tests | |
npx jest -c jest.integ.config.js src/input |
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 approach, thanks. Updated the docs too.
bfa5bd3
to
2c5a0d5
Compare
Description
Adjust NodeJS config so that jest tests can cope with ESM (see https://jestjs.io/docs/ecmascript-modules). This is so that we can upgrade webdriverio, which is distributed as ESM in recent versions.
Related links, issue #, if available: AWSUI-51276
How has this been tested?
Tested in local workspace with upgraded version of webdriverio (cloudscape-design/browser-test-tools#86)
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.