Closed
Bug 1402471
Opened 7 years ago
Closed 7 years ago
Massive Xpcshell test failure for tests from toolkit/mozapps/extensions/test/xpcshell on comm-beta
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(thunderbird57 fixed, thunderbird58 fixed)
RESOLVED
FIXED
Thunderbird 58.0
People
(Reporter: jorgk-bmo, Unassigned)
References
Details
Attachments
(1 file, 2 obsolete files)
59 bytes,
text/x-review-board-request
|
tomprince
:
review+
jorgk-bmo
:
approval-comm-beta+
|
Details |
TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_AddonRepository_cache.js | xpcshell return code: 0 [log…] TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_bug299716.js | xpcshell return code: 0 [log…] TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js | xpcshell return code: 0 [log…] and many more. This started after the latest merge. Only on comm-beta. Kris, any hints for us?
Flags: needinfo?(kmaglione+bmo)
Reporter | ||
Comment 1•7 years ago
|
||
Looking for example at test_install_strictcompat.js, that fails on line 74: 11:50:18 INFO - TEST-PASS | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js | run_test_1/< - [run_test_1/< : 73] 737 == 737 11:50:18 WARNING - TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js | run_test_1/< - [run_test_1/< : 74] false == true do_check_true(install.addon.hasResource("install.rdf")); do_check_eq(install.addon.install, install); 73 do_check_eq(install.addon.size, ADDON1_SIZE); 74 do_check_true(hasFlag(install.addon.operationsRequiringRestart, AddonManager.OP_NEEDS_RESTART_INSTALL));
Comment 2•7 years ago
|
||
Sorry, I don't know. My best guess is that it has something to do with the toolkit code that disables legacy extension support on release branches. I don't know how that flag affects Thunderbird, but in Firefox we get around it for tests on release branches by checking the Cu.isInAutomation flag.
Flags: needinfo?(kmaglione+bmo)
Reporter | ||
Comment 3•7 years ago
|
||
Or taking test_upgrade_strictcompat.js, fails at line 112: 11:50:44 INFO - TEST-PASS | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_upgrade_strictcompat.js | run_test_1/< - [run_test_1/< : 111] {} != null 11:50:44 WARNING - TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_upgrade_strictcompat.js | run_test_1/< - [run_test_1/< : 112] false == true AddonManager.getAddonsByIDs(["addon1@tests.mozilla.org", "addon2@tests.mozilla.org", "addon3@tests.mozilla.org", "addon4@tests.mozilla.org"], function([a1, a2, a3, a4]) { 111 do_check_neq(a1, null); 112 do_check_true(isExtensionInAddonsList(profileDir, a1.id));
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #2) > Sorry, I don't know. My best guess is that it has something to do with the > toolkit code that disables legacy extension support on release branches. I > don't know how that flag affects Thunderbird, but in Firefox we get around > it for tests on release branches by checking the Cu.isInAutomation flag. Thanks, Kris, I'm not sure what to make of this. Cu.isInAutomation is checked in toolkit http://searchfox.org/mozilla-central/search?q=isInAutomation&case=false®exp=false&path= and the failing tests are from toolkit. So I don't know what else we should do in TB. Maybe Tom can take a look.
Flags: needinfo?(mozilla)
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/logviewer.html#?job_id=132740257&repo=comm-beta&lineNumber=14872 looks like it might be the proximate cause.
Reporter | ||
Comment 6•7 years ago
|
||
You mean: 12:14:21 INFO - PID 23428 | JavaScript strict warning: resource://testing-common/httpd.js, line 800: ReferenceError: reference to undefined property "_stopCallback" 12:14:21 INFO - PID 23428 | !!! error running onStopped callback: TypeError: callback is not a function 12:14:21 INFO - "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "_stopCallback"" {file: "resource://testing-common/httpd.js" line: 800}]" 12:14:21 INFO - <<<<<<< This one? https://dxr.mozilla.org/comm-central/source/mozilla/services/sync/tps/extensions/mozmill/resource/stdlib/httpd.js#800 And why would that only fail on beta?
Comment 7•7 years ago
|
||
(In reply to Tom Prince [:tomprince] from comment #5) > https://treeherder.mozilla.org/logviewer.html#?job_id=132740257&repo=comm- > beta&lineNumber=14872 looks like it might be the proximate cause. Unlikely. That's pretty normal. (In reply to Jorg K (GMT+2) from comment #4) > Thanks, Kris, I'm not sure what to make of this. Cu.isInAutomation is > checked in toolkit > http://searchfox.org/mozilla-central/ > search?q=isInAutomation&case=false®exp=false&path= > and the failing tests are from toolkit. So I don't know what else we should > do in TB. The check is, but the ways that property winds up getting set are complicated. But it's possible the failure is just related to different legacy/unsigned extension configurations for Thunderbird release builds.
Comment 8•7 years ago
|
||
I did a similar patch for SeaMonkey in bug 1404638. Not sure if I should ask for review or if you would like the check it out first. Setting the option locally in the mozconfig works fine.
Reporter | ||
Comment 9•7 years ago
|
||
Comment on attachment 8914013 [details] [diff] [review] 1402471-legacyenable.patch Review of attachment 8914013 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, FRG. I'll let Tom look at this. Tom, SM deployed a similar patch successfully. ::: mail/config/mozconfigs/linux32/l10n-mozconfig @@ +1,2 @@ > . $topsrcdir/build/unix/mozconfig.linux32 > +. "$topsrcdir/mail/config/mozconfigs/common" I'm confused, what should have quotes and what shouldn't?
Attachment #8914013 -
Flags: review?(mozilla)
Comment 10•7 years ago
|
||
Comment on attachment 8914013 [details] [diff] [review] 1402471-legacyenable.patch Review of attachment 8914013 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. r+ after renaming the common file, with a green build. (There are potential ordering effects from changing the order, but I don't think they apply to any of the changes here). ::: mail/config/mozconfigs/common @@ +1,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +# This file is included by all mail mozconfigs I think we should put this at .../common/base or something. Down the line, it would make sense to abstract out the commonalities in each of ../nightly, ../release ../debug and ../l10n-mozconfig, and those probably all want to live in a ../common/ directory. ::: mail/config/mozconfigs/linux32/debug @@ +7,5 @@ > ac_add_options --disable-webrender > > ac_add_options --enable-application=mail > ac_add_options --enable-debug > ac_add_options --enable-calendar Probably this stanza (and several others too) could be moved into the common file, but I can do that in a follow up. ::: mail/config/mozconfigs/linux32/l10n-mozconfig @@ +1,2 @@ > . $topsrcdir/build/unix/mozconfig.linux32 > +. "$topsrcdir/mail/config/mozconfigs/common" Probably ~everything should be quoted.
Attachment #8914013 -
Flags: review?(mozilla) → review+
Updated•7 years ago
|
Flags: needinfo?(mozilla)
Reporter | ||
Comment 11•7 years ago
|
||
Renamed the file to common/base and added quotes everywhere. Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c37b3ce6527f8a1d27ef46c102166827eab296da Beta uplift if it works ;-)
Attachment #8916315 -
Flags: review+
Attachment #8916315 -
Flags: approval-comm-beta+
Reporter | ||
Comment 12•7 years ago
|
||
It didn't work, I can't tell what's gone wrong here: Error loading mozconfig: /builds/slave/tb-try-c-cen-l64-d-00000000000/build/.mozconfig Tom, can you please take a look.
Flags: needinfo?(mozilla)
Comment 13•7 years ago
|
||
It looks like when `mach` is invoked from a comm-central objdir, it evaluates mozconfig with $topsrcdir pointing at the mozilla-central directory, which causes the include to point to a non-existent file. This doesn't matter if the includes point to files shared between c-c and m-c (in build/ for example), since the results of reading the mozconfig are mostly ignored.
Reporter | ||
Comment 14•7 years ago
|
||
Thanks for the answer which I sadly don't understand. So what needs to change in the patch? SM landed their version here https://hg.mozilla.org/comm-central/rev/2f034b0789b5 https://hg.mozilla.org/comm-central/rev/c6fafa3db46a and it must be working for them.
Comment 15•7 years ago
|
||
That wasn't a complete answer, just a summary of my debugging I had managed to do. The patch is probably correct, but tickles a bug in how mach finds the topdir/processes mozconfig. SM probably doesn't tickle the bug because their build automation is different. I'll have a look at this tomorrow, but it will probably end up duplicating the code, unfortunately.
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8916315 -
Attachment is obsolete: true
Attachment #8916315 -
Flags: approval-comm-beta+
Reporter | ||
Comment 17•7 years ago
|
||
Comment on attachment 8914013 [details] [diff] [review] 1402471-legacyenable.patch Thanks, FRG, but it doesn't work for us.
Attachment #8914013 -
Attachment is obsolete: true
Flags: needinfo?(mozilla)
Comment 18•7 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #17) > Thanks, FRG, but it doesn't work for us. To be clear, the idea works, but due to unfortunate interaction between mach and mozconfig, we can't have mozconfig include files that don't exist in both c-c and m-c. This is because mach tries to load mozconfig's relative to m-c when run from objdir, which our build system currently does.
Comment 19•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=11173b757817aca465fac0dbbe33755fc7e95331 https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=66aa85d13a770d329a283b3e8b80607095256821
Comment 20•7 years ago
|
||
Comment on attachment 8917131 [details] Bug 1402471 - Enable legacy extensions in Thunderbird configs; Jorg, if the try runs fix enough of the test failures to your satisfaction, this can be pushed to comm-central and comm-beta.
Attachment #8917131 -
Flags: review?(mozilla) → review+
Reporter | ||
Comment 21•7 years ago
|
||
Hmm, how do you do that beta try magic? Looks like Xpcshell comes back to life with that patch which is good. You can also see that Mac builds on beta are broken, so simply uplift bug 1400533 comment #13?
Comment 22•7 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/e8fac029f7e4 Enable legacy extensions in Thunderbird configs; r=tomprince
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•7 years ago
|
||
I've landed that since it won't do damage on C-C and seems to fix things on C-B. As soon as I hear about the Mac build bug 1400533, I'll do a beta uplift round.
Target Milestone: --- → Thunderbird 58.0
Reporter | ||
Updated•7 years ago
|
Attachment #8917131 -
Flags: approval-comm-beta+
Comment 24•7 years ago
|
||
I rebased the patch onto comm-beta, and then used https://hg.mozilla.org/try-comm-central/rev/66aa85d13a770d329a283b3e8b80607095256821 (which is a bit of a hack) to ensure that the correct repository was checked out. I've filed Bug 1408204 to fix this in a more principled way (which has a patch attached that I am testing in https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bada39915381e77479cafbe95f90862639c7f182&selectedJob=136632647)
Reporter | ||
Comment 25•7 years ago
|
||
Beta (TB 57): https://hg.mozilla.org/releases/comm-beta/rev/ac60b8bf96cf9278d8069028cf7fbe619952928b
status-thunderbird57:
--- → fixed
status-thunderbird58:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•