Investigate autocomplete test unreliability on Linux/e10s

RESOLVED FIXED in Firefox 37

Status

()

Firefox
Address Bar
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Unfocused, Assigned: ttaubert)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 37
All
Linux
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(e10s+)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Bug 1072320 ended up being quite a rabbithole of inproductivity. We have a set of tests that aren't reliable on Linux (and so have been disabled there):

browser_action_keyword.js
browser_action_searchengine.js
browser_action_searchengine_alias.js

AFAICT, this is because when the test tries to interact with the autocomplete result, nothing happens. My assumption is that this is due to focus issues around popups on Linux, which we've had troubles with in the past.
Also browser_search_favicon.js.  I can see the same problem intermittently with e10s on Windows, and see no reason to believe they are different - so I'm opening a new bug to also disable these tests with e10s keeping this bug number as the reason.

As to the reason: I'm seeing that the failure in e10s is caused due to detachController in autocomplete.xml being called during the test, causing the search to stop before we've got the results we expect.  detachController is called via:

_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml:1251:9
_finalizeTabSwitch@chrome://browser/content/tabbrowser.xml:3273:11
set_selectedIndex/<@chrome://browser/content/tabbrowser.xml:5252:15
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:865:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:7

which is tabbrowser focusing the new tab asynchronously.
Summary: Investigate autocomplete test unreliability on Linux → Investigate autocomplete test unreliability on Linux/e10s
Blocks: 984139
tracking-e10s: --- → +
(Assignee)

Updated

4 years ago
Depends on: 1075450
(Assignee)

Comment 2

4 years ago
Created attachment 8527295 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch

This is based off of the patches from bug 1075450.
Attachment #8527295 - Flags: review?(mak77)
(Assignee)

Comment 3

4 years ago
Created attachment 8527296 [details] [diff] [review]
0002-Bug-1073339-Convert-from-CRLF-to-LF-for-browser_acti.patch

Windows -> Unix line endings.
Attachment #8527296 - Flags: review?(mak77)
Comment on attachment 8527295 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch

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

Thanks for looking into this :) It's been driving me batty.

Will also need to deal with browser_autocomplete_autoselect.js, added in bug 1067903.

::: browser/base/content/test/general/browser_action_searchengine.js
@@ +8,5 @@
>  
>    Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
>                                         "http://example.com/?q={searchTerms}");
>    let engine = Services.search.getEngineByName("MozSearch");
> +  let gOriginalEngine = Services.search.currentEngine;

Nit: Rename, so we don't accidentally assume it's a global?
Attachment #8527295 - Flags: review?(mak77) → review-
Attachment #8527296 - Flags: review?(mak77) → review+
Oh, would be good to also update browser_autocomplete_a11y_label.js too, for consistency.
(Assignee)

Comment 7

3 years ago
Created attachment 8530242 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v2
Assignee: nobody → ttaubert
Attachment #8527295 - Attachment is obsolete: true
Attachment #8527296 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8530242 - Flags: review?(bmcbride)
(Assignee)

Comment 8

3 years ago
Created attachment 8530243 [details] [diff] [review]
0002-Bug-1073339-Convert-from-CRLF-to-LF-for-autocomplete.patch, v2
Attachment #8530242 - Attachment is obsolete: true
Attachment #8530242 - Flags: review?(bmcbride)
Attachment #8530243 - Flags: review?(bmcbride)
(Assignee)

Comment 9

3 years ago
Try push with unifiedcomplete=false:

https://tbpl.mozilla.org/?tree=Try&rev=3ee73b9c370d

Try push with unifiedcomplete=true:

https://tbpl.mozilla.org/?tree=Try&rev=2d7106bda22e
(Assignee)

Updated

3 years ago
Iteration: --- → 37.1
Points: --- → 3
Flags: qe-verify-
Flags: firefox-backlog+
(Assignee)

Comment 10

3 years ago
Doesn't look great with e10s and unifiedcomplete enabled. If only I could run e10s tests locally...
you can by passing --e10s to mach mochitest-browser?
(Assignee)

Comment 12

3 years ago
Currently broken due to bug 1103159 unfortunately.
Attachment #8530243 - Flags: review?(bmcbride) → review+
Attachment #8530242 - Attachment is obsolete: false
Comment on attachment 8530242 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v2

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

I don't see any obvious reason why this is failing on Try :\
Attachment #8530242 - Flags: review-

Updated

3 years ago
Iteration: 37.1 → 37.2
(Assignee)

Comment 14

3 years ago
Created attachment 8535550 [details] [diff] [review]
0002-Bug-1073339-Convert-from-CRLF-to-LF-for-autocomplete.patch, v3

Updated due to test changes.
Attachment #8530243 - Attachment is obsolete: true
Attachment #8535550 - Flags: review+
(Assignee)

Comment 15

3 years ago
Created attachment 8535551 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v3

This is looking better:

https://tbpl.mozilla.org/?tree=Try&rev=497aac59ca3a
https://tbpl.mozilla.org/?tree=Try&rev=d98d73126544
Attachment #8530242 - Attachment is obsolete: true
Attachment #8535551 - Flags: review?(bmcbride)
Comment on attachment 8535551 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v3

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

::: browser/base/content/test/general/browser.ini
@@ -110,5 @@
>  [browser_aboutHome.js]
>  skip-if = e10s # Bug 1093153 - no about:home support yet
>  [browser_aboutSyncProgress.js]
>  [browser_action_keyword.js]
> -skip-if = os == "linux" || e10s # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s

Look like browser_action_keyword.js should get updated too.
Attachment #8535551 - Flags: review?(bmcbride) → review-
(Assignee)

Comment 17

3 years ago
Created attachment 8536635 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v4
Attachment #8535551 - Attachment is obsolete: true
Attachment #8536635 - Flags: review?(bmcbride)
(Assignee)

Updated

3 years ago
Blocks: 1104755
Attachment #8536635 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/d68830b2ce47
https://hg.mozilla.org/mozilla-central/rev/704ca3c0bb46
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
You need to log in before you can comment on or make changes to this bug.