@tsteur opened this Pull Request on October 8th 2021 Member

Description:

fix https://github.com/matomo-org/matomo/issues/18098

Use bionic instead of xenial.

Review

@tsteur commented on October 8th 2021 Member

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/

@tsteur commented on October 8th 2021 Member

see https://app.travis-ci.com/github/matomo-org/matomo/jobs/542073473#L1239

          the 'compare' command output could not be parsed, should be an integer, got: compare: ../../magick/quantum.c:216: DestroyQuantumInfo: Assertion `quantum_info != (QuantumInfo *) NULL' failed.

Maybe there's an issue somehow? sounds like we're maybe using an old version of image magick? https://github.com/ImageMagick/ImageMagick6/issues/56

See https://github.com/matomo-org/matomo/pull/15720#issuecomment-610422965 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

@sgiehl commented on October 8th 2021 Member

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.

@tsteur commented on October 11th 2021 Member

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.

@tsteur commented on October 11th 2021 Member

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.

@sgiehl commented on October 18th 2021 Member

Seems I found a proper solution to adjust the ImageMagick policy on bionic. See matomo-org/travis-scripts#75

@tsteur commented on October 18th 2021 Member

Awesome @sgiehl 🎉 that seems to work

@sgiehl commented on October 18th 2021 Member

Guess we should also change all our plugins to do the UI tests on bionic then as well

@tsteur commented on October 18th 2021 Member

@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?

@sgiehl commented on October 18th 2021 Member

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

@sgiehl commented on October 19th 2021 Member

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

@tsteur commented on October 19th 2021 Member

Great 👍

This Pull Request was closed on October 18th 2021
Powered by GitHub Issue Mirror