Skip to content

Fix an intermittent failure in the reorganize pages integration tests#21415

Open
calixteman wants to merge 1 commit into
mozilla:masterfrom
calixteman:issue21403
Open

Fix an intermittent failure in the reorganize pages integration tests#21415
calixteman wants to merge 1 commit into
mozilla:masterfrom
calixteman:issue21403

Conversation

@calixteman

Copy link
Copy Markdown
Contributor

waitForHavingContents shrinks to the minimum scale in WRAPPED mode so that every page is visible and renders its text layer. Since tests run with defaultViewport: null the window size is OS-dependent, so for the cut/copy tests (15-17 pages) some pages could stay outside the viewport and never render, making the helper wait forever.

It fixes #21403.

`waitForHavingContents` shrinks to the minimum scale in WRAPPED mode so that
every page is visible and renders its text layer. Since tests run with
`defaultViewport: null` the window size is OS-dependent, so for the cut/copy
tests (15-17 pages) some pages could stay outside the viewport and never
render, making the helper wait forever.

It fixes mozilla#21403.
@codecov-commenter

codecov-commenter commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (52a44a6) to head (350de91).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #21415   +/-   ##
=======================================
  Coverage   89.66%   89.66%           
=======================================
  Files         260      260           
  Lines       66010    66010           
=======================================
+ Hits        59188    59191    +3     
+ Misses       6822     6819    -3     
Flag Coverage Δ
integrationtest 66.77% <ø> (-0.01%) ⬇️
unittestcli 56.06% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

// Only visible pages render their text layer: enlarge the viewport so that,
// once shrunk to the minimum scale in WRAPPED mode below, all the pages fit
// on screen even for documents with many pages.
await page.setViewport({ width: 1500, height: 1500 });

@timvandermeij timvandermeij Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit unexpected to change the viewport size in the scope of a helper function. Wouldn't it be better to set a large enough viewport size in general for the tests then?

Alternatively, maybe we can find a better way to make sure all pages are rendered so we don't need to have this workaround of setting a custom viewer scroll mode and page scale altogether? For example we might be able to add a TESTING-only option that forces text rendering of all pages and emits an event if it's done that we can listen for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent failure in the should cut pages with Ctrl+X and paste them reorganize pages integration test

3 participants