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

Tolerance value warning documentation is not correct #282

Closed
adrinieto opened this issue Feb 3, 2022 · 1 comment · Fixed by #303
Closed

Tolerance value warning documentation is not correct #282

adrinieto opened this issue Feb 3, 2022 · 1 comment · Fixed by #303
Labels

Comments

@adrinieto
Copy link

In the tolerance sections states that:

If you configure tolerance = 1 it means the tolerance threshold will be 100% and all your tests will pass even if they should fail...so be careful when configuring this param.

Where it says tolerance = 1 should be tolerance = 100.

We have failing tests that need a value higher than 1 to pass and in the logs for the passing tests it shows "1.0%"

⚠️ Shot warning: There are some pixels changed in the screenshot named com.jobandtalent.android.candidates.opportunities.process.detail.process.ProcessNextStepsFragmentScreenshotTest_shouldShowNextStepsWithAllDoneState, but we consider the comparison correct because tolerance is configured to 1.0 % and the percentage of different pixels is 0.19463734567901234 %

Also, in the shot-consumer-compose build.gradle is set to 1.1

adrinieto added a commit to adrinieto/Shot that referenced this issue Feb 8, 2022
adrinieto added a commit to adrinieto/Shot that referenced this issue Feb 8, 2022
@pedrovgs pedrovgs added the bug label Apr 23, 2022
@pedrovgs
Copy link
Owner

I think you are right @adrinieto

I'm checking the code where tolerance is computed and it shows:

 val percentageOfDifferentPixels = differentPixels.length.toDouble / oldScreenshotPixels.length.toDouble
val percentageOutOf100        = percentageOfDifferentPixels * 100.0
val imagesAreDifferent        = percentageOutOf100 >= tolerance

If differentPixels.length.toDouble is 7 and oldScreenshotPixels.length.toDouble is 100. percentageOutOf100 will be 7. Representing 7% of the pixel values changed. As the value read from the config is not modified at all. If the shot config is like this:

shot {
   tolerance = 100

This means 100% of the tests are allowed to be different. I know you've sent a PR changing this combined with another huge change, but I'm going to send a PR changing only our docs. Thanks!

mhiew pushed a commit to mhiew/Shot that referenced this issue Oct 26, 2022
mhiew pushed a commit to mhiew/Shot that referenced this issue Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants