Visual regression testing #427
Replies: 17 comments 31 replies
-
|
Additional Features (maybe via plugin) could be
|
Beta Was this translation helpful? Give feedback.
-
|
Looking into a possible compare UI here: https://webcomponents.dev/edit/yXxe3Z8mQ6uCfZXTFulA/src/index.ts Not sure how we'd want to attach "approve" workflows, especially for tests that run in CI, but would love to hear more of your thoughts on that! |
Beta Was this translation helpful? Give feedback.
-
|
uh that looks really nice 💪 for approval, I could see a workflow similar to how we use changeset now. That could look something like this
|
Beta Was this translation helpful? Give feedback.
-
|
@daKmoR what's the benefit of saving approved images not in git but to an external host? MDC were using google storage (report example) for their screenshots tests before version 5.0 (they have now moved to an internal infra screenshots tests). |
Beta Was this translation helpful? Give feedback.
-
|
There are couple of things that can go awry why working with in repo image caches:
|
Beta Was this translation helpful? Give feedback.
-
|
for a bigger list of existing solutions, you can check this: |
Beta Was this translation helpful? Give feedback.
-
|
@MathieuPuech thanks for the links, that's really helpful. When you store images in git, each change retains the old image in history not the diff between the two images. This blows up repository size pretty quickly. However we don't force users here, they can choose how to store the images. We should just make it possible to hook it up to whatever you want. |
Beta Was this translation helpful? Give feedback.
-
|
When you do visual regression testing, you can have some challenges with the render done by browsers in different environments.
A solution can be to use docker to run tests but it's slower and the developer needs docker installed. |
Beta Was this translation helpful? Give feedback.
-
|
these are awesome points - would have never thought about them 🙈 removing scrollbars and using a transparent caret I think are awesome solutions 👍 the others are really tricky 🤔
|
Beta Was this translation helpful? Give feedback.
-
|
I would recommend to check how existing libraries handle some of the problems listed above. In particular, it has |
Beta Was this translation helpful? Give feedback.
-
|
I used this library for comparison and by experience the ignoreCaret option is not always working. But maybe the antialiasing tolerance is better |
Beta Was this translation helpful? Give feedback.
-
|
i've already been through the pain of finding a good VRT solution so you might be interested in my findings so you don't have to:
i tried fiddling with chrome flags, matching OS versions, browser versions, etc. but there will always inevitably be some minor difference between two machines, especially a CI server and a local machine. so the font smoothing mismatch has proven to be a problem every time. in the end, i used percy (now owned by browserstack) with a storybook plugin for it. this means it just uploads a static storybook to percy, percy does the visual diffing and produces a report in their dashboard. this is one case where doing it myself wasn't worth it tbh. of course it could still be worth implementing a looks-same solution here to give users the flexibility, but in the end i think a service is the right way to go and sits outside your test runner anyway. |
Beta Was this translation helpful? Give feedback.
-
|
thanks for sharing your experience here we should definitely look into percy. I like reusing storybook stories inside tests, but limiting this to only tests is unfortunate |
Beta Was this translation helpful? Give feedback.
-
|
I created a first implementation of visual regression in WTR: https://github.com/modernweb-dev/web/tree/master/packages/test-runner-visual-regression This implements taking screenshots from the browser, and failing when there are diffs. We can use this to experiment and test, and decide on the followup actions from there. I split up the discussion in some different topics, which are linked in the first post in this thread. |
Beta Was this translation helpful? Give feedback.
-
|
Hi, would like to see the ability to pass image data instead of dom node. Currently i'm using jest together with https://github.com/Prior99/jest-screenshot to do visual regression for pdf (generated in client side) screenshots. This is a few features that keep me from ditching jest completely |
Beta Was this translation helpful? Give feedback.
-
|
What do you mean by image data? The actual images? Tests are executed in the browser while the screenshots are taken server side. |
Beta Was this translation helpful? Give feedback.
-
Yes. pdf is generated client side (pdfkit/pdfmake), converted to image (png) using pdf.js and compared using jest. Here's one example: https://github.com/foliojs/pdfkit/blob/master/tests/visual/helpers.js#L23-L25 pdf2png returns an array of images as buffer. In case of using browser, the image would be a Blob or Uint8Array |
Beta Was this translation helpful? Give feedback.
-
|
Hi, we started using this in our project and discovered an issue we would like fixed. We've created a PR here: #1068 |
Beta Was this translation helpful? Give feedback.
-
|
Created a project with an example config of running visual tests in Sauce Labs: |
Beta Was this translation helpful? Give feedback.
-
|
Configuration options that I'd be interested in seeing:
I'm happy to dive into adding these if they make sense. If we can coalesce around API structure, I can start by converting this conversation into issues for tracking. |
Beta Was this translation helpful? Give feedback.
-
|
I've added the following PR to further the discussion around the possible API changes/additions that would be associated with the above: #1339 |
Beta Was this translation helpful? Give feedback.
-
|
With a tool like Screener.io, new screenshots are marked as "new" and not accepted by default, which requires manual acceptance to add them to the baseline. |
Beta Was this translation helpful? Give feedback.
-
|
Which I think is the right path, something not being nothing, and new results should fail. If you want to hard update the baseline (which happens..., new project, major upgrade, etc.) then you can do so with the The PR above would make that the default here, too. |
Beta Was this translation helpful? Give feedback.
-
|
I really like the idea of not storing baselines somewhere but running base (e.g. master) and compare (e.g. new feature/bugfix branch) at the same time and comparing... this would reduce all the pain point for needing multiple baselines for each system/browser combination... It's true it would require more CPU time... but it would work out of the box and if CPU/CI time is your issue then you can think of adding caching (e.g. storing baselines) on top 🤔
https://twitter.com/WestbrookJ/status/1365725266291666949?s=20 |
Beta Was this translation helpful? Give feedback.
-
|
@daKmoR do you know of any work being done on this feature? |
Beta Was this translation helpful? Give feedback.
-
|
Put together an update to the comparison on non-size matched screenshots, PTAL: #1352 |
Beta Was this translation helpful? Give feedback.
-
|
I've been experimenting with adding visual regression testing and was hoping this would be the easiest solution but it doesn't seem to work. I'm running the test from the README exactly and it always fails with: Error: Protocol error (Runtime.callFunctionOn): Execution context was destroyed.
at /home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/Connection.js:208:63
at new Promise (<anonymous>)
at CDPSession.send (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/Connection.js:207:16)
at ExecutionContext._evaluateInternal (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:201:50)
at ExecutionContext.evaluate (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:107:27)
at ElementHandle.evaluate (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/JSHandle.js:102:46)
at ElementHandle._scrollIntoViewIfNeeded (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/JSHandle.js:280:34)
at ElementHandle.screenshot (/home/espeed/projects/rikaikun/node_modules/puppeteer/lib/cjs/puppeteer/common/JSHandle.js:599:20)
at processTicksAndRejections (internal/process/task_queues.js:93:5)
at async Object.executeCommand (/home/espeed/projects/rikaikun/node_modules/@web/test-runner-visual-regression/dist/visualRegressionPlugin.js:40:45)
at async TestRunnerApiPlugin._onCommand (/home/espeed/projects/rikaikun/node_modules/@web/test-runner/node_modules/@web/test-runner-core/dist/server/plugins/api/testRunnerApiPlugin.js:145:32)The only custom part of my config for this test is the browser setup required to run tests at all: browsers: [
puppeteerLauncher({
launchOptions: {
executablePath: '/usr/bin/google-chrome',
headless: true,
// disable-gpu required for chrome to run for some reason.
args: ['--disable-gpu', '--remote-debugging-port=9333'],
},
}),
],I haven't been able to find many details about that error on the web but maybe others have come across it here... |
Beta Was this translation helpful? Give feedback.
-
|
Yeah, I pushed a branch with a real test that works except for the (configs are all at the top level if they're important) I had uploaded it to see if it would work in the Github action environment in case the problem was my WSL2 local env being weird. (However, for some reason even more tests failed though it's hard to tell given #1549 log spam) As I said though, it fails with just the code sample too so I could probably make a branch for that as well. |
Beta Was this translation helpful? Give feedback.
-
|
I did some more debugging and got it to successfully not hang in a Github action though it fails with: I could probably figure out how to make an action which can also update images but I'm surprised since documentation implied that the absence of baseline image would lead to baseline creation:
Locally, it times out but the error I get is: which probably is due to whatever limitations WSL gives me. It might be nice to fail fast with good error message when that happens though. |
Beta Was this translation helpful? Give feedback.
-
|
On the first run it will fail when there is no baseline to compare to, even as it creates a baseline for the next test. This is to clarify whether the new image is approved to enter the cache or not. Once approved and saved to your cache the test will work against that baseline going forward. Re: "The Cache", the plugin in un-opinionated as to how you manage your own cache. A naive approach is to run once, commit the baseline images to your repo and then run again for regressions, though that means that your golden images are saved to your repo, which can, over time, increase the size of your repo, the speed at which this occurs depending on the size of your test suite. Other options, like saving in a separate repo, saving in an archive in your CI provider, etc. need to be handled by you in your project. The |
Beta Was this translation helpful? Give feedback.
-
|
Thanks; that makes sense; though I would have expected some kind of success message if it also saved the screenshots (like Re 'error handling', I had assumed it was a matter of wrapping the calls to the pupeeteer page object since it seemed to be called from the handler. Either way, WSL2 is known to have various problems when it comes to puppeteer and browsers so I'll either wait for more updates to the windows linux kernel or figure something else out. Given it seems to work on regular linux in Github CI environment it's probably something out of scope for the plugin at least. Thanks for the help and chiming in. |
Beta Was this translation helpful? Give feedback.
-
|
I worked on this some more and got it working locally as well: My project isn't so big so storing the images in Github is probably fine but reviewing diffs sounds like it will be a pain in Github; my next plan is to run this tool with I actually tried to integrate percy directly but it does a weird thing where it tries to reload the test page causing many things to break. 😅 Thanks again! |
Beta Was this translation helpful? Give feedback.
-
|
Hey! I've been evaluating this tool for use. I have experience using jest/puppeteer to create visual snapshots. One thing that I immediately miss from a jest setup is storing the snapshots adjacent to the test file (I like both tests and snapshots to be as close as possible to my components). So that the directory stucture could be like: Another aspect related to this is automatically generating image file names from some combination of test names. e.g. given this test: describe('my-component', () => {
it('passes some test', async () => {
const element = document.createElement('p');
element.textContent = 'Hello world';
element.style.color = 'blue';
document.body.appendChild(element);
await visualDiff(element);
});
});Then the file could be automatically named It seems the current extensibility points do not allow for this. Is there any plan to support this kind of structure/setup? |
Beta Was this translation helpful? Give feedback.
-
|
Hi, currently it is possible by passing Here is a code that we use to get path to a test file from it. The test can look like this: it('disabled', async () => {
element.disabled = true;
await visualDiff(div, `${import.meta.url}_disabled`);
}); |
Beta Was this translation helpful? Give feedback.
-
|
@web-padawan interesting! This has given me an idea... Since export interface GetNameArgs {
browser: string;
name: string;
+ testFile: string;
}I would be happy to make a PR for this. Also, your linked |
Beta Was this translation helpful? Give feedback.
-
|
Made a PR here #1657 |
Beta Was this translation helpful? Give feedback.
-
|
it could be nice if we can pass a clip of what we want to screenshot instead of doing a screenshot only of the element. Sometimes, you want to add some margin around your element to screenshot shadows and sometimes your element open an overlay you want to screenshot too (for example an opened menu/select) |
Beta Was this translation helpful? Give feedback.
-
|
here is a snippet I've done to get a clip of around 2 elements ( async function getClip({ page, selector, endClipSelector }) {
const clip = await page.evaluate(
([sel, endSel]) => {
const el = document.querySelector(sel);
if (!el) {
throw new Error(`An el matching selector \`${sel}\` wasn't found`);
}
const range = document.createRange();
range.setStartBefore(el);
range.setEndAfter(endSel ? document.querySelector(endSel) : el);
const rect = range.getBoundingClientRect();
const margin = 8;
const x = rect.left - margin < 0 ? 0 : rect.left - margin;
const y = rect.top - margin < 0 ? 0 : rect.top - margin;
return {
x,
y,
width: rect.width + (rect.left - x) * 2,
height: rect.height + (rect.top - y) * 2,
};
},
[selector, endClipSelector]
);
return clip;
} |
Beta Was this translation helpful? Give feedback.
-
|
I really like how customizable this plugin is through its options. Using that customization, I made it so when new screenshots fail, the "baseline" screenshot gets overwritten with the new screenshoot but the test still fails (the first time). This makes it very easy to update screenshots. If the new baseline is actually bad, that will get caught in the git diff or pull request code review. Since the first test run fails, CI pipelines will still fail on legitimate errors. |
Beta Was this translation helpful? Give feedback.
-
visualRegressionPlugin({
// update is set to false so that the test fails for the first time. The modification to getFailedName causes the file to get overwritten even though update is set to false
update: false,
baseDir: 'test-screenshots',
getBaselineName: (args) => {
return getTestFileName(args, '');
},
getDiffName: (args) => {
return getTestFileName(args, 'diff');
},
getFailedName: (args) => {
return getTestFileName(args, '');
},
}),And then function getTestFileName(args, type) {
// most of this is just picking a screenshot structure that I like
const screenshotDir = relative(
join(rootDir, 'src'),
args.testFile.replace(/\.[jt]sx?$/, ''),
);
// the real magic is here, where extension, and ultimately screenshotName, are identical for both failed and baseline files
const extension = `${type ? `${type}.` : ''}png`;
const screenshotName = `${
args.name
}.${process.platform.toLowerCase()}.${args.browser.toLowerCase()}.${extension}`;
const dirs = [
screenshotDir,
args.name,
type === 'diff' ? 'failure-diff' : '',
].filter((a) => a);
return join(...dirs, screenshotName);
}Here's my source code: https://github.com/electrovir/tdd-html-challenges/blob/main/.virmator/web-test-runner.config.mjs |
Beta Was this translation helpful? Give feedback.
-
|
That's a great insight. having baseline and failed names be the same is an easy way to get this functionality! Given source control makes diffs easy to see and I never want to check-in failed screenshots it seems like there's no downside with this unless you need to do offline processing. |
Beta Was this translation helpful? Give feedback.
-
|
Could you guys also share the github actions you are running? |
Beta Was this translation helpful? Give feedback.
-
|
Mine is a simple one that runs tests and then if there's a failure uploads new screenshots for comparison. Here's the relevant section: https://github.com/melink14/rikaikun/blob/5b835f5f40bcb01f50a47cfcc936a6eb6d35707c/.github/workflows/presubmit.yml#L42 |
Beta Was this translation helpful? Give feedback.
-
|
Thank you very much. That is enough inspiration to get me going :) |
Beta Was this translation helpful? Give feedback.
-
|
Hi, I keep getting |
Beta Was this translation helpful? Give feedback.
-
We should support visual regression testing with the test runner. Because we run tests in a real browser, it will be relatively easy to support this within the test runner. Most of the basic building blocks are already present.
Some different topics:
Beta Was this translation helpful? Give feedback.
All reactions