LayoutTests/http/tests/serviceworker/waiting.html - Issue 324893002: Add tests for navigator.serviceWorker.waiting.

Side by Side Diff: LayoutTests/http/tests/serviceworker/waiting.html

Issue 324893002: Add tests for navigator.serviceWorker.waiting. (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: Created 6 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

« no previous file with comments | « LayoutTests/http/tests/serviceworker/resources/test-helpers.js ('k') | LayoutTests/http/tests/serviceworker/waiting-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <title>ServiceWorker: navigator.serviceWorker.waiting</title>

	3 <script src="../resources/testharness.js"></script>

	4 <script src="../resources/testharnessreport.js"></script>

	5 <script src="resources/test-helpers.js"></script>

	6 <body>

	7 <script>

	8 var t = async_test('waiting is set');

	9 t.step(function() {

	10 var url = 'resources/worker-no-op.js';

	11 var scope = 'resources/blank.html';

	12 var frame;

	13

	14 navigator.serviceWorker.unregister(scope)

	15 .then(function() { return with_iframe(scope); },
	jsbell 2014/06/10 17:59:32 Style nit: In other tests, we don't seem to start 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.) jsbell 2014/06/10 17:59:33 Needs t.step_func() wrapper (testharness.js is upd Needs t.step_func() wrapper (testharness.js is updated to pass the return value through) dominicc (has gone to gerrit) 2014/06/10 21:27:18 Yeah, I wrote it the style guide way, and then dec Show quoted text 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? jsbell 2014/06/10 22:36:23 Sounds good. It's not my personal preference for n 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.
	16 unreached_rejection(t, 'Unregister should not fail'))

	17 .then(function(f) {
	jsbell 2014/06/10 17:59:32 Needs t.step_func() wrapper Needs t.step_func() wrapper dominicc (has gone to gerrit) 2014/06/10 21:27:18 The only thing that does for me is fail the test i Show quoted text 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 Correct. However, from my experience trying to deb Show quoted text 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...
	18 frame = f;

	19 return navigator.serviceWorker.register(url, {scope: scope});

	20 })

	21 .then(function(serviceWorker) {
	jsbell 2014/06/10 17:59:32 Needs t.step_func() wrapper Needs t.step_func() wrapper
	22 // FIXME: Replace this with .ready when that supports the in-waiting

	23 // Service Worker.

	24 return new Promise(function(resolve, reject) {
	jsbell 2014/06/10 17:59:32 Needs t.step_func() wrapper Needs t.step_func() wrapper
	25 serviceWorker.onstatechange = function() {
	jsbell 2014/06/10 17:59:32 Needs t.step_func() wrapper Needs t.step_func() wrapper
	26 if (serviceWorker.state == 'installed') {

	27 resolve();

	28 }

	29 };

	30 });

	31 }, unreached_rejection(t, 'Registration should not fail'))

	32 .then(t.step_func(function() {

	33 var container = frame.contentWindow.navigator.serviceWorker;

	34 assert_equals(container.controller, null);

	35 assert_equals(container.waiting.url, normalizeURL(url));

	36

	37 // FIXME: Add a test for a frame created after installation.

	38 // Should the existing frame ("frame") block activation?

	39 }))

	40 .then(function() {
	jsbell 2014/06/10 17:59:32 Needs t.step_func() wrapper Needs t.step_func() wrapper
	41 frame.remove();

	42 return service_worker_unregister_and_done(t, scope);

	43 });

	44 });

	45 </script>

OLD	NEW