Add test coverage for the style property of places autocomplete results

RESOLVED FIXED in mozilla36

Status

()

Toolkit
Places
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Unfocused, Assigned: Unfocused)

Tracking

unspecified
mozilla36
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

Test coverage for the style property of places autocomplete items is woefully lacking - given UnifiedComplete, and an upcoming autocomplete controller rewrite, we really need to expand coverage in such areas.
Flags: qe-verify-
Marco: Yet another followup I'll pickup immediately.
Flags: needinfo?(mmucci)
Added to IT 35.1

(In reply to Blair McBride [:Unfocused] from comment #1)
> Marco: Yet another followup I'll pickup immediately.
Iteration: --- → 35.1
Flags: needinfo?(mmucci)

Updated

3 years ago
Flags: firefox-backlog+

Updated

3 years ago
Iteration: 35.1 → 35.2

Updated

3 years ago
Assignee: bmcbride → nobody
Status: ASSIGNED → NEW
Iteration: 35.2 → ---

Comment 3

3 years ago
Unfocused: Would you be willing to mentor a contributor through this, and flesh out the description a bit?
Flags: needinfo?(bmcbride)
Yes indeed!

So what we want is a new test added in toolkit/components/places/test/unifiedcomplete/, which tries a series of queries that will result in different types of results:
* autofill
* history
* bookmark
* tagged bookmark
* bookmark with keyword
* switch-to-tab
* search engine
* normal (but never visited/bookmarked) URL
* matching a previous search via a search engine

And ensures that the .style property on each result is correct.

Looking through _adjustAcItem in autocomplete.xml will likely help with understanding how the .style property (set as "type" in that binding) relates to the different types of results.
Mentor: bmcbride@mozilla.com
Flags: needinfo?(bmcbride)
Want this done ASAP, so taking this for myself.
Assignee: nobody → bmcbride
Mentor: bmcbride@mozilla.com
Status: NEW → ASSIGNED

Updated

3 years ago
Iteration: --- → 36.2
Created attachment 8519808 [details]
MozReview Request: bz://1065191/Unfocused
Attachment #8519808 - Flags: review?(mak77)
/r/359 - Bug 1065191 - Add test coverage for the style property of places autocomplete results.

Pull down this commit:

hg pull review -r e830269b3b1efc655c0317a240551dfd3ef96265
Attachment #8519808 - Flags: review?(mak77) → review?(paolo.mozmail)
/r/359 - Bug 1065191 - Add test coverage for the style property of places autocomplete results.

Pull down this commit:

hg pull review -r e830269b3b1efc655c0317a240551dfd3ef96265

Updated

3 years ago
Iteration: 36.2 → 36.3

Updated

3 years ago
Blocks: 995091

Updated

3 years ago
Attachment #8519808 - Flags: review?(paolo.mozmail) → review+

Comment 9

3 years ago
https://reviewboard.mozilla.org/r/357/#review173

Looks good!

::: toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
(Diff revision 1)
> +          let actualStyle = normalizeStyle(controller.getStyleAt(i));

I'd not use a helper function here (and the name is slightly misleading, because it takes a string and returns an array rather than a normlaized string).

::: toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
(Diff revision 1)
> +            Assert.equal(actualStyle.toString(), style.toString(), "Match should have expected style");

nit: I think xpcshell tests report line numbers of failures, so the description in the third parameter is unnecessary. Same below.
(In reply to :Paolo Amadini from comment #9)
> nit: I think xpcshell tests report line numbers of failures, so the
> description in the third parameter is unnecessary. Same below.

The message is used. eg:

 0:01.37 TEST_STATUS: Thread-1  FAIL Match should have expected style - "action,visiturl" == "action,visiturldfdff"
    d:/mozilla/central/obj-i686-pc-mingw32/_tests/xpcshell/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js:check_autocomplete:177

I lot of xpcshell tests don't provide a message, and it makes it much harder to deal with.
https://hg.mozilla.org/integration/fx-team/rev/fb67e4e32ef2
https://hg.mozilla.org/mozilla-central/rev/fb67e4e32ef2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment on attachment 8519808 [details]
MozReview Request: bz://1065191/Unfocused
Attachment #8519808 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.