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

Fix UI tests as it fails to fetch data from marketplace #18111

Merged
merged 6 commits into from Oct 18, 2021
Merged

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Oct 8, 2021

Description:

fix #18098

Use bionic instead of xenial.

Review

@tsteur tsteur changed the title Try bionic Fix UI tests as it fails to fetch data from marketplace Oct 8, 2021
@tsteur tsteur added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Oct 8, 2021
@tsteur tsteur marked this pull request as draft October 8, 2021 01:47
@tsteur tsteur added this to the 4.6.0 milestone Oct 8, 2021
@tsteur
Copy link
Member Author

tsteur commented Oct 8, 2021

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
Copy link
Member Author

tsteur commented Oct 8, 2021

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

@sgiehl
Copy link
Member

sgiehl commented Oct 8, 2021

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
Copy link
Member Author

tsteur commented Oct 11, 2021

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
Copy link
Member Author

tsteur commented Oct 11, 2021

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
Copy link
Member

sgiehl commented Oct 18, 2021

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

@sgiehl sgiehl marked this pull request as ready for review October 18, 2021 13:45
@tsteur
Copy link
Member Author

tsteur commented Oct 18, 2021

Awesome @sgiehl 🎉 that seems to work

@tsteur tsteur merged commit dc9f768 into 4.x-dev Oct 18, 2021
@tsteur tsteur deleted the travisyml2 branch October 18, 2021 19:13
@sgiehl
Copy link
Member

sgiehl commented Oct 18, 2021

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

@tsteur
Copy link
Member Author

tsteur commented Oct 18, 2021

@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
Copy link
Member

sgiehl commented Oct 18, 2021

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
Copy link
Member

sgiehl commented Oct 19, 2021

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
Copy link
Member Author

tsteur commented Oct 19, 2021

Great 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make UI tests work again as they fail regarding SSL certs
2 participants