Closed
Bug 1285173
Opened 8 years ago
Closed 8 years ago
Intermittent devtools/client/netmonitor/test/browser_net_statistics-01.js | Test timed out -
Categories
(DevTools :: Netmonitor, defect, P1)
DevTools
Netmonitor
Tracking
(firefox50 wontfix, firefox51 fixed, firefox52 fixed, firefox53 fixed)
RESOLVED
FIXED
Firefox 53
People
(Reporter: intermittent-bug-filer, Assigned: jsnajdr)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
Honza
:
review+
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details |
Filed by: tomcat https://treeherder.mozilla.org/logviewer.html#?job_id=1299464&repo=mozilla-beta http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-macosx64/1467867322/mozilla-beta_yosemite_r7_test-mochitest-e10s-devtools-chrome-5-bm135-tests1-macosx-build29.txt.gz
Updated•8 years ago
|
Component: Developer Tools → Developer Tools: Netmonitor
Comment 1•8 years ago
|
||
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 14•8 years ago
|
||
Honza, this test is failing 85 times a week, I think it needs re-prioritization and someone looking into it now.
Flags: needinfo?(odvarko)
Comment 15•8 years ago
|
||
Steve, your work on the Statistics view (e.g. bug 1308425) is closely related, could you please investigate the problem with this failing test? Will Statistics view refactoring have big impact on how the tests for this view are written? Or do you think they will stay pretty much the same? (there are 3 tests AFAIK) Honza
Flags: needinfo?(odvarko) → needinfo?(schung)
Priority: P3 → P1
Comment hidden (Intermittent Failures Robot) |
Comment 17•8 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] from comment #15) > Steve, your work on the Statistics view (e.g. bug 1308425) is closely > related, could you please investigate the problem with this failing test? > > Will Statistics view refactoring have big impact on how the tests for this > view are written? Or do you think they will stay pretty much the same? > (there are 3 tests AFAIK) > > Honza The patch for performance statistics view is mainly about the js code moving, and the xul structure is unchanged. Previous commits shows this intermittent issue happened month ago, and they are all stocked at this assertion: yield waitUntil( () => $all(".pie-chart-container:not([placeholder=true])").length == 2); ok(true, "Two real pie charts appear to be rendered correctly."); Hi Jarda, since you refactored this part before(bug 1297362), do you have any idea about the possible timeout error?
Flags: needinfo?(schung) → needinfo?(jsnajdr)
Comment 18•8 years ago
|
||
(In reply to Steve Chung [:steveck] from comment #17) This came up at our meeting today so, just for the record. I know that this intermittent isn't caused by your work on the Statistics view, it's just that your work might involve also tests refactoring (there are three for the Statistics view) and it would be great if the intermittent could be fixed as part of it. Honza
Assignee | ||
Comment 19•8 years ago
|
||
This intermittent happens because the function whenDataAvailable at [1] is wrong. It can be called at a time when no requests were sent yet, and the request list is empty. In that case, the "list.every()" call always returns true no matter what the predicated for "every" is. This is exactly what happens in the failing tests. The screenshot created when the test times out always looks like the one at [2]. whenDataAvailable was testing an empty list, and the chart shows the list status a moment later - there is only one request of type "other". That's the root document, and it has type "other" instead of "html" because the response hasn't arrived yet and the MIME type is unknown. The resulting chart is "empty", but the test is waiting for a nonempty chart to be displayed. That never happens. [1] http://searchfox.org/mozilla-central/rev/4b6cab91f93c73ae591dafaea40fd5704b41810e/devtools/client/netmonitor/netmonitor-view.js#1200 [2] https://public-artifacts.taskcluster.net/fzuZ4OXYR_WCnqYd-BNEdA/0/public/test_info//mozilla-test-fail-screenshot_JRmWOa.png
Assignee: nobody → jsnajdr
Flags: needinfo?(jsnajdr)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 21•8 years ago
|
||
Attached a patch that forces whenDataAvailable to wait at least for one request. Note that the exact event for which whenDataAvailable should be waiting is still ill-defined. It will end waiting whenever it finds out that all the currently outstanding requests are finished. Does this mean that the page load was finished? Maybe, but most likely not always. The whole performance stats is ripe for a complete redesign - it's very hard to understand what it's actually doing, and whatever it is, it probably does it wrong. See bug 1015201 for example. I agree with the sentiment expressed by Sole there.
See Also: → 1015201
Comment hidden (Intermittent Failures Robot) |
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8809806 [details] Bug 1285173 - Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list https://reviewboard.mozilla.org/r/92332/#review92678 R+, thanks Jarda Honza
Attachment #8809806 -
Flags: review?(odvarko) → review+
Comment 24•8 years ago
|
||
Pushed by jsnajdr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/afd22bdea63c Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list r=Honza
Comment 25•8 years ago
|
||
Backed out on request of developer for timeouts in browser_net_statistics-03.js on Linux x64 opt+pgo: https://hg.mozilla.org/integration/autoland/rev/222dd5537727a008fbb3fc299d6e3ef856761f46 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=afd22bdea63caf72d1ae956d2b2f28e99ab2a748 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=6585710&repo=autoland [task 2016-11-14T13:14:46.834693Z] 13:14:46 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_statistics-03.js | The requests-menu-filter-media-button button should not have a 'checked' class. - [task 2016-11-14T13:14:46.835819Z] 13:14:46 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_statistics-03.js | The requests-menu-filter-flash-button button should not have a 'checked' class. - [task 2016-11-14T13:14:46.842607Z] 13:14:46 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_statistics-03.js | The requests-menu-filter-ws-button button should have a 'checked' class. - [task 2016-11-14T13:14:46.842690Z] 13:14:46 INFO - The correct filtering predicates are used before entering perf. analysis mode. [task 2016-11-14T13:14:46.842751Z] 13:14:46 INFO - Buffered messages finished [task 2016-11-14T13:14:46.842852Z] 13:14:46 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_statistics-03.js | Test timed out -
Flags: needinfo?(jsnajdr)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 27•8 years ago
|
||
Submitting a new and improved version of the patch. Solved another problem with whenDataAvailable: it retrieved the request list from the view object only once (RequestsMenu.attachments returns a copy of the requests array), and then waited for its properties to be filled in. It failed in case when the array returned on first call was empty. Now, the RequestsMenu.attachments is called again each time a check is performed in the setInterval loop.
Flags: needinfo?(jsnajdr)
Comment 28•8 years ago
|
||
mozreview-review |
Comment on attachment 8809806 [details] Bug 1285173 - Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list https://reviewboard.mozilla.org/r/92332/#review93072 Looks good, thanks Jarda for working on this! Honza
Comment 29•8 years ago
|
||
Pushed by jsnajdr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b41ff8fe81c2 Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list r=Honza
Comment 30•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b41ff8fe81c2
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment 31•8 years ago
|
||
Please request Aurora/Beta approval on this when you get a chance.
status-firefox50:
--- → wontfix
status-firefox51:
--- → affected
status-firefox52:
--- → affected
Flags: needinfo?(jsnajdr)
Comment hidden (Intermittent Failures Robot) |
Comment 33•8 years ago
|
||
Comment on attachment 8809806 [details] Bug 1285173 - Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list Approval Request Comment [Feature/regressing bug #]: n/a [User impact if declined]: n/a [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: No risk for the user, the fix is related to automated test. [String/UUID change made/needed]: none
Attachment #8809806 -
Flags: approval-mozilla-aurora?
Comment 34•8 years ago
|
||
Comment on attachment 8809806 [details] Bug 1285173 - Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list Approval Request Comment [Feature/regressing bug #]: n/a [User impact if declined]: n/a [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: No risk for the user, the fix is related to automated test. [String/UUID change made/needed]: none
Attachment #8809806 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jsnajdr)
Comment 35•8 years ago
|
||
Comment on attachment 8809806 [details] Bug 1285173 - Netmonitor perf stats: whenDataAvailable should wait for a nonempty request list Fix an intermittent test failure. Beta51 and Aurora 52+. Should be in 51 beta 2.
Attachment #8809806 -
Flags: approval-mozilla-beta?
Attachment #8809806 -
Flags: approval-mozilla-beta+
Attachment #8809806 -
Flags: approval-mozilla-aurora?
Attachment #8809806 -
Flags: approval-mozilla-aurora+
Comment 36•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/f96c9373cf0f
Flags: in-testsuite+
Comment 37•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/83de2b07a573
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•