Issue 4838 - Use nodeunit framework for integration tests running in browser

browsertests.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2016 Eyeo GmbH

[Felix Dahlke, 2017/01/30 14:37:58]

It's 2017 :)

[Wladimir Palant, 2017/02/24 09:18:55]

Yes, but the copyright header hasn't been updated for the other files either.
Well, whatever...

[Felix Dahlke, 2017/02/24 10:43:38]

:)

+ * Copyright (C) 2006-2017 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()
 {
-  // TODO: This should use promises once PhantomJS supports them, this will make

[Felix Dahlke, 2017/01/30 14:37:58]

Could you reference issue 4796 here? In the other files where we are restricted
to ES5 because of PhantomJS, we have this comment, I'd personally just add it
the same way at the top in this file:

// We are currently limited to ECMAScript 5 in this file, because it is being
// used in the browser tests. See https://issues.adblockplus.org/ticket/4796

[Wladimir Palant, 2017/02/24 09:18:55]

Done.

-  // wrapping all async calls in order to catch errors unnecessary.
+  // We are currently limited to ECMAScript 5 in this file, because it is being
+  // used in the browser tests. See https://issues.adblockplus.org/ticket/4796
+  // Once this is resolved we should use promises here.
   function safeCall(callback)

[Felix Dahlke, 2017/01/30 14:37:58]

Have you considered using phantom.onError instead?

[Wladimir Palant, 2017/02/24 09:18:55]

I was using that originally, it wasn't catching all errors reliably.

[Felix Dahlke, 2017/02/24 10:43:38]

Acknowledged.

   {
     return function()
     {
       try
       {
         callback.apply(this, arguments);
       }
       catch (e)
       {
         var message = String(e);
         if (e.stack)
           message += "\n" + e.stack;
         console.log(message);

[Felix Dahlke, 2017/01/30 14:37:58]

Since it's an error message, how about console.error?

[Wladimir Palant, 2017/02/24 09:18:55]

nodeunit prints all output via console.log(), including errors. I guess this
makes redirection of test results easier.

[Felix Dahlke, 2017/02/24 10:43:39]

Acknowledged.

-        phantom.exit();

[Felix Dahlke, 2017/01/30 14:37:58]

phantom.exit(1) in this case?

[Wladimir Palant, 2017/02/24 09:18:55]

Done.

+        phantom.exit(1);
       }
-    }

[Felix Dahlke, 2017/01/30 14:37:58]

Missing semicolon?

[Wladimir Palant, 2017/02/24 09:18:55]

Done.

+    };
   }
 
   function loadScript(doc, url, callback)
   {
     var script = doc.createElement("script");
     script.src = url;
     script.async = false;
     doc.head.appendChild(script);
     if (callback)
       window.setTimeout(callback, 0);
-  }
-
-  function run()

[Felix Dahlke, 2017/01/30 14:37:58]

Nit: Move this below runTests() to have the functions declared in a consistent
order? (Maybe I'm being meticulous though.)

[Wladimir Palant, 2017/02/24 09:18:55]

Done.

-  {
-    var system = require("system");
-    var nodeunitUrl = system.args[1];
-    var urls = system.args.slice(2);
-
-    loadScript(document, nodeunitUrl, safeCall(function()
-    {
-      loadModules(urls, safeCall(function(modules)
-      {
-        runTests(modules, function()
-        {
-          phantom.exit();
-        });
-      }));
-    }));
   }
 
   function loadModules(urls, callback)
   {
     var modules = {};
 
     var loadNext = safeCall(function()
     {
       if (urls.length)
       {
@@ skipping 59 matching lines @@
           console.log("\u2714 " + name);
         else
         {
           console.log(error("\u2716 " + name) + "\n");
           errors.forEach(function(error)
           {
             console.log(String(error));
             if (error.stack)
               console.log(error.stack);
             console.log("");
-          })
+          });
         }
       },
       done: function(assertions)
       {
         var failures = assertions.filter(function(assertion)
         {
           return assertion.failed();
         });
         if (failures.length)
         {
           console.log(
             "\n" +
             bold(error("FAILURES: ")) +
             failures.length + "/" + assertions.length + " assertions failed"
           );
         }
         else
         {
           console.log(
             "\n" + bold(ok("OK: ")) +
             assertions.length + " assertions"
           );
         }
 
-        callback();
+        callback(!failures.length);
       }
     });
   }
 
+  function run()
+  {
+    var system = require("system");
+    var nodeunitUrl = system.args[1];
+    var urls = system.args.slice(2);
+
+    loadScript(document, nodeunitUrl, safeCall(function()
+    {
+      loadModules(urls, safeCall(function(modules)
+      {
+        runTests(modules, function(success)
+        {
+          phantom.exit(success ? 0 : 1);
+        });
+      }));
+    }));
+  }
+
   safeCall(run)();
 })();