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

Confusion of the percentage definition #270

Closed
bcjohnblue opened this issue Sep 10, 2024 · 1 comment · Fixed by #271
Closed

Confusion of the percentage definition #270

bcjohnblue opened this issue Sep 10, 2024 · 1 comment · Fixed by #271

Comments

@bcjohnblue
Copy link

The result of percentage makes me confused.

It says 0.2 means the pixel difference is 0.2% from README.md.

cy.get('h1').compareSnapshot('homePage', 0.2) // will compare only the image of h1 element and fail only if the percentage of pixels that are different is bigger than 0.2%

However, the percentage doesn't multiple by 100 in source code. I think the range of percentage is from 0 to 1 now.

const percentage = (mismatchedPixels / diffImage.width / diffImage.height) ** 0.5

In conclusion, 0.2 should not translated to 0.2%, it means20%.


By the way, I suggest to remove ** 0.5 when calculate percentage. It seems haim-io/cypress-image-diff#184 has similar request too.

@Zaista
Copy link
Collaborator

Zaista commented Sep 12, 2024

Thanks for raising the issue, you are right, it is confusing. It was always represented as a decimal range 0-1, and we've always referred to it as the percentage. In the following PR I reworded it a little bit to make it clearer and fixed my 0.2% mistake.

For the square root part, you are again right, removing ** 0.5 and calculating the percentage by hand gives the correct numbers, I can't imagine why it was there in the first place for a straightforward pixel difference calculation. @mjhea0 maybe you could weigh in on this one? 🤔

The PR should be ready soonish

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants