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

test/browser/_bootstrap.js

Index: test/browser/_bootstrap.js
diff --git a/test/browser/_bootstrap.js b/test/browser/_bootstrap.js
index 969c62ec1d05f42f9ba68ee72abc33c1fa23e5fe..960c0aec790967599d2657c2ca0f8a70a98d6d53 100644
--- a/test/browser/_bootstrap.js
+++ b/test/browser/_bootstrap.js
@@ -18,124 +18,87 @@
 "use strict";
 
 /* globals nodeunit */
+require("nodeunit");
 
-(function(nodeunitUrl, ...moduleUrls)
+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 loadScript(doc, url)
+  function bold(str)
   {
-    return new Promise((resolve, reject) =>
-    {
-      let script = doc.createElement("script");
-      script.src = url;
-      script.onload = resolve;
-      doc.head.appendChild(script);
-    });
+    return "\u001B[1m" + str + "\u001B[22m";
   }
 
-  function loadModules(urls)
+  function ok(str)
   {
-    let modules = {};
-
-    return (function loadNext()
-    {
-      if (urls.length)
-      {
-        // Load each module into a new frame so that their scopes don't clash
-        let frame = document.createElement("iframe");
-        document.body.appendChild(frame);
-
-        let wnd = frame.contentWindow;
-        wnd.loadScript = url => loadScript(wnd.document, url);
-        wnd.exports = {};
-        wnd.module = {exports: wnd.exports};
-
-        let url = urls.shift();
-        let name = url.split("/").pop();
-        return wnd.loadScript(url).then(() =>
-        {
-          modules[name] = nodeunit.testCase(wnd.module.exports);
-          return loadNext();
-        });
-      }
-
-      return Promise.resolve(modules);
-    })();
+    return "\u001B[32m" + str + "\u001B[39m";
   }
 
-  function runTests(modules)
+  function error(str)
   {
-    function bold(str)
-    {
-      return "\u001B[1m" + str + "\u001B[22m";
-    }
+    return "\u001B[31m" + str + "\u001B[39m";
+  }
 
-    function ok(str)
-    {
-      return "\u001B[32m" + 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.

 
-    function error(str)
-    {
-      return "\u001B[31m" + str + "\u001B[39m";
-    }
+  // 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.

 
-    return new Promise((resolve, reject) =>
-    {
-      nodeunit.runModules(modules, {
-        moduleStart(name)
-        {
-          console.log(bold(name));
-        },
-        testDone(name, assertions)
-        {
-          let errors = assertions.filter(assertion => assertion.failed())
-                                 .map(assertion => assertion.error);
+  return new Promise((resolve, reject) =>
+  {
+    nodeunit.runModules(testModules, {
+      moduleStart(name)
+      {
+        console.log(bold(name));
+      },
+      testDone(name, assertions)
+      {
+        let errors = assertions.filter(assertion => assertion.failed())
+                               .map(assertion => assertion.error);
 
-          if (errors.length == 0)
-            console.log("\u2714 " + name);
-          else
-          {
-            console.log(error("\u2716 " + name) + "\n");
-            errors.forEach(e =>
-            {
-              if (e.stack)
-                console.log(e.stack);
-              else
-                console.log(String(e));
-              console.log("");
-            });
-          }
-        },
-        done(assertions)
+        if (errors.length == 0)
+          console.log("\u2714 " + name);
+        else
         {
-          let failures = assertions.filter(assertion => assertion.failed());
-          if (failures.length)
-          {
-            console.log(
-              "\n" +
-              bold(error("FAILURES: ")) +
-              failures.length + "/" + assertions.length + " assertions failed"
-            );
-          }
-          else
+          console.log(error("\u2716 " + name) + "\n");
+          errors.forEach(e =>
           {
-            console.log(
-              "\n" + bold(ok("OK: ")) +
-              assertions.length + " assertions"
-            );
-          }
-
-          resolve();
+            if (e.stack)
+              console.log(e.stack);
+            else
+              console.log(String(e));
+            console.log("");
+          });
+        }
+      },
+      done(assertions)
+      {
+        let failures = assertions.filter(assertion => assertion.failed());
+        if (failures.length)
+        {
+          console.log(
+            "\n" +
+            bold(error("FAILURES: ")) +
+            failures.length + "/" + assertions.length + " assertions failed"
+          );
+        }
+        else
+        {
+          console.log(
+            "\n" + bold(ok("OK: ")) +
+            assertions.length + " assertions"
+          );
         }
-      });
-    });
-  }
 
-  return loadScript(document, nodeunitUrl).then(() =>
-  {
-    return loadModules(moduleUrls);
-  }).then(modules =>
-  {
-    return runTests(modules);
+        resolve();
+      }
+    });
   });
-});
+}
+
+module.exports = runTests;