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

Switch WebKitGTK testing to Ubuntu and add a nightly channel #19595

Merged
merged 2 commits into from Oct 14, 2019

Conversation

clopez
Copy link
Contributor

@clopez clopez commented Oct 9, 2019

This PR adds two changes:

  1. Switches testing with WebKitGTK to Ubuntu (removes the need for the extra dockerfile)

    Previously (Add Dockerfile for WebKitGTK testing (based on Debian) #18595 and Add webkitgtk_minibrowser runs to taskcluster daily. #19466) a Dockerfile based on Debian was added for WebKitGTK testing with the idea of reusing the built products that build.webkit.org is producing.

    That was done with the idea of having dozens of new nightly builds available at a day, which doesn't seem to match the needs for WPT testing/runs. It seems that having one new nightly a day (or even two a week) is more than enough. At that rate is more than reasonable to do new builds for testing with Ubuntu 18.04.

    And it seems that harmonizing the testing of the Linux based browsers on the same docker image has several benefits, including making easier future maintenance.

  2. Adds a nightly channel for testing with WebKitGTK on taskcluster.

    The nighly channel downloads the last nightly tarball available at https://webkitgtk.org/built-products and installs it on taskcluster when the test start.

    This tarball is generated using the webkitgtk internal JHBuild, that builds several third-party libraries needed for webkitgtk layout tests and then builds webkitgtk on top of this libraries.
    Because of this, to use this tarball it is required to installing quite a lot of extra dependencies (that are needed by this extra third-party libraries).

    A script is included inside the tarball to install this dependencies. This script is executed at test time in task cluster. But because installing them take quite a bit of time (several minutes), the dockerfile is modified to have them already installed and speed up this process. Having them already installed on the docker image is not strictly necessary, it just speed up the testing process.

//cc @jgraham @foolip

@clopez
Copy link
Contributor Author

clopez commented Oct 9, 2019

Example of wpt.fyi results with the new setup:

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good modulo the question about installing deps in the image, and I'd like @jgraham to review since this will involve another weekly run if my suggestion for stable/nightly cadence is accepted.

.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
tools/ci/run_tc.py Outdated Show resolved Hide resolved
tools/docker/Dockerfile Outdated Show resolved Hide resolved
@@ -44,6 +48,16 @@ RUN apt-get -qqy install \
libindicator3-7 \
libindicator7

# To speed up runs of webkitgtk_minibrowser nightly we install all the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much size does this add to the docker image, and how much time does it save?

Since this would only take effect when updating the image, which we don't do often, will the dependencies not become increasingly out of sync and need to be updated at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the docker image 1GB bigger (from 1.27GB to 2.2GB). It saves around 6-10 minutes:

  • 3-7 minutes of download (depends on the download speed of the ubuntu apt repositories)
  • ~3 minutes of installing packages

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that WebKitGTK is still in a bit of an experimental phase that sounds like it's adding a bit too much burden on the other uses of the same image and could increase total transfer size, but I'll let @jgraham weigh in on that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think I'd prefer to make the WebKitGtk runs bear that cost for now. If we think that adding 10 minutes per job is too much I guess we could have a second Docker image here which inherits from the base one and just adds the extra bits to do this update (yes, that undoes part of the motivation for this PR). That adds the maintainance burden of requiring us to update both dockerfiles when we change the base image.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clopez I see you've rebased now but these changes remain. Please ping when it's ready for final review.

@foolip foolip assigned jgraham and unassigned jugglinmike Oct 9, 2019
@clopez clopez force-pushed the pr-webkitgtk-nightly-ubuntu branch from 91e685d to ffea852 Compare October 9, 2019 16:26
tools/ci/run_tc.py Outdated Show resolved Hide resolved
@@ -44,6 +48,16 @@ RUN apt-get -qqy install \
libindicator3-7 \
libindicator7

# To speed up runs of webkitgtk_minibrowser nightly we install all the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think I'd prefer to make the WebKitGtk runs bear that cost for now. If we think that adding 10 minutes per job is too much I guess we could have a second Docker image here which inherits from the base one and just adds the extra bits to do this update (yes, that undoes part of the motivation for this PR). That adds the maintainance burden of requiring us to update both dockerfiles when we change the base image.

@clopez
Copy link
Contributor Author

clopez commented Oct 14, 2019

Right. Let's try to keep this simple for now and avoid bloating the image with this extra deps. I think the 10 minutes of extra time is something that we can live for now, since it runs daily.
If in the future there is timeouts or we want to speed up webkitgtk runs we can rethink this.
I will rebase the patches cleaned without changes to the dockerfile.

@clopez clopez force-pushed the pr-webkitgtk-nightly-ubuntu branch from ccbc614 to 5243fbb Compare October 14, 2019 11:43
clopez added a commit to clopez/wpt that referenced this pull request Oct 14, 2019
…eb-platform-tests#19595)

Previously a Dockerfile based on Debian was added for WebKitGTK testing
with the idea of reusing the built products that build.webkit.org is
producing.

That was done with the idea of having dozens of new nightly builds available
at a day, which doesn't seem to match the needs for WPT testing/runs.
It seems that having one new nightly a day (or even two a week) is more
than enough. At that rate is more than reasonable to do new builds for
testing with Ubuntu 18.04.

And it seems that harmonizing the testing of the Linux based browsers on
the same docker image has several benefits, including making easier future
maintenance.

This switchs the WebKitGTK testing to using Ubuntu 18.04.
clopez added a commit to clopez/wpt that referenced this pull request Oct 14, 2019
…-platform-tests#19595)

The nighly channel downloads the last nightly tarball available at
https://webkitgtk.org/built-products and installs it on taskcluster
when the test start.

This tarball is generated using the webkitgtk internal JHBuild, that
builds several third-party libraries needed for webkitgtk layout tests
and then builds webkitgtk on top of this libraries. Because of this
using this tarball requires installing quite a lot of extra dependencies
(that are needed by this extra third-party libraries). A script is
included inside the tarball to install this dependencies.
@clopez clopez force-pushed the pr-webkitgtk-nightly-ubuntu branch from 5243fbb to 13594b3 Compare October 14, 2019 11:44
…eb-platform-tests#19595)

Previously a Dockerfile based on Debian was added for WebKitGTK testing
with the idea of reusing the built products that build.webkit.org is
producing.

That was done with the idea of having dozens of new nightly builds available
at a day, which doesn't seem to match the needs for WPT testing/runs.
It seems that having one new nightly a day (or even two a week) is more
than enough. At that rate is more than reasonable to do new builds for
testing with Ubuntu 18.04.

And it seems that harmonizing the testing of the Linux based browsers on
the same docker image has several benefits, including making easier future
maintenance.

This switchs the WebKitGTK testing to using Ubuntu 18.04.
…-platform-tests#19595)

The nighly channel downloads the last nightly tarball available at
https://webkitgtk.org/built-products and installs it on taskcluster
when the test start.

This tarball is generated using the webkitgtk internal JHBuild, that
builds several third-party libraries needed for webkitgtk layout tests
and then builds webkitgtk on top of this libraries. Because of this
using this tarball requires installing quite a lot of extra dependencies
(that are needed by this extra third-party libraries). A script is
included inside the tarball to install this dependencies.
@clopez clopez force-pushed the pr-webkitgtk-nightly-ubuntu branch from 13594b3 to b26f18b Compare October 14, 2019 11:51
@clopez
Copy link
Contributor Author

clopez commented Oct 14, 2019

Rebased and cleaned patches. @jgraham does it look ok now?

@jgraham jgraham merged commit 60d0c91 into web-platform-tests:master Oct 14, 2019
jgraham pushed a commit that referenced this pull request Oct 14, 2019
…19595)

Previously a Dockerfile based on Debian was added for WebKitGTK testing
with the idea of reusing the built products that build.webkit.org is
producing.

That was done with the idea of having dozens of new nightly builds available
at a day, which doesn't seem to match the needs for WPT testing/runs.
It seems that having one new nightly a day (or even two a week) is more
than enough. At that rate is more than reasonable to do new builds for
testing with Ubuntu 18.04.

And it seems that harmonizing the testing of the Linux based browsers on
the same docker image has several benefits, including making easier future
maintenance.

This switchs the WebKitGTK testing to using Ubuntu 18.04.
@foolip
Copy link
Member

foolip commented Oct 16, 2019

@clopez looks like the daily build failed both times since this was landed:
https://tools.taskcluster.net/groups/c1CPobZVRQuTrZpqDRoyUw (for 20a55f6)
https://tools.taskcluster.net/groups/ABUsWSTDSxymeh5oR1MWBQ (for 50fa6ad)

It's wpt-webkitgtk_minibrowser-nightly-testharness-4 that timed out after 2 hours in both cases. It wasn't at the same point, not sure if things had hung or if it was still running. Can you investigate?

https://tools.taskcluster.net/groups/IZIeBnWZQg2hmBkDgIMT5Q (for 823aece) is the last daily run before these changes, which succeeded.

@clopez
Copy link
Contributor Author

clopez commented Oct 16, 2019

I think the timeout is caused because of this extra 10-15 minutes needed for the download of dependencies. I checked this with @jgraham and he suggests to first try to raise the number of chunks before trying to raise the timeout value (is set a 2 hours). I opened a PR for that in #19724
I also observed on some runs I have been trying on the triggers/webkitgtk_minibrowser_nightly branch that sometimes the download aborts. I opened a PR for that in #19725

foolip added a commit that referenced this pull request Oct 17, 2019
foolip added a commit that referenced this pull request Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants