Repository navigation
test: run the printer equivalence suite against Gecko in CI - #22155
Conversation
|
|
This PR is packaged and the instant preview is available (db556f3). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@db556f3
yarn add -D webpack@https://pkg.pr.new/webpack@db556f3
pnpm add -D webpack@https://pkg.pr.new/webpack@db556f3 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe syntax-equivalence workflow now runs Chrome, Firefox, and WebKit matrix entries. Browser launch supports Playwright-backed WebKit pages. Engine filtering, media handling, normalization, assertions, setup, and documentation were updated. ChangesCross-browser syntax-equivalence
Suggested reviewers: Priority: ⬇️ Low Change: Other Merge Risk: ⚪ Minimal · up to The expanded browser matrix and equivalence harness consistently select and run the intended browser-specific paths. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Full details: Title checkExplanation The title uses the allowed 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22155 +/- ##
==========================================
- Coverage 94.90% 94.90% -0.01%
==========================================
Files 733 733
Lines 103819 103819
Branches 31979 31979
==========================================
- Hits 98532 98530 -2
- Misses 5287 5289 +2
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 base (
Read No asset with an untouched test changed size — every change below comes with a test this pull request edited. 1 asset(s) changed size, test edited
No runtime gained or lost a runtime module. Built |
Merging this PR will degrade performance by 64.31%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "side-effects-reexport", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
155 KB | 903.7 KB | -82.84% |
| ❌ | Memory | unit benchmark "js-parser-unit", sourceType 'auto' |
3.5 MB | 4.7 MB | -25.73% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing test/equivalence-firefox-ci (8b57509) with main (4b2eed0)
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. ↩
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…ples Follow-up to #22151. `unsampledBy` treated every feature whose name ends in a viewport dimension as one the viewport bits already tell apart, so `device-width`, `device-height` and `device-aspect-ratio` — the screen's, which `setViewport` does not move — were dropped from the text and from the bits both, and two conditions naming different ones read as one. The media type is now read outside the parentheses as well, so a feature's own words cannot be taken for a type. A filed color rewrite is held exactly, the way every other filed set in the file is: a `FILED_COLOR_REWRITES` entry the engine stopped moving now fails rather than sitting there. It is counted before the within-a-byte tolerance so the tolerance cannot hide it, and the enumerated-fold comparison sorts both sides. Two JSDoc blocks had come apart from what they document: `installHelpers` had gained a second block instead of the `@param`, and `roundedOnly`'s had been left above `withinAByte`. `launchBrowser` and `install-firefox` read `PUPPETEER_CACHE_DIR` and build the path with `path.join`, so they agree on where the browser went; a caller's own `executablePath` is no longer overwritten, a platform puppeteer cannot detect reports no browser rather than throwing, and a failed install prints its error and exits non-zero. `TESTING_DOCS.md` still pointed at the deleted `launchChrome.js`.
`syntax-equivalence-firefox` runs the same corpus and the same assertions under `EQUIVALENCE_BROWSER=firefox`, beside the Chrome job rather than replacing it — a rewrite only one engine reads differently is invisible to a suite that asks one, and five of the defects filed today are ones only Gecko sees. It is a job of its own so the existing check keeps its name, installs the browser with `yarn setup:firefox`, and uploads no coverage: the Chrome job already reports this suite's lines under the `syntax-equivalence` flag. `unsampledBy` counted a feature once per occurrence, so `(monochrome) and (monochrome)` and the `(monochrome)` the printer folds it to read as two conditions where the viewport bits agreed. `and`, `or` and `,` are idempotent, so the parts are a set — its own documentation already said "named once".
The Chrome run was `syntax-equivalence` with nothing saying which browser it asked, while the engine added beside it said `-firefox` — so the pair read as a suite and an afterthought rather than as one suite over two engines. They are one matrix now, `syntax-equivalence (chrome)` and `syntax-equivalence (firefox)`, the shape `runtimes (bun)` and `runtimes (deno)` already use here. That also removes the copy of the job the second engine had brought with it: the corpus checkout, the three caches and both comparison installs were duplicated. Firefox is installed on its own row, Chrome reports the coverage (both cover the same lines, so a second uploader only leaves the report short), and `fail-fast: false` keeps one engine's failure from cancelling the other.
A matrix row carries the script and the coverage flag beside the browser, and the generated name prints every key — so the checks read `syntax-equivalence (chrome, cover:syntax-equivalence, true)`. An explicit `name` keeps the engine and drops the rest.
WebKit is the third engine and the one Safari ships, and puppeteer reaches Chrome and Firefox only — so `EQUIVALENCE_BROWSER=webkit` drives it through playwright, under a shim in `launchBrowser.js` presenting the nine calls the suites make. Playwright takes one argument where puppeteer takes several, and offers no `color-gamut`, so it declines media emulation outright and a condition carries what no viewport varies as text, the way Gecko already does. `forEngine` no longer assumes two engines: a reason opening `<engine> only:` is kept where it names the engine being run and dropped everywhere else, so a third filed set needs nothing further. The row is `continue-on-error` until its filed set is written from what it actually sees. Both other engines opened with hundreds of differences that were nearly all the harness rather than the printer, and the same is expected here; filing them before the first run would be inventing them.
The first WebKit run reported 24 differences over 5803 checks, and they are five causes. Each is filed with what was measured, so the row blocks like Chrome's and Firefox's rather than reporting past a CI flag. Three are the harness meeting a third engine. WebKit reads a different `document.compatMode` for the two forms of eleven files and the reverse for two, which a direction that flips per file says is how the document is built rather than what the printer wrote. It lays out in sixty-fourths of a pixel, so `112px` reads back as `111.984375px` in five framework stylesheets. And it serializes a computed number to six significant digits where Blink keeps more — which is the printer's own cap, so the whole-element comparison now reads both sides at that precision, Chromium excepted since the fixtures were measured against it. Two are the engine: no `fetchpriority` reflection, so `high` and `low` both read `auto`; no CSS Painting API, so a `paint()` image is dropped; and `shape-image-threshold` read as the number its percentage means. `attribute-tables` and `minimize-values` are filed by two engines for different causes now, so a reason may name more than one.
05894c2 to
8b57509
Compare
Types CoverageCoverage after merging test/equivalence-firefox-ci into main will be
Coverage Report |
Summary
#22151 made the equivalence suite able to ask Gecko, but nothing runs it, so the five defects only that engine sees are found by hand or not at all — this adds the
syntax-equivalence-firefoxjob beside the Chrome one. Following up on that PR it also fixes three harness defects it left:unsampledBytreateddevice-width/device-height/device-aspect-ratioas sampled by the viewport and counted a repeated feature twice, so distinct conditions collapsed and@media (monochrome) and (monochrome)read as different from the(monochrome)it folds to;FILED_COLOR_REWRITESwas tolerated rather than held exactly like every other filed set; and two JSDoc blocks had come apart from what they document.What kind of change does this PR introduce?
test
Did you add tests for your changes?
This is the test:
test/specCases/syntaxEquivalence.spectest.jsandtest/helpers/syntaxEquivalence.js, now run twice in CI.Does this PR introduce a breaking change?
No — nothing under
lib/changes.If relevant, what needs to be documented once your changes are merged or what have you already documented?
Documented here:
TESTING_DOCS.mdgainsEQUIVALENCE_BROWSER/yarn setup:firefox/PUPPETEER_CACHE_DIRand still pointed at the deletedlaunchChrome.js;AGENTS.mdnames both jobs as fetchingtest/wpt.Use of AI
AI was used. It reviewed #22151, reproduced each finding, and wrote the job and the fixes. The Gecko run itself could not be exercised where this was prepared — Mozilla's download hosts are blocked there — so the fallback branch was verified by forcing it on in Chrome over the same corpus, which is how the duplicate-feature defect was found; the first CI run of the new job is its first real run. I reviewed the result.
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation