@tsteur opened this Pull Request on June 21st 2015 Owner

fixes #8142 #8034

It looks like resemble causes the random crashes during UI tests on Travis CI. I can reproduce this locally and identified resemble as the problem. The tests crashed because of too much memory usage.
We now avoid loading the images into PhantomJS to compare them and we use resemble.js only as a fallback if there's a threshold defined (which is currently only the case for ImageGraph tests). The dashboard tests failed since the images are pretty long. By executing ImageMagick we seem to save a lot of memory and time. Eg currently on master the tests need more than 50 minutes whereas with this solution it takes only approx 37 minutes. This explains why the UI test build take longer the more failing tests there are as more pictures need to be compared using resemble (which is slow). So for green tests the time difference might be actually much less.

It should also fix a problem with resemble that we probably often not compared the whole image in tests. From the Resemble.js docs:

By default, the comparison algorithm skips pixels when the image width or height is larger than 1200 pixels. This is there to mitigate performance issues.

To fix this I do now set a largeImageThreshold parameter.

To improve performance of ImageMagick further I wanted to set an option to stop comparing as soon as there is at least one pixel difference but did not find such option.

FYI: https://developer.piwik.org/guides/tests-ui already mentions the requirement of ImageMagick. I was thinking about adding a check whether compare command exists in ./console tests:run-ui but decided not to as it I'm not sure re Windows etc. When compare does not exist there should be an error message once a screenshot is actually compared.

I'm not sure if we should merge directly. Ideally we would make master green before merging this one but in most cases the tests on master take more than 50 minutes which makes this a bit hard.

@tsteur commented on June 22nd 2015 Owner

This PR also fixes an issue that we did not detect all UI regressions. Eg there was always a default tolerance in resemble.js that ignored small changes of a color. Eg if one only made the border slightly lighter it was not detected by resemble.js. I changed the file directly here but also issued a PR: https://github.com/Huddle/Resemble.js/pull/44

This Pull Request was closed on June 22nd 2015
Powered by GitHub Issue Mirror