run wpt tests in e10s in automation

RESOLVED FIXED in Firefox 43

Status

Testing
web-platform-tests
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: bkelly, Assigned: jgraham)

Tracking

(Blocks: 1 bug)

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(e10s+, firefox43 fixed)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
There is a desire to move more of our main testing out of mochitests and into wpt tests.  This can't really happen, though, unless we run wpt on the same platforms as mochitests.  Getting wpt running on debug was a huge improvement.  The next hurdle seems to get wpt running on e10s in addition to non-e10s.

For example, I'm writing wpt tests for service workers.  Unfortunately, though, we have a fair number of e10s-only bugs due to the way the necko stack is architected differently in that mode.  Without running wpt in e10s we can't rely on the tests to provide coverage.
Blocks: 984139
tracking-e10s: --- → +
(Reporter)

Comment 1

3 years ago
Jonathan, I'm told this is a problem.  Can you explain why?
Flags: needinfo?(jgriffin)
(Assignee)

Comment 2

3 years ago
So I see three options here:

1) We continue as-is and people write tests for features that behave differently under e10s using mochitest (service workers, streams, anything else that touches the network or storage) rather than wpt because they get better coverage.
2) We duplicate all web-platform-tests in e10s enabled and e10s disabled configurations.
3) We duplicate a subset of wpt in an e10s-enabled configuration, according to the features that are most likely to vary between the two configurations (or, conversely, we switch all tests to run under e10s and duplicate a subset under non-e10s).

1) is not an option I like at all; it prevents us sharing our tests with other browser vendors so although it may help e10s ship it won't help us with the wider goal of having great implementation quality / compatibility. 2) is the obvious option, but may be prohibitively expensive in terms of machine time and number of added jobs. 3) is a compromise position that provides less coverage and manual work to identify the tests most likely to differ but will save infrastructure resources.

jgriffin: do you have a feeling for whether 2) is acceptable or whether we have to do 3)?
(Reporter)

Comment 3

3 years ago
I would rather wpt tests run in both e10s and non-e10s by default.  If you have some tests you think should not be effected by the processing mode, then specifically opt those into one mode or the other.

Lets not create extra stumbling blocks to writing new wpt tests like requesting full coverage for them.

(After all, if I write them as mochitests they automatically get e10s and non-e10s testing anyway.)
I think 2) is the preferred approach, with the caveat that we may not be able to roll it out completely for Windows, due to capacity limitations.  On Windows, I think we should start with win7-opt, assess the impact, then consider rolling out win7-debug or win8-opt, etc.
Flags: needinfo?(jgriffin)
(Assignee)

Comment 5

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=98bd705942e4 is wpt with all tests running under e10s. Clearly it's going to be necessary to have some different metadata for e10s vs not-e10s. I think the easiest way to do that is to manually add it to the run_info and allow the manifests to use 'e10s' as a boolean variable like 'debug'. The only other sensible option I can think of is to duplicate all the metadata, but that seems unpleasant for obvious reasons.
(Assignee)

Comment 6

3 years ago
Created attachment 8654186 [details] [diff] [review]
wpt-e10s.diff

Buildbot config changes to enable wpt-e10s on Try only. This depends on the wptrunner/mozharness changes (forthcoming) to add the --e10s option; presumably for builds missing that we'll just get an error. Since this is only Try I'm not that worried; the jobs will initially fail anyway and so should be hidden.
Attachment #8654186 - Flags: review?(jgriffin)
(Assignee)

Comment 7

3 years ago
Created attachment 8654202 [details] [diff] [review]
0001-Enable-e10s-for-web-platform-tests.patch

wptrunner bits need to be upstreamed, obviously.

This seems to work OK in some local testing, but the nature of the patch means I can't run it through try.
Attachment #8654202 - Flags: review?(ahalberstadt)
Comment on attachment 8654186 [details] [diff] [review]
wpt-e10s.diff

Review of attachment 8654186 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozilla-tests/config.py
@@ +747,5 @@
> +WEB_PLATFORM_REFTESTS_E10S = [
> +    ('web-platform-tests-reftests-e10s', {
> +        'use_mozharness': True,
> +        'script_path': 'scripts/web_platform_tests.py',
> +        'extra_args': ["--test-type=reftest"],

Is this missing an --e10s arg?
Attachment #8654186 - Flags: review?(jgriffin) → review+
Also, feel free to continue using cedar for this, we're keeping it around with a reduced job set for the time being.
Comment on attachment 8654202 [details] [diff] [review]
0001-Enable-e10s-for-web-platform-tests.patch

Review of attachment 8654202 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm! (I guess) :)

::: testing/web-platform/harness/wptrunner/products.py
@@ +43,5 @@
>      browser_kwargs = getattr(module, data["browser_kwargs"])
>      executor_kwargs = getattr(module, data["executor_kwargs"])
>      env_options = getattr(module, data["env_options"])()
> +    run_info_extras = (getattr(module, data["run_info_extras"])
> +                       if "run_info_extras" in data else lambda **kwargs:{})

Neat, I've never really thought of **kwargs in a lambda before.
Attachment #8654202 - Flags: review?(ahalberstadt) → review+
https://hg.mozilla.org/mozilla-central/rev/b5b60f000985
Assignee: nobody → james
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 13

3 years ago
Sorry, should have marked this leaveopen since it isn't actually running anywhere yet.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

3 years ago
Created attachment 8657218 [details] [diff] [review]
wpt-missing-configs.diff

This was broken in a couple of different ways :( Attached a patch to fix things up a bit.
Attachment #8657218 - Flags: review?(jgriffin)
Attachment #8657218 - Flags: review?(jgriffin) → review+
(Assignee)

Updated

3 years ago
Keywords: leave-open
(Assignee)

Comment 19

3 years ago
So I tried enabling this on nightly/aurora and seem to have run into the builder limit:

  File "/home/jgraham/.mozilla/releng/repos/braindump/buildbot-related/dump_master_json.py", line 165, in <module>
    main()
  File "/home/jgraham/.mozilla/releng/repos/braindump/buildbot-related/dump_master_json.py", line 146, in main
    dump = dump_master(args.masters[0])
  File "/home/jgraham/.mozilla/releng/repos/braindump/buildbot-related/dump_master_json.py", line 111, in dump_master
    c = loadMaster(path)
  File "/home/jgraham/.mozilla/releng/repos/braindump/buildbot-related/dump_master_json.py", line 26, in loadMaster
    execfile(path, g, g)
  File "/home/jgraham/.mozilla/releng/repos/buildbot-configs/test-output/bm51-tests1-linux64/master.cfg", line 246, in <module>
    assert count <= builderLimit, "%s has %i builders; limit is %i" % (s, count, builderLimit)
AssertionError: tst-linux64-spot-2089 has 4104 builders; limit is 4084

Any ideas?
Flags: needinfo?(catlee)
I'm removing all the builders on gum in bug 1206269.  Once this is merged to production, you should have available builders to work with.
Flags: needinfo?(catlee)
(Assignee)

Comment 21

3 years ago
Created attachment 8664534 [details] [diff] [review]
Enable wpt-e10s on default trees for nightly and aurora

Attached patch to enable tests on e10s-enabled trees. Obviously this depends on bug 1206016 to ensure that people can try their changes before landing to inbound.
Attachment #8664534 - Flags: review?(jlund)
(Assignee)

Comment 22

3 years ago
Created attachment 8664535 [details]
List of builders added
Comment on attachment 8664534 [details] [diff] [review]
Enable wpt-e10s on default trees for nightly and aurora

Review of attachment 8664534 [details] [diff] [review]:
-----------------------------------------------------------------

looks good to me :)
Attachment #8664534 - Flags: review?(jlund) → review+
(Assignee)

Comment 26

3 years ago
Created attachment 8671336 [details] [diff] [review]
wpt-e10s.diff

Updated the patch to enable this to remove the windows builders on non-try due to capacity issues. Tested again with braindump.
Attachment #8664534 - Attachment is obsolete: true
Attachment #8671336 - Flags: review?(jlund)
Comment on attachment 8671336 [details] [diff] [review]
wpt-e10s.diff

Review of attachment 8671336 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8671336 - Flags: review?(jlund) → review+
(Assignee)

Comment 28

3 years ago
https://hg.mozilla.org/build/buildbot-configs/rev/418daa4d6110ca27f444a58494201d6549990e2b
Bug 1197365 - Enable web-platform-tests-e10s tests on nightly and aurora branches, r=jlund
(Assignee)

Comment 29

3 years ago
Now running on nightly + aurora branches, except on Windows where we are having a capacity crisis.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.