Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

Don't run tests until the page is under SW control #84

Merged
merged 3 commits into from
Dec 16, 2015

Conversation

jeffposnick
Copy link
Contributor

R: @gauntface @wibblymat @addyosmani @ebidel

As it turns out, @guantface ran into a similar race condition with the .ready promise. Switching from it to explicitly waiting on navigator.serviceWorker.controller seems to clear up the lingering test flakiness we were seeing.

We'll be following up on the .ready promise flakiness out-of-band.

Closes #37

@jeffposnick
Copy link
Contributor Author

So this actually isn't a bug in .ready. I'd been assuming that .ready wouldn't resolve until the page was controlled, but that's not the case—the clients.claim() inside the SW's activate handler isn't enough to ensure that. See w3c/ServiceWorker#799

I'm going to add in some additional changes to this PR to disabuse myself of that assumption throughout all the tests, not just that one specific one that was failing.

@jeffposnick
Copy link
Contributor Author

I've moved all the tests over to use a window._controlledPromise helper that I put together, to make sure that they all wait until the page is controlled before executing. That seems like the cleanest approach for the time being.

On a happy note, with #85 merged, all the tests are passing on Firefox Nightly as well!

@jeffposnick jeffposnick changed the title Works around what looks to be a bug in the .ready promise. Don't run tests until the page is under SW control Dec 15, 2015
@gauntface
Copy link

LGTM.

This makes sense to me and I'll like use this code in the sw-unit-test repo - hopefully won't make a difference but it's an important distinction.

jeffposnick added a commit that referenced this pull request Dec 16, 2015
Don't run tests until the page is under SW control
@jeffposnick jeffposnick merged commit fb5a32d into master Dec 16, 2015
@jeffposnick jeffposnick deleted the fix-flaky-test branch December 16, 2015 15:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants