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
Fix UI tests as it fails to fetch data from marketplace #18111
Conversation
Not sure why these screenshots would fail they seem to be the same? https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/49818/ |
see https://app.travis-ci.com/github/matomo-org/matomo/jobs/542073473#L1239
Maybe there's an issue somehow? sounds like we're maybe using an old version of image magick? ImageMagick/ImageMagick6#56 See #15720 (comment) probably this is why the UI tests didn't run in bionic in first place. Not sure if we can somehow workaround these policies or use a different version |
I tried using bionic in the first place. But gave up while trying to fix that error. The problems are actually too big images. It seems in general be possible to change the config of imagick. But I didn't manage to do that on Travis. We maybe could try to manually install a newer version, but not sure if that would fix the limitations. Another possible solution would be to set a css zoom before making the screenshot, so the resulting image is smaller. But that also means loosing some accuracy and details. Iirc I did that for on premium plugin test, as a screenshot was even breaking the limits on xenial. |
Do we need to compare screenshots there at all? I don't know if that's something we do or the puppeteer. Because we won't actually need it as we're doing this later on build artifacts in browser anyway. |
Ignore last comment. We use it to compare the image if it's identical not for the diff. Maybe a different command could be used for this maybe also, not sure. Just a thought. |
29cd564
to
e55ccb3
Compare
Seems I found a proper solution to adjust the ImageMagick policy on bionic. See matomo-org/travis-scripts#75 |
Awesome @sgiehl 🎉 that seems to work |
Guess we should also change all our plugins to do the UI tests on bionic then as well |
@sgiehl I was thinking to wait until it was needed to delay this for a while until it's needed (so we can work on more impactful things). If they don't get data from marketplace or so it's probably not an issue? Or do you mean it's needed re the travis build upload? |
Guess we should at least add it to the Travis template, so newly generated files would use bionic. If we merge the PR that reverts the SSL ignore, we also need to use bionic for plugins to have a working artifacts upload |
Had a look at the plugin. And actually it was only core still using Xenial instead of Bionic, due to the failing ImageMagick policy problems. So guess we actually don't need to update anything else |
Great 👍 |
Description:
fix #18098
Use bionic instead of xenial.
Review