-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Dependency Extraction Webpack Plugin: Fix test issues #57332
Conversation
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
Flaky tests detected in be0babc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7297949922
|
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.
It looks good to me. I can confirm that hash changes should change whenever the configuration changes.
What?
Fix test issues in the Dependency Extraction Webpack Plugin.
This also updates the test name and snapshots in preparation for #57199.
We add explicit options
target: 'web'
andexperiments: {}
. These do not change compilation (except for the hash), but are necessary for #57199.Why?
While working on #57199 I struggled with tests that would hang without explanation. This made debugging the issues very difficult. We address that by correctly surfacing the error and relying on a simpler structure of promises.
How?
The issue was because of the way we use webpack in the tests but don't correctly handle errors. Instead of passing an error to the callback, webpack was completing with the errors collected in its stats.
This PR makes the entire test async, and awaits a small promise that just wraps webpack and its callback.
We then check for errors in the webpack stats, and if they're found we throw an
Error
with the descriptive string produced by webpack.Testing Instructions
CI passes.
I suggest reviewing the changes ignoring whitespace to better see the actual differences.
The snapshots will have different test names ("scripts" is added to the describe title) and the hash will be different due to configuration changes.
This can be tested with a patch like this:
The tests will fail with a message like this: