Closed Bug 1370461 Opened 7 years ago Closed 7 years ago

stylo: Add stylo config to WPT expectations

Categories

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

Version 3
enhancement

Tracking

(firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: manishearth, Assigned: jgraham)

References

Details

Attachments

(5 files)

We'd like to run WPT on stylo as well. We have a bunch of failures right now, and it would be nice to have WPT expectations up before we fix all of them.

I tried adding an `if stylo` mode but I can't really figure out the plumbing for this, through the taskcluster and wptrunner files.


Additionally, in parallel mode (which is what we do on e10s stylo builds), wpt exits with the following error:


 web_platform_tests.py: error: no such option: --parallel-stylo-traversal


https://treeherder.mozilla.org/logviewer.html#?job_id=104445139&repo=try&lineNumber=531#L557
James/Nathan, any idea how to do this? If it's too complicated I'd rather just work on fixing all the WPT bugs, but if not I'd like to have this in place so that we can benefit from the additional coverage.
Flags: needinfo?(nfroyd)
Flags: needinfo?(james)
Blocks: stylo-wpt
I have zero experience with WPT, so James is probably the better person for ideas on how to accomplish this.
Flags: needinfo?(nfroyd)
Priority: -- → P3
Summary: Add stylo config to WPT → stylo: Add stylo config to WPT expectations
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d07f02f140b199838ba3a227735005fa817e3d7&selectedJob=105217022

Looks like there is a little bit of instability in stylo-debug to sort out, but the tests could be enabled on opt right away.
Flags: needinfo?(james)
Any progress on landing this? Fine with it being opt-only for now.
Flags: needinfo?(james)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e482e37e0ac78ca8b3bca64591aa540003b03f3c to get another round of metadata updates. Reviews requested. Maja - if you are fed up of me requesting wptrunner reviews from you let me know, we can probably find some servo person to do the reviews in this case.
Comment on attachment 8878114 [details]
Bug 1370461 - Support --parallel-stylo-traversal for wpt in mozharness,

https://reviewboard.mozilla.org/r/149522/#review154124

::: testing/mozharness/scripts/web_platform_tests.py:75
(Diff revision 1)
> -         ]
> +         ],
> +        [["--parallel-stylo-traversal"], {
> +            "action": "store_true",
> +            "dest": "parallel_stylo_traversal",
> +            "default": False,
> +            "help": "Forcibly enable parallel traversal in Stylo with STYLO_THREADS=4"}

Any reason you hard code 4 instead of making this type int?
Attachment #8878114 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8878114 [details]
Bug 1370461 - Support --parallel-stylo-traversal for wpt in mozharness,

https://reviewboard.mozilla.org/r/149522/#review154124

> Any reason you hard code 4 instead of making this type int?

Because otherwise it wouldn't be compatible with the command line parameters passed to other testsuites, which would make updating the task definitions much harder. I agree it's an odd choice to hardcode it, but I guess it must be good enough for what the stylo people want to test.
Comment on attachment 8878112 [details]
Bug 1370461 - Run wpt tests on stylo builds,

https://reviewboard.mozilla.org/r/149518/#review154442
Attachment #8878112 - Flags: review?(jmaher) → review+
Still a couple of tests causing problems on debug (which ofc I can't reproduce); just need to disable those.
Depends on: 1373629
(In reply to James Graham [:jgraham] from comment #12)
> Comment on attachment 8878114 [details]
> Bug 1370461 - Support --parallel-stylo-traversal for wpt in mozharness,
> 
> https://reviewboard.mozilla.org/r/149522/#review154124
> 
> > Any reason you hard code 4 instead of making this type int?
> 
> Because otherwise it wouldn't be compatible with the command line parameters
> passed to other testsuites, which would make updating the task definitions
> much harder. I agree it's an odd choice to hardcode it, but I guess it must
> be good enough for what the stylo people want to test.

IIUC, we do this because the test machines have only 1 CPU, which would normally turn off parallel stylo.  So we have this command-line option and associated environment variable to force parallel stylo.  The precise value of STYLO_THREADS doesn't matter too much here (as long as it's not something ridiculous, like 30), but compatibility with other suites is probably a good idea.
Comment on attachment 8878115 [details]
Bug 1370461 - Update expected data for stylo,

https://reviewboard.mozilla.org/r/149524/#review154556
Attachment #8878115 - Flags: review?(manishearth) → review+
Comment on attachment 8878112 [details]
Bug 1370461 - Run wpt tests on stylo builds,

https://reviewboard.mozilla.org/r/149518/#review154558
Attachment #8878112 - Flags: review?(manishearth) → review+
Comment on attachment 8878111 [details]
Bug 1370461 - Add support for stylo metadata in wpt,

https://reviewboard.mozilla.org/r/149516/#review154618
Attachment #8878111 - Flags: review?(mjzffr) → review+
Comment on attachment 8878113 [details]
Bug 1370461 - Allow setting number of stylo threads in wpt,

https://reviewboard.mozilla.org/r/149520/#review154620
Attachment #8878113 - Flags: review?(mjzffr) → review+
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/a86a509f0d85
Add support for stylo metadata in wpt, r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/d3c249159ba5
Run wpt tests on stylo builds, r=jmaher,manishearth
https://hg.mozilla.org/integration/autoland/rev/3e8d584fd5db
Allow setting number of stylo threads in wpt, r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/04ecf78a1c4c
Support --parallel-stylo-traversal for wpt in mozharness, r=ahal
https://hg.mozilla.org/integration/autoland/rev/4c6bd08f65bd
Update expected data for stylo, r=manishearth
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/9b6a8d757dc2
Update expectations in label-attributes.html for stylo. r=emilio
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/b47db2569c5f
Update expectations in label-attributes.html for stylo, v2. r=me
Assignee: nobody → james
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: