-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Question] What does threshold mean in a method toMatchSnapshot #10219
Comments
@KirProkopchik Thank you for the input! Pixelmatch threshold is not very well explained in their docs, so we could've been confused. Let me look into this. |
I observe the same behavior. Subsequent runs can take new snapshots that differ by 1 pixel in resolution. toMatchSnapshot fails regardless of threshold value, even when bumped up to 1. |
In Pixelmatch which is used so far by the method If a single pixel exceeds that threshold, the images are considered different. |
old one ticket, where pics with big and small diffs are same after same threshold maybe its worth to investigate usage of some alternative? |
@dimkin-eu I developed a pixel-buffer-diff library that takes care of partial pixel differences to reduce false positive. Additionally it is 4x faster than pixelMatch ( the image comparison lib used in Playwright ). You can see the PR #10823 which tried to bump to an early version of the library. The current version is 1.3.2 addressed some issues found around that time. Could you try on your project and let us know if it helped ? @fanjum66 the screenshots in your example have different resolutions and therefore could not be diffed. |
@p01 - Thanks for your quick response, I think even though a different library is used the below line still throw an error as PNG library is used to compare the sizes https://github.com/p01/playwright/blob/5148c1dbf62ae2bc3c36083fc9d9fa54c9405e0d/packages/playwright-test/src/matchers/golden.ts#L63 It could be better if a flag can be used to ignore the size diff and focus only on the data change. And is there any playwright version I can use to test the mentioned fix ? |
@fanjum66 In your example, one image is 1px taller than the other. How can the pixel buffer diffing library make sense of that ? Is the whole page/UI really 1px taller ? Is everything shifted 1px to the bottom ? to the top ? Any way you look at it these snapshots won't match and need to be reviewed. It could be a true positive where your page/UI really did shrank by 1px but it is likely a real issue that needs to be investigated to figure why the screenshots have different resolution to fix the tests and ensure it is deterministic. To try pixel-buffer-diff in a recent version of Playwright, |
@aslushnikov @pavelfeldman @dgozman , it seems that Playwright uses a threshold of
If we look at the colors black, In our tests, we use |
Let's open a PR #11838 Add pixel-buffer-diff behind env variable opt-in, so anyone who want can try it without disrupting existing tests. |
@p01 Did our defaults result in some tests passing erroneously? With the new suggested |
This patch adds additional options to `toMatchSnapshot` method: - `pixelCount` - acceptable number of pixels that differ to still consider images equal. - `pixelRatio` - acceptable percentage of pixels that differ to still consider images equal. Since some anti-aliasing artifacts still can cripple in, we default `pixelCount` to some arbitrary small number - `17` - to improve tolerance. Fixes microsoft#12167, microsoft#10219
This patch adds additional options to `toMatchSnapshot` method: - `pixelCount` - acceptable number of pixels that differ to still consider images equal. - `pixelRatio` - acceptable percentage of pixels that differ to still consider images equal. Since some anti-aliasing artifacts still can cripple in, we default `pixelCount` to some arbitrary small number - `17` - to improve tolerance. Fixes microsoft#12167, microsoft#10219
This patch adds additional options to `toMatchSnapshot` method: - `pixelCount` - acceptable number of pixels that differ to still consider images equal. - `pixelRatio` - acceptable percentage of pixels that differ to still consider images equal. Since some anti-aliasing artifacts still can cripple in, we default `pixelCount` to some arbitrary small number - `17` - to improve tolerance. Fixes microsoft#12167, microsoft#10219
…12169) This patch adds additional options to `toMatchSnapshot` method: - `pixelCount` - acceptable number of pixels that differ to still consider images equal. Unset by default. - `pixelRatio` - acceptable ratio of all image pixels (from 0 to 1) that differ to still consider images equal. Unset by default. Fixes #12167, #10219
Everybody: We just landed two new options to
You can give them a try using |
@aslushnikov I gave them a try on a scenario and they work well for me, thanks very much. I found the options to actually be named as:
|
Hi all,
Now I am using jest as a test runner with jest-playwright-preset and jest-image-snapshot for image comparisons.
Versions:
jest: 27.0.4
jest-playwright-preset: 1.7.0
jest-image-snapshot: 4.5.1
playwright: 1.16.2
Screenshot comparisons looks like:
Where threshold is the ratio of differing to the total number of pixels. Or the maximum value of differing pixels, in case if threshold
is > 1.
When I started migrating to the native playwright runner, I faced with the image comparison issue. The expected and actual screenshots are not equals. But the difference is only a few pixels. For the image with resolution 1920x1080. I tried to set the threshold 0.5 or even more. Without any positive results.
I started looking the pixlematch source and find out that threshold determines how much each pixel can differ in color, and not the number of different pixels.
https://github.com/mapbox/pixelmatch/blob/b9261a447515f5aff37a15cfab9f4a491868f720/index.js#L45
Further, the number of different pixels is simply compared with zeros in playwright matcher.
playwright/packages/playwright-test/src/matchers/golden.ts
Line 69 in 2a0a44b
In my case, this approach leads to failures even for several pixels with a fairly high threshold.
The text was updated successfully, but these errors were encountered: