Port bug 1352539: to TB: Build error: Deleting file '.deps/generated_en-US/list.json' and 10 Xpcshell test failures

RESOLVED FIXED in Thunderbird 62.0

Status

defect
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: jorgk, Assigned: mkaply)

Tracking

unspecified
Thunderbird 62.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Thunderbird-testfailure: X all])

Attachments

(4 attachments, 2 obsolete attachments)

https://taskcluster-artifacts.net/KN-Qc_TuTX2pB73NxKKUJQ/0/public/logs/live_backing.log

[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-thunderbird/comm/mail/locales'
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: *** Deleting file '.deps/generated_en-US/list.json'
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-thunderbird/comm/mail/locales'
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  /builds/worker/workspace/build/src/obj-thunderbird/_virtualenvs/init/bin/python -m mozbuild.action.generate_searchjson /builds/worker/workspace/build/src/comm/mail/locales/search/list.json en-US .deps/generated_en-US/list.json
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  Error: Missing default searchDefault in list.json
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  Makefile:88: recipe for target '.deps/generated_en-US/list.json' failed
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: *** [.deps/generated_en-US/list.json] Error 1
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-thunderbird/comm/mail/locales'
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2018-05-09T09:44:50.364Z] 09:44:50     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-thunderbird/comm/mail/locales'

M-C last good: 7c83ceac4be6d055bebd870a82b78b76de
M-C first bad: 0cd106a2eb78aa04fd481785257e6f4f9b
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7c83ceac4be6d055bebd870a82b78b76de&tochange=0cd106a2eb78aa04fd481785257e6f4f9b
This could be from bug 1352539.
Looks like bug 1352539:
https://hg.mozilla.org/mozilla-central/rev/f8a023df39e2

Florian, Michael, what do we need to do here, all our builds are busted :-(
Blocks: 1352539
Flags: needinfo?(old-mozilla)
Flags: needinfo?(old-mozilla) → needinfo?(mozilla)
Oh, at times it helps to read the message ;-)

Missing default searchDefault in list.json

Could be this:
https://hg.mozilla.org/mozilla-central/rev/f8a023df39e2#l3.6
+    "searchDefault": "Google",
Missed one line.
Attachment #8974339 - Attachment is obsolete: true
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7eb9d52466f3
Port bug 1352539:  Move default search engine to list.json (add searchDefault). rs=bustage-fix
OK, this gives us a working build, but we get heaps of test failures as can me seen on the try runs:

TEST-UNEXPECTED-FAIL | toolkit/components/places/tests/unifiedcomplete/test_PlacesSearchAutocompleteProvider.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_selectedEngine.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_currentEngine_fallback.js | xpcshell return code: 0
TypeError: Services.search.currentEngine is null at /Users/cltbld/tasks/task_1525863775/build/tests/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_currentEngine_fallback.js:22
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_geodefaults.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_geodefaults.js | no_request_if_prefed_off - [no_request_if_prefed_off : 74] "Bing" == "bing"
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_addEngineWithDetails.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_addEngineWithDetailsObject.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_searchReset.js | xpcshell return code: 0

Michael, what are we missing?
Whiteboard: [Thunderbird-testfailure: X all]
Attachment #8974326 - Attachment is obsolete: true
Summary: Build error: Deleting file '.deps/generated_en-US/list.json' → Port bug 1352539: to TB: Build error: Deleting file '.deps/generated_en-US/list.json' and 10 Xpcshell test failures
Comment on attachment 8974342 [details] [diff] [review]
1460232-searchDefault.patch (v3)

Also, what do these lines do:
https://hg.mozilla.org/mozilla-central/rev/f8a023df39e2#l3.25
+      },
+      "KZ": {
+        "searchDefault": "Яндекс"
+      },
Do we need to add something like this as well?
Attachment #8974342 - Flags: review?(mozilla)
Oh, I see, you added |"searchDefault": "Google"| with an upper-case "Google". Let me try with an upper-case "Bing". I've also added default Google where there wasn't a "bing" in the list:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=47364f20f5815d227e8651f664684c2ccb297592
OK, with the follow-up we're down to two failures:
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_selectedEngine.js | xpcshell return code: 0

I'll take a look locally.
Attachment #8974380 - Flags: review?(mozilla)
Comment on attachment 8974342 [details] [diff] [review]
1460232-searchDefault.patch (v3)

Sorry, completely forgot about Thunderbird.

This is correct with the "Bing" you did in the other patch.
Flags: needinfo?(mozilla)
Attachment #8974342 - Flags: review?(mozilla) → review+
Attachment #8974380 - Flags: review?(mozilla) → review+
OK, thanks, but with those two patches applied, we still have two failures:
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_selectedEngine.js | xpcshell return code: 0

Running the first one locally gives a bunch of output and ends with:

 0:01.33 INFO "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "US"" {file: "c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/toolkit/components/search/tests/xpcshell/head_search.js" line: 249}]"

That code is:
  if (isUS && ("searchDefault" in searchSettings.US)) {
    defaultEngineName = searchSettings.US.searchDefault;
  }
  return defaultEngineName;

So somehow, searchSettings, which comes from searchSettings = parseJsonFromStream(chan.open2()); doesn't have a US property.

The second test test_selectedEngine.js fails at
 0:01.41 FAIL test_defaultEngine - [test_defaultEngine : 13] "Bing" == "undefined"
c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/toolkit/components/search/tests/xpcshell/test_selectedEngine.js:test_defaultEngine:13

So that looks like some inconsistency in the JSON file.
Flags: needinfo?(mozilla)
It shouldn't need a US property. We explicitly check for that. 

I'll debug this and get you a fix.
Flags: needinfo?(mozilla)
I just checked out thunderbird and you didn't apply the second patch. searchDefault is "bing"
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/16c11d312c0a
Follow-up: Fix spelling of default search engine and provide localised default where global default is not available. r=mkaply DONTBUILD
I've landed the second patch now so we're all on the same page.

Please run the failing tests:
mach xpcshell-test toolkit/components/search/tests/xpcshell/test_selectedEngine.js
mach xpcshell-test toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js

I noticed this:
    "ja": {
      "default": {
        "searchDefault": "Google",
        "visibleDefaultEngines": [
          "google-jp", "yahoo-jp", "amazon-jp", "rakuten", "yahoo-jp-auctions", "oshiete-goo", "twitter-ja", "wikipedia-ja"
        ]
      }
    },

So the default isn't in the list, and FF has "google" in the list instead of "google-jp". But using "google-jp" or removing "ja" entirely doesn't fix the test failure either.
google-jp still has an internal name of Google, so that's fine:

https://dxr.mozilla.org/l10n-central/source/ja-JP-mac/mail/searchplugins/google-jp.xml

I'm building Thunderbird now and will be able to take a look in a few minutes.
"searchDefault" uses the ShortName element, which is "Google" for google-jp.xml
https://hg.mozilla.org/l10n-central/ja/file/tip/mail/searchplugins/google-jp.xml#l6

The same is true for search.order. On the other hand, visibleDefaultEngines uses file names, minus the extension (".xml").

Hopefully we can get to some uniformity once everything is centralized. Historically, they were in two different places (list.txt, region.properties). Not that it would make things less confusing.
So the issue is that you don't have US at all which I didn't expect.

Fixing the test is the right thing. I'll submit a formal mozreview in a bit.
Thanks, did you have a chance to see why
mach xpcshell-test toolkit/components/search/tests/xpcshell/test_selectedEngine.js
fails. Or any hint how to debug this? More details in comment #13.
Running
  mach xpcshell-test toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js
with the patch applied gives a different failure:

 0:01.44 pid:9496 *** Search: NOTIFY: Engine: "Amazon.com"; Verb: "engine-current"
 0:01.44 pid:9496 [9496, LoadRoots] WARNING: This method is lossy. Use GetCanonicalPath !: file c:/mozilla-source/comm-central/xpcom/io/nsLocalFileWin.cpp, line 3378
 0:01.45 FAIL test_searchDefaultEngineUS - [test_searchDefaultEngineUS : 22] expected US default search engine - "Amazon.com" == "undefined"
c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js:test_searchDefaultEngineUS:22

which appears to be similar to the failure of test_selectedEngine.js:
toolkit/components/search/tests/xpcshell/test_selectedEngine.js
  FAIL test_defaultEngine - [test_defaultEngine : 13] "Bing" == "undefined"
c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/toolkit/components/search/tests/xpcshell/test_selectedEngine.js:test_defaultEngine:13
  FAIL toolkit/components/search/tests/xpcshell/test_selectedEngine.js - xpcshell return code: 0

Can you spot anything wrong in our list.json?
Severity: blocker → normal
With my change to head_search.js applied, both tests pass.
Is the empty regionOverrides needed in list.json? As far as I see, the code always check if that key is available, doesn't assume it is.
Hmm, not for me, but let me rebuild with M-C at tip. My build doesn't contain the last merge, but that doesn't have anything search related.
regionOverrides is read at build time, so it wouldn't cause a test failure.
(In reply to Mike Kaply [:mkaply] from comment #26)
> regionOverrides is read at build time, so it wouldn't cause a test failure.

Sorry, there was a question unrelated to the test failure. Is it needed at all?
No, regionOverrides is optional.
Sorry, both tests still fail for me locally. Here's a try build with your change included:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9c108fe8d5645e7b28be62986fd02c0d93e3f268

The trick is that I pointed the M-C directory to use try to where I've pushed this:
https://hg.mozilla.org/try/rev/adc646ffd176cf31f68a9952057b2d314af11654
Without your patch Aceman did a try run here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6672837a190a805ff7ea19706a76aa5f1ff37c48
On Mac he got two failures:
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_selectedEngine.js | xpcshell return code: 0
and on Linux only one:
TEST-UNEXPECTED-FAIL | toolkit/components/search/tests/xpcshell/test_list_json_searchdefault.js | xpcshell return code: 0

I guess that's the one you've fixed. Which platform are you on?
I'm on Mac. Nothing about this should be platform specific though. Waiting for your try run so I can see.
Check
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=28fb09316488db06ca1f606429a75c29d0bd856d
and click the X's to see that the platforms are different.
According to
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9c108fe8d5645e7b28be62986fd02c0d93e3f268
your fix worked. So I don't know what's going on locally.

Maybe we just land that and see.
Comment on attachment 8974523 [details]
Bug 1460232 - Update search test to handle missing region.

https://reviewboard.mozilla.org/r/242862/#review248760
Attachment #8974523 - Flags: review?(adw) → review+
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/117b148d7a02
Update search test to handle missing region. r=adw
Is this all working now?
Yes, thanks for your help. I'm not sure why I had local failures even with your patch. Does the test depend on the locale of the machine it runs on? That would explain why it passes on the en-US servers and not locally.
Status: NEW → RESOLVED
Closed: Last year
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 62.0
> Yes, thanks for your help. I'm not sure why I had local failures even with your patch. Does the test depend on the locale of the machine it runs on? That would explain why it passes on the en-US servers and not locally.

It shouldn't, but I guess that's possible.
Assignee: nobody → mozilla
You need to log in before you can comment on or make changes to this bug.