Closed Bug 1191216 Opened 9 years ago Closed 8 years ago

Intermittent browser_console_error_source_click.js | view source opened -

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox49 fixed, firefox50 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 50
Tracking Flags:
Tracking Status
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: cbook, Assigned: jsnajdr)

References

(
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 2 obsolete files)

Part 1: Intermittent browser_console_error_source_click.js - fix test
2.58 KB, patch
bgrins
: review+
Details | Diff | Splinter Review
Part 2: Intermittent browser_console_error_source_click.js - rewrite to Task.js
4.65 KB, patch
bgrins
: review+
Details | Diff | Splinter Review 
https://treeherder.mozilla.org/logviewer.html#?job_id=12503984&repo=mozilla-inbound

 22:25:19 INFO - 521 INFO TEST-UNEXPECTED-FAIL | browser/devtools/webconsole/test/browser_console_error_source_click.js | view source opened -
I needed to fix this in order to move forward with bug 1281732.

The test triggers two messages in console: one CSS warning and one JS error. Then waits for them to appear and tests if the source links are opened on click.

The CSS warning never even appears until devtools.browserconsole.filter.cssparser is set to true. Fixed this.

Then, after the waitForMessages is finished, the "for result of results" loop should check both messages, but instead it checks only the first one twice: "result" vs "results[0]". Fixed.

The test hooks up the hud.viewSourceInDebugger function. If the first message is the JS error, this works. If the first message is the CSS warning, then viewSourceInDebugger is not called and the test fails.

The right thing to do is to hook up the hud.viewSource function, because it's called as a fallback by viewSourceInDebugger and viewSourceInStyleEditor, because the Debugger and Style Editor tools are not available in the browser console.

I also changed the element on which the click is synthesized: from "frame-link-filename" to its parent element "frame-link-source", which is the actual <A> element with the click handler. If I didn't do this, the onClick handler wasn't always called. Probably some mystery with how synthetic mouse events do/don't bubble up.
Attachment #8764890 - Flags: review?(bgrinstead)
Assignee: nobody → jsnajdr
The test still fails on try in non-e10s mode. The screenshot indicates that the messages are filtered, i.e., they are hidden and not clickable.

It seems that maybe some previous test disables the "exception" and "cssparser" messages. I added explicit calls to hud.setFilterState and I'm also testing for filtered-by-type class on the message element.

Let's see how this goes on try now.
Attachment #8764949 - Flags: review?(bgrinstead)
Attachment #8764890 - Attachment is obsolete: true
Attachment #8764890 - Flags: review?(bgrinstead)
(In reply to Jarda Snajdr [:jsnajdr] from comment #28)
> Created attachment 8764949 [details] [diff] [review]
> Intermittent browser_console_error_source_click.js
> 
> The test still fails on try in non-e10s mode. The screenshot indicates that
> the messages are filtered, i.e., they are hidden and not clickable.
> 
> It seems that maybe some previous test disables the "exception" and
> "cssparser" messages. I added explicit calls to hud.setFilterState and I'm
> also testing for filtered-by-type class on the message element.

If possible we should modify the previous test to clean up state instead of making this one worry about it (either using pushPrefEnv or manually resetting the prefs in a cleanup function)
Final version of the fixed tests. Everything described in comment 26 above is still true. No filter.* flags are left in a wrong state from previous tests - it was a false alarm.

One thing to be careful about (and that caused failures in previous try runs) is the selector for ".message .message-location .frame-link-source". For error messages with a stack trace, this rule selected the first item from the stack trace, not the location node from the message. And because the stack trace is hidden, trying to click on it fails. So, the selector needs to be ".message > .message-location". This will change in bug 1281732, where the message-location changes. The fact I was fixing these two bugs together caused confusion.

Try run is green now.
Attachment #8765419 - Flags: review?(bgrinstead)
Attachment #8764949 - Attachment is obsolete: true
Attachment #8764949 - Flags: review?(bgrinstead)
Second part: rewrite the test to task.js after fixing the intermittent failures.
Attachment #8765420 - Flags: review?(bgrinstead)
Attachment #8765419 - Flags: review?(bgrinstead) → review+
Attachment #8765420 - Flags: review?(bgrinstead) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/73b33302591e
Part 1: Intermittent browser_console_error_source_click.js - fix test. r=bgrins
https://hg.mozilla.org/integration/fx-team/rev/461be9599b39
Part 2: Intermittent browser_console_error_source_click.js - rewrite to Task.js. r=bgrins
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/73b33302591e
https://hg.mozilla.org/mozilla-central/rev/461be9599b39
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Depends on: 1283901
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: