Run wpt tests from source checkout

ASSIGNED
Assigned to

Status

Testing
web-platform-tests
ASSIGNED
10 months ago
25 days ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks: 4 bugs)

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 wontfix)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments)

(Assignee)

Description

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

This bug is about testing that theory.
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

10 months ago
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.
Blocks: 1278680
(Assignee)

Updated

9 months ago
Depends on: 1291365
(Assignee)

Updated

9 months ago
Depends on: 1294234
(Assignee)

Updated

9 months ago
Depends on: 1295380
(Assignee)

Updated

8 months ago
Depends on: 1296397
(Assignee)

Updated

8 months ago
Depends on: 1302907
(Assignee)

Updated

7 months ago
Depends on: 1304176
(Assignee)

Updated

7 months ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 18

7 months ago
mozreview-review
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.)
Attachment #8793159 - Flags: review?(ted) → review+

Comment 19

7 months ago
mozreview-review
Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

https://reviewboard.mozilla.org/r/79956/#review79456
Attachment #8793160 - Flags: review?(ted) → review+

Comment 20

7 months ago
mozreview-review
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?
Attachment #8793161 - Flags: review?(ted) → review+

Comment 21

7 months ago
mozreview-review
Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review79460
Attachment #8793162 - Flags: review?(ted) → review+

Comment 22

7 months ago
mozreview-review
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!
Attachment #8793163 - Flags: review?(ted) → review+
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.
(Assignee)

Comment 24

7 months ago
(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.
(Assignee)

Comment 25

7 months ago
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...
(Assignee)

Updated

7 months ago
Blocks: 1305205
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 32

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

Needs re-review for mozinfo.json changes.
Attachment #8793162 - Flags: review+ → review?(ted)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 35

7 months ago
mozreview-review
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.
Attachment #8795070 - Flags: review?(ted) → review+

Comment 36

7 months ago
mozreview-review-reply
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.
(Assignee)

Comment 37

7 months ago
mozreview-review-reply
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

7 months ago
mozreview-review-reply
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

7 months ago
mozreview-review
Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review80000
Attachment #8793162 - Flags: review?(ted) → review+
(Assignee)

Comment 40

7 months ago
mozreview-review-reply
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

7 months ago
Pushed by gszorc@mozilla.com:
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

7 months ago
mozreview-review-reply
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).
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.
(Assignee)

Updated

7 months ago
Blocks: 1305795
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.
(Assignee)

Updated

7 months ago
Depends on: 1305903
Blocks: 1306502
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 57

7 months ago
Pushed by gszorc@mozilla.com:
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

7 months ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/748abfe6748e
Run Mercurial with PYTHONUNBUFFERED=1; r=me

Comment 59

7 months ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3fff88de2f2d
Set env explicitly because mozharness; r=me

Comment 60

7 months ago
bugherder
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
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
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
Status: RESOLVED → REOPENED
status-firefox52: fixed → affected
Flags: needinfo?(gps)
Resolution: FIXED → ---
Target Milestone: mozilla52 → ---
(Assignee)

Updated

7 months ago
Depends on: 1307798
(Assignee)

Comment 62

6 months ago
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.
Status: REOPENED → ASSIGNED
Flags: needinfo?(gps)
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.
(Assignee)

Comment 64

3 months ago
... good question. Needinfo me to follow up on this.
Flags: needinfo?(gps)
(Assignee)

Comment 65

3 months ago
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.
Flags: needinfo?(gps)
Too late for firefox 52, mass-wontfix.
status-firefox52: affected → wontfix
You need to log in before you can comment on or make changes to this bug.