Issue 6884 - Migrate from nodeunit to mocha

test/firefox.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * 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");
 const {Command} = require("selenium-webdriver/lib/command");
 const {ensureFirefox} = require("../adblockpluscore/test/runners/" +
                                 "firefox_download");
 
-function reportElements(test, driver, success)
+describe("Firefox", function()
 {
-  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))
-  )));
-}
+  this.timeout(0);
 
-exports.runFirefox = function(test)
-{
-  // https://stackoverflow.com/a/45045036
-  function installWebExt(driver, extension)
+  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(() =>
       // Wait for tests to finish
       driver.wait(() =>
         driver.findElement(By.id("qunit-testresult"))
           .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)
+    ]))
+  );
+});