Noissue - Allow identifying the element in case of error in test

Noissue - Allow identifying the element in case of error in test

This make debugging the tests easier.

[kzar, 2018-03-19 16:55:20]

Cool idea

https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHi...
File test/browser/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHi...
test/browser/elemHideEmulation.js:63: `The element's display property should be
set to 'none' ${elementId}`);
Please can you improve the message, like "The element with ID 'foo'...".

[hub, 2018-03-20 14:40:26]

https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHi...
File test/browser/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHi...
test/browser/elemHideEmulation.js:63: `The element's display property should be
set to 'none' ${elementId}`);
On 2018/03/19 16:55:20, kzar wrote:
> Please can you improve the message, like "The element with ID 'foo'...".

Done.

[kzar, 2018-03-21 18:12:00]

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
File test/browser/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
test/browser/elemHideEmulation.js:60: let message = id ?
This seems kind of messy, also I guess the ID might be zero. How about something
like this? (Though to be honest its still kind of messy :/)

function expectHidden(test, element, id)
{
  let withId = "";
  if (typeof id != "undefined")
    withId = ` with ID '${id}'`;

  test.equal(
    window.getComputedStyle(element).display, "none",
    "The element" + withId + "'s display property should be set to none"
  );
}

[hub, 2018-03-21 19:05:05]

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
File test/browser/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
test/browser/elemHideEmulation.js:60: let message = id ?
On 2018/03/21 18:11:59, kzar wrote:
> This seems kind of messy, 

Hence my original approach, since these are just test and not end-user visible
text.

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
test/browser/elemHideEmulation.js:60: let message = id ?
On 2018/03/21 18:11:59, kzar wrote:
> This seems kind of messy, also I guess the ID might be zero. How about
something
> like this? (Though to be honest its still kind of messy :/)
> 
> function expectHidden(test, element, id)
> {
>   let withId = "";
>   if (typeof id != "undefined")
>     withId = ` with ID '${id}'`;
> 
>   test.equal(
>     window.getComputedStyle(element).display, "none",
>     "The element" + withId + "'s display property should be set to none"
>   );
> }

Done.

[hub, 2018-03-21 19:06:14]

(sorry this time it is the right patch)

[kzar, 2018-03-22 09:40:05]

LGTM

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
File test/browser/elemHideEmulation.js (right):

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
test/browser/elemHideEmulation.js:60: let message = id ?
On 2018/03/21 18:11:59, kzar wrote:
> ...also I guess the ID might be zero...

Sorry I was talking nonsense here, the ID is a string not a number.

https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHi...
test/browser/elemHideEmulation.js:60: let message = id ?
On 2018/03/21 19:05:04, hub wrote:
> On 2018/03/21 18:11:59, kzar wrote:
> > This seems kind of messy, 
> 
> Hence my original approach, since these are just test and not end-user visible
> text.

Yea fair enough.