Issue 6884 - Migrate from nodeunit to mocha

test/firefox.js

Index: test/firefox.js
===================================================================
--- a/test/firefox.js
+++ b/test/firefox.js
@@ -15,11 +15,14 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* eslint-env mocha */
+
 "use strict";
 
 const FIREFOX_VERSION = "57.0";
 
 const path = require("path");
+const assert = require("assert");
 const webdriver = require("selenium-webdriver");
 const {By, until} = webdriver;
 const firefox = require("selenium-webdriver/firefox");
@@ -27,62 +30,58 @@
 const {ensureFirefox} = require("../adblockpluscore/test/runners/" +
                                 "firefox_download");
 
-function reportElements(test, driver, success)
-{
-  return driver.findElements(
-    By.css(`#qunit-tests ${success ? ".pass" : ".fail"} .test-name`)
-  ).then(elements => Promise.all(elements.map(elem =>
-    elem.getAttribute("innerHTML").then(data => test.ok(success, data))
-  )));
-}
-
-exports.runFirefox = function(test)
+describe("Firefox", function()
 {
-  // https://stackoverflow.com/a/45045036
-  function installWebExt(driver, extension)
+  this.timeout(0);
+
+  let driver;
+  let origin;
+
+  before(() =>
+    ensureFirefox(FIREFOX_VERSION).then(firefoxPath =>
+    {
+      let binary = new firefox.Binary(firefoxPath);
+      binary.addArguments("-headless");
+
+      driver = new webdriver.Builder()
+        .forBrowser("firefox")
+        .setFirefoxOptions(new firefox.Options().setBinary(binary))
+        .build();
+
+      let devenv = "./devenv.gecko";
+      let cmd = new Command("moz-install-web-ext")
+        .setParameter("path", path.resolve(devenv))
+        .setParameter("temporary", true);
+
+      driver.getExecutor().defineCommand(
+        cmd.getName(), "POST",
+        "/session/:sessionId/moz/addon/install"
+      );
+      driver.schedule(cmd, `installWebExt(${devenv})`);
+
+      return driver.wait(() =>
+        driver.getAllWindowHandles().then(handles => handles[1])
+      ).then(handle =>
+        driver.switchTo().window(handle)
+      ).then(() =>
+        driver.executeScript("return location.origin;")
+      ).then(result => { origin = result; });
+    })
+  );
+
+  after(() => driver.quit());

[tlucas, 2018/08/26 15:10:51]

Nit: IMHO this should go after it(), to reflect the actual execution order.

[Sebastian Noack, 2018/08/26 19:02:00]

Done. Though one could argue the other way around to keep the hooks together.

+
+  function reportElements(success)
   {
-    let cmd = new Command("moz-install-web-ext")
-      .setParameter("path", path.resolve(extension))
-      .setParameter("temporary", true);
-
-    driver.getExecutor()
-      .defineCommand(cmd.getName(), "POST",
-                     "/session/:sessionId/moz/addon/install");
-
-    return driver.schedule(cmd, `installWebExt(${extension})`);
+    return driver.findElements(
+      By.css(`#qunit-tests ${success ? ".pass" : ".fail"} .test-name`)
+    ).then(elements => Promise.all(elements.map(elem =>
+      elem.getAttribute("innerHTML").then(data => assert.ok(success, data))
+    )));
   }
 
-  ensureFirefox(FIREFOX_VERSION).then(firefoxPath =>
-  {
-    let binary = new firefox.Binary(firefoxPath);
-
-    binary.addArguments("-headless");
-
-    let options = new firefox.Options()
-      .setBinary(binary);
-
-    let driver = new webdriver.Builder()
-      .forBrowser("firefox")
-      .setFirefoxOptions(options)
-      .build();
-
-    installWebExt(driver, "./devenv.gecko");
-
-    driver.wait(() =>
-      // Wait for the firstrun-page to be loaded
-      driver.getAllWindowHandles().then(handles =>
-      {
-        if (handles.length > 1)
-        {
-          driver.switchTo().window(handles[1]);
-          return true;
-        }
-        return false;
-      })
-    ).then(() =>
-      // Navigate to the qunit index
-      driver.executeScript("location.href = \"qunit/index.html\";")
-    ).then(() =>
+  it("qunit", () =>
+    driver.navigate().to(origin + "/qunit/index.html").then(() =>

[tlucas, 2018/08/26 15:10:51]

Just FMI, why do you deem this more appropriate than
"driver.executeScript("location.href = \"qunit/index.html\";")",
especially since you run executeScript() anyway in line 67 to read to retrieve
origin?

[Sebastian Noack, 2018/08/26 15:42:18]

I need the extension URL prefix for the tests I'm going to add anyway. Also once
we add more tests (reusing the same browser instance), it will be a hassle to
guarantee that the first run page will be open and focused at the beginning of
each test. By getting the origin upfront, we have larger code reuse, and less
assumptions in the individual tests.

[tlucas, 2018/08/26 21:13:50]

Thanks for clarifying.

       // Wait for qunit-results to be present
       driver.wait(until.elementLocated(By.id("qunit-testresult")))
     ).then(() =>
@@ -92,11 +91,8 @@
           .getAttribute("innerHTML").then(data =>
              data.includes("Tests completed")))
     ).then(() => Promise.all([
-      reportElements(test, driver, true),
-      reportElements(test, driver, false)
-    ])).then(
-      () => driver.quit().then(() => test.done()),
-      err => driver.quit().then(() => { throw err; })
-    );
-  });
-};
+      reportElements(true),
+      reportElements(false)
+    ]))
+  );
+});