Closed Bug 1384582 Opened 8 years ago Closed 8 years ago

Re-enable browser_urlbar_search_reflows.js for macOS opt builds

Categories

(Firefox :: Address Bar, enhancement, P2)

Product:
Firefox
Component:
Address Bar
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox57 --- fix-optional
firefox58 --- fixed

People

(Reporter: mconley, Assigned: daleharvey)

Details

Attachments

(1 file)

Bug 1384582 - Ensure browser_urlbar_search_reflows.js test runs are isolated.
59 bytes, text/x-review-board-request
mconley
: review+
Details
browser_urlbar_search_reflows.js was added in bug 1356271, but disabled for opt macOS due to frequent failures with this stack: 21:09:21 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_search_reflows.js | unexpected uninterruptible reflow 21:09:21 INFO - [ 21:09:21 INFO - "_onLazyResize@resource:///modules/CustomizableUI.jsm:4376:1", 21:09:21 INFO - "_runTask@resource://gre/modules/DeferredTask.jsm:304:20", 21:09:21 INFO - "async*_timerCallback/<@resource://gre/modules/DeferredTask.jsm:277:13", 21:09:21 INFO - "async*_timerCallback@resource://gre/modules/DeferredTask.jsm:275:30", 21:09:21 INFO - "" 21:09:21 INFO - ] 21:09:21 INFO - - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reflow :: line 104 21:09:21 INFO - Stack trace: 21:09:21 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reflow:104 21:09:21 INFO - resource:///modules/CustomizableUI.jsm:_onLazyResize:4376 21:09:21 INFO - resource://gre/modules/DeferredTask.jsm:_runTask:304 21:09:21 INFO - Not taking screenshot here: see the one that was previously logged 21:09:21 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_search_reflows.js | unexpected uninterruptible reflow 21:09:21 INFO - [ 21:09:21 INFO - "_onLazyResize@resource:///modules/CustomizableUI.jsm:4376:1", 21:09:21 INFO - "_runTask@resource://gre/modules/DeferredTask.jsm:304:20", 21:09:21 INFO - "async*_timerCallback/<@resource://gre/modules/DeferredTask.jsm:277:13", 21:09:21 INFO - "async*_timerCallback@resource://gre/modules/DeferredTask.jsm:275:30", 21:09:21 INFO - "" 21:09:21 INFO - ] 21:09:21 INFO - - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reflow :: line 104 21:09:21 INFO - Stack trace: 21:09:21 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reflow:104 21:09:21 INFO - resource:///modules/CustomizableUI.jsm:_onLazyResize:4376 21:09:21 INFO - resource://gre/modules/DeferredTask.jsm:_runTask:304 We should figure out what's going on there and then re-enable the test.
Summary: Re-enabled browser_urlbar_search_reflows.js for macOS opt builds → Re-enable browser_urlbar_search_reflows.js for macOS opt builds
Been taking a look at this if nobody else is picking it up
Assignee: nobody → dharvey
Just some WIP comments, so this test has never failed on a single run, only as part of --verify, and in --verify it seems to fail fairly early on which makes me think that something isnt completely isolated during the test runs It is no longer failing with the above stack, its now runs an expected reflow stack (http://searchfox.org/mozilla-central/source/browser/base/content/test/performance/browser_urlbar_search_reflows.js#64) too many times, the current times is 6 however 60(!) is needed to pass, 60 is the exact minimum, if I set it to 59 it will fail pretty reliably after 219 tests Trying to figure out the reason for the 4th test run (test suite is 83 tests, 219 failing is the 4th run) being different from the other 3
ok so first bug after spending too long looking for it is simple, this is never going to pass with --verify as it assumes a fresh browser instance when first opening the awesomebar, the second time the tests run it has to clear the items from the awesomebar from the previous run @ http://searchfox.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#1970 (1st run childnodes.length === 1, 2nd run childnodes.length === 10), not entirely sure what the bet way to do cleanup here is
So the issue causing --verify to permanently fail is that places will return stale results to a query across new windows even with clearHistory being called, per maks suggestion I give each test its own unique id (just a timestamp) which prevents subsequent test runs from interfering with each others results
Comment on attachment 8926366 [details] Bug 1384582 - Ensure browser_urlbar_search_reflows.js test runs are isolated. https://reviewboard.mozilla.org/r/197652/#review202892 Thanks for figuring this out, daleharvey! ::: commit-message-57482:1 (Diff revision 1) > +Bug 1384582 - Ensure reflow test runs are isolated. r?mconley Nit: let's maybe make it clear that we're making sure that the browser_urlbar_search_reflows.js tests are isolated. Probably wouldn't hurt to briefly explain what you mean by "isolated" in a short paragraph below too, but I won't harp on that. :) ::: browser/base/content/test/performance/head.js:4 (Diff revision 1) > "use strict"; > > +XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils", > + "resource://gre/modules/PlacesUtils.jsm"); Nit: can you align this like the `defineLazyModuleGetter` on the next line?
Attachment #8926366 - Flags: review?(mconley) → review+
Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/09381b4fe070 Ensure browser_urlbar_search_reflows.js test runs are isolated. r=mconley
https://hg.mozilla.org/mozilla-central/rev/09381b4fe070
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: