Repository navigation
test(test262): assert two more errors webpack reports at build time - #21906
Conversation
|
|
This PR is packaged and the instant preview is available (d6e86c0). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@d6e86c0
yarn add -D webpack@https://pkg.pr.new/webpack@d6e86c0
pnpm add -D webpack@https://pkg.pr.new/webpack@d6e86c0 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21906 +/- ##
=======================================
Coverage 95.10% 95.10%
=======================================
Files 703 703
Lines 90747 90747
Branches 27390 27390
=======================================
Hits 86303 86303
Misses 4444 4444
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Generated code sizeComparing
No asset changed size. No runtime gained or lost a runtime module. Built |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughChangesTest262 classification
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change updates tests to cover behavior webpack already provides and does not modify production code or user-facing behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A module that fails to resolve is a build error whether or not the import is deferred, and assigning to a namespace import is one since #21867. Both were skipped as bugs; now the suite checks webpack reports them.
c56be98 to
6ff35e7
Compare
Merging this PR will regress 1 benchmark
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes |
alexander-akait
changed the title
Summary
Two entries in the test262 skip lists no longer describe what webpack does, so the suite was skipping behaviour it already has.
import-defer-of-missing-module-fails.jsandmodule-code/instn-star-binding.jsmove fromknownBugstolinkErrorsAtBuildTime, the existing list for cases where webpack — like rollup, esbuild and bun — resolves linking at build time and reports there rather than throwing at runtime. A module that fails to resolve is a build error whether or not the import is deferred, and assigning to a namespace import has been one since #21867. The suite now asserts webpack reports both instead of skipping them.What kind of change does this PR introduce?
test
Did you add tests for your changes?
This PR is only test changes —
test/test262.spectest.js. Both cases pass on this branch, across the development and production scenarios. Because membership oflinkErrorsAtBuildTimeis itself what turns onexportsPresence: "error", the classification would be self-confirming if checked through the suite alone, so both were also built with a realwebpack()under default options, where they error on their own:Module not found: Error: Can't resolve './missing.js'andAssignment to the imported binding 'ns'. A namespace import is immutable, ….Does this PR introduce a breaking change?
No. Nothing outside
test/changes.If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI (Claude Code) was used to re-probe the remaining skip-list entries with their filter removed, and to check each candidate against a real build rather than against the suite alone — which matters here, since the harness enables
exportsPresence: "error"forlinkErrorsAtBuildTimemembers and would otherwise make that classification self-confirming. Several candidates the first sweep called green were rejected on exactly that ground, and one further candidate — un-skipping the top-levelawait usingScript-goal case — was proposed, pushed, then reverted after CI disproved it: the local run had scored a hostSyntaxErrorfrom an older Node as the expected parse error. All findings and reasoning were reviewed by me before committing.