Issue 4726 - Add tests for the element hiding emulation content script

test/browser/elemHideEmulation.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * 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
@@ skipping 102 matching lines @@
     }
   );
 
   elemHideEmulation.load(function()
   {
     elemHideEmulation.apply();
     callback();
   });
 }
 
-QUnit.test("Verbatim property selector", function(assert)
-{
-  var done = assert.async();
-  var toHide = createElementWithStyle("{background-color: #000}");
-  applyElemHideEmulation(["[-abp-properties='background-color: rgb(0, 0, 0)']"],

[kzar, 2017/01/18 04:17:15]

Mind also moving the first argument of the applyElemHideemulation calls down in
line with the second?

applyElemHideEmulation(
  ["[-abp-properties='background-color: rgb(0, 0, 0)']"],
  function()
  {
    ...
  }
);

(That's generally how we do it, also it is nice to make the indentation
consistent between the test cases.)

[Felix Dahlke, 2017/01/19 09:56:51]

I think there's a range of how we generally do this, but if you find this more
readable (makes no relevant difference for me), I'll change it. Done.

[Sebastian Noack, 2017/01/19 10:13:00]

I agree with Dave. As I explained in my previous comment, it is harder to read
if the arguments are not aligned.

-    function()
-    {
-      assert.hidden(toHide);
-      done();
-    }
-  );
-});
-
-QUnit.test("Property selector with wildcard", function(assert)

[kzar, 2017/01/18 04:17:15]

Mind bringing the function call and test description down onto the next line?
That will make how the test case functions are indented consistent and as
mentioned above that's generally how we do it. It's kind of unusual IMO to mix
both styles of indentation.

So like

QUnit.test(
  "Property selector with wildcard",
  function(assert)
  {
    ...
  }
);

[Felix Dahlke, 2017/01/19 09:56:51]

There's no need for the wrapping in these cases, and I don't think we generally
wrap lines that don't have to be wrapped.

IMO it hurts readability, but since I wrote it and you're reading it, you're the
better judge of that. Done. (On the positive side, it does resolve the
blame/diff issues I mentioned.)

[Sebastian Noack, 2017/01/19 10:13:00]

FWIW, if all arguments fit on the same line as the function call, like here, I
generally wouldn't wrap it. Here, one might argue though that it's more
consistent to wrap all tests in the same way. I don't have a strong opinion.

-{
-  var done = assert.async();
-  var toHide = createElementWithStyle("{background-color: #000}");
-  applyElemHideEmulation(["[-abp-properties='*color: rgb(0, 0, 0)']"],
-    function()
-    {
-      assert.hidden(toHide);
-      done();
-    }
-  );
-});
-
-QUnit.test("Property selector with regular expression", function(assert)
-{
-  var done = assert.async();
-  var toHide = createElementWithStyle("{background-color: #000}");
-  applyElemHideEmulation(["[-abp-properties='/.*color: rgb\\(0, 0, 0\\)/']"],
-    function()
-    {
-      assert.hidden(toHide);
-      done();
-    }
-  );
-});
-
-QUnit.test("Property selector with regular expression containing escaped brace",
+QUnit.test(
+  "Verbatim property selector",
+  function(assert)
+  {
+    var done = assert.async();
+    var toHide = createElementWithStyle("{background-color: #000}");
+    applyElemHideEmulation(
+      ["[-abp-properties='background-color: rgb(0, 0, 0)']"],
+      function()
+      {
+        assert.hidden(toHide);
+        done();
+      }
+    );
+  }
+);
+
+QUnit.test(
+  "Property selector with wildcard",
+  function(assert)
+  {
+    var done = assert.async();
+    var toHide = createElementWithStyle("{background-color: #000}");
+    applyElemHideEmulation(
+      ["[-abp-properties='*color: rgb(0, 0, 0)']"],
+      function()
+      {
+        assert.hidden(toHide);
+        done();
+      }
+    );
+  }
+);
+
+QUnit.test(
+  "Property selector with regular expression",
+  function(assert)
+  {
+    var done = assert.async();
+    var toHide = createElementWithStyle("{background-color: #000}");
+    applyElemHideEmulation(
+      ["[-abp-properties='/.*color: rgb\\(0, 0, 0\\)/']"],
+      function()
+      {
+        assert.hidden(toHide);
+        done();
+      }
+    );
+  }
+);
+
+QUnit.test(
+  "Property selector with regular expression containing escaped brace",
   function(assert)
   {
     var done = assert.async();
     var toHide = createElementWithStyle("{background-color: #000}");
     applyElemHideEmulation(
       ["[-abp-properties='/background.\\x7B 0,6\\x7D : rgb\\(0, 0, 0\\)/']"],
       function()
       {
         assert.hidden(toHide);
         done();
       }
     );
   }
 );
 
-QUnit.test("Property selector with regular expression containing improperly \
-escaped brace",
+QUnit.test(
+  "Property selector with regular expression containing improperly escaped \
+brace",
   function(assert)
   {
     var done = assert.async();
     var toHide = createElementWithStyle("{background-color: #000}");
     applyElemHideEmulation(
       ["[-abp-properties='/background.\\x7B0,6\\x7D: rgb\\(0, 0, 0\\)/']"],
       function()
       {
         assert.visible(toHide);
         done();
       }
     );
   }
 );
 
-QUnit.test("Property selector works for dynamically changed property",
+QUnit.test(
+  "Property selector works for dynamically changed property",
   function(assert)
   {
     var done = assert.async();
     var toHide = createElementWithStyle("{}");
     applyElemHideEmulation(
       ["[-abp-properties='background-color: rgb(0, 0, 0)']"],
       function()
       {
         assert.visible(toHide);
         insertStyleRule("#" + toHide.id + " {background-color: #000}");
         setTimeout(function()
         {
           assert.hidden(toHide);
           done();
         });
       }
     );
   }
 );