1305795 - Use fewer files from common.tests.zip in web-platform-tests

Status: NEW

(Testing :: web-platform-tests, defect)

Description

[Gregory Szorc [:gps]]

Bug 1286900 started using most WPT files from a source checkout (namely tests and meta files). But we still rely on several files from common.tests.zip. This bug is about reducing that dependency, possibly even eliminating it.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Stop extracting web-platform directory;

This mozharness script is only extracting the common.tests.zip file
now. It doesn't have a web-platform directory. So remove this entry.

Review commit: https://reviewboard.mozilla.org/r/81522/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81522/

Comment 2

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Install Python packages from source checkout;

Previously, we extracted Python packages for mozbase, marionette, and
wptserve from common.tests.zip and installed them from an extracted
directory. These files in the test archive were copies of files from
a source checkout. Now that we have a source checkout in the WPT
execution environment, these files are redundant and we can simply
reference files from the source checkout instead.

We copy the existing marionette_requirements.txt to
marionette_in_tree_requirements.txt and modify relative paths to
reflect the source tree layout. We update the WPT mozharness
script to use this new requirements file from a source checkout.
And since we no longer need these Python files from the zip archive,
we stop extracting them.

Review commit: https://reviewboard.mozilla.org/r/81524/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81524/

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Use tooltool manifest from source checkout;

With the pip requirements file being used from the source checkout,
the only file from config/ that was still being referenced was a
releng.manifest tooltool manifest used for obtaining the minidump
binary.

This commit modifies TestingMixin to use the manifest from the
source checkout if available. Otherwise it assumes the manifest
is in pwd (preserves existing behavior). the WPT script has been
modified to no longer extract the config/ directory from
common.tests.zip.

Review commit: https://reviewboard.mozilla.org/r/81526/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81526/

Comment 4

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Sort buildbot properties for logging;

Not really related to the task at hand. But my brain was angry
I had to read unsorted buildbot properties in log files as part
of reverse engineering what mozharness is doing.

Review commit: https://reviewboard.mozilla.org/r/81528/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81528/

Comment 5

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Set mozinfoJsonUrl buildbot property;

We're uploading this file but not setting this property. Why, I don't
know. I'm porting WPT execution to not rely on common.tests.zip.
That will require the test environment to resolve the URL to
mozinfo.json. The way we do that in buildbot land is to set a
buildbot property with the URL.

Review commit: https://reviewboard.mozilla.org/r/81530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81530/

Comment 6

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Set mozinfo_json_url property;

We set the attribute from buildbot properties, iterating TaskCluster
artifacts, or inferring it from the path to test_packages.json.

Honestly, I'm not sure when these code paths are all used. The code
in testbase.py that iterates over the buildbot properties file's
['sourcestamp']['changes']['files'] is particularly hacky. Currently
the list only contains the package bz2 file and test_packages.json.
But I can't figure out what actually populates the list. I suspect
the code lives out of tree. While we should be adding the mozinfo.json
URL to this list, it seems just as effective to infer the URL from
the URL of test_packages.json. So I did.

Review commit: https://reviewboard.mozilla.org/r/81532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81532/

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Support downloading mozinfo.json from an artifact;

TestingMixin.download_and_extract() has been taught to download
mozinfo.json from an artifact (as opposed to extracting it from a zip
file as a side-effect).

The WPT script has been converted to obtain mozinfo.json this way.
We now only rely on common.tests.zip for bin/* files in the WPT script.

Review commit: https://reviewboard.mozilla.org/r/81534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81534/

Comment 8

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Produce a bin.tests.zip with binary files;

Previously, common.tests.zip contained a bin/ directory with binaries.
common.tests.zip also contained a bunch of other test files (such as
thousands of JS JIT tests).

During artifact builds, we'd download all of common.tests.zip and
extract a whitelist of bin/ files to their own cached file. When
running WPT in automation, we'd download all of common.tests.zip
and extract all of bin/ files alongside the files from the main
package artifact. These operations were inefficient because they
required us to download a ~20 MB zip file when we only needed ~4MB
from it (at least those are the sizes on Linux). Multiplied by
tens of thousands of test jobs a day or even throw in a slow
internet connection, and that overhead could matter.

This commit extracts the bin/ directory of common.tests.zip to a
bin.tests.zip file. The generated test_packages.json file has
been updated to include this file everywhere common.tests.zip
is being used. Artifact builds have been updated to download
bin.tests.zip instead of common.tests.zip.

The change to artifact builds means that once this lands, there
will be a window where artifact builds won't work until
automation has produced the bin.tests.zip file for an ancestor
build. This should be OK for end-users. But it will likely cause
a perma failure on the landing that introduces this commit. We
can't avoid that unless we have redundant packaging of the
bin/ files. I don't think this is worth the complexity.

Review commit: https://reviewboard.mozilla.org/r/81558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81558/

Comment 9

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Produce a test manifest entry for web-platform;

This change makes the WPT mozharness script only download the
bin.tests.zip file (instead of the default which is to download
common.tests.zip and bin.tests.zip).

With this change, the WPT jobs in automation only download the
package archive, the bin.tests.zip file. Everything else is
referenced from a source checkout!

Review commit: https://reviewboard.mozilla.org/r/81560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81560/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8795491 [details]
Bug 1305795 - Support downloading mozinfo.json from an artifact;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81534/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8795491 [details]
Bug 1305795 - Support downloading mozinfo.json from an artifact;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81534/diff/2-3/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8795536 [details]
Bug 1305795 - Produce a bin.tests.zip with binary files;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81558/diff/1-2/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8795537 [details]
Bug 1305795 - Produce a test manifest entry for web-platform;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81560/diff/1-2/

Comment 14

[Gregory Szorc [:gps]]

Attachment: Bug 1305795 - Remove pull action from WPT script;

Not sure why this was here in the first place. This action no-ops
in all the logs I've seen.

Review commit: https://reviewboard.mozilla.org/r/81562/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81562/

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8795538 [details]
Bug 1305795 - Remove pull action from WPT script;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81562/diff/1-2/

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8795485 [details]
Bug 1305795 - Stop extracting web-platform directory;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81522/diff/1-2/

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8795486 [details]
Bug 1305795 - Install Python packages from source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81524/diff/1-2/

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 8795487 [details]
Bug 1305795 - Use tooltool manifest from source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81526/diff/1-2/

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 8795488 [details]
Bug 1305795 - Sort buildbot properties for logging;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81528/diff/1-2/

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8795489 [details]
Bug 1305795 - Set mozinfoJsonUrl buildbot property;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81530/diff/1-2/

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8795490 [details]
Bug 1305795 - Set mozinfo_json_url property;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81532/diff/1-2/

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8795491 [details]
Bug 1305795 - Support downloading mozinfo.json from an artifact;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81534/diff/3-4/

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 8795536 [details]
Bug 1305795 - Produce a bin.tests.zip with binary files;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81558/diff/2-3/

Comment 24

[Gregory Szorc [:gps]]

Comment on attachment 8795537 [details]
Bug 1305795 - Produce a test manifest entry for web-platform;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81560/diff/2-3/

Comment 25

[Gregory Szorc [:gps]]

chmanchester: you may care about the patch that introduced bin.tests.zip.

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8795536 [details]
Bug 1305795 - Produce a bin.tests.zip with binary files;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81558/diff/3-4/

Comment 27

[Gregory Szorc [:gps]]

Comment on attachment 8795537 [details]
Bug 1305795 - Produce a test manifest entry for web-platform;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81560/diff/3-4/

Comment 28

[Gregory Szorc [:gps]]

Gah - mozinfo.json isn't working right on TC with this series. I'll look into it tomorrow. Buildbot is fine though. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1504587cb39f66d652292934ff078e0cd539c0d3 for the curious.

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 8795538 [details]
Bug 1305795 - Remove pull action from WPT script;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81562/diff/2-3/

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8795485 [details]
Bug 1305795 - Stop extracting web-platform directory;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81522/diff/2-3/

Comment 31

[Gregory Szorc [:gps]]

Comment on attachment 8795486 [details]
Bug 1305795 - Install Python packages from source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81524/diff/2-3/

Comment 32

[Gregory Szorc [:gps]]

Comment on attachment 8795487 [details]
Bug 1305795 - Use tooltool manifest from source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81526/diff/2-3/

Comment 33

[Gregory Szorc [:gps]]

Comment on attachment 8795488 [details]
Bug 1305795 - Sort buildbot properties for logging;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81528/diff/2-3/

Comment 34

[Gregory Szorc [:gps]]

Comment on attachment 8795489 [details]
Bug 1305795 - Set mozinfoJsonUrl buildbot property;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81530/diff/2-3/

Comment 35

[Gregory Szorc [:gps]]

Comment on attachment 8795490 [details]
Bug 1305795 - Set mozinfo_json_url property;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81532/diff/2-3/

Comment 36

[Gregory Szorc [:gps]]

Comment on attachment 8795491 [details]
Bug 1305795 - Support downloading mozinfo.json from an artifact;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81534/diff/4-5/

Comment 37

[Gregory Szorc [:gps]]

Comment on attachment 8795536 [details]
Bug 1305795 - Produce a bin.tests.zip with binary files;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81558/diff/4-5/

Comment 38

[Gregory Szorc [:gps]]

Comment on attachment 8795537 [details]
Bug 1305795 - Produce a test manifest entry for web-platform;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81560/diff/4-5/

Comment 39

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8795536 [details]
Bug 1305795 - Produce a bin.tests.zip with binary files;

https://reviewboard.mozilla.org/r/81558/#review80354

::: python/mozbuild/mozbuild/artifacts.py:168
(Diff revision 5)
> +    # TODO reconsider doing this.
> +    # When we introduced test artifact processing, the point was to extract
> +    # bin/* files from common.tests.zip into a smaller zip. Now that we have
> +    # bin.tests.zip, this extract step may not be necessary and we can perform
> +    # any filtering (if we need to do it at all) during archive extraction
> +    # time.

I don't think this comment is adding much. It would apply nearly equally to the packaged build we handle, so implies we should re-write that too. I don't agree with the premise, but if you'd like to discuss this I think it would be better off tracked in a follow up bug.

Comment 40

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8795490 [details]
Bug 1305795 - Set mozinfo_json_url property;

https://reviewboard.mozilla.org/r/81532/#review80602

::: testing/mozharness/mozharness/mozilla/taskcluster_helper.py:205
(Diff revision 3)
> -            p['packageUrl'] if p.get('packageUrl') else None,
> -            p['testPackagesUrl'] if p.get('testPackagesUrl') else None,
> -            p['symbolsUrl'] if p.get('symbolsUrl') else None
> +            p.get('packageUrl') or None,
> +            p.get('testPackagesUrl') or None,
> +            p.get('symbolsUrl') or None,
> +            p.get('mozinfoJsonUrl') or None,

Is the `or None` part even necessary?

Comment 41

[(not currently active) Ted Mielczarek]

Comment on attachment 8795485 [details]
Bug 1305795 - Stop extracting web-platform directory;

https://reviewboard.mozilla.org/r/81522/#review85088

Comment 42

[(not currently active) Ted Mielczarek]

Comment on attachment 8795538 [details]
Bug 1305795 - Remove pull action from WPT script;

https://reviewboard.mozilla.org/r/81562/#review85090

Comment 43

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jgraham, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 44

[James Graham [:jgraham]]

I'm not aware of any current plans to move to using the source checkouts for tests directly (but I still think we would benefit from doing that!).