Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(118)

Issue 29721681: Noissue - Allow identifying the element in case of error in test (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by hub
Modified:
2 months, 3 weeks ago
Reviewers:
kzar, Sebastian Noack
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Noissue - Allow identifying the element in case of error in test This make debugging the tests easier.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Review feedback #

Total comments: 5

Patch Set 3 : Review feeedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -8 lines) Patch
M test/browser/elemHideEmulation.js View 1 2 2 chunks +18 lines, -8 lines 0 comments Download

Messages

Total messages: 7
hub
3 months ago (2018-03-13 14:44:20 UTC) #1
kzar
Cool idea https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHideEmulation.js#newcode63 test/browser/elemHideEmulation.js:63: `The element's display property should be set ...
3 months ago (2018-03-19 16:55:20 UTC) #2
hub
https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29721681/diff/29721682/test/browser/elemHideEmulation.js#newcode63 test/browser/elemHideEmulation.js:63: `The element's display property should be set to 'none' ...
2 months, 4 weeks ago (2018-03-20 14:40:26 UTC) #3
kzar
https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHideEmulation.js#newcode60 test/browser/elemHideEmulation.js:60: let message = id ? This seems kind of ...
2 months, 4 weeks ago (2018-03-21 18:12:00 UTC) #4
hub
https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHideEmulation.js File test/browser/elemHideEmulation.js (right): https://codereview.adblockplus.org/29721681/diff/29728598/test/browser/elemHideEmulation.js#newcode60 test/browser/elemHideEmulation.js:60: let message = id ? On 2018/03/21 18:11:59, kzar ...
2 months, 4 weeks ago (2018-03-21 19:05:05 UTC) #5
hub
(sorry this time it is the right patch)
2 months, 4 weeks ago (2018-03-21 19:06:14 UTC) #6
kzar
2 months, 3 weeks ago (2018-03-22 09:40:05 UTC) #7
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5