Closed Bug 1040924 Opened 7 years ago Closed 6 years ago

Fix and re-enable the serviceworker tests on non-Windows

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: RyanVM, Assigned: baku)

References

Details

Attachments

(1 file, 3 obsolete files)

Per this bug's deps, the serviceworker tests are unreliable on every platform except Windows. Due to their ongoing issues and the upcoming merge, they are being disabled on affected platforms. This bug tracks fixing and re-enabling them.
https://hg.mozilla.org/integration/mozilla-inbound/rev/69615044a960
Whiteboard: [tests disabled on !Windows][leave open]
Depends on: 1047398
Hey bent --- is the cause of this timeout known already?  It's not clear from the context in bug 1037739.

p.s. I'd like to use rr to debug this ;).
Flags: needinfo?(bent.mozilla)
Chris, I think our best theory was bug 1042908. Maybe apply that first and then see if rr shows anything? Thanks!
Flags: needinfo?(bent.mozilla) → needinfo?(cjones.bugs)
Attached patch test.patch (obsolete) — Splinter Review
The dependence is fixed. Can we try to reenable the tests?

https://tbpl.mozilla.org/?tree=Try&rev=b0b3e94441af
Depends on: 1042908
This is the latest result: https://tbpl.mozilla.org/?tree=Try&rev=5d676000e336

To me we can re-enable it and fix the issue when we find it.
Flags: needinfo?(ryanvm)
Flags: needinfo?(bkelly)
Looks like Android 2.3 is still in pretty bad shape. I'd support re-enabling everywhere else, though. Changing the annotation to |skip-if = android_version == "10"| should do the trick. Would also be nice if bug 1042908 could be completed in nsm's absence.
Flags: needinfo?(ryanvm)
Looks like there's a new B2G debug intermittent as well:
https://tbpl.mozilla.org/php/getParsedLog.php?id=45534564&tree=Try
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #7)
> Looks like Android 2.3 is still in pretty bad shape. I'd support re-enabling
> everywhere else, though. Changing the annotation to |skip-if =
> android_version == "10"| should do the trick. Would also be nice if bug
> 1042908 could be completed in nsm's absence.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #8)
> Looks like there's a new B2G debug intermittent as well:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=45534564&tree=Try

Since the code is going to be changing a lot over the next few weeks I'm not sure its worth hunting these down right now.  There is a good chance they will get fixed as we finish implementing the rest of SW.

So lets just skip on android and b2g-emulator.  Please just file a follow-up bug to re-enable and hang it off bug 903441.
Flags: needinfo?(bkelly)
Just to clarify, I don't think its worth hunting down given how slow it is to investigate android / b2g-emulator issues remotely via try.  We should definitely hunt down any desktop intermittents, though.
Trying to keep things as targeted as possible, let's go with:
skip-if = android_version == "10" || (toolkit == "gonk" && debug) # <new bug for re-enabling>

We can always get more restrictive later if we find more intermittents occurring in production. Andrea, mind filing said bug and updating the patch? :)
Flags: needinfo?(amarchesini)
Sure. Before doing this I want to see the 'try' result of bug 1056259.
Attached patch test1.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=0389ba661302

Ok, the android implementation is still buggy.
And for b2g I have a separated patch.
Attachment #8468455 - Attachment is obsolete: true
Attachment #8476597 - Flags: review?(ryanvm)
Flags: needinfo?(amarchesini)
How does the Try push in comment 13 relate to the Try pushes for the other checkin-needed patches of yours I pushed earlier today that had failures you said were OK to ignore?
Yesterday we landed bug 1056259 and this fixed several failures of this try push: https://tbpl.mozilla.org/?tree=Try&rev=28d7624a47fe

Today the situation was much better, talking about ServiceWorkers.
There are still issues with b2g where window.open() fails, but my solution is still not enough:
https://tbpl.mozilla.org/?tree=Try&rev=ec72ac6fb344

Does it answer to your question?
Comment on attachment 8476597 [details] [diff] [review]
test1.patch

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

Worth a shot. We can always make it more restrictive later if need-be.
Attachment #8476597 - Flags: review?(ryanvm) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/171355fe87e1
Whiteboard: [tests disabled on !Windows][leave open] → [tests disabled on Android 2.3 and B2G debug][leave open]
Attached patch test1.patch (obsolete) — Splinter Review
This excludes b2g as well.
Attachment #8476597 - Attachment is obsolete: true
Attachment #8478336 - Flags: review?(ryanvm)
Assignee: nsm.nikhil → amarchesini
Comment on attachment 8478336 [details] [diff] [review]
test1.patch

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

No need for the (toolkit == "gonk" && debug) part if you're skipping all of B2G.
Attachment #8478336 - Flags: review?(ryanvm) → review-
Attached patch test1.patchSplinter Review
Attachment #8478336 - Attachment is obsolete: true
Attachment #8478355 - Flags: review?(ryanvm)
Attachment #8478355 - Flags: review?(ryanvm) → review+
Blocks: 1056702
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #4)
> Chris, I think our best theory was bug 1042908. Maybe apply that first and
> then see if rr shows anything? Thanks!

I was able to repro either this or another of the serviceworker intermittent-fails with the fix from bug 1042908, but I left this open to double-check this particular symptoms.  The odds of that being meaningful are pretty low at this point so clearing request.  Sorry bent.
Flags: needinfo?(cjones.bugs)
We already have these enabled on central for everything except b2g. Bug 1053275 will fix intermittents.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [tests disabled on Android 2.3 and B2G debug][leave open]
You need to log in before you can comment on or make changes to this bug.