Convert remaining places/tests/unifiedcomplete tests to urlbar tests
Categories
(Firefox :: Address Bar, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: bugzilla, Assigned: bugzilla)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Ahead of converting UnifiedComplete to being a modern provider, we should port its remaining tests into Urlbar tests. Porting unifiedcomplete tests has been handled on an as-needed basis for the providers split out from UnifiedComplete. Now that only the core functionality of UnifiedComplete remains, all the remaining tests can be ported. By porting them before porting UnifiedComplete itself, we reduce the risk of regressions and also shrink the size of the patch stack that will eventually port UnifiedComplete.
Assignee | ||
Comment 1•3 years ago
|
||
The last few subtests in test_tags_returnedInSearches.js got substantive changes. This is because urlbar tests reflect the results actually shown in the Urlbar and unifiedcomplete tests just tested what came out of UnifiedComplete. Those last few subtests tested that we show non-matching tags. While UnifiedComplete returns those non-matching tags, UrlbarProviderUnifiedComplete has filtered them out since bug 1522226.
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
Depends on D118635
Assignee | ||
Comment 3•3 years ago
|
||
There are two substantive changes to test_protocol_swap worth pointing out:
- Some subtests now search for <protocol>://sit instead of <protocol>://site. This is because the latter would make the heuristic result the same as the relevant history result and the history result would be deduped. We would thus lose test coverage for that history result.
- Tests that expected allMatches no longer expect uri5. The muxer dedupes https://www. URLs in favour of https:// URLs.
Depends on D118636
Assignee | ||
Comment 4•3 years ago
|
||
Changes to these tests are atomic, so I'm going to post them in batches as I finish them. That spreads out the review work and makes them more manageable. Try for the three patches that I just posted: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bfee83216ca2627c6ddc586b62c7a1445aced71
Assignee | ||
Comment 5•3 years ago
|
||
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 7•3 years ago
|
||
Apologies in advance for this review. It's the test I've had to rewrite the most. This is because the unifiedcomplete tests did not care about sorting, and urlbar tests do. Since this test does some complicated stuff with frecency, many of the expected matches had to be reordered in the test. The old test just listed all the uris in descending order in matches
, paying no mind to frecency. As I've been doing with other tests, I reversed the order which with they are added to history/bookmarks, to reduce the number of changes required in the sets of expected matches.
That yielded this order, in descending order of frecency:
uri11
uri1
uri4
uri6
uri5
uri7
uri8
uri9
uri10
uri12
uri2
uri3
Assignee | ||
Comment 8•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Comment 10•3 years ago
|
||
Assignee | ||
Comment 11•3 years ago
|
||
Updated•3 years ago
|
Comment 12•3 years ago
|
||
bugherder |
Comment 13•3 years ago
|
||
Comment 14•3 years ago
|
||
bugherder |
Comment 15•3 years ago
|
||
Comment 16•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Comment 17•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/78e13cba3313
https://hg.mozilla.org/mozilla-central/rev/b4d32b143c00
Description
•