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 400 matching lines @@
       now: () => currentTime
     }
   };
 };
 
 console.warn = console.log;
 
 exports.silenceWarnOutput = function(test, msg)
 {
   let warnHandler = console.warn;
-  console.warn = s =>
-  {
+  console.warn = (...args) =>
+  {
+    let s = (args[0] instanceof Error ? args[0].message : args[0]);
+
     if (s != msg)
-      warnHandler(s);
+      warnHandler(...args);
   };
   try
   {
     return test();
   }
   finally
   {
     console.warn = warnHandler;
   }
 };
 
 exports.silenceAssertionOutput = function(test, msg)
 {
   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.

-  {
-    let match = s.match(msgMatch);
+  console.error = (...args) =>
+  {
+    let s = (args[0] instanceof Error ? args[0].message : args[0]);
+    let match = s && s.match(msgMatch);
     if (!match || match[1] != msg)
-      errorHandler(s);
+      errorHandler(...args);
   };
   try
   {
     return test();
   }
   finally
   {
     console.error = errorHandler;
   }
 };
@@ skipping 13 matching lines @@
       }
     })
   };
 };
 
 exports.unexpectedError = function(error)
 {
   console.error(error);
   this.ok(false, "Unexpected error: " + error);
 };