Bug 1588193 - Fix broken tests, r=mccr8
☠☠ backed out by 5eaefee2f30d ☠ ☠
authorKashav Madan <kmadan@mozilla.com>
Tue, 05 Nov 2019 21:52:43 +0000
changeset 562570 0bbb1f92bb4721a0bc090a10d2c34ded75f8e01c
parent 562569 074bb8a6fd6851f9ebfc68e7505d6e30b4342bdb
child 562571 89e0d67e037b74f1e2ae8e9d7cbccc5a0f652397
push id12351
push userffxbld-merge
push dateMon, 02 Dec 2019 11:32:26 +0000
treeherdermozilla-beta@dba4410526a2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1588193
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1588193 - Fix broken tests, r=mccr8 Most of these tests relied on assumptions that were broken by the new content event utils (timing, being in a ContentTask, etc). Differential Revision: https://phabricator.services.mozilla.com/D51442
browser/base/content/test/general/browser_bug567306.js
browser/base/content/test/general/browser_search_discovery.js
browser/base/content/test/plugins/browser_bug744745.js
dom/indexedDB/test/head.js
dom/quota/test/head.js
toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
toolkit/mozapps/extensions/test/browser/browser_history_navigation.js
--- a/browser/base/content/test/general/browser_bug567306.js
+++ b/browser/base/content/test/general/browser_bug567306.js
@@ -9,17 +9,17 @@ add_task(async function() {
 
   let selectedBrowser = newwindow.gBrowser.selectedBrowser;
   await new Promise((resolve, reject) => {
     BrowserTestUtils.waitForContentEvent(
       selectedBrowser,
       "pageshow",
       true,
       event => {
-        return content.location.href != "about:blank";
+        return event.target.location != "about:blank";
       }
     ).then(function pageshowListener() {
       ok(
         true,
         "pageshow listener called: " + newwindow.gBrowser.currentURI.spec
       );
       resolve();
     });
--- a/browser/base/content/test/general/browser_search_discovery.js
+++ b/browser/base/content/test/general/browser_search_discovery.js
@@ -1,11 +1,19 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
+/* eslint-disable mozilla/no-arbitrary-setTimeout */
+
+// Bug 1588193 - BrowserTestUtils.waitForContentEvent now resolves slightly
+// earlier than before, so it no longer suffices to only wait for a single event
+// tick before checking if browser.engines has been updated. Instead we use a 1s
+// timeout, which may cause the test to take more time.
+requestLongerTimeout(2);
+
 add_task(async function() {
   let url =
     "http://mochi.test:8888/browser/browser/base/content/test/general/discovery.html";
   info("Test search discovery");
   await BrowserTestUtils.withNewTab(url, searchDiscovery);
 });
 
 let searchDiscoveryTests = [
@@ -68,17 +76,17 @@ async function searchDiscovery() {
       link.rel = test.rel || "search";
       link.href = test.href || "http://so.not.here.mozilla.com/search.xml";
       link.type = test.type || "application/opensearchdescription+xml";
       link.title = test.title;
       head.appendChild(link);
     });
 
     await promiseLinkAdded;
-    await new Promise(resolve => executeSoon(resolve));
+    await new Promise(resolve => setTimeout(resolve, 1000));
 
     if (browser.engines) {
       info(`Found ${browser.engines.length} engines`);
       info(`First engine title: ${browser.engines[0].title}`);
       let hasEngine = testCase.count
         ? browser.engines[0].title == testCase.title &&
           browser.engines.length == testCase.count
         : browser.engines[0].title == testCase.title;
@@ -107,17 +115,17 @@ async function searchDiscovery() {
     link.title = "Test Engine";
     let link2 = link.cloneNode(false);
     link2.href = "http://second.mozilla.com/search.xml";
     head.appendChild(link);
     head.appendChild(link2);
   });
 
   await promiseLinkAdded;
-  await new Promise(resolve => executeSoon(resolve));
+  await new Promise(resolve => setTimeout(resolve, 1000));
 
   ok(browser.engines, "has engines");
   is(browser.engines.length, 1, "only one engine");
   is(
     browser.engines[0].uri,
     "http://first.mozilla.com/search.xml",
     "first engine wins"
   );
--- a/browser/base/content/test/plugins/browser_bug744745.js
+++ b/browser/base/content/test/plugins/browser_bug744745.js
@@ -1,30 +1,16 @@
 var gTestRoot = getRootDirectory(gTestPath).replace(
   "chrome://mochitests/content/",
   "http://127.0.0.1:8888/"
 );
 var gTestBrowser = null;
-var gNumPluginBindingsAttached = 0;
-
-function pluginBindingAttached() {
-  gNumPluginBindingsAttached++;
-  if (gNumPluginBindingsAttached != 1) {
-    ok(false, "if we've gotten here, something is quite wrong");
-  }
-}
 
 add_task(async function() {
   registerCleanupFunction(function() {
-    gTestBrowser.removeEventListener(
-      "PluginBindingAttached",
-      pluginBindingAttached,
-      true,
-      true
-    );
     clearAllPluginPermissions();
     setTestPluginEnabledState(Ci.nsIPluginTag.STATE_ENABLED, "Test Plug-in");
     setTestPluginEnabledState(
       Ci.nsIPluginTag.STATE_ENABLED,
       "Second Test Plug-in"
     );
     gBrowser.removeCurrentTab();
     window.focus();
@@ -33,35 +19,30 @@ add_task(async function() {
 });
 
 add_task(async function() {
   gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
   gTestBrowser = gBrowser.selectedBrowser;
 
   setTestPluginEnabledState(Ci.nsIPluginTag.STATE_CLICKTOPLAY, "Test Plug-in");
 
-  BrowserTestUtils.addContentEventListener(
+  let promisePluginBindingAttached = BrowserTestUtils.waitForContentEvent(
     gTestBrowser,
     "PluginBindingAttached",
-    pluginBindingAttached,
-    { capture: true, wantUntrusted: true }
+    true,
+    null,
+    true
   );
 
-  let testRoot = getRootDirectory(gTestPath).replace(
-    "chrome://mochitests/content/",
-    "http://127.0.0.1:8888/"
-  );
   await promiseTabLoadEvent(
     gBrowser.selectedTab,
-    testRoot + "plugin_bug744745.html"
+    gTestRoot + "plugin_bug744745.html"
   );
 
-  await promiseForCondition(function() {
-    return gNumPluginBindingsAttached == 1;
-  });
+  await promisePluginBindingAttached;
 
   await ContentTask.spawn(gTestBrowser, {}, async function() {
     let plugin = content.document.getElementById("test");
     if (!plugin) {
       Assert.ok(false, "plugin element not available.");
       return;
     }
     // We can't use MochiKit's routine
--- a/dom/indexedDB/test/head.js
+++ b/dom/indexedDB/test/head.js
@@ -80,33 +80,37 @@ function dismissNotification(popup) {
 
 function waitForMessage(aMessage, browser) {
   // We cannot capture aMessage inside the checkFn, so we override the
   // checkFn.toSource to tunnel aMessage instead.
   let checkFn = function() {};
   checkFn.toSource = function() {
     return `function checkFn(event) {
       let message = ${aMessage.toSource()};
-      is(event.data, message, "Received: " + message);
       if (event.data == message) {
         return true;
       }
       throw new Error(
        \`Unexpected result: \$\{event.data\}, expected \$\{message\}\`
       );
     }`;
   };
 
   return BrowserTestUtils.waitForContentEvent(
     browser.selectedBrowser,
     "message",
     /* capture */ true,
     checkFn,
     /* wantsUntrusted */ true
-  );
+  ).then(() => {
+    // An assertion in checkFn wouldn't be recorded as part of the test, so we
+    // use this assertion to confirm that we've successfully received the
+    // message (we'll only reach this point if that's the case).
+    ok(true, "Received message: " + aMessage);
+  });
 }
 
 function dispatchEvent(eventName) {
   info("dispatching event: " + eventName);
   let event = document.createEvent("Events");
   event.initEvent(eventName, false, false);
   gBrowser.selectedBrowser.contentWindow.dispatchEvent(event);
 }
--- a/dom/quota/test/head.js
+++ b/dom/quota/test/head.js
@@ -102,33 +102,37 @@ function dismissNotification(popup, win)
 
 function waitForMessage(aMessage, browser) {
   // We cannot capture aMessage inside the checkFn, so we override the
   // checkFn.toSource to tunnel aMessage instead.
   let checkFn = function() {};
   checkFn.toSource = function() {
     return `function checkFn(event) {
       let message = ${aMessage.toSource()};
-      is(event.data, message, "Received: " + message);
       if (event.data == message) {
         return true;
       }
       throw new Error(
        \`Unexpected result: \$\{event.data\}, expected \$\{message\}\`
       );
     }`;
   };
 
   return BrowserTestUtils.waitForContentEvent(
     browser.selectedBrowser,
     "message",
     /* capture */ true,
     checkFn,
     /* wantsUntrusted */ true
-  );
+  ).then(() => {
+    // An assertion in checkFn wouldn't be recorded as part of the test, so we
+    // use this assertion to confirm that we've successfully received the
+    // message (we'll only reach this point if that's the case).
+    ok(true, "Received message: " + aMessage);
+  });
 }
 
 function removePermission(url, permission) {
   let uri = Cc["@mozilla.org/network/io-service;1"]
     .getService(Ci.nsIIOService)
     .newURI(url);
   let ssm = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(
     Ci.nsIScriptSecurityManager
--- a/toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
@@ -197,17 +197,22 @@ function testOnWindow(aTestData) {
         onContentBlockingEvent(aWebProgress, aRequest, aEvent) {
           expected = aTestData.reportUrl;
         },
         QueryInterface: ChromeUtils.generateQI(["nsISupportsWeakReference"]),
       };
       wp.addProgressListener(progressListener, wp.NOTIFY_CONTENT_BLOCKING);
 
       await BrowserTestUtils.loadURI(browser, aTestData.url);
-      await BrowserTestUtils.waitForContentEvent(browser, "DOMContentLoaded");
+      await BrowserTestUtils.browserLoaded(
+        browser,
+        false,
+        `http://${aTestData.url}`,
+        true
+      );
       checkResults(aTestData, expected);
       win.close();
       resolve();
     })();
   });
 }
 SpecialPowers.pushPrefEnv(
   {"set": [
--- a/toolkit/mozapps/extensions/test/browser/browser_history_navigation.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_history_navigation.js
@@ -148,32 +148,18 @@ function is_in_discovery(aManager, url, 
   check_state(canGoBack, canGoForward);
 }
 
 async function expand_addon_element(aManager, aId) {
   var addon = get_addon_element(aManager, aId);
   addon.click();
 }
 
-function wait_for_page_show(browser) {
-  let promise = new Promise(resolve => {
-    let removeFunc;
-    let listener = () => {
-      removeFunc();
-      resolve();
-    };
-    removeFunc = BrowserTestUtils.addContentEventListener(
-      browser,
-      "pageshow",
-      listener,
-      {},
-      event => event.target.location == "http://example.com/"
-    );
-  });
-  return promise;
+function wait_for_page_load(browser) {
+  return BrowserTestUtils.browserLoaded(browser, false, "http://example.com/");
 }
 
 // Tests simple forward and back navigation and that the right heading and
 // category is selected
 add_task(async function test_navigate_history() {
   let aManager = await open_manager("addons://list/extension");
   info("Part 1");
   is_in_list(aManager, "addons://list/extension", false, false);
@@ -395,17 +381,17 @@ add_task(async function test_navigate_ba
     set: [["security.allow_eval_with_system_principal", true]],
   });
 
   let aManager = await open_manager("addons://list/plugin");
   info("Part 1");
   is_in_list(aManager, "addons://list/plugin", false, false);
 
   BrowserTestUtils.loadURI(gBrowser, "http://example.com/");
-  await wait_for_page_show(gBrowser.selectedBrowser);
+  await wait_for_page_load(gBrowser.selectedBrowser);
 
   info("Part 2");
 
   await new Promise(resolve =>
     executeSoon(function() {
       ok(gBrowser.canGoBack, "Should be able to go back");
       ok(!gBrowser.canGoForward, "Should not be able to go forward");
 
@@ -419,17 +405,17 @@ add_task(async function test_navigate_ba
 
         aManager = await wait_for_view_load(
           gBrowser.contentWindow.wrappedJSObject
         );
         info("Part 3");
         is_in_list(aManager, "addons://list/plugin", false, true);
 
         executeSoon(() => go_forward());
-        wait_for_page_show(gBrowser.selectedBrowser).then(() => {
+        wait_for_page_load(gBrowser.selectedBrowser).then(() => {
           info("Part 4");
 
           executeSoon(function() {
             ok(gBrowser.canGoBack, "Should be able to go back");
             ok(!gBrowser.canGoForward, "Should not be able to go forward");
 
             go_back();