Try: Visual regression test coverage for Cover block #28554
Conversation
ockham
added
[Status] In Progress
| @@ -0,0 +1,13 @@ | |||
| /** | |||
There was a problem hiding this comment.
|
Size Change: +1.81 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
|
I wish I could run these locally to generate the initial snapshot, but unfortunately, e2e tests are locally failing with @vindl seems to experience something similar. Curious what's wrong 🤔 |
I found the answer 😅 and posted it here. |
|
FYI @tellthemachines 👋 (b/c WordPress/wordpress-develop#209) |
|
We better don't include images in the repository. It's huge already. You don't want to download all those data on every git clone run. 😅 |
Well, that's basically how We could probably do some contortions to run the tests against |
|
I don’t think you should be worried about the general approach here regarding how the tests work. It’s mostly about how the process of storing and comparing visual changes could work. The tools I listed store images externally and provided advanced UI with workflows that help the team to decide whether proposed visual changes are intended. Anyway, for v1, we can use the approach you have. In the long run, it won’t scale because every change to standard UI components will trigger updates for existing image snapshots, and since those are binary files, it will replace all the content. Could we maybe use another repository to keep those images or some special instance of WordPress where every post links to the GitHub PR? I’m just sharing ideas on how to mitigate this issue. We already store all images used in documentation outside of the Gutenberg repository, but still, we see reports that people have to download over 1 GB of data to run Gutenberg locally (npm + git) 😅 |
This is the approach I'm taking for Core, not because of storage (though that definitely should be taken into account), but to solve the problem of cross-environment false positives. Even a very basic snapshot such as the editor one in this PR is failing on CI, and I can reproduce that failure locally. I think the only reason the front end snapshot doesn't fail is it's a monochrome image 😅 I tried fiddling with all the settings: image size, failure thresholds, even blurring the images, and wasn't able to come up with a way of getting the tests to pass on CI. So for Core, the plan is to run the tests on trunk to generate the snapshots and then again on the feature branch. For local runs, it'll be done manually for now, but might be nice to automate it sometime in the future. Note: in my PR, I set up the visual regression tests as a separate task from the e2e tests, so that we can have the choice of not running them for changesets that don't touch anything browser-rendered, because the tests are pretty slow to run, but that probably doesn't make sense with the approach in this PR. |
ockham
force-pushed
the
add/visual-diff-testing
branch
2 times, most recently
from
8bc8924 to
253fa50
Compare
|
Hey @tellthemachines 👋 Thanks for chiming in!
Interesting, the visual regression test added by my PR seemed to pass reliably on CI at first, but lately, it always fails 😕 I had a look at the visual diffs from the latest run (ZIP file, look for Editor:
Frontend:
FWIW, I've been more inclined to co-locate and couple visual regression tests with e2e tests rather tightly, since it turns out that we need to use quite a few e2e tools to insert and customize the block. Maybe indvidual |
ockham
force-pushed
the
add/visual-diff-testing
branch
from
253fa50 to
5311460
Compare
Hmm, we're using system fonts so that makes sense; I'm getting much the same diff when running them locally. This is another argument for building a custom theme to use in e2e tests 😄 With the Core tests, I got all sorts of diffs when comparing local snapshots with CI-generated ones, but we're testing whole pages there so there'll always be a higher chance of failure. I'd say let's give this approach a go (assuming we can work around the font issue), and if it starts failing enough to be annoying we can scrap it and try something else. |
|
In my past experience, the font difference is always going to happen even if we use the same font. The problem is that different OS have different font rendering engine and will produce slightly different outputs. The solution is to always run the tests in the same environment, i.e. a docker instance. This is definitely going to be tedious in local development though, not sure how we can do that. Another possible solution is to use external tools like Chromatic or Percy. |
|
Chromatic might be in theory free for WordPress. At the bottom of the
|
|
Chromatic is heavily coupled with Storybook though. Depending on what we want to test, it might not be the perfect fit for now. But if our goal is to create more stories then it should be the recommended solution. AFAIK, there aren't many stories for blocks. We can start by adding a story for the cover block and see if it makes sense? |
|
I've looked around a bit, it seems like lack of cross-platform, pixel-perfect rendering is a problem for pretty much everyone attempting to use puppeteer for visual regression testing (see puppeteer/puppeteer#661). One recurring mitigation strategy was to set Chrome's I'll give that a quick try. For the record, currently, our snapshots differ by 887 (editor) and 486 (frontend) pixels, respectively. |
Still at 887/486. It also didn't make a change when I ran it locally. I'm starting to wonder if puppeteer is respecting |
|
Your answer is here: Line 251 in 0105c9a |
Thanks! That points me to which then takes me to this file: https://github.com/WordPress/gutenberg/blob/0105c9a2312272d768f1ecb77c0b9e6dca77d0a0/packages/scripts/config/jest-e2e.config.js But that's a Jest config, not a puppeteer config 🤔 How can I pass the puppeteer arg through that? Unless you meant the gutenberg/packages/scripts/scripts/test-e2e.js Lines 42 to 47 in 0105c9a |
|
Right, there are two config files, and both of them can be overridden. I think you are changing the correct file. |
Description
Add basic visual regression testing for the Cover block by taking a screenshot of the block (and nothing else) both in the editor and on the frontend, and comparing the screenshots to image snapshots stored in git.
Visual regression testing is added to the exisiting end-to-end test for the Cover block. This seems to make sense to me and is analogous to how 'textual' snapshot testing is currently done in unit tests: In both cases, some preparatory steps are required to create the conditions under which the snapshot can be produced (and then compared to the stored one).
If the snapshot is found to differ from what is stored in git, a visual diff is created. When run in CI (i.e. via a GitHub Actions workflow), that visual diff is included with the workflow's artifacts.
Uses Jest Image Snapshot (h/t @johngodley) which basically adds a new
expect(image).toMatchImageSnapshot()assertion to Jest. Combining this with Puppeteer's ability to take a screenshot, this fits in very nicely with our existing test framework.Fixes #28477.
How has this been tested?
Locally:
Visual diffs
The following diffs were created by the test by checking out
packages/block-library/src/cover/style.scssfrom a commit (3de1114) that was known to suffer from a visual regression. To reproduce, run the following while on this branch:This gives
(plus the same error for the frontend, omitted for brevity).
Refer to the error message to locate the generated visual diffs.
In each of the diffs, the expected image is on the left, the actual image is on the right, and the diff is shown in the middle (differences highlighted in red):
Editor
Frontend
Possible future additions:
Cover block: Add visual regression tests for different alignments, and different themes, to prevent the kind of regressions documented in #28404 (comment).
Prior art / related projects
Prior art (using systems hosted on 3rd party servers) (h/t @gziolo):
And finally, there's a
wordpress-developPR to add visual regresston testing to Core by @tellthemachines.