LayoutTests/http/tests/push_messaging/register-document.html - Issue 783983003: Push API: move PushManager from Navigator to ServiceWorkerRegistration [switchover 6/6]

Side by Side Diff: LayoutTests/http/tests/push_messaging/register-document.html

Issue 783983003: Push API: move PushManager from Navigator to ServiceWorkerRegistration [switchover 6/6] (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: Created 6 years 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

« LayoutTests/http/tests/push_messaging/has-permission-document.html ('K') | « LayoutTests/http/tests/push_messaging/has-permission-document.html ('k') | LayoutTests/http/tests/push_messaging/resources/empty-worker.js » ('j') | LayoutTests/http/tests/push_messaging/resources/push_manifest.json » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>

	4 <title>Push API: register in document context</title>

	5 <link rel="manifest" href="resources/push_manifest.json">

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

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

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

	9 </head>

	10 <body>

	11 <script>

	12 async_test(function(test) {
	Peter Beverloo 2014/12/08 21:40:50 This is a bit of a weird test. Also, "register in This is a bit of a weird test. Also, "register in document context" doesn't describe what it's doing. Currently, it: (1) Unregisters a Service Worker, then registers a new one. (2) Grants permission for using the Push API. (3) Registers for Push messaging. (4) Verifies that the endpoint and registration Id are as defined in the TestRunner. (5) Resets existing Push API registrations, revokes permissions. (6) Registers for Push messaging. (7) Verifies that an AbortError gets thrown. That's a bit silly. Firstly, (4) and beyond test the TestRunner rather than the feature. It's also not obvious what's going on by reading the test. Instead, I'd propose having a few more focused tests: (1) - Registers for Push messaging when permission has been granted. (2) - Registers for Push messaging when permission has not been granted. (3) - Verify that the endpoint received for a valid Push Registration is a URL. (Orthogonal of which URL.) (4) - Verify that registering, then unregistering, then registering again for Push messaging results in a different push registration Id. Assuming that (4) is the reason that resetPushMessagingRegistrations() is needed at all, it may be worthwhile to postpone introducing that method and wait until the unregister() call works as expected, which really shouldn't be too much longer. Think of the TestRunner as tools to instrument your tests, rather than tools which must be used in your tests. Michael van Ouwerkerk 2014/12/09 18:47:45 It was mixed up because multiple async tests on th Show quoted text On 2014/12/08 21:40:50, Peter Beverloo wrote: > This is a bit of a weird test. Also, "register in document context" doesn't > describe what it's doing. > > Currently, it: > > (1) Unregisters a Service Worker, then registers a new one. > (2) Grants permission for using the Push API. > (3) Registers for Push messaging. > (4) Verifies that the endpoint and registration Id are as defined in the > TestRunner. > (5) Resets existing Push API registrations, revokes permissions. > (6) Registers for Push messaging. > (7) Verifies that an AbortError gets thrown. > > That's a bit silly. Firstly, (4) and beyond test the TestRunner rather than the > feature. It's also not obvious what's going on by reading the test. > > Instead, I'd propose having a few more focused tests: > > (1) > - Registers for Push messaging when permission has been granted. > > (2) > - Registers for Push messaging when permission has not been granted. > > (3) > - Verify that the endpoint received for a valid Push Registration is a URL. > (Orthogonal of which URL.) > > (4) > - Verify that registering, then unregistering, then registering again for Push > messaging results in a different push registration Id. > > > Assuming that (4) is the reason that resetPushMessagingRegistrations() is needed > at all, it may be worthwhile to postpone introducing that method and wait until > the unregister() call works as expected, which really shouldn't be too much > longer. > > > Think of the TestRunner as tools to instrument your tests, rather than tools > which must be used in your tests. It was mixed up because multiple async tests on the same page don't work correctly with service workers and permissions management. I've split the following into separate test pages now: * register success * register failure due to permission denied * register failure due to worker not being active yet The relevant API surfaces are verified in there as well, doing that separately would duplicate an awful lot of code. It does seem fine to wait with (4) until unregister() and getRegistration() have landed, so we can avoid unnecessary TestRunner methods.
	13 assert_true(!!window.testRunner, 'This test requires the TestRunner.');

	14 var url = 'resources/empty-worker.js';

	15 var scope = 'resources/scope/register';

	16 var origin = 'http://127.0.0.1:8000';

	17 var swRegistration;

	18 service_worker_unregister_and_register(test, url, scope)

	19 .then(function(serviceWorkerRegistration) {

	20 swRegistration = serviceWorkerRegistration;

	21 testRunner.setPushMessagingPermission(origin, true);

	22 return swRegistration.pushManager.register();

	23 })

	24 .then(function(pushRegistration) {

	25 assert_equals(pushRegistration.pushEndpoint, 'https://example.com/La youtTestEndpoint');

	26 assert_equals(pushRegistration.pushRegistrationId, 'layoutTestRegist rationId_0');

	27 testRunner.resetPushMessagingRegistrations();

	28 testRunner.setPushMessagingPermission(origin, false);

	29 return swRegistration.pushManager.register();

	30 })

	31 .then(function(pushRegistration) {

	32 assert_unreached('Registration must not succeed after permission is denied.');

	33 }, function(e) {

	34 assert_equals(e.name, 'AbortError');

	35 assert_equals(e.message, 'Registration failed - permission denied');

	36 test.done();

	37 })

	38 .catch(unreached_rejection(test));

	39 }, 'Push API: register in document context');

	40 </script>

	41 </body>

	42 </html>

OLD	NEW