feat(webdriver-utils): add scaleToFit for full-page Automate captures (PER-10530) - #2388
Open
rishigupta1599 wants to merge 6 commits into
Open
feat(webdriver-utils): add scaleToFit for full-page Automate captures (PER-10530)#2388rishigupta1599 wants to merge 6 commits into
rishigupta1599 wants to merge 6 commits into
Conversation
… (PER-10530) Full-page POA captures are capped at a 50,000px stitched-image budget, which mobile-common enforces by truncating the DOM walk at MAX_PIXEL_HEIGHT_LIMIT/dpr -- 16,666 CSS px at DPR 3. Pages taller than that lose their tail silently. scaleToFit opts a capture into downscaling every tile by a fixed 1/dpr instead, so a page of up to 50,000 CSS px fits the same budget. The factor is fixed at construction and deliberately NOT derived from the measured page height: Percy diffs pixel to pixel and a uniform resize moves both dimensions, so a height-derived factor would make output WIDTH a function of page height. A page drifting across a threshold would then change width, leaving snapshot and baseline with different dimensions -- unalignable, not merely different. Two pieces here: - Schema entry on /config/snapshot. The section sets additionalProperties: false, so without a declaration the key is rejected before it ever reaches the Automate session. Gated onlyAutomate, like fullPage. - PERCY_SCALE_TO_FIT in addDefaultOptions(), so a whole run can opt in without editing per-snapshot config -- how support enables this for a customer hitting the truncation. Coerced to a real boolean, because mobile-common compares with `== true` and a truthy string would silently no-op. No provider changes: automateProvider already forwards `options` wholesale, and SeleniumHub passes it through verbatim, so the camelCase spelling is the contract with mobile-common. Requires browserstack/mobile-common#1255. Verified: webdriver-utils 240/240, core unit/config 21/21, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…metadata The previous commit plumbs the option INTO the Automate session. This carries the result back OUT: mobile-common reports `scale_to_fit` and `applied_scale_factor` in its response, but getTiles() built metadata from `screenshotType` alone, so neither ever reached percy-api. percy-api needs the factor to relax calculate_max_tiles_limit. A scaleToFit capture walks a taller page and therefore returns roughly 1/factor times the usual tile count -- so without this the extended walk is pointless: the page is rejected at the tile-count gate instead of being truncated upstream. Sending only a boolean would leave the API guessing a worst-case DPR, so send the magnitude that was actually applied. Both keys are added only when mobile-common reports it really scaled. They land in comparison_details.metadata -- a 1:1 child of comparisons and the largest table on the platform -- so the default path must not grow every row with constants. Verified: webdriver-utils 242/242, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment-only change, no behaviour difference. Keeps the load-bearing "why" on each -- why the schema entry is required, why the boolean is coerced, why the metadata keys are conditional -- and drops the surrounding exposition. Verified unchanged: webdriver-utils 242/242, core unit/config 21/21, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One block was still 3 lines after the previous pass. 242/242, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failed with "Cannot read property 'map' of undefined": PercyConfig.validate returns undefined when there are no errors, and onlyAutomate only errors when PERCY_TOKEN is set to a non-automate token. My local run had one set, CI does not -- so the spec asserted an absolute outcome that only held locally. Assert scaleToFit is treated the SAME as fullPage instead. Both are onlyAutomate, so the comparison holds whether the token is absent, web, or automate, and an undeclared key would still draw 'unknown property' where fullPage drew none. Verified passing with PERCY_TOKEN unset, web_*, and auto_*; and verified BOTH schema specs fail in all three states when the scaleToFit entry is removed, so neither can pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…te options percyAutomateRequestHandler builds the provider options by ENUMERATING each global snapshot config key, then merging per-screenshot options on top. Adding scaleToFit to the schema made `.percy.yml` accept it, but because it was not in that list it was silently dropped on the way to the provider -- the page truncated exactly as before while validation and the build stayed green. So of the two levels only per-screenshot actually worked. Both work now. The failure mode is the reason for the test: it asserts global-only, a per-screenshot override of a global true, and per-screenshot opt-in with global unset. Verified it fails when the merge line is removed, so it cannot rot. Verified: core utils 140/140, core unit/config 21/21, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Full-page POA captures are bounded by a 50,000px stitched-image budget. mobile-common enforces it by truncating the DOM walk at
MAX_PIXEL_HEIGHT_LIMIT / dpr— 16,666 CSS px at DPR 3 — so any page taller than that silently loses its tail. That is the PER-10298 / PPLT-5851 dealblocker (Adecco, Akkodis).scaleToFitopts a capture into downscaling every tile by a fixed1/dprinstead, so a page of up to 50,000 CSS px lands exactly on the same budget:page_height × dpr × (1/dpr) == page_height.Why the factor is fixed, not height-derived
Percy diffs pixel to pixel, and a uniform resize moves both dimensions. A height-derived factor would make output width a function of page height — so a page drifting from 16,600 to 18,000 px would change width, leaving snapshot and baseline with different dimensions: unalignable, not merely different.
Constant width forces a constant factor, and the ceiling forces
f ≤ 50000/(50000 × 3) = 1/3. The three constraints (constant width, ≤ 50,000px, coverage to 50,000 CSS px) are only jointly satisfiable atf = 1/dpr. It is also opt-in, so quality never changes for anyone who does not ask for it.Changes
packages/core/src/config.js— declarescaleToFiton the snapshot schema. That section setsadditionalProperties: false, so without a declaration the key is rejected before it ever reaches the Automate session. GatedonlyAutomate, exactly likefullPage.packages/webdriver-utils/src/providers/genericProvider.js—PERCY_SCALE_TO_FITinaddDefaultOptions(), so a whole run can opt in without editing per-snapshot config (how support would enable this for an affected customer). Coerced to a real boolean, because mobile-common compares with== trueand a truthy string would silently no-op.No provider changes needed.
automateProvider.getTiles()already forwardsoptionswholesale and SeleniumHub passes it through verbatim, so the camelCase spelling is the contract with mobile-common.Notes for review
Two things I got wrong first and corrected, worth knowing if you extend these tests:
/config/snapshot, not/snapshot— the latter$refs only a hand-picked subset and reports evenfullPageasunknown property. My first specs validated against/snapshotand passed vacuously.onlyAutomateis evaluated when AJV compiles the schema, not per-validation, so flippingPERCY_TOKENinside a spec cannot change the outcome. The specs now assert the declaration structurally plus the real validation error, and explicitly assert the message is notunknown propertyso they cannot pass vacuously again.Testing
packages/webdriver-utils: 240/240packages/coretest/unit/config.test.js: 21/21eslintclean on all four filesThe full
packages/coresuite does not pass in my environment — 155 failures, allFailed to launch browser / Timed out after 30000ms, i.e. Chrome cannot start here. Unrelated to this change, but I could not use it as a signal, so I ran the config specs in isolation.Depends on
browserstack/mobile-common#1255 — the capture-side implementation. This PR only plumbs the option; without it the option is accepted and ignored.
🤖 Generated with Claude Code