After we start building Stylo support in regular Linux64 builds (pref'd off) in bug 1356991, we can: 1. Run Stylo tests using regular builds (with a pref flip or environment variable like WebRender's linux64-qr tests) instead of the linux64-stylo builds. 2. Stop building linux64-stylo. Running Stylo tests on platforms other than Linux64 can happen later.

Running Stylo tests using regular builds will also affect Talos.

The pref to enable Stylo is "layout.css.servo.enabled" = true.

patch to disable stylo specific builds and run the stylo-refests with a pref enabled I also disabled the sequential tests please confirm that this is expected

Builder diff if these changes were implemented on m-c cpeterson please confirm that this is what you are looking for

Also I noticed that there are stylo jobs running on beta now, can these be disabled?

(In reply to Kim Moir [:kmoir] from comment #5) > Also I noticed that there are stylo jobs running on beta now, can these be > disabled? Yes. We don't need to run Stylo jobs on Beta 55 (or Beta 56 next cycle). We will want them for Beta 57 and thereafter, if all goes well.

(In reply to Kim Moir [:kmoir] from comment #3) > Created attachment 8880856 [details] [diff] [review] > bug1374748.patch > > patch to disable stylo specific builds and run the stylo-refests with a pref > enabled > > I also disabled the sequential tests please confirm that this is expected We will still want to run Stylo some tests in parallel (default) mode and sequential mode (bug 1356122) and some in e10s and non-e10s mode. I updated our test matrix in the Stylo spreadsheet: https://docs.google.com/spreadsheets/d/1HmBsxrEpFfCgpLBG5Mvj5wEA9oEq5N8m4cxQYsSo4S8/edit#gid=1737640065 (In reply to Kim Moir [:kmoir] from comment #4) > Created attachment 8880859 [details] > m-cbuilder.diff > > Builder diff if these changes were implemented on m-c > > cpeterson please confirm that this is what you are looking for It looks like this diff removes Stylo crashtests, mochitests, Talos, and web-platform-tests and non-e10s mode. We will still want to run all the test suites we're running now.

So this is a bit tricky because you want the stylo tests to run in parallel with the non-stylo tests but they have all have the same name. Is this an an accurate assessment? For instance Run crashtest against linux64/opt Run crashtest against linux64/opt with '--parallel-stylo-traversal and (enable stylo pref) and layout.css.servo.enabled=true for parallel tests Run crashtest against linux64/opt with '--setperf=layout.css.servo.enabled=true' for sequential tests

(In reply to Kim Moir [:kmoir] from comment #8) > So this is a bit tricky because you want the stylo tests to run in parallel > with the non-stylo tests but they have all have the same name. Is this an an > accurate assessment? > > For instance > Run crashtest against linux64/opt > Run crashtest against linux64/opt with '--parallel-stylo-traversal and > (enable stylo pref) and layout.css.servo.enabled=true for parallel tests > Run crashtest against linux64/opt with > '--setperf=layout.css.servo.enabled=true' for sequential tests I think that is correct. Would adding new names for these Stylo tests make configuration easier? It looks like Treeherder has separate test names for linux64-stylo's reftests: opt-reftest-e10s (R1-R8) and opt-reftest-stylo-e10s (Rs1-Rs16).

patch to disable stylo jobs from running on beta. will attach diff Some of the tests weren't limited by branch

Pushed by kmoir@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c923a5e8169f Run Stylo tests using regular builds and stop building linux64-stylo builds r=aki DONTBUILD

Given that we need to set a couple of properties and run the regular set of tests against linux64 builds as well as the stylo ones, I think we need to set a property in the same way that they e10s tests run. i.e. setting this flag essentially allows the tests to be duplicated and copied with new properties like in @transforms.add def split_e10s(config, tests): in taskcluster/taskgraph/transforms/tests.py and give them new names as well to distinguish them from non-stylo tests.

I tried a few permutations of this but wasn't able to get it working before I left for PTO.

I took a quick look at this, and found one problem that's going to make this hard to do: there's no common way to enable stylo across test suites. It would be nice if --enable-stylo worked for mozharness and the various underlying suites, but that's not the case right now. Another approach would be to enable stylo if an environment variable is set, e.g. MOZ_STYLO=1, but that also is not supported right now.

(In reply to Chris AtLee [:catlee] from comment #17) > It would be nice if --enable-stylo worked for mozharness and the various > underlying suites, but that's not the case right now. Another approach would > be to enable stylo if an environment variable is set, e.g. MOZ_STYLO=1, but > that also is not supported right now. So an environment variable would be adequate? How is Quantum Render enabled for the linux64-qr test runs? Setting an environment variable sounds a lot easier than adding --enable-stylo flags to multiple test suites for a feature that will soon by enabled by default.

The QR tests use an environment variable MOZ_WEBRENDER, which is set in configs in places like this: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/desktop_unittest.py#709 And then used in the code here: https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#2388 I agree that using an environment various sounds a lot easier right now. In general though, I do think it would be nice if test harnesses all supported setting arbitrary prefs. Some already do, judging by some green test runs on this try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9159ff2650470993fc39126ab279589521b03b23

Given that MOZ_STYLO is already the name of our #define, I think FORCE_ENABLE_STYLO is a better name. Easiest way is to check the env var here: http://searchfox.org/mozilla-central/rev/238406d4c1b3f147522ce0a45a4c6f84a8115781/layout/base/nsLayoutUtils.cpp#7798 If the var is set, we just set sStyloEnabled to true and avoid hooking it up to the pref system.

Or maybe FORCE_STYLO_ENABLED, I guess.

How about using STYLO_THREADS as a proxy? If you set it to a positive value, it uses that number of threads, if you set it to 0 (or -1, maybe), it finds a good number of threads, and you don't set it, stylo is not enabled.

STYLO_THREADS is already overloaded and used in a entirely different part of the code. I don't see any advantage to overloading it more for this very specific and short-lived use-case of overriding the normal pref mechanism.

I filed bug 1379300 to add an env var for Stylo.

With bug 1379300 landed, you should be able to use STYLO_FORCE_ENABLED=1 for this purpose.

Now that we have Stylo support in Nightly builds for Linux64, Mac, and Windows, we should run our Stylo tests on those platforms. We should run all the same test configurations as we do for linux64-stylo on inbound, autoland, and central: https://docs.google.com/spreadsheets/d/1HmBsxrEpFfCgpLBG5Mvj5wEA9oEq5N8m4cxQYsSo4S8/edit#gid=1737640065 We don't need to run the Mac and Windows tests in Stylo sequential mode, just parallel mode.

Simply setting the env var isn't enough. The mozinfo file claims that stylo is off, which means that all of the skip-if=stylo tests run.

:catlee attempted a try run to use the STYLO_FORCE_ENABLED=1 env var, but there's an additional issue of the test annotations. Some tests use mozinfo to decide what tests should run, and this checks MOZ_STYLO_ENABLE from build time when setting the `stylo` field, which will be false if we use only the STYLO_FORCE_ENABLED=1 env var. This means in the current state, Stylo is getting enabled, but the test harness thinks it is disabled, so it tries to run tests that are known to fail with Stylo. :ahal, is there a good place to override mozinfo during test runs to adjust mozinfo based on an env var?

Is this for mochitest? Looks like there's an --extra-mozinfo-json argument that will do what you want: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/mochitest_options.py#356

Okay, I filed bug 1380082 to update the test harnesses to run the right tests with the env var.

(In reply to Andrew Halberstadt [:ahal] from comment #29) > Is this for mochitest? Looks like there's an --extra-mozinfo-json argument > that will do what you want: > https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/ > mochitest_options.py#356 We currently run mochitests, reftests, crashtests, and web-platform-tests with Stylo enabled. Also, we run reftests in two different Stylo modes: comparing Stylo-vs-Stylo (similar to the default Gecko-vs-Gecko mode) and Stylo-vs-Gecko. This spreadsheet describes the various combinations of tests and Stylo/e10s modes we run on the inbound, autoland, and central repos: https://docs.google.com/spreadsheets/d/1HmBsxrEpFfCgpLBG5Mvj5wEA9oEq5N8m4cxQYsSo4S8/edit#gid=1737640065

Ah ok, it's probably best to handle each of those within the respective harnesses since they use different manifest formats. I think this is most analogous to e10s, if you foresee developers wanting to run this locally a lot, it may be worth adding a --stylo flag to the harnesses or something.

Here's a tweaked version of :catlee's changes to switch back to regular builds using the env var. This depends on other patches in bug 1380082. Here's a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=66695eac04fe2aa9e5514c05edd85714a896cf97 I am not sure what other steps might be needed, so I'll leave this part here for someone else to pick up.

Kim, Ryan is configuring the Stylo tests to use regular builds (instead of linux64-stylo) and to run on Mac and Windows. Once he has the tests running, you can turn off the linux64-stylo custom builds. In the meantime, I don't think you need to do anything for this bug.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #33) > Created attachment 8885934 [details] [diff] [review] > Use regular builds for Stylo via env var > > Here's a tweaked version of :catlee's changes to switch back to regular > builds using the env var. > > This depends on other patches in bug 1380082. Here's a try run: > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=66695eac04fe2aa9e5514c05edd85714a896cf97 > > I am not sure what other steps might be needed, so I'll leave this part here > for someone else to pick up. Linux64 appears to work in this mode (with just the env var). I am still triaging the state of other desktop platforms in bug 1380053.

After talking on IRC, updating the scope of this bug to focus on the Linux64 part only. For Stylo tests on other desktop platforms, we'll use the separate bug 1380053 (first to check tests, perhaps another is needed for actual task changes). With Linux64, the tests appear green in my last try run from comment 33: https://treeherder.mozilla.org/#/jobs?repo=try&revision=66695eac04fe2aa9e5514c05edd85714a896cf97 However, the special Stylo builds still ran, so it seems more steps are needed to actually remove them. I don't think I am best placed to make this change, or at least I would need some pointers on what remains. :catlee, since you started the WIP patch I used above, do you know what's left here?

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #36) > However, the special Stylo builds still ran, so it seems more steps are > needed to actually remove them. Ah, my mistake, the linux64-stylo builds do appear to be gone in the try run. So maybe it's ready to go, but there could be other bits to clean up that I am unaware of.

The try run in comment 33 was tests only, let's verify Talos as well: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfb836a87a9c17f9b0aeb63b7356ae5e8762b0c2

Talos is driven by Buildbot, so we need a different way to enable Stylo. Trying that based on :catlee's suggestion: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb76d4a7687b282e50a52b95647051ecf707270c

Another attempt to activate Stylo for Talos doesn't seem to be working: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a547800299e604ee7f7810edb044b4b76106dc9

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #40) > Another attempt to activate Stylo for Talos doesn't seem to be working: > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=9a547800299e604ee7f7810edb044b4b76106dc9 Ah, my mistake, I must have opened the wrong log at first. :( Okay, I'll fold this version into my patch.

Comment on attachment 8887711 [details] Bug 1374748 - Use regular builds for Stylo via env var. https://reviewboard.mozilla.org/r/158616/#review164244

Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c21ec4c07f82 Use regular builds for Stylo via env var. r=catlee

Backing this out for breaking talos on at least Mac platforms. Regular mac builds: https://treeherder.mozilla.org/logviewer.html#?job_id=115672376&repo=autoland Devedition mac builds: https://treeherder.mozilla.org/logviewer.html#?job_id=115672403&repo=autoland https://hg.mozilla.org/integration/autoland/rev/22e24b1ada9431c21c7f1f8a950d68ec94842c83

Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6c77cacaf82d Use regular builds for Stylo via env var. r=catlee