Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(options): add option to pass on size missmatch #174

Merged
merged 1 commit into from
Apr 17, 2020
Merged

feat(options): add option to pass on size missmatch #174

merged 1 commit into from
Apr 17, 2020

Conversation

adrianjost
Copy link
Contributor

if the option allowSizeMismatch is set, a build will not allways fail on
images with different sizes. Missing or Added Pixel will be counted as a
missmatch and respected by the set threshold.

Related #83, #85


I am not sure if this addition is welcome because of the discussion in the issues mentioned but we needed to implement it anyway because we want our test to run on different operating systems which results in different image sizes if we crop images using 3rd party libraries. The reason for this is different shadow and text rendering of the operating systems and browsers.

@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

@anescobar1991
Copy link
Member

I may be missing something but couldnt you make sure to crop the images prior to passing them in to jest-image-snapshot? Declining as I dont think this is necessary.

@adrianjost
Copy link
Contributor Author

adrianjost commented Apr 6, 2020

@anescobar1991 that's not really possible. We crop our screenshots to the actual content before passing them in by removing any whitespace around our components we screenshot. Therefore we do not know the size of the screenshot beforehand and hardcode the size. Because shadows are rendered differently per browser, system and so on, the shadow might be 1 or 2px larger/smaller between systems and all the tests will fail, even though the diff is below the defined threshold (even when all missing/additional pixels are counted as a mismatch).

@anescobar1991 anescobar1991 reopened this Apr 9, 2020
@anescobar1991
Copy link
Member

@adrianjost I see now - I think this does make sense to add then. Can you resolve the conflicts in this PR?

@adrianjost
Copy link
Contributor Author

perfect. Yes I will resolve the conflicts later today :)

@sanajaved7
Copy link

Seconding that this would be a very helpful feature! 🙏

@adrianjost
Copy link
Contributor Author

Sorry, I did something wrong during the rebase and now the tests are failing. I will fix this later this weekend.

@adrianjost
Copy link
Contributor Author

adrianjost commented Apr 13, 2020

@anescobar1991 All conflicts are now resolved

src/diff-snapshot.js Outdated Show resolved Hide resolved
anescobar1991
anescobar1991 previously approved these changes Apr 15, 2020
if the option `allowSizeMismatch` is set, a build will not allways fail on
images with different sizes. Missing or Added Pixel will be counted as a
missmatch and respected by the set threshold.

Related #83, #85
@adrianjost
Copy link
Contributor Author

@anescobar1991 update the branch from the americanexpress repo and removed the Boolean in the latest commit and instead set the default value on the method attribute.

@anescobar1991 anescobar1991 merged commit cee46b1 into americanexpress:master Apr 17, 2020
oneamexbot added a commit that referenced this pull request Apr 17, 2020
# [3.1.0](v3.0.1...v3.1.0) (2020-04-17)

### Features

* **options:** add option to pass on size missmatch ([#174](#174)) ([cee46b1](cee46b1)), closes [#83](#83) [#85](#85)
@oneamexbot
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

goverdhan07 pushed a commit to goverdhan07/jest-image-snapshot that referenced this pull request Jul 23, 2023
)

if the option `allowSizeMismatch` is set, a build will not always fail on
images with different sizes. Missing or Added Pixel will be counted as a
mismatch and respected by the set threshold.

Related americanexpress#83, americanexpress#85
goverdhan07 pushed a commit to goverdhan07/jest-image-snapshot that referenced this pull request Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants