Closed
Bug 1065191
Opened 10 years ago
Closed 10 years ago
Add test coverage for the style property of places autocomplete results
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
People
(Reporter: Unfocused, Assigned: Unfocused)
References
Details
Attachments
(1 obsolete file)
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-
Assignee | ||
Comment 1•10 years ago
|
||
Marco: Yet another followup I'll pickup immediately.
Flags: needinfo?(mmucci)
Comment 2•10 years ago
|
||
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•10 years ago
|
Flags: firefox-backlog+
Updated•10 years ago
|
Iteration: 35.1 → 35.2
Updated•10 years ago
|
Assignee: bmcbride → nobody
Status: ASSIGNED → NEW
Iteration: 35.2 → ---
Comment 3•10 years ago
|
||
Unfocused: Would you be willing to mentor a contributor through this, and flesh out the description a bit?
Flags: needinfo?(bmcbride)
Assignee | ||
Comment 4•10 years ago
|
||
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
Flags: needinfo?(bmcbride)
Assignee | ||
Comment 5•10 years ago
|
||
Want this done ASAP, so taking this for myself.
Assignee: nobody → bmcbride
Mentor: bmcbride
Status: NEW → ASSIGNED
Updated•10 years ago
|
Iteration: --- → 36.2
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8519808 -
Flags: review?(mak77)
Assignee | ||
Comment 7•10 years ago
|
||
/r/359 - Bug 1065191 - Add test coverage for the style property of places autocomplete results. Pull down this commit: hg pull review -r e830269b3b1efc655c0317a240551dfd3ef96265
Assignee | ||
Updated•10 years ago
|
Attachment #8519808 -
Flags: review?(mak77) → review?(paolo.mozmail)
Assignee | ||
Comment 8•10 years ago
|
||
/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•10 years ago
|
Iteration: 36.2 → 36.3
Updated•10 years ago
|
Blocks: UnifiedComplete
Updated•10 years ago
|
Attachment #8519808 -
Flags: review?(paolo.mozmail) → review+
Comment 9•10 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.
Assignee | ||
Comment 10•10 years ago
|
||
(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.
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/fb67e4e32ef2
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fb67e4e32ef2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #8519808 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•