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 commented Oct 9, 2019

This PR adds two changes:

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

    Previously (#18595 and #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

This comment has been minimized.

Copy link
Contributor Author

clopez commented Oct 9, 2019

Example of wpt.fyi results with the new setup:

Copy link
Contributor

foolip left a comment

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

This comment has been minimized.

Copy link
@foolip

foolip Oct 9, 2019

Contributor

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?

This comment has been minimized.

Copy link
@clopez

clopez Oct 9, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@foolip

foolip Oct 9, 2019

Contributor

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.

This comment has been minimized.

Copy link
@jgraham

jgraham Oct 14, 2019

Contributor

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.

This comment has been minimized.

Copy link
@foolip

foolip Oct 14, 2019

Contributor

@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 clopez:pr-webkitgtk-nightly-ubuntu branch from 91e685d to ffea852 Oct 9, 2019
tools/ci/run_tc.py Outdated Show resolved Hide resolved
@foolip
foolip approved these changes Oct 10, 2019
@@ -44,6 +48,16 @@ RUN apt-get -qqy install \
libindicator3-7 \
libindicator7

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

This comment has been minimized.

Copy link
@jgraham

jgraham Oct 14, 2019

Contributor

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

This comment has been minimized.

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 clopez:pr-webkitgtk-nightly-ubuntu branch from ccbc614 to 5243fbb Oct 14, 2019
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 clopez:pr-webkitgtk-nightly-ubuntu branch from 5243fbb to 13594b3 Oct 14, 2019
clopez added 2 commits Oct 9, 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.
)

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 clopez:pr-webkitgtk-nightly-ubuntu branch from 13594b3 to b26f18b Oct 14, 2019
@clopez

This comment has been minimized.

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
18 checks passed
18 checks passed
build-and-publish
Details
build-and-tag
Details
Azure Pipelines Build #20191014.40 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (infrastructure/ tests: macOS) infrastructure/ tests: macOS succeeded
Details
Azure Pipelines (tools/ unittests: Windows Python 3) tools/ unittests: Windows Python 3 succeeded
Details
Azure Pipelines (tools/ unittests: Windows) tools/ unittests: Windows succeeded
Details
Azure Pipelines (tools/ unittests: macOS) tools/ unittests: macOS succeeded
Details
Azure Pipelines (tools/wpt/ tests: Windows) tools/wpt/ tests: Windows succeeded
Details
Azure Pipelines (tools/wpt/ tests: macOS) tools/wpt/ tests: macOS succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: Windows) tools/wptrunner/ unittests: Windows succeeded
Details
Azure Pipelines (tools/wptrunner/ unittests: macOS) tools/wptrunner/ unittests: macOS succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
wpt.fyi - safari[experimental] Safari results
Details
jgraham added 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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

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
`apt-get upgrade` changes taken from #19595.
foolip added a commit that referenced this pull request Oct 18, 2019
`apt-get upgrade` changes taken from #19595.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.