1197365 - run wpt tests in e10s in automation

Status: RESOLVED FIXED

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

Description

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Jonathan, I'm told this is a problem.  Can you explain why?

Comment 2

[James Graham [:jgraham]]

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)?

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

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.)

Comment 4

[Jonathan Griffin (:jgriffin)]

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.

Comment 5

[James Graham [:jgraham]]

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.

Comment 6

[James Graham [:jgraham]]

Attachment: 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.

Comment 7

[James Graham [:jgraham]]

Attachment: 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.

Comment 8

[Jonathan Griffin (:jgriffin)]

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?

Comment 9

[Jonathan Griffin (:jgriffin)]

Also, feel free to continue using cedar for this, we're keeping it around with a reduced job set for the time being.

Comment 10

[Andrew Halberstadt [:ahal]]

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.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5b60f000985

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b5b60f000985

Comment 13

[James Graham [:jgraham]]

Sorry, should have marked this leaveopen since it isn't actually running anywhere yet.

Comment 14

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/2e7e0e4e5257

Comment 15

[James Graham [:jgraham]]

Attachment: wpt-missing-configs.diff

This was broken in a couple of different ways :( Attached a patch to fix things up a bit.

Comment 16

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/00bc20c89c57

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/080075cb97f4

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/080075cb97f4

Comment 19

[James Graham [:jgraham]]

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?

Comment 20

[Jonathan Griffin (:jgriffin)]

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.

Comment 21

[James Graham [:jgraham]]

Attachment: 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.

Comment 22

[James Graham [:jgraham]]

Attachment: List of builders added

Comment 23

[Jordan Lund (:jlund)]

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 :)

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/86a670f08a64
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d6d89e01c5

Comment 25

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

https://hg.mozilla.org/mozilla-central/rev/86a670f08a64
https://hg.mozilla.org/mozilla-central/rev/36d6d89e01c5

Comment 26

[James Graham [:jgraham]]

Attachment: 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.

Comment 27

[Jordan Lund (:jlund)]

Comment on attachment 8671336 [details] [diff] [review]
wpt-e10s.diff

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

lgtm

Comment 28

[James Graham [:jgraham]]

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

Comment 29

[James Graham [:jgraham]]

Now running on nightly + aurora branches, except on Windows where we are having a capacity crisis.

Comment 30

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.