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

test/_common.js

Index: test/_common.js
===================================================================
--- a/test/_common.js
+++ b/test/_common.js
@@ -86,20 +86,16 @@
         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
 };
 
@@ -416,48 +412,49 @@
     }
   };
 };
 
 console.warn = console.log;
 
 exports.silenceWarnOutput = function(test, msg)
 {
-  let warnHandler = globals.console.warn;
-  globals.console.warn = s =>
+  let warnHandler = console.warn;
+  console.warn = s =>
   {
     if (s != msg)
       warnHandler(s);
   };
   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]");

[hub, 2017/11/28 19:49:45]

Another alternative is:

let msgMatch = new RegExp("^Error: (.*)$", "m");

But then we are no longer matching the first line, just any line.

[Wladimir Palant, 2017/11/28 20:00:57]

So, what about looking at s.message only?

+  let errorHandler = console.error;
+  console.error = s =>

[Wladimir Palant, 2017/11/28 20:00:57]

Nit: Why is this parameter called s? It's not necessarily a string, typically it
will be an Error object. Call it err maybe?

[sergei, 2017/11/28 20:34:24]

It's Error.prototype.stack
(https://github.com/adblockplus/adblockpluscore/blob/4939e6a861dba5a9327829629...)
what is string.

I would like to add that in general it would be better to interrupt a JS
function corresponding to C++ `assert` instead of console.error, but there is no
such function, at least so far. In that function it would be fair to expect an
Error object or any another special object, but since it's expected that
console.error uses string representation of its arguments, strictly speaking we
should not try to dig the nature of arguments and to treat them differently in
order to avoid a potential side effect, though it's pure theoretical. BTW, that
was one of the reasons that I mentioned that it would be better to use
msgMatch.exec(s) than `s.match(msgMatch)` because the latter can result in
anything.

So, if you decide to make changes here to expect an Error object then I would
ask to add JS `assert` function and silence an expected output only for that
function.

[hub, 2017/11/28 20:54:33]

Oh. I see.

In our code, assert2() will always call console.error() with a string as a
parameter.
But in general it could be call with any object and any parameters. And that
would lead to a failure. But we don't go that route. I'll update the patch to
make sure we deal properly with the situation.

   {
-    if (!msgMatch.test(s))
+    let match = s.match(msgMatch);
+    if (!match || match[1] != msg)
       errorHandler(s);
   };
   try
   {
     return test();
   }
   finally
   {
-    globals.console.error = errorHandler;
+    console.error = errorHandler;
   }
 };
 
 exports.setupRandomResult = function()
 {
   let randomResult = 0.5;
   Object.defineProperty(this, "randomResult", {
     get: () => randomResult,