Closed Bug 741972 Opened 8 years ago Closed 7 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)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17
Tracking Status
firefox-esr10 13+ fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: regression, Whiteboard: [qa-])

Attachments

(2 files, 3 obsolete files)

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.
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 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-
Attached patch ESR-only fixSplinter Review
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)
Attachment #611897 - Attachment is obsolete: true
Attached patch mozilla-central fix (obsolete) — Splinter Review
I think this is the right thing to do. Will push it to try server.
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 on attachment 614756 [details] [diff] [review]
ESR-only fix

Review of attachment 614756 [details] [diff] [review]:
-----------------------------------------------------------------

Great - thankyou!
Attachment #614756 - Flags: review?(bmcbride) → review+
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)
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?
Attachment #614765 - Flags: review?(bmcbride) → review+
Checked into inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/993d0e9edd3f
Whiteboard: [inbound]
Target Milestone: --- → mozilla14
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]
(this doesn't affect the ESR patch, as the ESR one is doing the mochitest-other changes in a simpler way).
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+
Checked in to esr:

https://hg.mozilla.org/releases/mozilla-esr10/rev/22a079b539fc

I'll pick up the m-c fix soon.
Mark: Any progress here?
Status: NEW → ASSIGNED
Mark, can you please verify this is now working in latest-mozilla-esr10?
Whiteboard: [qa-]
Attached patch mozilla-central fix v2 (obsolete) — Splinter Review
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 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-
Addresses the review comments.
Attachment #651682 - Attachment is obsolete: true
Attachment #651766 - Flags: review?(bmcbride)
Attachment #651766 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b0193cd6bf9
Target Milestone: mozilla14 → mozilla17
https://hg.mozilla.org/mozilla-central/rev/6b0193cd6bf9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.