Closed Bug 1082294 Opened 5 years ago Closed 5 years ago

TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/places/tests/unifiedcomplete/test_autoFill_default_behavior.js | test failed (with xpcshell return code: 0), see following log

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- unaffected
firefox35 --- fixed
firefox36 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: hiro, Assigned: hiro)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

18:33:26  WARNING -  TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js | Didn't find the current result ('moz-action:searchengine,{"engineName":"Bing","input":"visited/v","searchQuery":"visited/v"}', 'Bing') in matches - See following stack:
18:33:26     INFO -      /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js:check_autocomplete:179
18:33:26     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:865:23
18:33:26     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:7
18:33:26     INFO -      _do_main@/builds/slave/test/build/tests/xpcshell/head.js:191:5
18:33:26     INFO -      _execute_test@/builds/slave/test/build/tests/xpcshell/head.js:405:5
18:33:26     INFO -      @-e:1:1
18:33:26     INFO -  TEST-INFO | (xpcshell/head.js) | exiting test
Blocks: 1067888
Comment on attachment 8504444 [details] [diff] [review]
bug1082294.patch

Try server results seem to be fine.
Failures on B2G ICS opt/debug were picked from wrong error message. It's a commit log, 'Bug 1082294 - TEST_UNEXPECTED-FAILURE ...'.
Attachment #8504444 - Flags: review?(mak77)
Comment on attachment 8504444 [details] [diff] [review]
bug1082294.patch

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

::: toolkit/components/places/UnifiedComplete.js
@@ +872,5 @@
>    },
>  
>    _matchCurrentSearchEngine: function* () {
> +    if (!Prefs.autofillSearchEngines)
> +      return;

Unfortunately this pref explicitly refers to autoFill, but _matchCurrentSearchEngine is not about autofill, so reusing the same pref would be confusing..

what I noticed is that the first time we call _matchHeuristicFallback we check this.pending && this._enableActions && !hasFirstResult

The second time we call it here http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/UnifiedComplete.js#746 we don't seem to check this._enableActions.

I wonder if checking that would solve the problem.
Attachment #8504444 - Flags: review?(mak77) → review-
Thanks Marco!
Now the failure has gone.

I wanted to create a new function in this patch just returning (this.pending && this._enableActions && !hasResult) but I have no good name for it.
Assignee: nobody → hiikezoe
Attachment #8504444 - Attachment is obsolete: true
Attachment #8505306 - Flags: review?(mak77)
Comment on attachment 8505306 [details] [diff] [review]
Also checking this._enableAction

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

thanks.
Attachment #8505306 - Flags: review?(mak77) → review+
(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)
> I wanted to create a new function in this patch just returning (this.pending
> && this._enableActions && !hasResult) but I have no good name for it.

I don't think it's worth it cause it would end up hiding the logic.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e75bd2a311d6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla36
Please request Aurora approval on this when you get a chance.

Also, in the future, please use commit messages that explain what the patch is actually doing. Would have helped you avoid comment 2 as well.
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
Flags: needinfo?(hiikezoe)
Comment on attachment 8505306 [details] [diff] [review]
Also checking this._enableAction

I am sorry for the commit log.

Approval Request Comment
[Feature/regressing bug #]: bug 1067888
[User impact if declined]: an xpcshell test in mozilla-aurora has been failing on comm-aurora builds.
[Describe test coverage new/current, TBPL]: The failed test on comm-central gets passed.
[Risks and why]: Low risk, autocompletion will work as expected
[String/UUID change made/needed]: None
Flags: needinfo?(hiikezoe)
Attachment #8505306 - Flags: approval-mozilla-aurora?
Attachment #8505306 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.