garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.67k stars 605 forks source link

Inconsistent behaviour after viewport resize #1449

Open DEfusion opened 1 year ago

DEfusion commented 1 year ago

I have a couple of scenarios that have multiple viewports, sometimes it captures them after the resize a little too fast not giving the CSS enough time to be repainted. Sometimes it doesn't, but it happens more on CI, with us using docker on all environments.

I can replicate the behaviour in the browser by manually resizing in responsive mode and see the flash that I sometimes get in the backstop failures. I've tried increasing delay on the scenarios to ridiculous values but it doesn't seem to apply after the viewport resize, just on the first take i.e. if I have 3 viewport definitions and a delay of 10 seconds I'd expect the capture to take ~30 seconds but it only takes 10 seconds.

Is it possible to make the delay apply after the viewport has been resized or should there be another option? I'm happy to submit a PR for either of these if they seem reasonable.

martiniert commented 1 year ago

same problem here, I need a delay after resizing, especially when debouncing/throttling methods, the screenshot is (sometimes) taken too fast while the page is still rerendering.

Something like this would most probably fix it:

await page.setViewport({ width: VP_W, height: VP_H });
+ await page.waitForTimeout(1000);
rossjrw commented 1 year ago

This is very weird - I'm seeing errors in my screenshots that look exactly like they could be being caused by the above, but looking at the code, it doesn't make any sense.

The renderer (either runPuppet or runPlaywright) is called once per 'scenarioView' which is a combination of a single scenario and a single view:

https://github.com/garris/BackstopJS/blob/7fc512c4168bfbe6e5c8b8d072f5d4be4c3d374d/core/util/createBitmaps.js#L114-L121

https://github.com/garris/BackstopJS/blob/7fc512c4168bfbe6e5c8b8d072f5d4be4c3d374d/core/util/createBitmaps.js#L133

For Puppeteer at least, not only is each test run using a single scenario and a single viewport, but each test uses its own browser instance altogether. The viewport size never actually changes mid-test, assuming you're just using the scenarios and viewports parameters in the config and not doing anything yourself.

Which is really weird, because I swear the errors I'm seeing really do look like they could be explained by that.


Update: I've been able to resolve this on my end.

I needed to disable CSS transitions which could have been causing layout shifts to be delayed until after the screenshot had actually been taken:

/* backstop_data/engine_scripts/backstop.css */
* {
  /* Remove transitions outright where possible */
  transition: none !important;
  /* Otherwise (e.g. when the original has !important) try to make it a noop */
  transition-duration: 0 !important;
  transition-delay: 0 !important;
}
// backstop_data/engine_scripts/puppet/onReady.js
const fs = require("fs")

module.exports = async (page, scenario, vp) => {
  // Disable transitions
  await page.addStyleTag({ content: fs.readFileSync("backstop_data/engine_scripts/backstop.css", "utf-8") })

  // Prompt lazy-loaded images to load
  await page.evaluate(() => document.querySelectorAll("img[loading=lazy]").forEach(img => img.loading = "eager"))

  await page.waitForNetworkIdle({ idleTime: 250 })
}

Based on my assessment of the Backstop engine above, which I still think is accurate, I have no idea how this fix could possibly help... but it did for me!