Issue 6887 - add Chrome to "npm test"

test/all.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 glob = require("glob");
 const path = require("path");
 const {exec} = require("child_process");
 
 for (let browser of glob.sync("./test/browsers/*.js"))
 {
   let module = require(path.resolve(browser));
 
   describe(module.platform, function()
   {
     this.timeout(0);
 
-    let driver;
-    let origin;
-
-    before(() => new Promise((resolve, reject) =>

[Sebastian Noack, 2018/08/30 02:02:40]

I just noticed, getting the driver and setting up the devenv can actually happen
in parallel:

  before() => Promise.all([
    module.getDriver(path.resolve(`./devenv.${module.platform}`),
    new Promise((resolve, reject) =>
      ...
    )
  ]).then(([d]) =>
  {
    ...
  })

[tlucas, 2018/09/01 09:38:31]

Correct me if i'm wrong - but couldn't it then happen, that getDriver() tries to
load a not yet existent devenv?

[Sebastian Noack, 2018/09/01 10:42:47]

My bad, you are right. However, what we can do, and I like that idea, is
splitting out the part that makes sure the browser is there (this is the
expensive part on first run) and parallelize it with creating the devenv. Then
browsers/chromium.js for example would look like this:

  exports.ensureBrowser = function()
  {
    return ensureChromium(CHROMIUM_REVISION);
  };

  exports.getDriver = function(browserBinary, devenvPathAbsolute)
  {
    let options = new chrome.Options()
      .setChromeBinaryPath(browserBinary)
      .addArguments("--no-sandbox")
      .addArguments(`load-extension=${devenvPathAbsolute}`);

    return new webdriver.Builder()
      .forBrowser("chrome")
      .setChromeOptions(options)
      .build();
  };

And here we get this:

  before(function()
  {
    return Promise.all([
      module.ensureBrowser(),
      new Promise((resolve, reject) =>
        ...
      )
    ]).then([browserBinary] =>
    {
      this.driver = module.getDriver(
        browserBinary, 
        path.resolve(`./devenv.${module.platform}`)
      );

      return this.driver.wait(() =>            
        this.driver.getAllWindowHandles().then(handles => handles[1])
      );
    })

[tlucas, 2018/09/01 11:54:23]

Done (almost, nodejs complained about an unexpected token in

     ]).then([browserBinary] =>
                           ^
)

[Sebastian Noack, 2018/09/01 12:26:33]

Right, you have to use parenthesis if you want to destructure any elements in
the arguments list of an arrow function:

  ]).then(([browserBinary]) =>

[tlucas, 2018/09/01 14:46:44]

Done.

+    before(function()
     {
-      exec(`python build.py devenv -t ${module.platform}`,
-        (error, stdout, stderr) =>
+      return Promise.all([
+        module.ensureBrowser(),
+        new Promise((resolve, reject) =>
         {
-          if (error)
-          {
-            console.error(stderr);
-            reject(error);
-          }
-          else resolve(stdout);
-        });
-    }).then(() => module.getDriver(path.resolve(`./devenv.${module.platform}`))
-      .then(d =>

[Sebastian Noack, 2018/08/29 15:22:12]

Nit: The indentation seems a little inconsistent. Perhaps change it like this:

  before(() => new Promise((resolve, reject) =>
  {
    ...
  }).then(() =>
    ...
  ).then(d =>
    ...
  )

[tlucas, 2018/09/01 09:38:31]

Should be resolved now.

+          exec(`python build.py devenv -t ${module.platform}`,
+            (error, stdout, stderr) =>
+            {
+              if (error)
+              {
+                console.error(stderr);
+                reject(error);
+              }
+              else resolve(stdout);
+            });
+        })
+      ]).then(([browserBinary]) =>
       {
-        driver = d;
-        return driver.wait(() =>
-          driver.getAllWindowHandles().then(handles => handles[1])
+        this.driver = module.getDriver(
+          browserBinary,
+          path.resolve(`./devenv.${module.platform}`)
+        );
+        return this.driver.wait(() =>
+          this.driver.getAllWindowHandles().then(handles => handles[1])
         );
       }).then(handle =>
-        driver.switchTo().window(handle)
+        this.driver.switchTo().window(handle)
       ).then(() =>
-        driver.executeScript("return location.origin;")
-      ).then(o =>
+        this.driver.executeScript("return location.origin;")
+      ).then(origin =>
       {
-        origin = o;
-      }))
-    );
+        this.origin = origin;
+      });
+    });
 
     for (let file of glob.sync("./test/wrappers/*.js"))
     {
-      let testModule = require(path.resolve(file));
-
-      it(testModule.title, () =>
-      {
-        return testModule.it(driver, origin);
-      });
+      // Reload the module(s) for every browser
+      let modulePath = path.resolve(file);
+      delete require.cache[require.resolve(modulePath)];
+      require(modulePath);
     }
 
-    after(() => driver.quit());
+    after(function()
+    {
+      this.driver.quit();
+    });
   });
 }