Issue 4223 - Migrate some more of adblockplustests

test_runner.js

Index: test_runner.js
diff --git a/lib/coreUtils.js b/test_runner.js
similarity index 50%
copy from lib/coreUtils.js
copy to test_runner.js
index b9e9a2f050252ae0ac6d4d7d1a1919ff812c1d7a..987b4a41d16f3e382a6e547d0dcf62ca4d0a636e 100644
--- a/lib/coreUtils.js
+++ b/test_runner.js
@@ -15,20 +15,29 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-function desc(properties)
-{
-  let descriptor = {};
-  let keys = Object.keys(properties);
-
-  for (let key of keys)
-    descriptor[key] = Object.getOwnPropertyDescriptor(properties, key);
+"use strict";

[Wladimir Palant, 2016/10/04 11:02:58]

Gotta love Git, it considers your new test_wrapper.js a copy of lib/coreUtils.js
("similarity index 50%"). I suggest using -M100 -C100 when exporting your patch.

[kzar, 2016/10/04 12:06:24]

Acknowledged.

 
-  return descriptor;
-}
-exports.desc = desc;
+let fs = require("fs");
+let path = require("path");
+let process = require("process");
+let nodeunit = require("nodeunit");
 
-function extend(cls, properties)
+let files = [];
+function addTestPaths(testPaths, depth)

[Wladimir Palant, 2016/10/04 11:02:57]

I wonder whether depth really needs to be a number. How about having a boolean
called recursive that will always be set to false on the recursive call? This
will also make sure that we only recurse once but be more obvious.

[kzar, 2016/10/04 12:06:23]

Done.

 {
-  return Object.create(cls.prototype, desc(properties));
+  if (depth < 1)
+    return;
+
+  for (let testPath of testPaths)
+  {
+    let stat = fs.statSync(testPath);
+    if (stat.isDirectory())
+      addTestPaths(fs.readdirSync(testPath).map(file => path.join(testPath, file)),

[Wladimir Palant, 2016/10/04 11:02:58]

1) This is still longer than 80 characters. Maybe get directory entries in a
separate statement, with a temporary variable?

2) Please always put brackets around a block if it spans multiple lines - even
if it is a single statement.

[kzar, 2016/10/04 12:06:23]

Done.

+                   depth -1);
+    else if (path.extname(testPath) == ".js" && path.basename(testPath) != "common.js")

[Wladimir Palant, 2016/10/04 11:02:58]

How about something more generic than ignoring common.js explicitly? Maybe
rename it into _common.js and ignore anything starting with an underscore?

[kzar, 2016/10/04 12:06:23]

Done.

+      files.push(testPath);
+  }
 }
-exports.extend = extend;
+addTestPaths(process.argv.length > 2 ? process.argv.slice(2) : [path.join(__dirname, "test")], 2);

[Wladimir Palant, 2016/10/04 11:02:59]

No point having this overly long line. Please split it to multiple lines, this
will also make it easier to read:

  if (process.argv.length > 2)
    addTestPaths(process.argv.slice(2), 2);
  else
    addTestPaths([path.join(__dirname, "test")], 2);

[kzar, 2016/10/04 12:06:24]

Done.

+
+nodeunit.reporters.default.run(files);