Address tab-related flakiness in integration tests (#2754)

* Redo webdriver tab stuff * whitespace * Cleanup * Wait on activation and controlling * Skip a test in Safari
GoogleChrome · Feb 14, 2021 · 9c15dc7 · 9c15dc7
1 parent 6581108
commit 9c15dc7
Show file tree

Hide file tree

Showing 8 changed files with 161 additions and 204 deletions.
diff --git a/infra/testing/webdriver/TabManager.js b/infra/testing/webdriver/TabManager.js
@@ -0,0 +1,49 @@
+/*
+  Copyright 2021 Google LLC
+
+  Use of this source code is governed by an MIT-style
+  license that can be found in the LICENSE file or at
+  https://opensource.org/licenses/MIT.
+*/
+
+/**
+ * Wraps methods in the underlying webdriver API to create, switch between,
+ * and close tabs in a browser.
+ */
+class TabManager {
+  /**
+   * @param {WebDriver} driver
+   *
+   * @private
+   */
+  constructor(driver) {
+    this._driver = driver;
+    this._openedHandles = new Set();
+    this._initialHandle = null;
+  }
+
+  async openTab(url) {
+    if (this._initialHandle === null) {
+      this._initialHandle = await this._driver.getWindowHandle();
+    }
+
+    await this._driver.switchTo().newWindow('tab');
+    this._openedHandles.add(await this._driver.getWindowHandle());
+
+    await this._driver.get(url);
+  }
+
+  async closeOpenedTabs() {
+    for (const handle of this._openedHandles) {
+      await this._driver.switchTo().window(handle);
+      await this._driver.close();
+    }
+    this._openedHandles = new Set();
+
+    if (this._initialHandle) {
+      await this._driver.switchTo().window(this._initialHandle);
+    }
+  }
+}
+
+module.exports = {TabManager};
diff --git a/infra/testing/webdriver/getLastWindowHandle.js b/infra/testing/webdriver/getLastWindowHandle.js
diff --git a/infra/testing/webdriver/openNewTab.js b/infra/testing/webdriver/openNewTab.js
diff --git a/infra/testing/webdriver/windowLoaded.js b/infra/testing/webdriver/windowLoaded.js
@@ -17,8 +17,7 @@ const windowLoaded = async () => {
   await executeAsyncAndCatch(async (cb) => {
     const loaded = () => {
       if (!window.Workbox) {
-        const error = new Error('Workbox not yet loaded...');
-        cb({error: error.stack});
+        cb({error: `window.Workbox is undefined; location is ${location.href}`});
       } else {
         cb();
       }

diff --git a/test/workbox-background-sync/sw/test-BackgroundSyncPlugin.mjs b/test/workbox-background-sync/sw/test-BackgroundSyncPlugin.mjs
@@ -9,6 +9,11 @@
 import {Queue} from 'workbox-background-sync/Queue.mjs';
 import {BackgroundSyncPlugin} from 'workbox-background-sync/BackgroundSyncPlugin.mjs';
 
+let count = 0;
+function getUniqueQueueName() {
+  return `queue-${count++}`;
+}
+
 describe(`BackgroundSyncPlugin`, function() {
   const sandbox = sinon.createSandbox();
 
@@ -27,7 +32,7 @@ describe(`BackgroundSyncPlugin`, function() {
   describe(`constructor`, function() {
     it(`should implement fetchDidFail and add requests to the queue`, async function() {
       const stub = sandbox.stub(Queue.prototype, 'pushRequest');
-      const queuePlugin = new BackgroundSyncPlugin('a');
+      const queuePlugin = new BackgroundSyncPlugin(getUniqueQueueName());
 
       queuePlugin.fetchDidFail({request: new Request('/one')});
       expect(stub.callCount).to.equal(1);

diff --git a/test/workbox-broadcast-update/integration/test-all.js b/test/workbox-broadcast-update/integration/test-all.js
@@ -7,15 +7,15 @@
 */
 
 const expect = require('chai').expect;
-const activateAndControlSW = require('../../../infra/testing/activate-and-control');
+
 const {runUnitTests} = require('../../../infra/testing/webdriver/runUnitTests');
-const {openNewTab} = require('../../../infra/testing/webdriver/openNewTab');
-const {getLastWindowHandle} = require('../../../infra/testing/webdriver/getLastWindowHandle');
+const {TabManager} = require('../../../infra/testing/webdriver/TabManager');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
+const cleanSWEnv = require('../../../infra/testing/clean-sw');
 const templateData = require('../../../infra/testing/server/template-data');
 
-
 // Store local references of these globals.
-const {webdriver, server} = global.__workbox;
+const {webdriver, server, seleniumBrowser} = global.__workbox;
 
 describe(`[workbox-broadcast-update]`, function() {
   it(`passes all SW unit tests`, async function() {
@@ -29,11 +29,13 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
   const swURL = `${testingURL}sw.js`;
   const apiURL = `${testServerAddress}/__WORKBOX/uniqueETag`;
 
-  it(`should broadcast a message when there's a cache update to a regular request`, async function() {
-    await webdriver.get(testingURL);
+  beforeEach(async function() {
+    // Navigate to our test page and clear all caches before this test runs.
+    await cleanSWEnv(global.__workbox.webdriver, testingURL);
     await activateAndControlSW(swURL);
-    await clearAllCaches();
+  });
 
+  it(`should broadcast a message when there's a cache update to a regular request`, async function() {
     // Fetch `apiURL`, which should put it in the cache (but not trigger an update)
     const err1 = await webdriver.executeAsyncScript((apiURL, cb) => {
       fetch(apiURL).then(() => cb()).catch((err) => cb(err.message));
@@ -68,13 +70,9 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
   });
 
   it(`should broadcast a message when there's a cache update to a navigation request`, async function() {
-    await webdriver.get(testingURL);
-    await activateAndControlSW(swURL);
-    await clearAllCaches();
-
     templateData.assign({
       title: 'Broadcast Cache Update Test',
-      body: '',
+      body: 'Second test, initial body.',
       script: `
         window.__messages = [];
         navigator.serviceWorker.addEventListener('message', (event) => {
@@ -83,7 +81,7 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
       `,
     });
 
-    const dynamicPageURL = testingURL + 'integration.html.njk';
+    const dynamicPageURL = testingURL + 'integration.html.njk?second';
 
     // Navigate to a dynamic page whose content can be updated from with this
     // test, and wait until the cache is populated.
@@ -95,9 +93,9 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
     });
 
     // Update the template data with new content,
-    // then refresh and wait until the udpate message is received.
+    // then refresh and wait until the update message is received.
     templateData.assign({
-      body: 'New content to change Content-Length!',
+      body: 'Second test, with and updated body.',
     });
 
     await webdriver.get(webdriver.getCurrentUrl());
@@ -124,13 +122,17 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
   });
 
   it(`should broadcast a message to all open window clients`, async function() {
-    await webdriver.get(testingURL);
-    await activateAndControlSW(swURL);
-    await clearAllCaches();
+    // This test doesn't work in Safari:
+    // https://github.com/GoogleChrome/workbox/issues/2755
+    if (seleniumBrowser.getId() === 'safari') {
+      this.skip();
+    }
+
+    const tabManager = new TabManager(webdriver);
 
     templateData.assign({
       title: 'Broadcast Cache Update Test',
-      body: '',
+      body: 'Third test, initial body.',
       script: `
         window.__messages = [];
         navigator.serviceWorker.addEventListener('message', (event) => {
@@ -139,71 +141,44 @@ describe(`[workbox-broadcast-update] Plugin`, function() {
       `,
     });
 
-    const dynamicPageURL = testingURL + 'integration.html.njk';
+    const dynamicPageURL = testingURL + 'integration.html.njk?third';
 
     // Navigate to a dynamic page whose content can be updated from with this
     // test, and wait until the cache is populated.
     await webdriver.get(dynamicPageURL);
-    const tab1Handle = await getLastWindowHandle();
     await webdriver.wait(async () => {
       return webdriver.executeAsyncScript(async (url, cb) => {
         cb(await caches.match(url));
       }, dynamicPageURL);
     });
 
-    // Update the template data with new content,
-    // then open a new tab and wait until the udpate message is received.
+    // Update the template data, then open a new tab to trigger the update.
     templateData.assign({
-      body: 'New content to change Content-Length!',
+      body: 'Third test, with an updated body.',
     });
-    await openNewTab(dynamicPageURL);
+
+    await tabManager.openTab(dynamicPageURL);
+
+    // Go back to the initial tab to assert the message was received there.
+    await tabManager.closeOpenedTabs();
+
     await webdriver.wait(() => {
       return webdriver.executeScript(() => {
         return window.__messages.length > 0;
       });
     });
 
-    const tab2Messsages = await webdriver.executeScript(() => {
+    const tab1Messages = await webdriver.executeScript(() => {
       return window.__messages;
     });
 
-    expect(tab2Messsages.length).to.equal(1);
-    expect(tab2Messsages[0]).to.deep.equal({
+    expect(tab1Messages).to.eql([{
       type: 'CACHE_UPDATED',
       meta: 'workbox-broadcast-update',
       payload: {
         cacheName: 'bcu-integration-test',
         updatedURL: dynamicPageURL,
       },
-    });
-
-    // Also assert a message was received on the first tab.
-    await webdriver.switchTo().window(tab1Handle);
-    const tab1Messsages = await webdriver.executeScript(() => {
-      return window.__messages;
-    });
-
-    expect(tab1Messsages.length).to.equal(1);
-    expect(tab1Messsages[0]).to.deep.equal({
-      type: 'CACHE_UPDATED',
-      meta: 'workbox-broadcast-update',
-      payload: {
-        cacheName: 'bcu-integration-test',
-        updatedURL: dynamicPageURL,
-      },
-    });
+    }]);
   });
 });
-
-/**
- * Clears all caches for the origin of the currently open page.
- */
-async function clearAllCaches() {
-  await webdriver.executeAsyncScript(async (cb) => {
-    const cacheNames = await caches.keys();
-    for (const name of cacheNames) {
-      await caches.delete(name);
-    }
-    cb();
-  });
-}