Module: functional Test: /testAwesomeBar/testAccessLocationBar.js | testAccessLocationBarHistory Failure: 05:07:42 ERROR | Test Failure | { 05:07:42 "exception": { 05:07:42 "message": "The second item in the drop down list should be selected", 05:07:42 "lineNumber": 27, 05:07:42 "name": "TimeoutError", 05:07:42 "fileName": "resource://mozmill/modules/errors.js" 05:07:42 } 05:07:42 } Report: http://mozmill-daily.blargon7.com/#/functional/report/53ec3148fca67aea1b1c3528f92c8a03 Branches: Nightly (Firefox 35.0a1 (35.0a1, en-US, 20141001030205)) Platforms: Linux 13.10 This looks like a new intermitent failure caused by landing of the bug 1067888 . We came with a fix in bug 1074076 (the same changes developer did within their tests). I couldn't reproduce this locally but I suspect why it's failing: we should wait for the results to load first, then handle the 2nd row, but we currently don't to that. That will be similar to how we're waiting in testSwitchToTab: >> assert.waitFor(function () { >> return locationBar.autoCompleteResults.visibleResults.length > 1; >> }, "Waiting for autocomplete results to load"); But first I'll try to reproduce this on CI.

We had 2 failures in 2 days.

On CI reproduces 50% of the time. With the fix all works great (no failure in 100 runs).

Comment on attachment 8498741 [details] [diff] [review] fix.patch Review of attachment 8498741 [details] [diff] [review]: ----------------------------------------------------------------- ::: firefox/tests/functional/testAwesomeBar/testAccessLocationBar.js @@ +62,5 @@ > }, "Autocomplete results should be visible"); > > + assert.waitFor(function () { > + return locationBar.autoCompleteResults.visibleResults.length > 1; > + }, "Waiting for autocomplete results to load"); Please combine this with the above waitFor. We can also switch to the fat arrow function.

Today I've also seen a new failure in > testAwesomeBar\testSuggestHistory.js | testSuggestHistoryAndBookmarks 06:36:36 ERROR | Test Failure | { 06:36:36 "fail": { 06:36:36 "message": "Expected to be two visible results in the autocomplete list - '1' should equal '2'", 06:36:36 "fileName": "file:///c:/jenkins/workspace/mozilla-central_functional/data/mozmill-tests/firefox/tests/functional/testAwesomeBar/testSuggestHistory.js", 06:36:36 "name": "testSuggestHistoryAndBookmarks", 06:36:36 "lineNumber": 60 06:36:36 } 06:36:36 } 06:36:36 TEST-UNEXPECTED-FAIL | testAwesomeBar\testSuggestHistory.js | testSuggestHistoryAndBookmarks Report: http://mozmill-daily.blargon7.com/#/functional/report/53ec3148fca67aea1b1c3528f940957b This is intermitent, but let's also fix this. I will just hijack this bug to take care of all the failures related to the awesomebar so we fix them all at once and don't have duplicated work in testing fixes.

And another failure in /testAwesomeBar/testCheckItemHighlight.js Those are intermitent failures and reproduces on CI and on local machine if we slow down the proccessor speed. Report: http://mozmill-daily.blargon7.com/#/functional/report/53ec3148fca67aea1b1c3528f942b4d9 Incresing the priority to P1 as it's causing failures pretty often and it reproduces on slower machines.

We had 2 failures over weekend in testAccessLocationBar.js, on Linux 13.10.

There's another affected test here: http://mozmill-daily.blargon7.com/#/functional/report/c1ae8473ee5b384b83bbfde33108c69b

This patch should fix the reported issues. I think it will be better to use a new method to check if the history has been populated using history API instead of using *.sleep() method for that.

Comment on attachment 8506867 [details] [diff] [review] b1076722.patch Review of attachment 8506867 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch Teodor. Please update your mercurial settings so the generated patch includes you as the author. Also the commit message should follow a certain pattern: > Bug XXX - Description. r=reviewer In this case something like: > Bug 1076722 - Correctly wait for AwesomeBar results. r=aeftimie ::: firefox/tests/functional/testAwesomeBar/testAccessLocationBar.js @@ +57,5 @@ > // the most recent visit first, then double arrow down again to the second entry, > // in this case mozilla_projects.html > controller.keypress(locationBar.urlbar, "VK_DOWN", {}); > + assert.waitFor(() => locationBar.autoCompleteResults.isOpened, > + "Autocomplete results should be visible"); Please check your indentation as this uses tabs mixed with spaces. @@ +59,5 @@ > controller.keypress(locationBar.urlbar, "VK_DOWN", {}); > + assert.waitFor(() => locationBar.autoCompleteResults.isOpened, > + "Autocomplete results should be visible"); > + > + // wait for autocomplete list to be populated nit: // Wait for the [...] @@ +71,1 @@ > Since we're here please also remove these trailing whitespaces. ::: firefox/tests/functional/testAwesomeBar/testCheckItemHighlight.js @@ +54,5 @@ > > // Type the page name into the location bar > locationBar.type(TEST_DATA.name); > > + nit: please remove this extra empty newline added. @@ +67,5 @@ > }, "Autocomplete popup has been opened - expected 'true'"); > > + // Wait for the autocomplete to be populated > + assert.waitFor(() => > + (locationBar.autoCompleteResults.visibleResults.length === 2), We treat 80 characters as a soft wrap point and 100 as a hard one. In certain cases we should break 80 characters if it is more readable, and I feel this is one. In this particular case I'd say to leave the whole function on one line. @@ +92,5 @@ > // Get a list of any underlined autocomplete results > var underlined = locationBar.autoCompleteResults. > getUnderlinedText(aResult, aType); > > + nit: same here, please remove this extra added empty newline @@ +97,3 @@ > // Check that there is only 1 entry > + assert.waitFor(() => (underlined.length === 1), > + "Only one autocompleted result is underlined"); We already wait to have everything shown, I think it should be safe to leave this as it was.

Updated patch after review notes.

Teodor could you have a quick look at bug 1085328 if it seems to have the same cause as the ones you are fixing here? If they are, we can / should issue a fix for testSuggestBookmarks.js

I'll look into it.

Comment on attachment 8507817 [details] [diff] [review] b1076722v2.patch Review of attachment 8507817 [details] [diff] [review]: ----------------------------------------------------------------- Small changes. Have you checked the reports to make sure only these 3 tests remained affected? ::: firefox/tests/functional/testAwesomeBar/testAccessLocationBar.js @@ +61,5 @@ > + "Autocomplete results should be visible"); > + > + // Wait for autocomplete list to be populated > + assert.waitFor(() => (locationBar.autoCompleteResults.length > 1), > + "Expected to be more than one results in the autocomplete list"); nit: one result @@ +71,1 @@ > Trailing whitespaces, please remove them.

Checked the reports only these 3 tests have issues related to this bug. The test reported by Andrei may be a different related issue, requires further investigation. Updated the patch following the review notes.

For aurora I also see an issue in testSwitchTabs: http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2014-10-15&to=2014-10-22&test=%2FtestAwesomeBar%2FtestSwitchToTab.js&func=testSwitchToTab Please check this as well. Might be a good enhancement for nightly as well.

Due to Bug 1081297 - Disable UnifiedComplete for Firefox 35, we have a few crashes on the latest aurora. Added a skip test patch for the mozilla-aurora branch. Skipped: testCheckItemHighlight.js, testSuggestBookmarks.js, testSuggestHistory.js, testSwitchToTab.js.

As give per bug 1087334 the unified location bar code has been disabled. So we should revert those changes on the aurora branch to be able to test the default behavior of Firefox.

Comment on attachment 8509521 [details] [diff] [review] skipawesomebartestsaurora.patch I've backed out the changes in the original issue: bug 1074076 No need to disable them on Aurora anymore.

Comment on attachment 8509293 [details] [diff] [review] b1076722v3.patch Review of attachment 8509293 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, I updated the commit message and the user. Teodor, make sure to update your .hgrc file to include this information if you haven't already. See https://developer.mozilla.org/en-US/docs/Mercurial_FAQ Landed: https://hg.mozilla.org/qa/mozmill-tests/rev/6f2385cd113b (default)