Add tests for navigator.serviceWorker.waiting.

Add tests for navigator.serviceWorker.waiting.

BUG=379012
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176157

[dominicc (has gone to gerrit), 2014-06-10 05:04:15]

PTAL

This is the integration test for https://codereview.chromium.org/317953004/

[jsbell, 2014-06-10 17:59:33]

lgtm with nits.

I don't know whether an Oprah meme ("everyone get a t.step_func() wrapper") or
Buzz Lightyear meme ("t.step_func() wrappers... everywhere!") is more
appropriate.

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/waiting.html (right):

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:15: .then(function() { return
with_iframe(scope); },
Needs t.step_func() wrapper (testharness.js is updated to pass the return value
through)

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:15: .then(function() { return
with_iframe(scope); },
Style nit: In other tests, we don't seem to start lines with `.then(...`
elsewhere, but instead wrap before the closing parenthesis, i.e. `).then(...` or
`}).then(...` etc.

(That may just be due to *my* style being copied rather than
Blink/Chromium/Google style, though, so all of the existing formatting may be
"wrong" if it is documented somewhere! Google JS style, at least, says the .
should go on the preceding line if it's going to wrap there.)

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:17: .then(function(f) {
Needs t.step_func() wrapper

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:21:
.then(function(serviceWorker) {
Needs t.step_func() wrapper

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:24: return new
Promise(function(resolve, reject) {
Needs t.step_func() wrapper

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:25:
serviceWorker.onstatechange = function() {
Needs t.step_func() wrapper

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:40: .then(function() {
Needs t.step_func() wrapper

[dominicc (has gone to gerrit), 2014-06-10 21:27:19]

Dissent inline :O

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/waiting.html (right):

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:15: .then(function() { return
with_iframe(scope); },
On 2014/06/10 17:59:32, jsbell wrote:
> Style nit: In other tests, we don't seem to start lines with `.then(...`
> elsewhere, but instead wrap before the closing parenthesis, i.e. `).then(...`
or
> `}).then(...` etc.
> 
> (That may just be due to *my* style being copied rather than
> Blink/Chromium/Google style, though, so all of the existing formatting may be
> "wrong" if it is documented somewhere! Google JS style, at least, says the .
> should go on the preceding line if it's going to wrap there.)

Yeah, I wrote it the style guide way, and then decided to break the rules. I
think the leading .then is clearer because it means single-line thens don't get
"lost"; the resolve and reject functions that belong to a single "then" are
guaranteed to be broken by the unindented "then". Compare:

f().then(
  function() { g },
  function() { h }).then(
  function() { i }).then(
  function() { j },
  function() { k });

f()
.then(function() { g },
      function() { h })
.then(function() { i })
.then(function() { j },
      function() { k });

I think this latter one makes it easier to see that the happy path takes
f,g,i,j. WDYT?

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:17: .then(function(f) {
On 2014/06/10 17:59:32, jsbell wrote:
> Needs t.step_func() wrapper

The only thing that does for me is fail the test if one of these two lines
throw, right? But the test will fail anyway (albeit more slowly) by timing out
in that case.

Ditto below... You only need step_func for making assertions.

[jsbell, 2014-06-10 22:36:23]

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/waiting.html (right):

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:15: .then(function() { return
with_iframe(scope); },
Sounds good. It's not my personal preference for now, but when we get ES6
lambdas the then-bodies will be even shorter so this format will be even more
readable.

https://codereview.chromium.org/324893002/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/waiting.html:17: .then(function(f) {
On 2014/06/10 21:27:18, dominicc wrote:
> On 2014/06/10 17:59:32, jsbell wrote:
> > Needs t.step_func() wrapper
> 
> The only thing that does for me is fail the test if one of these two lines
> throw, right? But the test will fail anyway (albeit more slowly) by timing out
> in that case.
> 
> Ditto below... You only need step_func for making assertions.

Correct. However, from my experience trying to debug these tests when the
t.step_func() wrappers are missing, I would try and put them in everywhere an
async function is used. If someone adds a new assertion that turns out to be
flaky I'd rather get useful failure messages on the bots rather than timeouts.

Not that I've ever done that to my own tests...

[dominicc (has gone to gerrit), 2014-06-11 18:21:36]

OK, we'll split the difference and keep left-aligned .then and have every
function be a step for more reliable failures.

I feel like we are breaking new ground here in testing style which is necessary
and good. Probably need to make a clean up pass soon to ensure consistency. I am
thinking it might make sense to just adopt the style in testharness.js. I expect
W3C could be interested in our Promise-based testing helpers after we upstream,
for example.

[dominicc (has gone to gerrit), 2014-06-12 02:01:07]

Quick update: This depends on https://codereview.chromium.org/317953004/ going
first and is expected to fail until that goes in.

[dominicc (has gone to gerrit), 2014-06-12 20:28:32]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-06-12 20:29:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominicc@chromium.org/324893002/20001

[commit-bot: I haz the power, 2014-06-12 23:23:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/11284)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/11831)

[commit-bot: I haz the power, 2014-06-12 23:53:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-12 23:53:21]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)

[dominicc (has gone to gerrit), 2014-06-13 21:02:25]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-06-13 21:02:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominicc@chromium.org/324893002/40001

[commit-bot: I haz the power, 2014-06-13 22:14:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/11963)

[commit-bot: I haz the power, 2014-06-13 23:55:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-13 23:55:01]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/11963)

[dominicc (has gone to gerrit), 2014-06-14 19:44:23]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2014-06-14 19:45:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominicc@chromium.org/324893002/40001

[commit-bot: I haz the power, 2014-06-14 19:45:41]

Change committed as 176157