Issue 6717 - Part 2: run qunit in headless firefox

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/>.
+ */
+
+"use strict";
+
+const FIREFOX_VERSION = "57.0";
+
+const path = require("path");
+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");
+

[Sebastian Noack, 2018/08/24 13:27:59]

Nit: We usually only put one blank line after the imports.

[tlucas, 2018/08/24 15:26:00]

Done.

+
+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)
+{
+  // https://stackoverflow.com/a/45045036
+  function installWebExt(driver, extension)
+  {
+    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})`);
+  }
+
+  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(() =>
+      // 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();
+      test.done();
+    }).catch((err) =>

[Sebastian Noack, 2018/08/24 13:27:59]

I'd suggest to change the error handling like that:

  .then(
    () =>
    {
      driver.quit();
      test.done();
    },
    err =>
    {
      driver.quit();
      throw err;
    }
  )

1. There is no reason to call driver.quit() again in case driver.quit()
   or test.done() (after calling driver.quit()) failed for some reason.
2. We don't need to wait for driver.quit() to complete before emitting
   the error.
3. Nit: The parenthesis are redundant (and we usually omit them) in
   arrow functions with exactly one argument.

Even better would be to call the Promise's finally() method, but that would
require Node.js >=10, and it doesn't seem to be worthwile to bump up the minimum
version just for that.

[tlucas, 2018/08/24 15:26:00]

Done. (Still waiting for driver.quit() to finish before throwing the error, as
discussed)

+    {
+      driver.quit().then(() =>
+      {
+        throw err;
+      });
+    });
+  });
+};