Polish PR based on feedbacks

Author: zwu52

integration/messaging/download-browsers.js

@@ -18,8 +18,9 @@
 const seleniumAssistant = require('selenium-assistant');
 
 console.log('Starting browser download - this may take some time.');
-// TODO: enable firefox testing once figure out how to give notification permission with SE webdriver.
-// TODO: Run the integration test against multiple major chrome versions to ensure backward compatibility
+// TODO: enable firefox testing once figure out how to give notification permission with SE
+// webdriver. TODO: Run the integration test against multiple major chrome versions to ensure
+// backward compatibility
 Promise.all([seleniumAssistant.downloadLocalBrowser('chrome', 'stable', 80)])
   .then(() => {
     console.log('Browser download complete.');

integration/messaging/manual-test-server.js

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

integration/messaging/test/static/helpers.js

@@ -40,7 +40,8 @@ async function addPayloadToDb(payload) {
 }
 
 async function addPayloadToDbInternal(db, payload) {
-  // onsuccess might race with onupgradeneeded. Consequently causing "object stores was not found" error. Therefore, wait briefly for db.createObjectStore to complete
+  // onsuccess might race with onupgradeneeded. Consequently causing "object stores was not found"
+  // error. Therefore, wait briefly for db.createObjectStore to complete
   const delay = ms => new Promise(res => setTimeout(res, ms));
   await delay(/* milliseconds= */ 30000);
 

integration/messaging/test/test-deleteToken.js

@@ -41,7 +41,7 @@ describe('Firebase Messaging Integration Tests > get and delete token', function
 
   const availableBrowsers = seleniumAssistant.getLocalBrowsers();
   availableBrowsers.forEach(assistantBrowser => {
-    //TODO: enable testing for firefox
+    // TODO: enable testing for firefox
     if (assistantBrowser.getId() !== 'chrome') {
       return;
     }

integration/messaging/test/test-send.js

@@ -29,16 +29,21 @@ const TEST_DOMAINS = ['valid-vapid-key', 'valid-vapid-key-modern-sw'];
 const TEST_PROJECT_SENDER_ID = '750970317741';
 const DEFAULT_COLLAPSE_KEY_VALUE = 'do_not_collapse';
 const FIELD_FROM = 'from';
-const FIELD_COLLAPSE_KEY = 'collapse_key';
+const FIELD_COLLAPSE_KEY_LEGACY = 'collapse_key';
+const FIELD_COLLAPSE_KEY = 'collapseKey';
+
 const FIELD_DATA = 'data';
 const FIELD_NOTIFICATION = 'notification';
 
-// 4 minutes. The fact that the flow includes making a request to the Send Service, storing/retrieving form indexedDb asynchronously makes these test units to have a execution time variance. Therefore, allowing these units to have a longer time to work is crucial.
+// 4 minutes. The fact that the flow includes making a request to the Send Service,
+// storing/retrieving form indexedDb asynchronously makes these test units to have a execution time
+// variance. Therefore, allowing these units to have a longer time to work is crucial.
 const TIMEOUT_BACKGROUND_MESSAGE_TEST_UNIT_MILLISECONDS = 240000;
 
 const TIMEOUT_FOREGROUND_MESSAGE_TEST_UNIT_MILLISECONDS = 120000;
 
-// 1 minute. Wait for object store to be created and received message to be stored in idb. This waiting time MUST be longer than the wait time for adding to db in the sw.
+// 1 minute. Wait for object store to be created and received message to be stored in idb. This
+// waiting time MUST be longer than the wait time for adding to db in the sw.
 const WAIT_TIME_BEFORE_RETRIEVING_BACKGROUND_MESSAGES_MILLISECONDS = 60000;
 
 const wait = ms => new Promise(res => setTimeout(res, ms));
@@ -55,7 +60,7 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
     await testServer.stop();
   });
 
-  //TODO: enable testing for firefox
+  // TODO: enable testing for firefox
   seleniumAssistant.getLocalBrowsers().forEach(assistantBrowser => {
     if (assistantBrowser.getId() !== 'chrome') {
       return;
@@ -72,7 +77,10 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
         it('Background app can receive a {} empty message from sw', async function() {
           this.timeout(TIMEOUT_BACKGROUND_MESSAGE_TEST_UNIT_MILLISECONDS);
 
-          // Clearing the cache and db data by killing the previously instantiated driver. Note that ideally this call is placed inside the after/before hooks. However, Mocha forbids operations longer than 2s in hooks. Hence, this clearing call needs to be inside the test unit.
+          // Clearing the cache and db data by killing the previously instantiated driver. Note that
+          // ideally this call is placed inside the after/before hooks. However, Mocha forbids
+          // operations longer than 2s in hooks. Hence, this clearing call needs to be inside the
+          // test unit.
           await seleniumAssistant.killWebDriver(globalWebDriver);
 
           globalWebDriver = createPermittedWebDriver(
@@ -94,7 +102,8 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
           checkMessageReceived(
             await getReceivedBackgroundMessages(globalWebDriver),
             /* expectedNotificationPayload= */ null,
-            /* expectedDataPayload= */ null
+            /* expectedDataPayload= */ null,
+            /* isLegacyPayload= */ false
           );
         });
 
@@ -137,9 +146,7 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
           /* browser= */ assistantBrowser.getId()
         );
 
-        await globalWebDriver.get(
-          `${testServer.serverAddress}/${TEST_DOMAIN}/`
-        );
+        await globalWebDriver.get(`${testServer.serverAddress}/${domain}/`);
 
         let token = await retrieveToken(globalWebDriver);
         checkSendResponse(
@@ -164,9 +171,7 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
           /* browser= */ assistantBrowser.getId()
         );
 
-        await globalWebDriver.get(
-          `${testServer.serverAddress}/${TEST_DOMAIN}/`
-        );
+        await globalWebDriver.get(`${testServer.serverAddress}/${domain}/`);
 
         checkSendResponse(
           await sendMessage({
@@ -191,9 +196,7 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
           /* browser= */ assistantBrowser.getId()
         );
 
-        await globalWebDriver.get(
-          `${testServer.serverAddress}/${TEST_DOMAIN}/`
-        );
+        await globalWebDriver.get(`${testServer.serverAddress}/${domain}/`);
 
         checkSendResponse(
           await sendMessage({
@@ -218,9 +221,7 @@ describe('Starting Integration Test > Sending and Receiving ', function() {
           /* browser= */ assistantBrowser.getId()
         );
 
-        await globalWebDriver.get(
-          `${testServer.serverAddress}/${TEST_DOMAIN}/`
-        );
+        await globalWebDriver.get(`${testServer.serverAddress}/${domain}/`);
 
         checkSendResponse(
           await sendMessage({
@@ -250,7 +251,10 @@ function checkMessageReceived(
   const message = receivedMessages[0];
 
   expect(message[FIELD_FROM]).to.equal(TEST_PROJECT_SENDER_ID);
-  expect(message[FIELD_COLLAPSE_KEY]).to.equal(DEFAULT_COLLAPSE_KEY_VALUE);
+  const collapseKey = !!message[FIELD_COLLAPSE_KEY_LEGACY]
+    ? message[FIELD_COLLAPSE_KEY_LEGACY]
+    : message[FIELD_COLLAPSE_KEY];
+  expect(collapseKey).to.equal(DEFAULT_COLLAPSE_KEY_VALUE);
 
   if (expectedNotificationPayload) {
     expect(message[FIELD_NOTIFICATION]).to.deep.equal(
@@ -285,13 +289,14 @@ function getTestDataPayload() {
 async function prepareBackgroundApp(globalWebDriver, domain) {
   await globalWebDriver.get(`${testServer.serverAddress}/${domain}/`);
 
-  // TODO: remove the try/catch block once the underlying bug has been resolved.
-  // Shift window focus away from app window so that background messages can be received/processed
+  // TODO: remove the try/catch block once the underlying bug has been resolved. Shift window focus
+  // away from app window so that background messages can be received/processed
   try {
     await openNewTab(globalWebDriver);
   } catch (err) {
-    // ChromeDriver seems to have an open bug which throws "JavascriptError: javascript error: circular reference".
-    // Nevertheless, a new tab can still be opened. Hence, just catch and continue here.
+    // ChromeDriver seems to have an open bug which throws "JavascriptError: javascript error:
+    // circular reference". Nevertheless, a new tab can still be opened. Hence, just catch and
+    // continue here.
     console.log('FCM (ignored on purpose): ' + err);
   }
 }

integration/messaging/test/test-updateToken.js

@@ -43,7 +43,7 @@ describe('Firebase Messaging Integration Tests > update a token', function() {
   });
 
   const availableBrowsers = seleniumAssistant.getLocalBrowsers();
-  //TODO: enable testing for edge and firefox if applicable
+  // TODO: enable testing for edge and firefox if applicable
   availableBrowsers.forEach(assistantBrowser => {
     if (assistantBrowser.getId() !== 'chrome') {
       return;

integration/messaging/test/utils/getReceivedBackgroundMessages.js

@@ -18,7 +18,10 @@
 const TEST_DB = 'FCM_INTEGRATION_TEST_DB';
 const BACKGROUND_MESSAGES_OBJECT_STORE = 'background_messages';
 
-/** Getting received background messages are trickier than getting foreground messages from app. It requires idb object store creation with the service worker. Idb operations are fired as async events. This method needs to be called after the idb operations inside sw is done. In tests, consider adding a brief timeout before calling the method to give sw some time to work.
+/** Getting received background messages are trickier than getting foreground messages from app. It
+ * requires idb object store creation with the service worker. Idb operations are fired as async
+ * events. This method needs to be called after the idb operations inside sw is done. In tests,
+ * consider adding a brief timeout before calling the method to give sw some time to work.
  */
 module.exports = async webdriver => {
   console.log('Getting received background messages from idb: ');

integration/messaging/test/utils/sendMessage.js

@@ -17,7 +17,9 @@
 
 const fetch = require('node-fetch');
 const FCM_SEND_ENDPOINT = 'https://fcm.googleapis.com/fcm/send';
-// Rotatable fcm server key. It's generally a bad idea to expose server keys. The reason is to simplify testing process (no need to implement server side decryption of git secret). The justification is that a) this is a disposable test project b)  the key itself is rotatable.
+// Rotatable fcm server key. It's generally a bad idea to expose server keys. The reason is to
+// simplify testing process (no need to implement server side decryption of git secret). The
+// justification is that a) this is a disposable test project b)  the key itself is rotatable.
 const FCM_KEY =
   'AAAArtlRq60:APA91bHFulW1dBpIPbArYXPbFtO9M_a9ZNXhnj9hGArfGK55g8fv5s5Qset6984xRIrqhZ_3IlKcG9LgSk3DiTdHMDIOkxboNJquNK1SChC7J0ULTvHPg7t0V6AjR1UEA21DXI22BM5N';
 

integration/messaging/test/utils/test-server.js

@@ -61,8 +61,8 @@ class MessagingTestServer {
     });
   }
 
-  // Sometimes the server doesn't trigger the callback due to
-  // currently open sockets. So call close this._server
+  // Sometimes the server doesn't trigger the callback due to currently open sockets. So call close
+  // this._server
   async stop() {
     if (this._server) {
       this._server.close();