Skip to content

chore: stabilize vrt in pipeline#2115

Draft
spliffone wants to merge 8 commits into
mainfrom
chore/stabilize-vrt
Draft

chore: stabilize vrt in pipeline#2115
spliffone wants to merge 8 commits into
mainfrom
chore/stabilize-vrt

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented May 29, 2026

@spliffone spliffone requested review from a team as code owners May 29, 2026 13:23
@spliffone spliffone marked this pull request as draft May 29, 2026 13:23
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces changes to stabilize visual regression tests in Playwright by forcing deterministic software rendering, limiting workers on CI to prevent CPU saturation, and managing animations during screenshot capture. The review feedback points out a critical issue where the newly added 5000ms timeout in waitForAllAnimationsToComplete will consistently trigger and fail tests if the page contains infinite animations (like loading spinners). It is recommended to filter out infinite animations from the check to ensure robust test execution.

Comment on lines 259 to 263
await this.page.waitForFunction(
count => window.document.getAnimations().length <= count,
threshold
threshold,
{ timeout: 5000 }
);
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.

high

If the page contains any infinite animations (such as loading spinners, infinite progress bars, or animated icons), window.document.getAnimations() will always return a non-empty array. This will cause waitForAllAnimationsToComplete to consistently time out after 5000ms and fail the test.

To prevent this, we should filter out infinite animations (where iterations is Infinity) so that we only wait for finite transitions and animations to complete.

    await this.page.waitForFunction(
      count => window.document.getAnimations().filter(anim => {
        const iterations = anim.effect?.getTiming().iterations;
        return iterations !== Infinity;
      }).length <= count,
      threshold,
      { timeout: 5000 }
    );

spliffone added 7 commits May 29, 2026 16:47
…n examples

The waitForAllAnimationsToComplete settle-wait polls until
document.getAnimations() drains to zero, which never happens for examples
with infinite CSS animations (loading spinner, shimmer/typing indicators),
causing a 5s timeout and test failure in the VRT update job.

Add an opt-in disableAnimations option to StaticTestOptions that disables
CSS transitions/animations before the settle-wait so getAnimations()
drains and the wait passes. Enable it on the affected examples.
Add a shared valueInputSize computed in SiFilteredSearchValueBase and
bind it to the typeahead and free-text criterion inputs, so the editing
input width tracks its value (min 20) instead of the browser default.
Add a 20ch min-inline-size to the search input so flex cannot collapse
it to a sliver when criteria fill the row; overflow scrolling and
freeTextFocus keep it visible.
- Remove --disable-gpu / --in-process-gpu from chromeLaunchOptions: these flags
  switched rasterization from GPU to software rendering, invalidating every
  baseline snapshot that predates the change (typography, markdown-renderer,
  loading-button height diffs). The existing --disable-oop-rasterization and
  workers:isCI?2:4 are sufficient to keep rendering deterministic.
- Guard filtered-search keystroke race: after typing 'annover' assert that the
  Location combobox actually holds 'Hannover' before pressing Enter. Under CI
  CPU contention keystrokes can be dropped, causing the snapshot to capture
  a partial value ('Ha') and a layout shift of 2425px.
- Add disableAnimations:true to si-dashboard/si-value-widget static test: the
  value widget has an infinite CSS animation that prevents
  waitForAllAnimationsToComplete from settling, causing a 5 s TimeoutError.
… update workflow

npx playwright test --update-snapshots=changed exits 1 whenever it detects
and updates snapshot differences (the expected outcome of the VRT update job).
With bash -e the subsequent archive/upload steps were silently skipped, so
updated snapshots were never committed back. Add '|| true' to ignore the
expected non-zero exit; only fail if no snapshots exist at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant