We are getting closer to having the service worker e10s redesign complete. While not ready to enable yet, many tests are green right now. Ultimately we will probably have the pref enabled on nightly for a release or so while things are polished, but still need to let the legacy mode ride the trains. To avoid regressions we will likely need a separate branch to run tests in whatever mode nightly is not running. So to start it would be a branch with the pref flipped on. After we enable on nightly it would be a branch with the pref flipped off. The reason we probably need a branch here is that service workers touch a huge amount of code: * Network * Document loading * Devtools * Its own complex state machine It is easy to regress whatever mode is not enabled by default on nightly. We want to catch this before the regression hits beta branch. Joel, how do we make this happen?

a few options come to mind: 1) run this in parallel on m-c only (downsides are regressions are harder to track down) 2) run this on a single platform (i.e. not OSX) per commit and let SETA help manage the load I assume the test suites would be: * xpcshell (a lot of networking) * mochitest-browser-chrome-devtools * wpt || mochitest (document loading) would we need everything? perf tests? To get something in parallel we need to do a couple things: 1) edit the .yml files that define a test https://searchfox.org/mozilla-central/source/taskcluster/ci/test/mochitest.yml#210 and make a copy of the suite you want, then add a mozharness option for the pref you want. 2) pick which platforms you want to run this on: https://searchfox.org/mozilla-central/source/taskcluster/ci/test/test-sets.yml#366 I am sure I am overlooking some details, but this should get the conversation moving in the right direction.

I guess I was thinking something similar to how stylo was done. Probably linux 32-bit debug would be a reasonable place to start since it runs e10s and non-e10s tests. (While the pref makes us handle e10s better, it also changes how we work in non-e10s.) I don't think we could just change normal test running for that platform to use the new pref yet, though. There is going to be a period where that mode is mostly green, but still has a bit of orange while things get polished. For example, we will probably have some broken devtools tests for a while since they need to be able to work with the final implementation to know how to adjust devtools to fix them.

the request here was for sw-e10s, so do we need to duplicate e10s and non-e10s tests? as soon as we are running geckoview in packet.net with e10s and web-platform-tests on fennec/geckoview, I see us turning off non-e10s tests for linux- that is still at least a month out. Do we need this on opt/debug? I would really focus on linux64, as linux32 as a platform has fewer tests in general than linux64.

I can't speak to future plans to migrate tier 1 platforms to e10s. If that is really happening in the near term, then great! If mozilla is going to be shipping a non-e10s tier 1 platform, though, then I think its important to run tests for both e10s and non-e10s. This code will be following a different path when the sw pref is flipped whether the browser is in e10s or non-e10s. There could be regressions in either case. Anyway, I just wanted to get the ball rolling here. I'll leave the specifics to Blake, Andrew, and Marion.

asuth: I think this may be worthwhile to setup

(In reply to Joel Maher ( :jmaher ) (UTC+2) from comment #3) > the request here was for sw-e10s, so do we need to duplicate e10s and > non-e10s tests? as soon as we are running geckoview in packet.net with e10s > and web-platform-tests on fennec/geckoview, I see us turning off non-e10s > tests for linux- that is still at least a month out. Re-stating what Ben said here: Yes. We need test coverage for both e10s and non-e10s for both states of the "dom.serviceWorkers.parent_intercept" boolean preference. The preference switches between two radically different implementations, and each implementation behaves radically differently for e10s and non-e10s. > Do we need this on opt/debug? I would really focus on linux64, as linux32 > as a platform has fewer tests in general than linux64. We want debug for assertion checking. We don't really need opt at this time. linux64 is fine. Thanks!

thanks for clarifying this. So linux64 unittests with tests in both e10s/non-e10s (where applicable) and having this preference set: dom.serviceWorkers.parent_intercept to value: ?? this will be on mozilla-central only, or all integration branches?

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #7) > thanks for clarifying this. > > So linux64 unittests with tests in both e10s/non-e10s (where applicable) and > having this preference set: > dom.serviceWorkers.parent_intercept > > to value: > ?? Currently: true (And then we'll flip this in bug 1456995. When that lands, setting the pref to `true` to nightly, we'll set it to `false` for this set of tests.) > this will be on mozilla-central only, or all integration branches? Just mozilla-central.

:ahal can you put this bug on your list to work on this month?

Sure, I'll keep the needinfo for now as a reminder. Aside (which is totally out of scope for this bug): we should really make "e10s" the default configuration and remove all those e10s labels from our treeherder symbols.

Just noting that enabling this for mochitest, reftest and web-platform (desktop only), we add ~1800 new tasks. This will be higher if we include mobile/additional suites. Andrew, is there a code name (like e10s) that developers will recognize this change by? I'm looking for something better than "serviceworker redesign" for things like cli flags and task descriptions.

(to clarify, I'm already using -sw for task symbols/labels)

So, we've been calling this "ServiceWorkers-e10s" which is what the meta-bug Bug 1231208's alias is. And our cool wiki tracking board is similarly named at https://wiki.mozilla.org/DOM/Workers-Storage/projects/e10s-sw. If you think "sw-e10s" is too ambiguous then "ServiceWorker Parent Intercept" is an also appropriate if somewhat misleading term that also happens to be what we named the preference, so that works out well. And then "sw-pi" or "swpi" maybe works when you need to distill that down. "-sw" seems great as a task symbol/label suffix. In terms of 1800 new tasks, is that after limiting this to only linux64 debug?

(In reply to Andrew Sutherland [:asuth] from comment #13) > So, we've been calling this "ServiceWorkers-e10s" which is what the meta-bug > Bug 1231208's alias is. And our cool wiki tracking board is similarly named > at https://wiki.mozilla.org/DOM/Workers-Storage/projects/e10s-sw. > > If you think "sw-e10s" is too ambiguous then "ServiceWorker Parent > Intercept" is an also appropriate if somewhat misleading term that also > happens to be what we named the preference, so that works out well. And > then "sw-pi" or "swpi" maybe works when you need to distill that down. > "-sw" seems great as a task symbol/label suffix. Thanks, those should cover all the bases. > In terms of 1800 new tasks, is that after limiting this to only linux64 > debug? Oops, missed that spec. Yeah that'll cut it down a lot more.

I almost forgot to update the regexes in moztest.resolve when creating the -sw variant of task. This adds a test to make sure we don't forget more things in the future.

These regexes are used for things like determining which tasks to run given a "path" int |mach try|. Previously, we used patterns like: mochitest-chrome-(?:e10s)?(?:-1)?$ This would match both e10s and non-e10s versions of a task with either no chunks, or only selecting chunk 1. But we keep adding other configurations, e.g -gpu, -no-accel, -sw, etc. Each time we create a new possibility we need to remember to update these task regexes (or else lose test coverage when using paths with |mach try|). Instead of individually listing every possibility, let's use a pattern like this: mochitest-chrome($|.*(-1|[^0-9])$) This also selects tasks that are either chunk 1 or don't have any chunks. But it allows for arbitrary strings in-between. This regex doesn't need to be updated when we add configurations like -sw. Depends on D7119

Comment on attachment 9012689 [details] Bug 1470266 - [moztest.resolve] Create a unittest for the task regexes, r?jmaher Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Comment on attachment 9012690 [details] Bug 1470266 - [moztest.resolve] Make task regexes more resilient to change, r?jmaher Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67da8157c1d6 [moztest.resolve] Create a unittest for the task regexes, r=jmaher https://hg.mozilla.org/integration/autoland/rev/3de6b966a246 [moztest.resolve] Make task regexes more resilient to change, r=jmaher

Here's an initial try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f45e0f532b0a589e7cf0d1604966225c1c98f88 Still have a few things left to implement: 1. Adding an 'sw' key to the manifests so we can skip tests based on this mode 2. Adding logging to the various runners so we can tell if it's enabled by inspecting a log Note, I decided to implement this by passing in --setpref (rather than creating a custom argument for it). Though I can definitely create a custom cli argument if desired. I therefore skipped some harnesses that don't have the ability to accpet prefs on the command line yet, like xpcshell, marionette, raptor and compiled tests. If some or all of these are important to have, I'd propose implementing them in a follow-up. Finally, do we care about talos for this configuration yet?

Looks like there are a few serviceworker test failures in that try run that'll need to be fixed or disabled.

(In reply to Andrew Halberstadt [:ahal] from comment #20) > I therefore skipped some harnesses that don't have > the ability to accpet prefs on the command line yet, like xpcshell, > marionette, raptor and compiled tests. If some or all of these are important > to have, I'd propose implementing them in a follow-up. We can get by without xpcshell tests initially. We're probably fine without marionette/raptor/compiled tests for now. But there are a ton of xpcshell tests under netwerk/test/unit that are potentially relevant and whose breakage would be _very bad_, so it'd want to be a fairly prompt follow-up. > Finally, do we care about talos for this configuration yet? I think we can handle talos via try pushes for our performance-related needs.

The mozharness scripts have a lot of special case arguments for one off configurations, stuff like --e10s, --enable-webrender and --gpu-required. Many of these command line args ultimately only end up setting an extra pref in the test harnesses. Instead, let's just give mozharness the ability to set prefs directly via --setpref. This way we can pass them through from taskgraph without needing to add extra configuration to mozharness when making changes like this.

(In reply to Andrew Sutherland [:asuth] from comment #22) > We can get by without xpcshell tests initially. We're probably fine without > marionette/raptor/compiled tests for now. But there are a ton of xpcshell > tests under netwerk/test/unit that are potentially relevant and whose > breakage would be _very bad_, so it'd want to be a fairly prompt follow-up. Ok, I'll handle it in this bug, though it might not land at the same time as the rest (might as well land what's finished).

Btw it looks like we don't run xpcshell with e10s *anywhere* at the moment: https://searchfox.org/mozilla-central/source/taskcluster/ci/test/xpcshell.yml#58 So that might lower the priority of getting them running (but I'll tackle it nonetheless).

(In reply to Andrew Halberstadt [:ahal] from comment #25) > Btw it looks like we don't run xpcshell with e10s *anywhere* at the moment: > https://searchfox.org/mozilla-central/source/taskcluster/ci/test/xpcshell. > yml#58 The xpcshell tests do this themselves via run_test_in_child(...) at https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/testing/xpcshell/head.js#1225 Here's a particularly important example netwerk test doing it for example: https://searchfox.org/mozilla-central/source/netwerk/test/unit_ipc/test_synthesized_response_wrap.js Other components do it slightly different ways; for example, indexedDB does some trickery with this manifest: https://searchfox.org/mozilla-central/source/dom/indexedDB/test/unit/xpcshell-child-process.ini and this helper logic: https://searchfox.org/mozilla-central/source/dom/indexedDB/test/unit/xpcshell-head-child-process.js

https://hg.mozilla.org/mozilla-central/rev/67da8157c1d6 https://hg.mozilla.org/mozilla-central/rev/3de6b966a246

Comment on attachment 9012928 [details] Bug 1470266 - [mozharness] Add --setpref argument to main desktop scripts, r?jmaher Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f251a29c5846 [mozharness] Add --setpref argument to main desktop scripts, r=jmaher

From my earlier try run, looks like reftest doesn't have any problems so I'm going to get that landed while I attempt to green up the other suites.

This adds a transform which will evaluate the 'serviceworker-e10s' key. If False, the task is returned as is. If True, the task is run with the 'dom.serviceWorkers.parent_intercept' pref enabled. If 'both' the task is split into two tasks, one with the pref, and one without.

This duplicates all the reftest tasks except with dom.serviceWorkers.parent_intercept set to true. For now they are only run on mozilla-central with linux64/debug. Depends on D7480

Comment on attachment 9013705 [details] Bug 1470266 - [taskgraph] Add ability to schedule serviceworker-e10s test tasks in taskgraph, r?jmaher Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Comment on attachment 9013706 [details] Bug 1470266 - [ci] Schedule serviceworker-e10s reftest tasks with linux64/debug on mozilla-central, r?jmaher Joel Maher ( :jmaher ) (UTC-4) has approved the revision.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3e6405e7631c [taskgraph] Add ability to schedule serviceworker-e10s test tasks in taskgraph, r=jmaher https://hg.mozilla.org/integration/autoland/rev/6eec41ce0c94 [ci] Schedule serviceworker-e10s reftest tasks with linux64/debug on mozilla-central, r=jmaher

https://hg.mozilla.org/mozilla-central/rev/f251a29c5846 https://hg.mozilla.org/mozilla-central/rev/3e6405e7631c https://hg.mozilla.org/mozilla-central/rev/6eec41ce0c94

This duplicates all the mochitest, based tests except with dom.serviceWorkers.parent_intercept set to true. For now they are only run on mozilla-central with linux64/debug.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/423ba8ffc8d7 [ci] Schedule serviceworker-e10s mochitest tasks with linux64/debug on mozilla-central, r=jmaher

Andrew, Ben, I ended up needing to disable the tests in dom/serviceworkers/test/mochitest.ini, which is obviously not ideal. But it was causing sporadic test timeouts and memory leaks so figured I'd get most of the tests going and leave it to you guys to re-enable that directory later. There were some other misc tests disabled as well, you'll be able to see them all at this link (once the latest commit merges and searchfox updates): https://searchfox.org/mozilla-central/search?q=serviceworker_e10s&case=true&path=ini%24 I'll work on wpt next, and finish off with xpcshell (which will be the harder).

Backed out changeset 423ba8ffc8d7 (Bug 1470266) for robocop failures. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=423ba8ffc8d742062721bd9d2d994b4aa8de21af Backout link: https://hg.mozilla.org/integration/autoland/rev/cff04dbdad5134c2583c86bb7f8472828f0afc49 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=203551896&repo=autoland&lineNumber=1372

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bf17ad5b022f [ci] Schedule serviceworker-e10s mochitest tasks with linux64/debug on mozilla-central, r=jmaher

Backed out for breaking test-verify. Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success,testfailed&fromchange=bf17ad5b022fd82b89695a945a687b1a6aef5f7d&tochange=28cbada91897aad669cee5b8780bcda5eb081a06&searchStr=tv&selectedJob=203667449 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=203667449&repo=autoland&lineNumber=1662 Backout link: https://hg.mozilla.org/integration/autoland/rev/28cbada91897aad669cee5b8780bcda5eb081a06 [task 2018-10-05T15:22:28.927Z] 15:22:28 INFO - options.extraPrefs = self.parseExtraPrefs(options.extraPrefs) [task 2018-10-05T15:22:28.928Z] 15:22:28 INFO - File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 915, in parseExtraPrefs [task 2018-10-05T15:22:28.929Z] 15:22:28 INFO - prefs = dict(parseKeyValue(prefs, context='--setpref=')) [task 2018-10-05T15:22:28.929Z] 15:22:28 INFO - File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 804, in parseKeyValue [task 2018-10-05T15:22:28.930Z] 15:22:28 INFO - (context, ','.join(missing)), errors=missing) [task 2018-10-05T15:22:28.931Z] 15:22:28 INFO - TypeError: not all arguments converted during string formatting [task 2018-10-05T15:22:28.975Z] 15:22:28 ERROR - Return code: 1 [task 2018-10-05T15:22:28.975Z] 15:22:28 INFO - TinderboxPrint: mochitest-browser-chrome<br/>11/0/0 [task 2018-10-05T15:22:28.975Z] 15:22:28 ERROR - # TBPL FAILURE # [task 2018-10-05T15:22:28.976Z] 15:22:28 WARNING - setting return code to 2 [task 2018-10-05T15:22:28.976Z] 15:22:28 ERROR - TinderboxPrint: Per-test run of .../browser_ext_tabs_highlight.js<br/>: FAILURE

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a3d2029e7b26 [ci] Schedule serviceworker-e10s mochitest tasks with linux64/debug on mozilla-central, r=jmaher

Hey Ben, just want to make sure someone is aware that dom/serviceworkers/test/mochitest.ini had to be disabled to get the mochitests running (due to frequent timeouts and memory leaks). You can see the full set of disabled mochitests here: https://searchfox.org/mozilla-central/search?q=serviceworker_e10s&case=true&path=ini%24 I'm not going to spend time greening these up, so please pass this on to whoever needs to know. Thanks!

301 :asuth

I started looking into WPT, but it makes bug 1400716 permafail. That bug isn't associated with any particular test, so I guess we'll have to fix it before we can enable wpt. At least we have a way to reproduce it now.

On closer inspection I don't think I'm hitting bug 1400716 (both issues just happen to have the same failure summary string)

This duplicates all web-platform-test mozharness based tests except with dom.serviceWorkers.parent_intercept set to true.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c77616575b61 [ci] Schedule serviceworker-e10s web-platform-test tasks with linux64/debug on mozilla-central, r=jgraham,jmaher

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13577 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Upstream PR merged

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67ff9b8c78bc [ci] Schedule serviceworker-e10s xpcshell tasks with linux64/debug on mozilla-central, r=jmaher

Why is this not running on try or autoland? I was just backed out of central, which seems like a bug to me (it wasted a lot of my time and delayed an important landing).

I don't think you're supposed to get backed out if you make these builds orange?

(In reply to Johann Hofmann [:johannh] from comment #58) > Why is this not running on try or autoland? I was just backed out of > central, which seems like a bug to me (it wasted a lot of my time and > delayed an important landing). I don't really have a stake in this besides being the one to implement the configuration changes, I made it central-only based on comment 8. (In reply to Andrew Sutherland [:asuth] from comment #59) > I don't think you're supposed to get backed out if you make these builds > orange? Tier 2 means that bustage can be tolerated for short periods of time (hours or days if the owner of the task says it's alright and fix is being worked on). Sometimes if a sheriff can't find anyone to clarify, they'll just back out to be safe. But typically tier-2 issues still need to be fixed in a reasonable time frame. Tier 3 is the one where bustage doesn't get backed out (and it's invisible by default).

Actually, looking at central these are tier 1 anyway. I agree with Johann they should either run on the integration branches or be bumped to tier 3.