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,61 @@
     }
   };
 };
 
 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,