tests: add smoketest for slow service worker #6648
Conversation
| } | ||
|
|
||
| if (window.location.search.includes('slow')) { | ||
| setTimeout(() => registerServiceWorker('?delay=5000&slow'), 500); |
There was a problem hiding this comment.
would it make any difference to do this on "load", where the load event is deferred by a significant amount of time?
There was a problem hiding this comment.
This setTimeout is mostly useless I can actually remove it since it's complicating the understanding of what's going on. Switching to fire on the load event is a good idea 👍
the load event is currently being delayed by an image farther down that takes 2s, the only way LH doesn't catch service worker AFAICT is if the page doesn't call .register quickly enough, and this test was meant to prove that fact.
There was a problem hiding this comment.
AFAIK this (registering SW on "load") is basically what pintrest.com (which LH says has no SW) does. not sure if the key difference is how long until "load" fires - or something else entirely.
There was a problem hiding this comment.
If that's all they're doing, then I'm not sure what's going on there. We always wait until load so unless their load event handlers take a very long time such that we don't get to .register in time, then it must be something else.
There was a problem hiding this comment.
I can take a look closer at pinterest if there's not already too many cooks in the kitchen :)
There was a problem hiding this comment.
There's at most 1 :) IDK if @wardpeet ever started on it.
There was a problem hiding this comment.
Haha, alright I'll poke around.
| console.log('service worker registration complete'); | ||
|
|
||
| registration.addEventListener('statechange', e => { | ||
| console.log('sw registration is now', e.target.state); | ||
| }); | ||
| }).catch(function(e) { | ||
| } catch (e) { | ||
| console.error('service worker is not so cool.', e); | ||
| throw e; |
There was a problem hiding this comment.
do we check for this anywhere? Or is the hope that a smoke test will fail and this would be used for debugging.
Maybe we should be pulling in errors-in-console into all our smoke tests :)
There was a problem hiding this comment.
do we check for this anywhere?
nope
Or is the hope that a smoke test will fail and this would be used for debugging.
Bingo!
Maybe we should be pulling in errors-in-console into all our smoke tests :)
Not a bad idea, followup! :D
yeppers |
Summary
Curious what types of late service worker we don't catch. I saw @paulirish's comment that it's just calling
.registerbut I was skeptical based on the other reports. I should not have been skeptical it lives up, and this just adds a smoketest to prove it. We wait for delays in loading the service worker script itself and delays in the install event too.Related Issues/PRs
#2924 (comment)
#5527