Upgrade backstopjs to 5.3.2
#68
Merged
Conversation
This file contains 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
On backstopjs 5.1 and above we were seeing test failures with very minor rendering differences to the reference images, and also fonts not always loading. We are not the only people seeing these problems - see [[1]] and [[2]]. In those issues, the author of backstopjs suggests that this is actually an issue with puppeteer, and another user suggests adding these lines to `onReady.js` to force the engine to wait for the fonts to load and ensure the window is always in the same place before taking a screenshot. [1]:garris/BackstopJS#1303 [2]:garris/BackstopJS#1318
bjgill
approved these changes
Jun 18, 2021
alex9smith
pushed a commit
that referenced
this pull request
Jun 21, 2021
On backstopjs 5.1 and above we were seeing test failures with very minor rendering differences to the reference images, and also fonts not always loading. We are not the only people seeing these problems - see [[1]] and [[2]]. In those issues, the author of backstopjs suggests that this is actually an issue with puppeteer, and another user suggests adding these lines to `onReady.js` to force the engine to wait for the fonts to load and ensure the window is always in the same place before taking a screenshot. We already merged this in #68, however reverted it in #69 because we thought it hadn't fixed the problems. Some more research showed that the CSS problems with text overflow on mobile are actually a separate bug with the GOV.UK Design System[[3]]. Therefore we can merge this again and it'll hopefully fix the problems we can address and allow us to stay up to date with backstop. [1]:garris/BackstopJS#1303 [2]:garris/BackstopJS#1318 [3]:alphagov/govuk-frontend#2233
alex9smith
pushed a commit
that referenced
this pull request
Jun 21, 2021
On backstopjs 5.1 and above we were seeing test failures with very minor rendering differences to the reference images, and also fonts not always loading. We are not the only people seeing these problems - see [1] and [2]. In those issues, the author of backstopjs suggests that this is actually an issue with puppeteer, and another user suggests adding these lines to onReady.js to force the engine to wait for the fonts to load and ensure the window is always in the same place before taking a screenshot. We already merged this in #68, however reverted it in #69 because we thought it hadn't fixed the problems. Some more research showed that the CSS problems with text overflow on mobile are actually a separate bug with the GOV.UK Design System[3]. Therefore we can merge this again and it'll hopefully fix the problems we can address and allow us to stay up to date with backstop.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
On
backstopjs5.1 and above we were seeing test failures with very minor rendering differences to the reference images, and also fonts not always loading.We are not the only people seeing these problems - see [1] and [2]. In those issues, the author of
backstopjssuggests that this is actually an issue withpuppeteer, and another user suggests adding these lines toonReady.jsto force the engine to wait for the fonts to load and ensure the window is always in the same place before taking a screenshot.This should fix the issues we've been having. I've tested it locally against my local version of the DMp and have been unable to see the same problems over several runs. I've also tried running the tests locally against preview which also seemed fine. The real test, however, will be on Jenkins as that runs
backstopjsin the Docker container which will runpuppeteerslightly differently to on my laptop.