Issue 5079, 5516 - Use webpack for browser tests, modules for content scripts

test/browser/_bootstrap.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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";
 
 /* globals nodeunit */
 require("nodeunit");
 
-let browserTestModuleContext = require.context("./", true, /^[^_].*\.js$/);
-
-function runTests(browserTestModules)

[Wladimir Palant, 2017/08/17 10:05:38]

Nit: Frankly, I'm a bit annoyed by the frequent repetition of "browserTest" -
this is redundant, the directory name already says so. moduleNames maybe?

[kzar, 2017/08/17 12:40:22]

Done.

+function runTests(moduleNames)
 {
   function bold(str)
   {
     return "\u001B[1m" + str + "\u001B[22m";
   }
 
   function ok(str)
   {
     return "\u001B[32m" + str + "\u001B[39m";
   }
 
   function error(str)
   {
     return "\u001B[31m" + str + "\u001B[39m";
   }
 
-  if (browserTestModules.length == 0)
-    browserTestModules = browserTestModuleContext.keys();

[Wladimir Palant, 2017/08/17 10:05:38]

Why do we need this? test_runner.js is where we are compiling the list of
modules, here we should just run what we get.

[kzar, 2017/08/17 12:40:22]

Done.

-
-  // We don't know which browser test modules are required at the point we
-  // bundle everything together using webpack, so we bundle them all but
-  // only use the necessary ones here.

[Wladimir Palant, 2017/08/17 10:05:38]

Why is this comment here? That's not where the bundling happens.

[kzar, 2017/08/17 12:40:22]

Done.

-  let testModules = {};

[Wladimir Palant, 2017/08/17 10:05:38]

Nit: tests should be unambiguous enough as a name for this variable?

[kzar, 2017/08/17 12:40:21]

Done.

-  for (let module of browserTestModules)
-    testModules[module] = nodeunit.testCase(browserTestModuleContext(module));

[Wladimir Palant, 2017/08/17 10:05:38]

As mentioned in comment on chrome_process.js, require("./" + module + ".js")
should do here without an explicit context.

[kzar, 2017/08/17 12:40:22]

Done.

+  let tests = {};
+  for (let module of moduleNames)
+    tests[module] = nodeunit.testCase(require("./" + module + ".js"));
 
   return new Promise((resolve, reject) =>
   {
-    nodeunit.runModules(testModules, {
+    nodeunit.runModules(tests, {
       moduleStart(name)
       {
         console.log(bold(name));
       },
       testDone(name, assertions)
       {
         let errors = assertions.filter(assertion => assertion.failed())
                                .map(assertion => assertion.error);
 
         if (errors.length == 0)
@@ skipping 30 matching lines @@
           );
         }
 
         resolve();
       }
     });
   });
 }
 
 module.exports = runTests;