Issue 4223 - Migrate some more of adblockplustests

test_runner.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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/>.
  */
 
-function desc(properties)
+"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.

+
+let fs = require("fs");
+let path = require("path");
+let process = require("process");
+let nodeunit = require("nodeunit");
+
+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.

 {
-  let descriptor = {};
-  let keys = Object.keys(properties);
+  if (depth < 1)
+    return;
 
-  for (let key of keys)
-    descriptor[key] = Object.getOwnPropertyDescriptor(properties, key);
+  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);
+  }
+}
+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.

 
-  return descriptor;
-}
-exports.desc = desc;
-
-function extend(cls, properties)
-{
-  return Object.create(cls.prototype, desc(properties));
-}
-exports.extend = extend;
+nodeunit.reporters.default.run(files);