Closed Bug 1619173 Opened 5 years ago Closed 5 years ago

toolkit/components/search xpcshell test failures

Categories

(MailNews Core :: General, defect)

Product:
MailNews Core
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 75.0

People

(Reporter: benc, Assigned: benc)

References

Details

Attachments

(1 file, 3 obsolete files)

Bug 1619173 - Bypass telemetry product check in search xpcshell tests.
(deleted), text/x-phabricator-request
Details

I think this is a case of firefox-specific telemetry keys not being compiled in to the Thunderbird build.

Assignee: nobody → benc
Summary: test_searchSuggest.js → toolkit/components/search/tests/xpcshell/test_searchSuggest.js failure
Attachment #9130059 - Attachment description: Bug 1619173 - Bypass telemetry product check in test_searchSuggest.js. r=xeonchen → Bug 1619173 - Bypass telemetry product check in search xpcshell tests.
Summary: toolkit/components/search/tests/xpcshell/test_searchSuggest.js failure → toolkit/components/search xpcshell test failures
Attached patch 1619173-fix-search-tests.patch (obsolete) (deleted) — — Splinter Review

Geoff: this is just your patch from the try build. With the telemetry product-check bypass in my M-C patch, this does indeed seems to fix all the search tests except test_sort_orders.js.

Flags: needinfo?(geoff)

test_sort_orders.js might be covered by Bug 1619073, but I'll take a peek tomorrow anyway.

Attached patch 1619173-fix-search-tests.patch (obsolete) (deleted) — — Splinter Review

Let's reduce that to just the necessary bit.

Attachment #9130073 - Attachment is obsolete: true
Flags: needinfo?(geoff)
Comment on attachment 9130088 [details] [diff] [review] 1619173-fix-search-tests.patch Please don't do this. We're working towards removing dependence on that service, and we don't want more things pinging it. Additionally, this will probably be returning the wrong data for Thunderbird. I'll try and have a detailed look at the failures in a bit.
Attachment #9130088 - Flags: review-
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/16bd6dfbf190 Bypass telemetry product check in search xpcshell tests. r=xeonchen,Standard8

The patch I'm attaching to bug 1619073 will fix the test_sort_orders.js failures.

Depends on: 1619073

(In reply to Mark Banner (:standard8) from comment #7)

The patch I'm attaching to bug 1619073 will fix the test_sort_orders.js failures.

Thanks, Mark!
Yes - that patch does seem to do the trick nicely for test_sort_orders.js here.

https://hg.mozilla.org/mozilla-central/rev/16bd6dfbf190

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 75.0
Attached patch 1619173-fix-search-tests-1.diff (obsolete) (deleted) — — Splinter Review

This fixes the remaining failures, which are happening because of a missing pref. Just an empty string is enough to fix the tests but I've disabled the whole thing as well.

Attachment #9130308 - Flags: review?(benc)
Attachment #9130308 - Flags: feedback?(standard8)
Attachment #9130088 - Attachment is obsolete: true
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 9130308 [details] [diff] [review] 1619173-fix-search-tests-1.diff Review of attachment 9130308 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/app/profile/all-thunderbird.js @@ +483,5 @@ > #endif > #endif > +// This two prefs should go away with changes to the search component. > +// No known bug number but it will be a dependency of bug 1542235. > +pref("browser.search.geoSpecificDefaults", false); This is already set to false by default: https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/modules/libpref/init/all.js#4488 and for tests: https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/testing/profiles/xpcshell/user.js#6 So I don't think you need to do that here. @@ +484,5 @@ > #endif > +// This two prefs should go away with changes to the search component. > +// No known bug number but it will be a dependency of bug 1542235. > +pref("browser.search.geoSpecificDefaults", false); > +pref("browser.search.geoSpecificDefaults.url", ""); Ah, bug 1589618 changed the search setup here. Does it fix this if we changed https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/toolkit/components/search/tests/xpcshell/head_search.js#430 to `let originalURL = defaultBranch.getCharPref(PREF_SEARCH_URL, "");` (sorry, I don't have a TB build at the moment).
Attachment #9130308 - Flags: feedback?(standard8)
Comment on attachment 9130308 [details] [diff] [review] 1619173-fix-search-tests-1.diff Yes that works and it's a much tidier solution. Let's do that.
Attachment #9130308 - Attachment is obsolete: true
Attachment #9130308 - Flags: review?(benc)

I can do that a bit later today.

Depends on: 1619587

I believe bug 1619587 has fixed the remaining issues here.

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: