Issue 6090 - Properly use regexp for message matching and proper console override

test/_common.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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
@@ skipping 73 matching lines @@
       let resourceName = match && match[1];
       if (resourceName && resources.hasOwnProperty(resourceName))
         return {[resourceName]: resources[resourceName]};
 
       throw new Error(
         "Attempt to import unknown JavaScript module " + resource
       );
     },
     reportError(e) {}
   },
-  console: {
-    log: console.log.bind(console),
-    error: console.error.bind(console)
-  },
   navigator: {
   },
   onShutdown: {
     add() {}
   },
   URL
 };
 
 let knownModules = new Map();
 for (let dir of [path.join(__dirname, "stub-modules"),
@@ skipping 306 matching lines @@
     Date: {
       now: () => currentTime
     }
   };
 };
 
 console.warn = console.log;
 
 exports.silenceWarnOutput = function(test, msg)
 {
-  let warnHandler = globals.console.warn;
-  globals.console.warn = s =>
+  let warnHandler = console.warn;
+  console.warn = (...args) =>
   {
+    let s;
+    if (typeof args[0] === "string")

[sergei, 2017/11/29 09:31:53]

To be consistent with other JS code it should not be a strict comparison, just
`==`.

+      s = args[0];
+    else
+      s = args[0].message;

[sergei, 2017/11/29 09:31:54]

args[0] can be undefined and before accessing `message` property we should check
that it's an instance of Error. However, IMO it's a dead code and
overengineering at that moment, here and below.

[Wladimir Palant, 2017/11/29 10:27:31]

Yes, the better approach is:

  let s = (args[0] instanceof Error ? args[0].message : args[0]);

Same thing below.

[hub, 2017/11/29 15:22:32]

Done.

+
     if (s != msg)
-      warnHandler(s);
+      warnHandler(args);

[sergei, 2017/11/29 09:31:53]

strictly speaking it should be warnHandler.warn.apply(console, args)

[Wladimir Palant, 2017/11/29 10:27:31]

This should be `warnHandler(...args)` - pass the original parameters as
parameters, not as an array. Same with errorHandler below.

[hub, 2017/11/29 15:22:32]

Done.

   };
   try
   {
     return test();
   }
   finally
   {
-    globals.console.warn = warnHandler;
+    console.warn = warnHandler;
   }
 };
 
 exports.silenceAssertionOutput = function(test, msg)
 {
-  let msgMatch = new RegExp(`^Error: ${msg}[\r\n]`);
-  let errorHandler = globals.console.error;
-  globals.console.error = s =>
+  let msgMatch = new RegExp("^Error: (.*)[\r\n]");
+  let errorHandler = console.error;
+  console.error = (...args) =>
   {
-    if (!msgMatch.test(s))
-      errorHandler(s);
+    let s;
+    if (typeof args[0] === "string")
+      s = args[0];
+    else
+      s = args[0].message;
+
+    let match = s && s.match(msgMatch);
+    if (!match || match[1] != msg)

[Wladimir Palant, 2017/11/29 10:27:31]

So why still use the regular expression if you are looking a the message only?
It can be a normal string comparison now, same as with silenceWarnOutput.

  if (s != msg)

[hub, 2017/11/29 15:22:32]

We want to silence an assert (call to our assert2()) so we do get as the error
message the whole stack trace. 
While I could put "Error: " part of the message instead of the matching Regexp.
I still need to match and not equal. Other option include susbtr(), split the
lines and equality on the first one.

This also mean that here we are unlikely to ever get a something that is not a
string as the only argument.

+      errorHandler(args);
   };
   try
   {
     return test();
   }
   finally
   {
-    globals.console.error = errorHandler;
+    console.error = errorHandler;
   }
 };
 
 exports.setupRandomResult = function()
 {
   let randomResult = 0.5;
   Object.defineProperty(this, "randomResult", {
     get: () => randomResult,
     set: value => randomResult = value
   });
 
   return {
     Math: Object.create(Math, {
       random: {
         value: () => randomResult
       }
     })
   };
 };
 
 exports.unexpectedError = function(error)
 {
   console.error(error);
   this.ok(false, "Unexpected error: " + error);
 };