1286900 - Run wpt tests from source checkout

Status: NEW

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

Description

[Gregory Szorc [:gps]]

We believe that running WPT tests from a Mercurial checkout will be faster than from test archives.

This bug is about testing that theory.

Comment 1

[(not currently active) Ted Mielczarek]

Specifically, we'd have the mozharness script for the test job clone the Mercurial repository, download the build as usual, possibly download the tests.common.zip (if WPT needs any of the binaries from there), and run the test command directly from the source directory (perhaps even using ./mach test web-platform-tests).

We might need to either fix `mach test` to do a minimal configure or have the build script run `mach configure` first, presumably with `--disable-compile-environment` so it's fast.

Comment 2

[James Graham [:jgraham]]

certutil is required to set up the profile. I don't recall any other tests.common.zip requirements. If we use |mach web-platform-tests| directly (which I think would be good), we'll need to allow the certutil and binary to be in a different place compared to a local build.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Ensure WPT tests have a version control checkout;

We add a mozharness action to the TestingMixin base class that ensures
we're running from a VCS checkout and we add this action to the WPT
script.

This ensures that we always have a source checkout available, even
in buildbot. (Before, we only had a source checkout in TaskCluster.)

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

Comment 4

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Reformat WPT command argument additions;

list.extend() is favored over +=. Also use single quotes, fixup
indentation, and factor out a common variable.

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

Comment 5

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Inline WPT arguments into script;

4 WPT config files all contained the same config options boilerplate.
Inline it into the WPT mozharness script.

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

Comment 6

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Use WPT files from source checkout;

We're starting to execute mozharness scripts from source checkouts
in automation. This means we can stop downloading and extracting
files that are already available in the source checkout and just
reference files from the checkout.

Web platform tests (WPT) are a logical place to start because they
are pretty well isolated. This commit modifies the mozharness script
for WPT execution to use files from a source checkout (previous
commits have guaranteed that a pristine source checkout is available
to the test execution environment).

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

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Stop producing web-platform tests zip file;

Now that we access WPT related files from a source checkout, we no
longer need the web-platform tests zip file produced or consumed by
automation. So stop producing it.

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

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

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

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

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

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

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

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

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

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

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

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

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

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

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

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

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

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

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

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

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

Comment 18

[(not currently active) Ted Mielczarek]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

https://reviewboard.mozilla.org/r/79954/#review79454

So in buildbot we'll bootstrap from mozharness then do a separate hg clone to get the full source repo? Not great, but if it's better in Taskcluster it's probably not worth over-optimizing.

::: testing/mozharness/mozharness/mozilla/testing/testbase.py:137
(Diff revision 3)
> +            dirs['checkout'] = os.path.join(dirs['base_work_dir'], 'checkout')
> +
> +        if 'HG_SHARE_BASE_DIR' in os.environ:
> +            dirs['hg_shared'] = os.environ['HG_SHARE_BASE_DIR']
> +        else:
> +            dirs['hg_shared'] = os.path.join(dirs['base_work_dir'], 'hg-shared')

I assume all of this is consumed by VCSMixin? It'd be nice to mention that. (I always have a hard time keeping track of what code does what in mozharness with all the mixins.)

Comment 19

[(not currently active) Ted Mielczarek]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

https://reviewboard.mozilla.org/r/79956/#review79456

Comment 20

[(not currently active) Ted Mielczarek]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

https://reviewboard.mozilla.org/r/79958/#review79458

We are really good at giving ourselves way too many options, aren't we?

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review79460

Comment 22

[(not currently active) Ted Mielczarek]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

https://reviewboard.mozilla.org/r/79962/#review79466

\o/ one down, a few more to go!

Comment 23

[(not currently active) Ted Mielczarek]

This bug is good as-is, maybe we should file a followup to run the tests directly from the mach command? That ought to let us remove a whole bunch of stuff from the mozharness invocation.

Comment 24

[Gregory Szorc [:gps]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #23)
> This bug is good as-is, maybe we should file a followup to run the tests
> directly from the mach command? That ought to let us remove a whole bunch of
> stuff from the mozharness invocation.

Yes, that is very tempting. There are a number of mozharness scripts that IMO add very little value, especially now that we can do source checkouts easily. It's at least worth having the bug on file.

Comment 25

[Gregory Szorc [:gps]]

I have to hold off landing this because there's a few lingering failures on Try. I suspect some files aren't in the locations the tests expect them to be in. I may have to poke jgraham for help on Monday...

Comment 26

[Gregory Szorc [:gps]]

Attachment: Bug 1286900 - Add environment variable to define location of mozinfo.json;

As part of converting WPT to run from a source checkout in automation,
I ran into problems where mozinfo.json wasn't being loaded, causing
several tests to fail. This is because mozinfo.json is only found
from a source directory or from a parent of the WPT meta directory.

Before, mozinfo.json was provided by the "common" tests zip file
and extracted to the base of the destination directory. WPT files
were extracted to a "web-platform" sub-directory. So mozinfo.json
was in a parent directory of "web-platform/meta" and was loaded
automatically.

Now that we want to run WPT out of the source directory, there is
no mozinfo.json and the source directory and no mozinfo.json can be
loaded.

This commit teaches mozinfo.json to look in a MOZINFO_PATH environment
variable for the path to mozinfo.json. This allows mozinfo.json
to exist anywhere on the filesystem. This provides a lot more
flexibility for loading the file.

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

Comment 27

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

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

Comment 28

[Gregory Szorc [:gps]]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

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

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

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

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

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

Comment 31

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

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

Comment 32

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

Needs re-review for mozinfo.json changes.

Comment 33

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

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

Comment 34

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

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

Comment 35

[(not currently active) Ted Mielczarek]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

This makes sense. We've been relying on the crappy "package mozinfo.json next to the main .py file" behavior since we started using it.

::: testing/mozbase/mozinfo/mozinfo/mozinfo.py:210
(Diff revision 1)
> -                 searched after first looking in the root of the objdir
> -                 if the current script is being run from a Mozilla objdir.
> +
> +    1) MOZINFO_PATH environment variable.
> +    2) objdir (via a build system context)
> +    3) Traversing directories ``dirs`` for a mozinfo.json file.
>  
>      Returns the full path to mozinfo.json if it was found, or None otherwise.

Did you also want to change this to raise if it can't find mozinfo.json? I don't know why we wrote it this way originally, but I suspect none of the test harnesses that use it are going to do the right thing if they are run without a mozinfo.json present.

Comment 36

[James Graham [:jgraham]]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> Did you also want to change this to raise if it can't find mozinfo.json? I don't know why we wrote it this way originally, but I suspect none of the test harnesses that use it are going to do the right thing if they are run without a mozinfo.json present.

I think that would break running things locally? I'm sure it would break running wpt outside mozilla-* at least.

Comment 37

[Gregory Szorc [:gps]]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> I think that would break running things locally? I'm sure it would break running wpt outside mozilla-* at least.

I was tempted to make this raise. But I figured it would be best deferred to a follow-up, as I didn't want to scope bloat and risk backout.

I made the new environment code fail fast because it isn't subject to backwards compatible behavior.

Comment 38

[(not currently active) Ted Mielczarek]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> I was tempted to make this raise. But I figured it would be best deferred to a follow-up, as I didn't want to scope bloat and risk backout.
> 
> I made the new environment code fail fast because it isn't subject to backwards compatible behavior.

In a local build we always have a mozinfo.json in the objdir. Does wpt work with Firefox outside of a mozilla repo if it doesn't have mozinfo? Surely all the manifest conditionals would be missing the proper info?

Comment 39

[(not currently active) Ted Mielczarek]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review80000

Comment 40

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

https://reviewboard.mozilla.org/r/79954/#review79454

> I assume all of this is consumed by VCSMixin? It'd be nice to mention that. (I always have a hard time keeping track of what code does what in mozharness with all the mixins.)

Yes, VCSMixin does all the magic.

At some point I want to start extracting the useful methods of mixins to functions so the code is more obvious to read and can be reused in other contexts (such as mach commands).

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b5f059a85ad
Add environment variable to define location of mozinfo.json; r=ted
https://hg.mozilla.org/integration/autoland/rev/01c3527edde5
Ensure WPT tests have a version control checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/5b0f69c86028
Reformat WPT command argument additions; r=ted
https://hg.mozilla.org/integration/autoland/rev/3928e945b584
Inline WPT arguments into script; r=ted
https://hg.mozilla.org/integration/autoland/rev/4b08ed30e0a5
Use WPT files from source checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/859457467e3c
Stop producing web-platform tests zip file; r=ted

Comment 42

[James Graham [:jgraham]]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> In a local build we always have a mozinfo.json in the objdir. Does wpt work with Firefox outside of a mozilla repo if it doesn't have mozinfo? Surely all the manifest conditionals would be missing the proper info?

It works in the sense that it runs, and that's a supported use case. It is true that not all the expectation data will work (although most of it will because most does not depend on the extra build-added mozinfo fields, as you see from the fact that not everything failed before this was added to the patch).

Comment 43

[(not currently active) Ted Mielczarek]

That seems like a strange and perilous thing! I guess ideally the harness would just error if you tried to use something in a conditional that wasn't defined.

Comment 44

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/aed585286446d6d39996d8c878a6ccabdef0d4f5 for needing who knows how many dependencies including but not limited to bug 1305877 to make it be a win, rather than a loss which runs the worst chunk over maxtime on Windows debug when it doesn't just hang while cloning or just pulling from mozilla-unified.

Comment 45

[Gregory Szorc [:gps]]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

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

Comment 46

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

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

Comment 47

[Gregory Szorc [:gps]]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

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

Comment 48

[Gregory Szorc [:gps]]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

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

Comment 49

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

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

Comment 50

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

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

Comment 51

[Gregory Szorc [:gps]]

Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

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

Comment 52

[Gregory Szorc [:gps]]

Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

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

Comment 53

[Gregory Szorc [:gps]]

Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

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

Comment 54

[Gregory Szorc [:gps]]

Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

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

Comment 55

[Gregory Szorc [:gps]]

Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79960/diff/6-7/

Comment 56

[Gregory Szorc [:gps]]

Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79962/diff/6-7/

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ed46e8ac86c
Add environment variable to define location of mozinfo.json; r=ted
https://hg.mozilla.org/integration/autoland/rev/43df1e962f8e
Ensure WPT tests have a version control checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/2de97e3cfcb3
Reformat WPT command argument additions; r=ted
https://hg.mozilla.org/integration/autoland/rev/a1311218621b
Inline WPT arguments into script; r=ted
https://hg.mozilla.org/integration/autoland/rev/bde587e47d00
Use WPT files from source checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/b4844ee1e542
Stop producing web-platform tests zip file; r=ted

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/748abfe6748e
Run Mercurial with PYTHONUNBUFFERED=1; r=me

Comment 59

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3fff88de2f2d
Set env explicitly because mozharness; r=me

Comment 60

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ed46e8ac86c
https://hg.mozilla.org/mozilla-central/rev/43df1e962f8e
https://hg.mozilla.org/mozilla-central/rev/2de97e3cfcb3
https://hg.mozilla.org/mozilla-central/rev/a1311218621b
https://hg.mozilla.org/mozilla-central/rev/bde587e47d00
https://hg.mozilla.org/mozilla-central/rev/b4844ee1e542
https://hg.mozilla.org/mozilla-central/rev/748abfe6748e
https://hg.mozilla.org/mozilla-central/rev/3fff88de2f2d

Comment 61

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

As philor points out, this was causing timeouts on win7vm wpt tests, so all of this backed out in https://hg.mozilla.org/mozilla-central/rev/cda7c3e91ad7eb8119e1ae352cf362d9afa19918

https://treeherder.mozilla.org/logviewer.html#?job_id=4451408&repo=autoland

Comment 62

[Gregory Szorc [:gps]]

This is blocked on EBS performance on Windows testers. I don't need the needinfo set. I'll reland this as soon as it is unblocked.

Comment 63

[James Graham [:jgraham]]

So how does the wontfix in bug 1307798 affect this? I am blocking on this change for some of the wpt stability linting that I'd like to try.

Comment 64

[Gregory Szorc [:gps]]

... good question. Needinfo me to follow up on this.

Comment 65

[Gregory Szorc [:gps]]

I'm not sure what we can do here. We may just have to deal with whatever performance issues this presents.

Since I'll be away for ~1 month, I recommend following up with Ted. I /think/ the old commits should rebase pretty cleanly. Hopefully it isn't much work to flip the switch if we decide to move forward.

As long as automation is using `hg robustclone` and caches are configured properly, there shouldn't be a capacity problem for hg.mozilla.org. But it is something to watch if we enable this.

Comment 66

[Julien Cristau [:jcristau]]

Too late for firefox 52, mass-wontfix.

Comment 67

[Gregory Szorc [:gps]]

Moving the tracker bugs to bug 1432287.

Comment 68

[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 69

[James Graham [:jgraham]]

I'm not aware of any current plans to revive the work to move to a source checkout.