jest-image-snapshot
jest-image-snapshot copied to clipboard
Error: "Error running image diff." for large images
Hey,
for some of my tests I get the error Error running image diff..
As those are the longest pages e.g. 14252px I assume the length is the deciding factor.
Other pages work fine. The diff file was also created and is in the folder.
Can I do something to fix this (e.g. is there a config option I missed)?
Not sure, have not seen this one before. Can you try to come up with a test to easily replicate and see if there is something we can do? Likely this is an issue downstream with pixelmatch but we could take a look.
Hey, sure here is a test case that replicates the error. I hope that helps. Please let me know if anything in my code does not make sense and I can test if changes there fix it.
const puppeteer = require('puppeteer')
const { toMatchImageSnapshot } = require('jest-image-snapshot')
let browser
let page
beforeAll(async () => {
// start Puppeteer with a custom configuration, see above the setup
browser = await puppeteer.launch({
ignoreHTTPSErrors: true,
headless: true,
args: ['--no-sandbox', '--disable-setuid-sandbox'],
})
expect.extend({ toMatchImageSnapshot })
// load page
page = await browser.newPage()
await page.goto(`https://www.theverge.com/`, {waitUntil: 'load'});
// scroll to bottom and back up
await page.waitFor(1000)
await page.evaluate(() => {
window.scrollTo(0, Number.MAX_SAFE_INTEGER)
})
await page.waitFor(500)
await page.evaluate(() => {
window.scrollBy(0, 0)
})
await page.waitFor(500)
})
test(`Testing viewport: 1400`, async () => {
// set viewport
await page.setViewport({
width: 1400,
height: 900,
deviceScaleFactor: 1
})
// take full screen screenshot
let image = await page.screenshot({
path: `${__dirname}/test_snaps/verge-1440.png`,
type: 'png',
fullPage: true
})
// compare screenshot
expect(image).toMatchImageSnapshot({
customDiffConfig: {
threshold: 0.01
},
customDiffDir: `${__dirname}/test_snaps/verge-1440.png`,
customSnapshotsDir: `${__dirname}/baseline/`,
customSnapshotIdentifier: `verge-1440`,
noColors: true
})
}, 15000)
afterAll(async () => {
await browser.close()
})
Hi, this is happening to me to a lot. Curious though I cannot reproduce this issue locally, only when running on CI environment through TeamCity build pipeline
Hey, sure here is a test case that replicates the error. I hope that helps. Please let me know if anything in my code does not make sense and I can test if changes there fix it.
Are you able to print a stack trace of where it is throwing?
Hey @omnisip what I get is the following:
Testing Page: privacy › Testing viewport: desktop-extra-large
Error running image diff.
67 | })
68 | // compare screenshot
> 69 | expect(image).toMatchImageSnapshot(config.setConfig({
| ^
70 | filename: `${viewport}`,
71 | snapshotPath: `${config.basePath}/baseline/${currentCase.folder}`,
72 | diffPath: `${config.testSnaps}/${currentCase.folder}`
at runDiffImageToSnapshot (node_modules/jest-image-snapshot/src/diff-snapshot.js:277:11)
at Object.toMatchImageSnapshot (node_modules/jest-image-snapshot/src/index.js:194:7)
at tests/integration/ui.baseTest.js:69:19
Does that help?
Yes. Very helpful.
There's an option to run jest image snapshot in process.
Can you add runInProcess: true as soon of the options you pass toMatchSnapshot, and paste the next stack trace here as well?
On Wed, Jul 1, 2020, 00:33 Lukas Oppermann [email protected] wrote:
Hey @omnisip https://github.com/omnisip what I get is the following:
Testing Page: privacy › Testing viewport: desktop-extra-large
Error running image diff.
67 | })
68 | // compare screenshot
69 | expect(image).toMatchImageSnapshot(config.setConfig({
| ^70 | filename:
${viewport},71 | snapshotPath:
${config.basePath}/baseline/${currentCase.folder},72 | diffPath:
${config.testSnaps}/${currentCase.folder}at runDiffImageToSnapshot (node_modules/jest-image-snapshot/src/diff-snapshot.js:277:11)
at Object.toMatchImageSnapshot (node_modules/jest-image-snapshot/src/index.js:194:7)
at tests/integration/ui.baseTest.js:69:19
Does that help?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-652221204, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJXNA3SNDRIRW3TDAA3RZLKC5ANCNFSM4NE6LOWQ .
Hey @omnisip, running with runInProcess: true takes a loooot longer but does not actually produce this error (and thus no stack trace).
Does that make sense?
Hmm...
On Fri, Jul 3, 2020, 02:09 Lukas Oppermann [email protected] wrote:
Hey @omnisip https://github.com/omnisip, running with runInProcess: true takes a loooot longer but does not actually produce this error (and thus no stack trace).
Does that make sense?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-653414792, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJSTVGF7NHMEQH7ILM3RZWG5BANCNFSM4NE6LOWQ .
Hey @omnisip, running with
runInProcess: truetakes a loooot longer but does not actually produce this error (and thus no stack trace).Does that make sense?
Yes. I wonder if you're running out of memory in the sub process running jest-image-snapshot.
Try rerunning with the original settings but cut the jest concurrency to half or a quarter of what it currently is.
Hey @omnisip could you please tell me how I can reduce concurrency in jest?
--maxWorkers=50%
On Sun, Jul 12, 2020, 06:01 Lukas Oppermann [email protected] wrote:
Hey @omnisip https://github.com/omnisip could you please tell me how I can reduce concurrency in jest?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-657212373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJWVWB2LDWDQAE3IRGTR3GQYNANCNFSM4NE6LOWQ .
Sadly even with --maxWorkers=10% I still get the error for some tests. (I test with 50% first and had the same issue.)
And just for grins, can you set --maxConcurrency=1 as well?
On Mon, Jul 20, 2020, 06:01 Lukas Oppermann [email protected] wrote:
Sadly even with --maxWorkers=10% I still get the error for some tests. (I test with 50% first and had the same issue.)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-660983994, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJSOXZEMN23X3FKFOQLR4QWYBANCNFSM4NE6LOWQ .
We're going to fix this if we can. I'll submit a patch today if we can get to the bottom of it.
So even with --maxConcurrency=1 I get the issue.
What time is it there? Would you have time for a screenshare in 45 minutes to an hour?
Sadly not, sorry. It's getting late and I have to get up really early. Is there anything more I can provide as info?
- I am running on a macbook pro 16", 16GB on Mojave.
- latest jest & puppeteer version
- [email protected]
Yeah. You provided a test case, but it's not integrated into the jest image snapshot test suite.
Are you able to provide that test as a patch so we can play with it?
On Mon, Jul 20, 2020, 13:25 Lukas Oppermann [email protected] wrote:
Sadly not, sorry. It's getting late and I have to get up really early. Is there anything more I can provide as info?
- I am running on a macbook pro 16", 16GB on Mojave.
- latest jest & puppeteer version
- [email protected]
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-661286687, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJTC7WNNF7SOPNKPNX3R4SKZFANCNFSM4NE6LOWQ .
What does "as a patch" mean? Do you mean a "PR" to this repo with my test case?
The original is here: https://github.com/lukasoppermann/veare/tree/master/tests/integration
Yes. A PR would be perfect.
On Mon, Jul 20, 2020, 13:41 Lukas Oppermann [email protected] wrote:
What does "as a patch" mean? Do you mean a "PR" to this repo with my test case?
The original is here: https://github.com/lukasoppermann/veare/tree/master/tests/integration
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-661294035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJT7QTILPJR6HIJAZYDR4SMVNANCNFSM4NE6LOWQ .
Hey, sorry it took a moment, but now I got a case showing the error: https://github.com/americanexpress/jest-image-snapshot/pull/227
Hey Lukas (@lukasopperman),
Np. Thanks for submitting this. Before I do the deep dive, can you answer these questions?
-- Quick Questions/Comments --
- Is Puppeteer version 5 a hard requirement?
- Do you usually test against live sites?
I'm curious because Puppeteer 5 might become a breaking change for the library, and we can't keep live sites as part of the standard test suite. However... using jest-image-snapshot to do integration testing of live sites is a really cool use case.
Dan
This is failing before it finishes writing back to the parent process. So it's a genuine bug.
Do you mind if we retain a copy of your large image for the purposes of fixing it and adding it as a test case?
The particular issue is that the output size of the stringified test failure from diff-process is 16205416 bytes which is much larger than the 10MB previously allocated. I'm going to put together a pull request to do a calculation for output size instead which should resolve this issue.
@lukasoppermann I've created a pull request with the appropriate fix. Please test it and see how it works for you.
On a separate but important note, it's currently using snapshots I derived from your test case. If this is not okay, please let us know, so we can find a suitable replacement image.
Hey @omnisip,
to answer your questions:
- Puppeteer version 5 is NOT a hard requirement
- Do you usually test against live sites? No, but against a local version of the page.
- Are you asking about the image generated by the test? This is than a screenshot of theverge, which I have no legal rights to (am not working for, I just used it as a test case). I guess to be on the safe side, you should use a different page, or something.
I tested the fix by copying your changes to diff-snapshot.js into the file in the repo where I found the issue and this seems to have solved it. 👍
Thank you very much for the help & support!
You're welcome. Let's see what we can do about finding a really large PNG file...
On Thu, Jul 23, 2020 at 8:06 PM Lukas Oppermann [email protected] wrote:
Hey @omnisip https://github.com/omnisip,
to answer your questions:
- Puppeteer version 5 is NOT a hard requirement
- Do you usually test against live sites? No, but against a local version of the page.
- Are you asking about the image generated by the test? This is than a screenshot of theverge, which I have no legal rights to (am not working for, I just used it as a test case). I guess to be on the safe side, you should use a different page, or something.
I tested the fix by copying your changes to diff-snapshot.js into the file in the repo where I found the issue and this seems to have solved it. 👍
Thank you very much for the help & support!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-663206481, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJWQVFJT47Y4MIGZOSDR5CJ3XANCNFSM4NE6LOWQ .
@omnisip --
Not trying to hijack the thread, but I was working through memory-related issues that appear to be caused by jest-image-snapshot and discovered that setting runInProcess to true (per your advise to @lukasoppermann above) solved the problem. I've filed a separate PR here (#231), but I thought I'd mention it in case the two issues were related.
Feel free to try this pull request branch without runInProcess and let me know.
On Thu, Jul 23, 2020 at 9:45 PM John Hildenbiddle [email protected] wrote:
@omnisip https://github.com/omnisip --
Not trying to hijack the thread, but I was working through memory-related issues that appear to be caused by jest-image-snapshot and discovered that setting runInProcess to true (per your advise to @lukasoppermann https://github.com/lukasoppermann above) solved the problem. I've filed a separate PR here (#231 https://github.com/americanexpress/jest-image-snapshot/issues/231), but I thought I'd mention it in case the two issues were related.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/americanexpress/jest-image-snapshot/issues/210#issuecomment-663247795, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKQJUF2VAD65EJUBPKUODR5CVOPANCNFSM4NE6LOWQ .
@omnisip -- Tested your branch. At first it seemed like the changes helped, but after a few more runs I'm starting to think there's just general flakiness with macOS CI on GitHub.
Apologies again for jumping into the thread. I was hoping for two birds with one stone, not divert attention away from @lukasoppermann's original issue.
Thx!