1610977 - Figure out what to do with mochitest-sw

Status: RESOLVED FIXED

(Testing :: General, task, P3)

Description

[Ryan VanderMeulen [:RyanVM]]

Right now, we're running mochitest-sw in addition to regular mochitests on Trunk and Try. My understanding is that the original intent for this test suite was to test SW-e10s while they were still the non-default option.

However, it isn't clear to me what use case they are meant to be serving now with SW-e10s and DocumentChannel enabled by default.

Should they be testing the opposite configuration of whatever the current enabled state is? If so, they should probably be riding the trains with whatever Gecko version is changing the defaults. If not, do we really need them anymore?

Comment 1

[Geoff Brown [:gbrown]]

mochitest-sw specifies --setpref="dom.serviceWorkers.parent_intercept=true", but as RyanVM says, it looks like that is set for regular mochitest also. When I check logs for mochitest (or mochitest-sw), I see:

runtests.py | Running with serviceworker_e10s: True

Comment 2

[Geoff Brown [:gbrown]]

:asuth - Do you know if there is still value in mochitest-sw, or should I retire them now?

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

There is no value as-is. The original intent was to cause the mochitest-sw jobs/tasks to have the opposite value of the default at all times. So when we transitioned the default to true, we should have transitioned mochitest-sw to false. This didn't happen.

Because sw-e10s and documentchannel are interdependent, I think if we were to make the tasks useful, we'd also want to disable documentchannel. Effectively, we want the disabling of sw-e10s/documentchannel on beta in bug 1610888 to be continually tested by the job. I think the main complexity is still the exposure of the "sw-e10s" flag to the mochitest logic, which ended up being hardcoded in bug 1588154. It would instead want to have some extra logic in there to be able to be disabled.

:RyanVM, is this something release management would like?

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Probably worth doing until DC/SW-e10s make it to release. We certainly had CI friction from the disabling in 73.

Comment 5

[Geoff Brown [:gbrown]]

I don't quite understand comment 3 / not sure what's needed here. Probably better for someone closer to the issues to implement the required changes (or I can pick this up again if someone needinfos me with specific requirements).

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

it sounds like we want no-sw instead of sw? If we did that, how long would we run these tests?

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

The flipped tests would want to exist until DocumentChannel and ServiceWorkers-e10s' enablings have stuck on release and we're super sure we won't have to turn them off. Right now they've both just re-ridden the trains to 74 beta. (They previously rode on 73 and were disabled late in the cycle in bug 1610888.) I'd presume we'd want the tests available until we've had them enabled for an entire release, which would mean that we if they don't get disabled again, we could start removing them once release=75 on 2020-04-06.

I think it will be the end of the week before we (Workers team) could have a flipped variant. Maybe it makes sense to remove the "-sw" tests now and then we can create the new "-no-dcsw" variant to realize the cost savings immediately.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

as a note the -sw tests are not running on mozilla-beta, only on mozilla-central. So the tests provide little safe guards, but the good news is they are low cost.

I think it is OK to keep -sw around this week and just change the few pieces needed for them to be -no-dcsw when the tests are ready.

Please ask if you need help with anything.

Comment 9

[Bob Clary [:bc] (inactive)]

As far as I can tell there is nothing to do until a flipped variant is available.

Comment 10

[Bob Clary [:bc] (inactive)]

Attachment: Bug 1610977 - disable service worker testing until alternatives are available, r=jmaher

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Wasn't the intent of comment 8 to modify the existing test suite rather than shutting them off?

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

I think the idea is that Perry or I or :mattwoodrow need to update the tests, and until we do, we're just burning moneys.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12c7490d8dfd
disable service worker testing until alternatives are available, r=jmaher

Comment 14

[Andreea Pavel [:apavel]]

Backed out for failing android test_error_reporting.html

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=12c7490d8dfd50a00351f4679ae42704b6051965&selectedJob=290390381

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290390381&repo=autoland&lineNumber=5216

Backout: https://hg.mozilla.org/integration/autoland/rev/784a923d204be20d57b0b3b1827b44593952d477

Comment 15

[Andreea Pavel [:apavel]]

There is also this mochitest chrome failure https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290390979&repo=autoland&lineNumber=53232

Comment 16

[Bob Clary [:bc] (inactive)]

The failing tests are skipped normally with Running with serviceworker_e10s: True. With the patch, serviceworker_e10s is not set and the associated tests are then run and fail. Note that I also reproduced this locally via a ./mach mochitest dom/serviceworkers without and with the patch. I have a patch that seems to work locally. Am doing a try run now and will push to phab if it looks good.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/337320a761ef
disable service worker testing until alternatives are available, r=jmaher

Comment 18

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/337320a761ef