Issue 4726 - Add tests for the element hiding emulation content script

test_runner.js

Index: test_runner.js
===================================================================
--- a/test_runner.js
+++ b/test_runner.js
@@ -16,36 +16,47 @@
  */
 
 "use strict";
 
 let fs = require("fs");
 let path = require("path");
 let process = require("process");
 let nodeunit = require("nodeunit");
+let qunit = require("node-qunit-phantomjs");
 
-let files = [];
+let nodeunitFiles = [];
+let qunitFiles = [];
 function addTestPaths(testPaths, recurse)
 {
   for (let testPath of testPaths)
   {
     let stat = fs.statSync(testPath);
     if (stat.isDirectory())
     {
       if (recurse)
       {
         addTestPaths(fs.readdirSync(testPath).map(
           file => path.join(testPath, file)));
       }
+      continue;
     }
-    else if (path.extname(testPath) == ".js" &&
-             !path.basename(testPath).startsWith("_"))
+    if (path.basename(testPath).startsWith("_"))
+      continue;
+    if (testPath.split(path.sep).includes("browser"))
     {
-      files.push(testPath);
+      if (path.extname(testPath) == ".html")
+        qunitFiles.push(testPath);
     }
+    else if (path.extname(testPath) == ".js")
+      nodeunitFiles.push(testPath);
   }
 }
 if (process.argv.length > 2)
   addTestPaths(process.argv.slice(2), true);
 else

[kzar, 2017/01/10 11:05:33]

Nit: Since the addTestPaths call goes over multiple lines we should add the
braces to the else clause. Since we're adding braces to the else clause we
should add it to the if clause too.

[Felix Dahlke, 2017/01/17 19:41:08]

Done.

-  addTestPaths([path.join(__dirname, "test")], true);
+  addTestPaths(
+    [path.join(__dirname, "test"), path.join(__dirname, "test", "browser")],
+    true);

[kzar, 2017/01/10 11:05:34]

Nit: We would normally put the closing parenthesis on the next line in line with
addTestsPaths here.

[Felix Dahlke, 2017/01/17 19:41:08]

We use the style above a lot, last time I checked at least, but changed it
anyway.

 
-nodeunit.reporters.default.run(files);
+if (nodeunitFiles.length)
+  nodeunit.reporters.default.run(nodeunitFiles);
+qunitFiles.forEach(file => qunit(path.resolve(file)));

[kzar, 2017/01/10 11:05:34]

Nit: We usually use "for of" loops instead of forEach where possible.

[Felix Dahlke, 2017/01/17 19:41:08]

I prefer forEach when it comes down to actually invoking a function for each
element in an iterable. Here I had to put an anonymous function inbetween, so
I've switched to for...of. (I realised the path.resolve is unnecessary, but
since forEach passes on more than just the value, I couldn't go for
`qunitFiles.forEach(qunit)`.)