Closed
Bug 741972
Opened 13 years ago
Closed 12 years ago
Test failures on ESR when channel is set to ESR, TEST-UNEXPECTED-FAIL | test_AddonRepository_compatmode.js | compatmode-strict@tests.mozilla.org == compatmode-ignore@tests.mozilla.org and more
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: standard8, Assigned: standard8)
References
Details
(Keywords: regression, Whiteboard: [qa-])
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
Unfocused
:
review+
lsblakk
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
Since bug 734848 landed on ESR, we've been seeing test failures on the Thunderbird boxes. This is because they set the update channel to "esr" for dep builds. The Firefox builders don't do that.
Example failure:
TEST-UNEXPECTED-FAIL | /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_AddonRepository_compatmode.js | compatmode-strict@tests.mozilla.org == compatmode-ignore@tests.mozilla.org - See following stack:
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_throw :: line 453
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: _do_check_eq :: line 547
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_check_eq :: line 568
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_AddonRepository_compatmode.js :: <TOP_LEVEL> :: line 93
JS frame :: resource://gre/modules/AddonRepository.jsm :: <TOP_LEVEL> :: line 860
TEST-UNEXPECTED-FAIL | /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_bug470377_2.js | null != null - See following stack:
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_throw :: line 453
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: _do_check_neq :: line 509
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_check_neq :: line 530
JS frame :: /buildbot/comm-esr10-linux-opt-unittest-xpcshell/build/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_bug470377_2.js :: <TOP_LEVEL> :: line 88
Requesting tracking for ESR as this will show up for the next round of Firefox builds released from ESR channel (and is a perma orange on the Thunderbird boxes).
Patches coming up in a moment.
Assignee | ||
Comment 1•13 years ago
|
||
This patch unifies the various checks in the xpcshell-tests for whether or not we're on the nightly channel into one function. It also adds the esr channel to the list, which is the actual fix for this bug.
I was going to do this as two patches, but then I realised we'd probably have another ESR branch alongside gecko 17. So I think this can just land in mozilla-central and mozilla-esr10 (it can filter through the aurora/beta branches naturally).
Attachment #611897 -
Flags: review?(bmcbride)
Comment 2•13 years ago
|
||
Comment on attachment 611897 [details] [diff] [review]
Unify the check for nightly channel
Review of attachment 611897 [details] [diff] [review]:
-----------------------------------------------------------------
Ugh, that was exceedingly stupid of me. I'd forgotten that I hadn't converted tests to use the new AddonManager.checkCompatibility getter/setter (which isn't on ESR yet anyway, so it doesn't help here).
Anyway, you appear to be missing some tests here:
- browser_globalwarnings.js
- browser_searching.js
- test_bug470377_3.js
- test_bug521905.js
- test_pref_properties.js
- test_updatecheck.js
Attachment #611897 -
Flags: review?(bmcbride) → review-
Updated•13 years ago
|
Assignee | ||
Comment 3•13 years ago
|
||
Ok, given your comments about AddonManager.checkCompatibility I'm currently working on a version of this specifically for trunk which uses that function.
Hence this is a patch for the ESR only.
test_pref_properties.js isn't in esr, so hence I've not updated the check in that file, and will do it in the trunk patch.
Attachment #614756 -
Flags: review?(bmcbride)
Assignee | ||
Updated•13 years ago
|
Attachment #611897 -
Attachment is obsolete: true
Assignee | ||
Comment 4•13 years ago
|
||
I think this is the right thing to do. Will push it to try server.
Comment 5•13 years ago
|
||
Try run for 84b537361ee5 is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=84b537361ee5
Results (out of 16 total builds):
success: 15
warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-84b537361ee5
Comment 6•13 years ago
|
||
Comment on attachment 614756 [details] [diff] [review]
ESR-only fix
Review of attachment 614756 [details] [diff] [review]:
-----------------------------------------------------------------
Great - thankyou!
Attachment #614756 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 614765 [details] [diff] [review]
mozilla-central fix
This passed on try server, the only warning was for 10.7 xpcshell tests builds that are ignored by default, and didn't fail on these specific tests.
Attachment #614765 -
Attachment is patch: true
Attachment #614765 -
Flags: review?(bmcbride)
Assignee | ||
Comment 8•13 years ago
|
||
Comment on attachment 614756 [details] [diff] [review]
ESR-only fix
[Approval Request Comment]
Regression caused by (bug #): General - moving to an ESR channel
User impact if declined: If there is an ESR specific problem with the updater after a release build, this may be hidden until later testing or release.
Testing completed (on m-c, etc.): Run tests locally, and via Thunderbird's try server
Risk to taking this patch (and alternatives if risky): None.
This is a test-only change that fixes xpcshell and browser-chrome update tests to take account of the ESR update channel.
Thunderbird sees these failures all the time, Firefox will see these on release builds if the results are analysed as I believe they should be.
Attachment #614756 -
Flags: approval-mozilla-esr10?
Updated•13 years ago
|
Attachment #614765 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 9•13 years ago
|
||
Checked into inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/993d0e9edd3f
Whiteboard: [inbound]
Target Milestone: --- → mozilla14
Assignee | ||
Comment 10•13 years ago
|
||
Unfortunately I had to back this out due to test failures in the mochitest-other tests (which somehow I'd neglected to run on try server).
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_globalwarnings.js | an unexpected uncaught JS exception reported through window.onerror - ReferenceError: pref is not defined at chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_globalwarnings.js:30
Stack trace:
JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 983
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
Is the main error that's reasonably easy to fix. I need to figure out if the others are follow-ups to this or if they are a separate issue.
Whiteboard: [inbound]
Assignee | ||
Comment 11•13 years ago
|
||
(this doesn't affect the ESR patch, as the ESR one is doing the mochitest-other changes in a simpler way).
Comment 12•13 years ago
|
||
Comment on attachment 614756 [details] [diff] [review]
ESR-only fix
[Triage Comment]
Test only change. Please go ahead and land to ESR.
Attachment #614756 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Assignee | ||
Comment 13•13 years ago
|
||
Checked in to esr:
https://hg.mozilla.org/releases/mozilla-esr10/rev/22a079b539fc
I'll pick up the m-c fix soon.
status-firefox-esr10:
--- → fixed
Comment 15•12 years ago
|
||
Mark, can you please verify this is now working in latest-mozilla-esr10?
Assignee | ||
Comment 16•12 years ago
|
||
Sorry for the delay in getting back to this, this fixes up the previous patch so that it'll pass.
The issues were that browser_globalwarnings.js needed an additional change to use AddonManager.checkCompatibility, and browser_searching.js had a typo in the checkCompatibility function name.
Attachment #614765 -
Attachment is obsolete: true
Attachment #651682 -
Flags: review?(bmcbride)
Comment 17•12 years ago
|
||
Comment on attachment 651682 [details] [diff] [review]
mozilla-central fix v2
Review of attachment 651682 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js
@@ +26,4 @@
>
> var TEST_UNPACKED = false;
>
> +function isNightlyChannel(channel) {
All the uses of this seem to only get the channel so it can be passed to this function. Better to just have this function figure that out.
::: toolkit/mozapps/extensions/test/xpcshell/test_checkcompatibility.js
@@ +163,1 @@
> restartManager("2.1a4");
This doesn't look right - restartManager("2.1a4") changes nsIXULAppInfo.version, so setting AddonManager.checkCompatibility before that uses the old version ("2.2.3"). So this should require using the original hardcoded pref for it to pass.
Attachment #651682 -
Flags: review?(bmcbride) → review-
Assignee | ||
Comment 18•12 years ago
|
||
Addresses the review comments.
Attachment #651682 -
Attachment is obsolete: true
Attachment #651766 -
Flags: review?(bmcbride)
Updated•12 years ago
|
Attachment #651766 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 19•12 years ago
|
||
Target Milestone: mozilla14 → mozilla17
Comment 20•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•