-
Notifications
You must be signed in to change notification settings - Fork 50
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
Diff fails due to a few missing rows of pixels #9
Comments
I've been using a different solution of forcing the browser viewport to a |
@acdha Excellent, thank you. I tried my own solution to force the browser viewport to a consistent size, but my solution didn't actually work. |
@treyhunner Actually, I forgot that my branch was already merged. Are you using the version on PyPI or something later which includes acdha/needle@fcda78d (which was merged shortly after the current release)? If you are running that, we need to revisit the logic in https://github.com/bfirsh/needle/blob/fcda78d30f7fd32adb363fa8b880ac4329457dd5/needle/cases.py#L54-63 which attempts to measure the viewport. |
@acdha yes I am using that version. I'm actually only seeing a few pixel difference in height between. I attempted to modify that code to include a height difference but that didn't resolve the issue (though it did reveal that the actual viewport height is much smaller than 768, probably because that height is window height and not viewport height). |
I should note that I'm using Firefox using the example from #8. I couldn't figure out how to get Firefox to work without using that method. I'm considering switching to the PhantomJS web driver for my needle tests but it doesn't seem to run much faster than Firefox and it would mean another dependency for my project than already uses selenium with Firefox elsewhere. |
I'd like to check, is this still an issue using the latest versions of Needle/Selenium? |
I encountered a similar issue where if a divider is slightly enlarged after a build, I get a message: "FAIL: Image dimensions do not match" and no diff.png file is produced. It would be better if there is an option such that a diff.png file is produced even if image dimensions don't match.
|
I agree that it would be nice to still generate a diff file if the images have a different size. But I'm not sure perceptualdiff supports that. It would indeed be quite difficult and/or time-consuming for it to figure out how the new image got cropped or extended. If perceptualdiff really can't handle this, then perhaps another engine does and would be worth supporting? |
I suspect it'd be best to have a simple solution for this – maybe something as easy as using Pillow to expand the smaller image to match the larger and accepting that you'll have a border of 100% different pixels? There's an interesting option using something like OpenCV to try to determine how the image has changed but unless someone is eager to start a big project I'd assume that's complete overkill. |
The thing is that I'm not sure there's a way of knowing in advance where to add the border. Is it to the top, left, right or bottom, or all of the above? If we picked the wrong one, then the diff file would not represent the actual difference. UPDATE: Also, what to do if the new version is in fact smaller than the original? This all sounds pretty hard, which is why I think perceptualdiff doesn't do it. |
Yeah, it's a mess – we could very quickly return a noisy comparison – which is arguably correct from a certain standpoint – or you're looking at something at least as expensive as template matching: http://docs.opencv.org/doc/tutorials/imgproc/histograms/template_matching/template_matching.html This could be done in PIL, of course, but is obviously not a fast process. I wonder whether something like phash might be a reasonable compromise – I used that awhile back for something similar and it was a reasonably fast way to get a single number between 0 and 1 for similarity which tolerated positioning, color, etc. differences. Testing that would be pretty simple:
I think that might be a good fuzzy matcher with a suitably high threshold – it'd tolerate slight differences in spacing or e.g. text anti-aliasing – but it might be too liberal. |
I think that Resemble.js does the comparison pretty well: http://huddle.github.io/Resemble.js/ perhaps a new engine could be built based on similar logic |
@acdha Fascinating article, nice work! :) @aricearice Resemble.js looks pretty good too, thanks for the link! |
@jphalip you're welcome. Btw, for more of an introduction: I'm an automation tester looking for a tool for visual regression testing. I was really excited to find Needle because of the multiple-browser compatibility, so I'd love to see this tool grow and perhaps make some contributions. |
(@acdha: Before copying github links to source code, press 'y' to get a URL that includes the current commit so that the links stay valid over time.) |
@mbertheau That link predates that particular Github feature. Let me edit the comment for posterity… |
I ran my tests on:
The height of the captured images were 3 pixels shorter during one test than during the other so the tests failed.
Fixing the cause would be ideal, but this seems like it might vary from machine-to-machine (these were both on the same machine) so mitigating the issue with an optional size difference allowance could stop the tests from failing without cause.
The text was updated successfully, but these errors were encountered: