search

wswebcreation / webdriver-image-comparison

MIT License
43 stars 36 forks source link

The percentage difference is rounded and passes the tests #103

Closed jesusfj710 closed 2 years ago

jesusfj710 commented 2 years ago

Environment (please complete the following information):

Describe the bug If you use the default configuration, the value of rawMisMatchPercentage is false.

If you have an image that fails by a mistmatch of 0.001%, it will be rounded to 0.00% and the service will return that the two images are the same (consequently, you can enter visual failures and not see them because they are so minuscule).

To Reproduce Steps to reproduce the behavior:

  1. Run a test by taking a screenshot of a large web page (e.g. 1920 × 2483). -> The images match.
  2. Make a change to the front end of the website. .> The image should not match,
  3. Undo the change and make a smaller change until you get a percentage less than 0.01%. -> The change is not detected even though it exists.
  4. Set rawMisMatchPercentage to true. And run the test again. It should now fail with a percentage less than 0.01%.

Expected behavior There are two ways to handle this if rawMisMatchPercentage is set to false:

One option is to have the test fail and indicate that the failure rate is below 0.00%. I think this would be the correct one.

Another option would be that the test passes, but a warning is thrown in the logs indicating that there is a difference below 0.00%. This should also be indicated in the documentation, so that people do not encounter this confusing behaviour.

Log

With rawMisMatchPercentage set to false:

[2022-04-22T08:47:29.888Z] [0-2]   Then he should see the expected full web-page
[2022-04-22T08:47:29.888Z] [0-2] ✓ Execution successful (28s 487ms)

With rawMisMatchPercentage set to true:

...
[2022-04-26T09:25:03.374Z] [0-0]     ✗ Error: The image tagged as "web-page" does not match its baseline 📸 Mismatch: 0.00014467592592592592%
[2022-04-26T09:25:03.374Z] [0-0] ✗ Execution failed with error (32s 812ms)
...
jesusfj710 commented 2 years ago

I can provide more info if needed, but I think it's pretty clear that the error is that it is making an assertion that if the percentage of mistmach is 0, it passes the test. This is not correct, as you are rounding the number, and should take the primitive value.

jesusfj710 commented 2 years ago

Another possible option would be to see to what digit you want to round to, because you may be interested in thousandths but no more than 3 decimal places.

wswebcreation commented 2 years ago

Hi @jesusfj710

thanks for this issue and totally agree. In also feel for this

to have the test fail and indicate that the failure rate is below 0.00%

But that would be a breaking change and thus a major release

We can also go for

Another option would be that the test passes, but a warning is thrown in the logs indicating that there is a difference below 0.00%. This should also be indicated in the documentation, so that people do not encounter this confusing behaviour.

But that would only pollute the logs and the question would be if people would always read the logs.

My suggestion would be to go for option one, mark it as a breaking change and I will release a new major. Do you mind creating a PR and a doc update for it?

jesusfj710 commented 2 years ago

Hello @wswebcreation

Thank you for your answer. I will try to think about how to approach the PR and see if I am able to do it and in case I am not able to do it. In case I am not capable I will try to provide as much feedback or testing as I can.

wswebcreation commented 2 years ago

Closing this and we will proceed the conversation in the PR