test(vrt): force-disable animations during VRT#2156
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates several visual regression tests by removing custom pixel difference thresholds and updating the corresponding PNG snapshots. It also introduces animation stabilization logic in the test helpers when updating snapshots. The reviewer recommends applying this animation stabilization logic to all visual regression test runs, rather than only when updating snapshots, to prevent test flakiness due to discrepancies between snapshot generation and verification.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
9fd0256 to
acf1fcc
Compare
spike-rabbit
left a comment
There was a problem hiding this comment.
What about just having a forceDisableAnimationsFlag?
This is only needed in rare cases.
Add least the overall job run seemed to be 30s slowser than before
|
Overall 30s slower isn't realistic, that from what, one run on unpredictable CI? Some timings, locally on M1 Max with 4 workers and 5 vCPUs in the VM:
So force-disable is 30s slower without shards. Combined 0.3 minutes slower, that 18s (that's like 6s per job) while making everything more reliable, i.e. what Playwright should do in the first place. So I would argue that is the best approach. And CI timings vary anyway so reliability over a few saved seconds. |
|
you are right, then lets do the combined one. |
acf1fcc to
9abfd56
Compare
|
Done. Slowest e2e job this run was 5m36s, so not bad at all. |
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: