Repository navigation
test(javascript): run what the minifier writes for swc's execution tests - #22355
aryanraj45 wants to merge 2 commits into
Conversation
The readers for swc's fixtures, exec.rs and mangle.rs, and the probe of which options terser reads, move out of the js-minify suite unchanged, so the stdout suite can run the same tests. js-minify still passes all 232.
swc's exec.rs tests and its fixtures with an expected.stdout now run in the stdout suite under their own options and webpack's: 497 and 135 whose input terser's sandbox runs. 24 outputs print otherwise, each as terser's own does, so an output fails only where the reference's does not. A broken scope phase fails 18 runs here, unused 72.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: webpack/webpack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe test workflow now checks out the SWC corpus. Shared helpers read SWC cases and options, and the stdout suite checks eligible SWC cases alongside the existing Terser cases. ChangesMinifier corpus tests
Suggested reviewers: Priority: ⬇️ Low Change: Other Merge Risk: ⚪ Minimal · up to This change only extends test coverage: webpack's minifier output is now also checked against SWC's execution tests, and CI checks out the SWC corpus for that job. Production behavior is unchanged, and no merge-blocking issue was found. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The added execution is confined to the CI test flow and uses its existing sandbox. The external corpus is pinned and the job has read-only repository permissions. The sandbox’s isolation could not be independently verified, so the risk is not assessed as minimal. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Full details: Title checkExplanation The title uses valid Conventional Commit syntax with the allowed type Warning Some tools did not complete. Review the errors below. 🔧 ESLint
test/helpers/swcMinifierTests.jsESLint failed to execute (timeout). test/specCases/js-minify-stdout.spectest.jsESLint skipped: the matched ESLint configuration already failed (timeout). test/specCases/js-minify.spectest.jsESLint skipped: the matched ESLint configuration already failed (timeout). 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 |
|
Reported by Types Coverage #239 for 4af7359, computed by this pull request's own code. Types CoverageCoverage after merging test/js-minify-swc-exec into main will be
Coverage Report |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22355 +/- ##
==========================================
+ Coverage 94.50% 95.74% +1.23%
==========================================
Files 767 770 +3
Lines 109227 109487 +260
Branches 34138 34199 +61
==========================================
+ Hits 103230 104825 +1595
+ Misses 5997 4662 -1335
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:
|
|
Reported by Code Size #3231 for 4af7359, computed by this pull request's own code. Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "asset-modules-source", scenario '{"name":"mode-production","mode":"production"}' |
5.9 MB | 8.4 MB | -29.92% |
| ⚡ | Memory | unit benchmark "js-parser-unit", source 'lodash-es' |
305.8 KB | 106.5 KB | ×2.9 |
| ⚡ | Memory | benchmark "css-modules", scenario '{"name":"mode-development","mode":"development"}' |
12.3 MB | 8.9 MB | +38.23% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing aryanraj45:test/js-minify-swc-exec (4af7359) with main (d7f7e56)
Footnotes
-
6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
Refactored and shipped in #22389 (building on #22352), thanks @aryanraj45! Generated by Claude Code |
Summary
The stdout suite now runs swc's execution tests as well as terser's. There are two sets. The first is the tests in
exec.rs: swc's harness runs each input with node and requires the minified output to print the same. The second is the fixtures that carry anexpected.stdout, leaving out swc's port of terser's own tests, which the suite already runs. Each test is minified by webpack under its own options and under the options webpack ships with (compress: { passes: 2 }, mangle: true), run in terser's sandbox, and has to print what its input prints.Of the 502 exec tests, the sandbox runs 497 inputs; the other 5 throw or print nothing there. Of the 150 fixtures, 135 print their
expected.stdoutin the sandbox. Of the other 15, 13 read what only node gives them, such asprocessor a timer, and 2 are modules. A floor on each count keeps a reader that stopped matching from passing quietly.24 outputs print something other than their input. Published terser's output prints the same thing in every one of them, so they are terser's own miscompiles, and webpack adds none. An output therefore fails only where the reference's output does not fail the same way. The suite takes about 30 s, up from 25 s.
The readers for swc's fixtures,
exec.rsandmangle.rs, and the probe of which options terser reads, move unchanged from the js-minify suite intotest/helpers/swcMinifierTests.jsso both suites share them. The js-minify leg still passes all 232 tests. Thejs-minify-stdoutCI leg now also checks out swc.What kind of change does this PR introduce?
test
Did you add tests for your changes?
Yes, the corpus is the test. I broke the checks and each phase on purpose, then restored them:
eval's scopes fails 18 runs here, where terser's corpus fails 5 files.Does this PR introduce a breaking change?
No. Only tests and CI change.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI was used. I found the execution tests in swc's tree and reproduced how its harness runs them. It checked each of the 24 differing outputs against published terser, measured how many inputs terser's sandbox can run, and broke the new phases one by one to measure what each corpus catches. I reviewed the result.
Summary by CodeRabbit