Issue 6822 - Report browser test errors

test/runners/webdriver.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 function executeScript(driver, name, script, scriptName, scriptArgs)
 {
   let realScript = `let f = ${script}
                     let callback = arguments[arguments.length - 1];
                     return Promise.resolve()
                       .then(() => f(...arguments))
                       .then(() => callback());`;
   return driver.executeScript(`let oldLog = console.log;
                                console.log = msg => {
-                                 window.consoleLogs.log.push(msg);
+                                 window._consoleLogs.log.push(msg);
                                  oldLog.call(this, msg);
                                };`)
     .then(() => driver.executeAsyncScript(realScript, scriptArgs))
-    .then(() => driver.executeScript("return window.consoleLogs;"))
+    .then(() => driver.executeScript("return window._consoleLogs;"))
     .then(result =>
     {
       console.log(`\nBrowser tests in ${name}\n`);
-      result.log.forEach(item => console.log(item));

[Manish Jethani, 2018/09/05 19:47:22]

By the way, `console.log` works even when called without the correct `this`, so
`result.log.forEach(console.log)` should work fine too; although they way you've
written it is also easier to follow. It's your call.

[hub, 2018/09/06 01:00:13]

Acknowledged.

-      if (result.failures != 0)
-        return Promise.reject("Test failure");
-    })
-    .finally(() => driver.quit());

[Manish Jethani, 2018/09/05 19:47:21]

Is `Promise.finally` already supported on the platform on which this code is
going to run? It's a new feature, I didn't know it was available already. Just
checking.

[hub, 2018/09/06 01:00:13]

excellent question. 

I can't find any mention the NodeJS documentation, while MDN say it is available
in Node 10 only.

I have tested with Node 7 and Node 8 and it works.

So I assume it is ok since Node 7 is the minimum we require.

I know also this contradict 
https://codereview.adblockplus.org/29860555/diff/29863569/test/firefox.js#new...

+      for (let item of result.log)
+        console.log(item);
+      return driver.quit().then(() =>
+      {
+        if (result.failures != 0)
+          throw name;
+      });
+    }, error =>
+    {
+      // This could be replaced by Promise.finally()
+      // once we require NodeJS >= 10
+      return driver.quit().then(() =>
+      {
+        throw error;
+      });
+    });
 }
 
 module.exports.executeScript = executeScript;