"Image size must match baseline" error

[americanexpress]

"Image size must match baseline" error

[anescobar1991]

@thomasbertet @sergeybekrin let's discuss here. I definitely see the need now for what you guys are all requesting (now that I understand fully).

[sbekrin]

Thanks @anescobar1991 for bringing this up. I agree with @thomasbertet on that option "a" would work best, we're using fork with almost same solution and it works super smooth for us.

[anescobar1991]

Resolved with #42! Will publish in next few days!

[anescobar1991]

Published as part of v2.4.0

[anescobar1991]

@dtipson would this help?

[thomasbertet]

I can back @dtipson on this @anescobar1991. This is the exact same thing I would love to have :)

[anescobar1991]

So in my mind what I am thinking is that we don't throw on size mismatches as is done today, but instead automatically fail the test and then either:

a). fill in the "extra" space on the smaller of the images and then pass it to pixelmatch (which would produce a diff image that is pretty meaningless but at least includes both the baseline image and the actual image for human eye comparison). If this approach is taken we need to make sure that the image that is then stored as a new baseline snapshot (if --update flag is passed) is the original and not the one we modified in order to match the baseline's dimensions.

or

b). skip pixelmatch comparison and generate a "diff image" that includes the baseline image, an image with a message letting you know that no diff was generated because of size mismatch (in place of where the diff would typically go, and the actual image.

@dtipson @thomasbertet @sergeybekrin What do you guys think?

[thomasbertet]

I would be happy with both solutions.

• Option "b" seems less work and less bug prone... so it might be sufficient, but requires more human eye comparison. That's OK for me :)

• Option "a" seems smarter but in a case where the page is different from the top of the image, it won't help much to have a comparison, since everything will be red (as you mentioned) ... but might help a lot if only a bottom part of the image has changed (ie. new content at the bottom). Not sure it worth the effort ...

Really happy that we will have something to handle this, every option is better than what we have now 👍 thanks @anescobar1991

[isilweo]

in HTML world i don't see option B as usable. Sometimes the images might differ with 1px that comes from some added padding around html element. Human eye won't catch this and in the end we'll have to check differences using some kind of pixel comparison software. So I would be happy with option A.

[anescobar1991]

@isilweo In cases like that wouldn't you just accept the image (via updating the snapshot) if there is no human determinable difference? Not ideal I know, but I ask because with option A, the diff that is generated will not be meaningful: It will always highlight almost the entire image as being different (as pixel by pixel they would be different which still wouldn't be helpful in your use case. What do you think?

For that reason I am leaning towards option B, @sergeybekrin I'd like to get your thoughts on this approach, what do you think?

[sbekrin]

@anescobar1991 I would say having at least some diff is really valuable, essentially on CI when it's only way to check what's wrong. And diff wouldn't be always that bad actually since most common use-case is spacing changes. Below is worst case scenario:

[isilweo]

I agree with you that if there is no human determinable difference it shouldn't a problem... but I have some customers who's testers are comparing PSD designs to what they see in browser on pixel to pixel level and I can't do nothing about it....

Maybe it'll be better if I describe my use case a bit more so you will know how we're using it.
Currently we use React Storybook + jest snapshots from rendered react components. Currently my test suite is almost 170 components, if you add different stories to this we have almost 800 rendered snapshots. Now there are some basic components like Button, if you change Button width by 1 pixel it has big impact on other components so let's say that 100 components will have to be checked to see if everything looks ok. This is quite time consuming. Jest snapshots will show difference and you can catch the change if it was unintentional. But if it was intentional you'll have to check every component that had button rendered to inspect if it is ok.

Now having this handled by image snapshots it could be a lot easier - you will end with 100 images to check but it should be quicker to see if everything is correct.
Below you can see output from pixel match and two screenshots, difference is 1px change in horizontal padding. Not whole image is highlighted and you can clearly see what has changed. Of course new empty area will always be highlighted but it also gives you a hing that size has been changed and you even know if it was height change or width

[anescobar1991]

@isilweo That only works if the new image is smaller than the baseline image it will not work if the opposite is true. Instead pixelmatch will fail to compare and throw a cryptic error message. That is why I throw my own friendly error message instead right now on size differences.

@sergeybekrin on CI you would still have an output image which would show you the baseline image and the actual image so you could still see the changes, albeit there would not be an image with highlighted differences.

The reason I prefer this is that the diff will be misleading a LOT of the time. Not all cases are like what you and @isilweo are seeing and I would rather make it very explicit that the test has failed because the image sizes are different and display the 2 images side by side to allow for a user to decide whether to update the snapshots or not as opposed to spend time creating a diff image that may or may not even be meaningful to a lot of users.

UPDATE: pixelmatch is not what throws the cryptic error. pngjs does. Either way @isilweo @sergeybekrin although today pixelmatch will create a (misleading) diff, in the next release it will not.

[sbekrin]

@anescobar1991 yeah I got your idea, but that feels inconsistent and could be confusing for users who expects diff. Depending on tools and workflow you use, you might not be able to say what's actually changed in specific commit.

Having just baseline and received images is fine, but diff would save us lots of time. I believe machines should help us to identify problems and point at it to let us focus on resolving it instead.

[anescobar1991]

@sergeybekrin I agree with you about the confusion which is why we'd have an image with a message specifying that no diff was created because image size was different. We would also modify the assertion message to specify that it failed because the image dimensions were different. Hopefully that messaging helps.

Depending on tools and workflow you use, you might not be able to say what's actually changed in specific commit.

What do you mean by that?

And I agree that the diff helps but what about the cases where the diff is not meaningful. If a user is not clear that a diff "looks" funny because there is a size difference then they will be confused.

[anescobar1991]

@sergeybekrin I spent some time investigating today and you are right. I am going to open #42 back up :)