Closed Bug 1252641 Opened 8 years ago Closed 8 years ago

Intermittent browser_net_image-tooltip.js | Test timed out - | Found a tab after previous test timed out: http://example.com/browser/devtools/client/netmonitor/test/html_content-type-without-cache-test-page.html -

Categories

(DevTools :: Netmonitor, defect)

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

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Firefox 49

People

(Reporter: KWierso, Unassigned)

References

Details

(Keywords: intermittent-failure, Whiteboard: [fixed by bug 1270111])

Attachments

(1 file)

netmonitor-tooltip-test-cleanup.patch
5.62 KB, patch
vporof
: review+
Details | Diff | Splinter Review 
Timing coincides closely with bug 1223037 landing and this is spiking pretty severely already. Can you please take a look, Tim?
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1252641
Flags: needinfo?(ntim.bugs)
I won't be able to look at this any further, but here's a patch that cleans up the test and that makes the failure permanent.

It seems like the tooltip is not showing for some reason.

Victor, any chance you can take a look at this?
Flags: needinfo?(ntim.bugs) → needinfo?(vporof)
A possible reason is that anchor is not visible (out of the viewport) because the toolbar position changed.
Either way, the failure is happening because the toolbar position changed.
Comment on attachment 8725616 [details] [diff] [review]
netmonitor-tooltip-test-cleanup.patch

Review of attachment 8725616 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/netmonitor/test/browser_net_image-tooltip.js
@@ +4,5 @@
>  /**
>   * Tests if image responses show a popup in the requests menu when hovered.
>   */
>  
> +"use strict";

Do we really need the "use strict" here?

@@ +12,3 @@
>  
> +  let { $, EVENTS, NetMonitorView: {RequestsMenu},
> +        NetMonitorController } = aMonitor.panelWin;

Nit: this destructuring is a bit gross. Maybe just do it across multiple lines.

@@ +17,5 @@
>  
> +  yield promise.all([
> +    waitForNetworkEvents(aMonitor, 7),
> +    waitFor(aMonitor.panelWin, EVENTS.RESPONSE_IMAGE_THUMBNAIL_DISPLAYED)
> +  ]);

Probably unlikely to fail, but should start waiting for those events before performingRequests()

@@ +27,4 @@
>  
> +  let anchor = $(".requests-menu-file", requestItem.target);
> +  let tooltip = yield showTooltipOn(requestTooltip, anchor);
> +  alert();

...?

A leftover I assume?

@@ +37,5 @@
>  
> +  yield promise.all([
> +    waitForNetworkEvents(aMonitor, 7),
> +    waitFor(aMonitor.panelWin, EVENTS.RESPONSE_IMAGE_THUMBNAIL_DISPLAYED)
> +  ]);

Ditto here, listen for the event before doing the work.

@@ +58,3 @@
>  
> +function* reloadAndPerformRequests(debuggee, controller) {
> +  let activity = ACTIVITY_TYPE.RELOAD.WITH_CACHE_ENABLED;

Is ACTIVITY_TYPE a global?
Attachment #8725616 - Flags: review+
Flags: needinfo?(vporof)
I don't know why the tooltip would not be showing up now. I don't see why such a small change in XUL layout could possibly affect how tooltip.js works.
With any luck, you'll be able to re-enable this on Win64 as well once it's sorted out.
See Also: → 1234341
This test failure happens because of the following facts:
1. The tooltip is hidden when the RequestsMenu list scrolls
2. When updates arrive to the RequestsMenu, they are throttled by 50ms, unless RequestsMenu.lazyUpdate is set to true (many tests set this, but not this one).
3. The events that the test waits for (waitForNetworkEvents etc) are fired after the view updates are put into the throttled queue, but before they are actually executed.

The sequence of events during the test execution is:
1. Test finishes waitForNetworkEvents, tries to show the popup.
2. Popup is being shown, probably in several async internal steps.
3. The view updates are being executed, causing the view to scroll.
4. Popup is hidden before the "show" process finishes.
5. The "shown" event never fires, the test will wait for it forever.

I'll be refactoring and reenabling this test as part of bug 1270111. I'll close this one after I'm done.
There's one more reason why the test fails: the second waitForNetworkEvents at [1] needs to wait for 8 events (1 document load + 7 XHRs), not 7. If it waits only for 7 events, an 8th event arrives in parallel with the tooltip showing and causes the view to scroll.

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_image-tooltip.js#16,36
Status: NEW → RESOLVED
Closed: 8 years ago
Depends on: 1270111
Keywords: leave-open
Resolution: --- → FIXED
Whiteboard: [fixed by bug 1270111]
Target Milestone: --- → Firefox 49
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: