Issue 4878 - Start using ESLint for adblockpluscore

test/_common.js

Index: test/_common.js
diff --git a/test/_common.js b/test/_common.js
index 2d9cf1ebf66eed7f92f5fb7bf499270ee1042bad..ad902f85b5065e65f8c8673ab16d2fe0596ffb78 100644
--- a/test/_common.js
+++ b/test/_common.js
@@ -30,7 +30,11 @@ const Cr = exports.Cr = {
 const MILLIS_IN_SECOND = exports.MILLIS_IN_SECOND = 1000;
 const MILLIS_IN_MINUTE = exports.MILLIS_IN_MINUTE = 60 * MILLIS_IN_SECOND;
 const MILLIS_IN_HOUR = exports.MILLIS_IN_HOUR = 60 * MILLIS_IN_MINUTE;
-const MILLIS_IN_DAY = exports.MILLIS_IN_DAY = 24 * MILLIS_IN_HOUR;
+
+function URL(urlString)
+{
+  return require("url").parse(urlString);
+}
 
 let globals = {
   atob: data => new Buffer(data, "base64").toString("binary"),
@@ -38,28 +42,24 @@ let globals = {
   Ci: {
   },
   Cu: {
-    import: () => {},
-    reportError: e => undefined
+    import() {},
+    reportError(e) {}
   },
   console: {
-    log: () => undefined,
-    error: () => undefined,
+    log() {},
+    error() {}
   },
   navigator: {
   },
   onShutdown: {
-    add: () =>
-    {
-    }
+    add() {}
   },
   Services: {
     obs: {
-      addObserver: () =>
-      {
-      }
+      addObserver() {}
     },
     vc: {
-      compare: (v1, v2) =>
+      compare(v1, v2)
       {
         function comparePart(p1, p2)
         {
@@ -69,8 +69,7 @@ let globals = {
             return 1;
           else if (p1 == p2)
             return 0;
-          else
-            return parseInt(p1, 10) - parseInt(p2, 10);
+          return parseInt(p1, 10) - parseInt(p2, 10);
         }
 
         let parts1 = v1.split(".");
@@ -86,22 +85,21 @@ let globals = {
     }
   },
   XPCOMUtils: {
-    generateQI: () =>
-    {
-    }
+    generateQI() {}
   },
-  URL: function(urlString)
-  {
-    return require("url").parse(urlString);
-  }
+  URL
 };
 
 let knownModules = new Map();
 for (let dir of [path.join(__dirname, "stub-modules"),
                  path.join(__dirname, "..", "lib")])
+{
   for (let file of fs.readdirSync(path.resolve(dir)))
+  {
     if (path.extname(file) == ".js")
       knownModules[path.basename(file, ".js")] = path.resolve(dir, file);
+  }
+}
 
 function addExports(exports)
 {
@@ -109,9 +107,13 @@ function addExports(exports)
   {
     let extraExports = exports[path.basename(this.filename, ".js")];
     if (extraExports)
+    {
       for (let name of extraExports)
+      {
         source += `
           Object.defineProperty(exports, "${name}", {get: () => ${name}});`;
+      }
+    }
     return source;
   };
 }
@@ -144,7 +146,7 @@ exports.createSandbox = function(options)
   // function which can be used to load further modules into the sandbox.
   return SandboxedModule.require("./_common", {
     globals: Object.assign({}, globals, options.globals),
-    sourceTransformers: sourceTransformers
+    sourceTransformers
   }).require;
 };
 
@@ -160,7 +162,7 @@ exports.setupTimerAndXMLHttp = function()
     delay: -1,
     nextExecution: 0,
 
-    initWithCallback: function(callback, delay, type)
+    initWithCallback(callback, delay, type)
     {
       if (this.callback)
         throw new Error("Only one timer instance supported");
@@ -172,7 +174,7 @@ exports.setupTimerAndXMLHttp = function()
       this.nextExecution = currentTime + delay;
     },
 
-    trigger: function()
+    trigger()
     {
       if (currentTime < this.nextExecution)
         currentTime = this.nextExecution;
@@ -186,7 +188,7 @@ exports.setupTimerAndXMLHttp = function()
       }
     },
 
-    cancel: function()
+    cancel()
     {
       this.nextExecution = -1;
     }
@@ -198,9 +200,8 @@ exports.setupTimerAndXMLHttp = function()
     this._host = "http://example.com";
     this._loadHandlers = [];
     this._errorHandlers = [];
-  };
-  XMLHttpRequest.prototype =
-  {
+  }
+  XMLHttpRequest.prototype = {
     _path: null,
     _data: null,
     _queryString: null,
@@ -210,7 +211,7 @@ exports.setupTimerAndXMLHttp = function()
     readyState: 0,
     responseText: null,
 
-    addEventListener: function(eventName, handler, capture)
+    addEventListener(eventName, handler, capture)
     {
       let list;
       if (eventName == "load")
@@ -224,7 +225,7 @@ exports.setupTimerAndXMLHttp = function()
         list.push(handler);
     },
 
-    removeEventListener: function(eventName, handler, capture)
+    removeEventListener(eventName, handler, capture)
     {
       let list;
       if (eventName == "load")
@@ -239,7 +240,7 @@ exports.setupTimerAndXMLHttp = function()
         list.splice(index, 1);
     },
 
-    open: function(method, url, async, user, password)
+    open(method, url, async, user, password)
     {
       if (method != "GET")
         throw new Error("Only GET requests are supported");
@@ -269,7 +270,7 @@ exports.setupTimerAndXMLHttp = function()
       }
     },
 
-    send: function(data)
+    send(data)
     {
       if (!this._data && !this._path)
         throw new Error("No request path set");
@@ -300,7 +301,7 @@ exports.setupTimerAndXMLHttp = function()
       }));
     },
 
-    overrideMimeType: function(mime)
+    overrideMimeType(mime)
     {
     },
 
@@ -316,9 +317,9 @@ exports.setupTimerAndXMLHttp = function()
   };
 
   XMLHttpRequest.requestHandlers = {};
-  this.registerHandler = (path, handler) =>
+  this.registerHandler = (requestPath, handler) =>
   {
-    XMLHttpRequest.requestHandlers[path] = handler;
+    XMLHttpRequest.requestHandlers[requestPath] = handler;
   };
 
   function waitForRequests()
@@ -332,8 +333,7 @@ exports.setupTimerAndXMLHttp = function()
         console.error(e);
       }).then(() => waitForRequests());
     }
-    else
-      return Promise.resolve();
+    return Promise.resolve();
   }
 
   function runScheduledTasks(maxMillis)
@@ -344,13 +344,8 @@ exports.setupTimerAndXMLHttp = function()
       currentTime = endTime;
       return Promise.resolve();
     }
-    else
-    {
-      fakeTimer.trigger();
-      return waitForRequests().then(() => runScheduledTasks(endTime - currentTime));
-    }
-
-    currentTime = endTime;
+    fakeTimer.trigger();
+    return waitForRequests().then(() => runScheduledTasks(endTime - currentTime));
   }
 
   this.runScheduledTasks = (maxHours, initial, skip) =>
@@ -399,7 +394,7 @@ exports.setupTimerAndXMLHttp = function()
         TYPE_REPEATING_SLACK: 1,
         TYPE_REPEATING_PRECISE: 2
       },
-      nsIHttpChannel: () => null,
+      nsIHttpChannel: () => null
     },
     Cr,
     XMLHttpRequest,
@@ -409,6 +404,10 @@ exports.setupTimerAndXMLHttp = function()
   };
 };
 
+/* eslint-disable no-console */

[Sebastian Noack, 2017/02/21 09:19:32]

Why is this necessary anyway? We shouldn't have any occurrence of console.log()
anymore. And ESLint makes sure that we don't regress.

[kzar, 2017/02/21 10:37:03]

Honestly I'm not sure why but when running our unit tests console.warn is not
defined, so I map it to console.log here.

[Sebastian Noack, 2017/02/21 11:11:56]

Please first figure out what's going on here. Perhaps another limitation of
PhantomJS? At least, Node.js does have console.warn(). But then, don't we only
run content scripts with PhantomJS, while we have no consle.warn() in there?

If there is really no way around that hack, I might prefer to just disable the
no-console rule for the tests, as you already have an .eslintrc.json in the
tests directory, where you already disable other rules anyway.

[kzar, 2017/02/21 11:27:41]

Well I've tried but I couldn't figure it out. I start Node.js and confirmed
console.warn worked as I expected. Then I ran the tests and I get this trace (I
stripped the paths):

TypeError: console.warn is not a function
    at verifySignature (.../adblockpluscore/lib/rsa.js:200:13)
    at Object.exports.testVeryLongData
(.../adblockpluscore/test/signatures.js:129:12)
    at .../adblockpluscore/node_modules/nodeunit/lib/core.js:232:20
    at .../adblockpluscore/node_modules/nodeunit/deps/async.js:168:13
    at .../adblockpluscore/node_modules/nodeunit/deps/async.js:131:25
    at .../adblockpluscore/node_modules/nodeunit/deps/async.js:165:17
    at .../adblockpluscore/node_modules/nodeunit/deps/async.js:463:34
    at Object.exports.setUp (.../adblockpluscore/test/signatures.js:31:3)
    at .../adblockpluscore/node_modules/nodeunit/lib/core.js:260:35
    at .../adblockpluscore/node_modules/nodeunit/deps/async.js:458:21
Done.

+console.warn = console.log;
+/* eslint-enable no-console */
+
 exports.setupRandomResult = function()
 {
   let randomResult = 0.5;