test: make them stable#20898
Conversation
🦋 Changeset detectedLatest commit: b91111d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (25f937c). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@25f937c
yarn add -D webpack@https://pkg.pr.new/webpack@25f937c
pnpm add -D webpack@https://pkg.pr.new/webpack@25f937c |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes. Additional details and impacted files@@ Coverage Diff @@
## main #20898 +/- ##
==========================================
+ Coverage 91.29% 91.31% +0.02%
==========================================
Files 562 562
Lines 55615 55661 +46
Branches 14705 14719 +14
==========================================
+ Hits 50772 50829 +57
+ Misses 4843 4832 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| expect(link.href).toBe("https://example.com/public/path/chunk1-a.js"); | ||
| expect(link.crossOrigin).toBe("anonymous"); | ||
|
|
||
| const promise2 = import( |
| expect(link.href).toBe("https://example.com/public/path/chunk1-a.js"); | ||
| expect(link.crossOrigin).toBe("anonymous"); | ||
|
|
||
| const promise2 = import( |
There was a problem hiding this comment.
Pull request overview
This PR updates several config-case tests around dynamic imports/prefetch/preload behavior and unhandled-rejection handling. However, the PR title/description indicate a CI change to apply ulimit -n on macOS integration tests, and no such CI/workflow change is present in the reviewed diffs.
Changes:
- Convert
prefetch-preloadtests fromreturn promise.then(...)toasync/awaitfor clearer sequencing. - In the ESM JSONP variant,
await import(/* webpackIgnore */ ...)before firingscript.onload()to avoid a microtask ordering race on older V8. - Add a
.catch(() => {})to a top-level dynamic import in the manifest-plugin config case to prevent unhandled rejections on older Node versions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/configCases/web/prefetch-preload/index.js | Refactors the test to async/await while preserving the existing assertions and sequencing. |
| test/configCases/web/prefetch-preload-module-jsonp/index.mjs | Adds await for webpackIgnore imports to stabilize ordering before script.onload(); converts the rest of the test to async/await. |
| test/configCases/plugins/manifest-plugin/index-2.js | Catches a top-level dynamic import to avoid unhandled rejections in the test harness. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The .mjs test runs through vm.SourceTextModule's importModuleDynamically callback. Whether that host callback executes synchronously during the import() expression or is queued as a microtask is V8-version-dependent. On Node 22+ V8 it runs synchronously, so the chunk's webpackChunk.push lands before the next-line script.onload(). On Node 10/12/14/16/18/20 V8 the callback is deferred — script.onload() then runs first, sees installedChunks[id] still as [resolve, reject, promise], and the JSONP runtime rejects with "Loading chunk chunk1 failed". await the import(webpackIgnore) so the chunk install is observed before script.onload() regardless of V8 version, and flatten the nested then() chain into async/await.
Mirror the structure of prefetch-preload-module-jsonp so the test no longer relies on a nested .then() chain that's hard to reason about across V8 versions. The CJS variant uses __non_webpack_require__ which is synchronous, but the awaited form makes microtask ordering explicit and matches the rest of the prefetch-preload tests.
The top-level import("./file.txt?foo") exists so ManifestPlugin records
the chunk in the emitted manifest; nothing awaits it, and on target:web
the JSONP runtime never invokes script.onload in this harness so the
promise sits in [resolve, reject, promise] forever. On older Node the
unhandled-rejection plumbing differs, and any eventual rejection (e.g.
through the chunk-load timeout path) surfaces as a test failure.
.catch keeps the chunk creation while making the dangling promise inert.
c1d4245 to
881900d
Compare
The trailing `import("./chunk1.css")` / `import("./chunk2.css")` in
prefetch-preload-module-jsonp and prefetch-preload each install a
stylesheet link and a <script> for the JS half. The harness never
invokes those scripts' onload, so the JSONP runtime sits on a
setTimeout(onScriptComplete, chunkLoadTimeout=120000ms). On Node 10
the integration suite runs longer than 120s, the timer fires while a
later test is executing, loadingEnded builds
ChunkLoadError: Loading chunk 945 failed.
(timeout: https://example.com/public/path/chunk1-css.js)
and the unhandled rejection lands inside the unrelated test
(TestCasesProdGlobalUsed > coffee-loader > should compile). Newer Node
finishes the suite before 120s elapses, which is why this only shows
up on old Node. Attach a no-op catch so the eventual rejection is
observed and dropped.
When __webpack_require__.e iterates ensureChunkHandlers, the prefetch trigger handler creates Promise.all(promises).then(...) and returns it, but the reduce in EnsureChunkRuntimeModule discards return values. If the chunk load rejects (chunkLoadTimeout, network error, etc.) that discarded chain has no rejection handler and surfaces as an unhandled rejection — which on Windows + Node 10 lands as 'Test suite failed to run' in jest-circus, blowing up unrelated suites. Attach a no-op rejection handler to the .then. Prefetch is best-effort (browsers don't surface link-prefetch errors either), so silencing the chain matches user expectation.
Types CoverageCoverage after merging claude/improve-macos-ci-performance-J0ne8 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
make tests more stable