LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html - Issue 1321643002: Fix notifications tests' use of assert_object_equals

Side by Side Diff: LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-data-reflection.html

Issue 1321643002: Fix notifications tests' use of assert_object_equals (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 5 years, 3 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

« no previous file with comments | « LayoutTests/http/tests/notifications/notification-data-property.html ('k') | LayoutTests/http/tests/notifications/serviceworker-notificationclick-event-reflection.html » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!doctype html>	1 <!doctype html>

2 <html>	2 <html>

3 <head>	3 <head>

4 <title>Notifications: data property reflection in the "notificationclick" ev ent.</title>	4 <title>Notifications: data property reflection in the "notificationclick" ev ent.</title>

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

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

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

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

9 </head>	9 </head>

10 <body>	10 <body>

(...skipping 46 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
57 return;	57 return;

58 }	58 }

59	59

60 // (3) Listen for confirmation from the Service Worker that th e	60 // (3) Listen for confirmation from the Service Worker that th e

61 // notification has been clicked on. Make sure that all proper ties	61 // notification has been clicked on. Make sure that all proper ties

62 // set on the Notification object are as expected.	62 // set on the Notification object are as expected.

63 assert_equals(event.data.command, 'click', 'The notification w as expected to be clicked.');	63 assert_equals(event.data.command, 'click', 'The notification w as expected to be clicked.');

64	64

65 var pos = event.data.notification.tag;	65 var pos = event.data.notification.tag;

66	66

67 assert_object_equals(event.data.notification.data, notificatio nDataList[pos], 'The data field must be the same.');	67 if (Number.isNaN(notificationDataList[pos])) {

	68 // TODO(peter): NaN should round trip to NaN rather than n ull.
	Peter Beverloo 2015/08/27 14:17:11 Let's remove this clause and the NaN value test. W Let's remove this clause and the NaN value test. We should not be testing SerializedScriptValue, and certainly not have TODOs for bugs in that code here :-). johnme 2015/08/27 14:42:14 Done - filed https://crbug.com/525652. Show quoted text On 2015/08/27 14:17:11, Peter Beverloo wrote: > Let's remove this clause and the NaN value test. We should not be testing > SerializedScriptValue, and certainly not have TODOs for bugs in that code here > :-). Done - filed https://crbug.com/525652.
	69 assert_equals(event.data.notification.data, null);

	70 } else if (typeof notificationDataList[pos] === 'object')

	71 assert_object_equals(event.data.notification.data, notific ationDataList[pos], 'The data field must be the same.');

	72 else

	73 assert_equals(event.data.notification.data, notificationDa taList[pos], 'The data field must be the same.');

68	74

69 if (pos < notificationDataList.length)	75 if (++pos < notificationDataList.length)
	Peter Beverloo 2015/08/27 14:17:11 Why this change? I find this less readable. Why this change? I find this less readable. johnme 2015/08/27 14:42:14 Because it fixes pos going out of bounds! [which w Show quoted text On 2015/08/27 14:17:11, Peter Beverloo wrote: > Why this change? I find this less readable. Because it fixes pos going out of bounds! [which was masked by assert_object_equals(someobject, undefined) passing]
70 assertNotificationDataReflects(++pos);	76 assertNotificationDataReflects(pos);

71 else	77 else

72 test.done();	78 test.done();

73 });	79 });

74	80

75 assertNotificationDataReflects(0);	81 assertNotificationDataReflects(0);

76 }).catch(unreached_rejection(test));	82 }).catch(unreached_rejection(test));

77	83

78 }, 'Clicking on a notification displayed by a Service Worker the notificat ionclick event.');	84 }, 'Clicking on a notification displayed by a Service Worker the notificat ionclick event.');

79 </script>	85 </script>

80 </body>	86 </body>

81 </html>	87 </html>

OLD	NEW