1239810 - test_UpdateUtils_url.js is going to permafail on Android 4.3 debug when Gecko 45 merges to Beta

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Merge day test failures

This has hit multiple pushes, but only on Android 4.3 debug?! I've verified that the opt runs show it running and passing.

https://treeherder.mozilla.org/logviewer.html#?job_id=15459747&repo=try

TEST-UNEXPECTED-FAIL | toolkit/modules/tests/xpcshell/test_UpdateUtils_url.js | test_locale - [test_locale : 200] the url param for %LOCALE% should equal the expected value - "null" == "en-US"

Comment 1

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

Review commit: https://reviewboard.mozilla.org/r/30973/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30973/

Comment 2

[Dave Townsend [:mossop]]

I'm completely confused over how it is passing on the opt build, it should be failing on all android.

Comment 3

[Phil Ringnalda (:philor)]

There's some weirdness around the way the updater is enabled/disabled/meant to be enabled but kinda disabled/meant to be disabled but enabled on Android, which was one of the bustages from the 44 merge to beta, it's possible that didn't get fixed above beta.

Comment 4

[Phil Ringnalda (:philor)]

Bug 1232798, which did intend to fix it above beta, and has forgotten.

Comment 5

[Robert Strong (they/them - no direct email)]

(In reply to Phil Ringnalda (:philor) from comment #3)
> There's some weirdness around the way the updater is enabled/disabled/meant
> to be enabled but kinda disabled/meant to be disabled but enabled on
> Android, which was one of the bustages from the 44 merge to beta, it's
> possible that didn't get fixed above beta.

As far as app update is concerned it no longer is enabled on Android as of Firefox 40 with bug 1152997 fixed. Android inappropriately reused MOZ_UPDATER but app update isn't compiled for Android and there is bug 1152634 to get Android to use a new define so I can remove the one-off code that makes Android not compile app update. The issue that Android has with
toolkit/modules/tests/xpcshell/test_UpdateUtils_url.js

is separate from app update which primarily lives in

toolkit/mozapps/update/
and
toolkit/xre/nsUpdateDriver.cpp

As for test_UpdateUtils_url.js does Android actually use UpdateUtils.jsm? If not, then I'd suggest excluding it on Android and disabling the test on Android.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #5)
> As for test_UpdateUtils_url.js does Android actually use UpdateUtils.jsm? If
> not, then I'd suggest excluding it on Android and disabling the test on
> Android.

In general I'd like all general toolkit modules to be supported and work in all platforms where possible. Specifically here though we have been talking about wanting system add-ons to work for Android and that relies on UpdateUtils.jsm so we want this to work.

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 45+ to keep an eye on this as we have a week left till merge day.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8708112 [details]
MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

Looks good on Try.

Comment 9

[Robert Strong (they/them - no direct email)]

Dave, I'm fine with this patch (it is simple enough afterall) but you should get someone on the Android team to also review it.

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 8708112 [details]
MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

https://reviewboard.mozilla.org/r/30973/#review28175

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8708112 [details]
MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

https://reviewboard.mozilla.org/r/30973/#review28211

wfm.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9004cc522119

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9004cc522119

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this.

Comment 15

[Dave Townsend [:mossop]]

Comment on attachment 8708112 [details]
MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

Approval Request Comment
[Feature/regressing bug #]: System add-ons
[User impact if declined]: Broken system add-on updates
[Describe test coverage new/current, TreeHerder]: Automated tests are catching the failures
[Risks and why]: Low risk, just adds a file to the distribution
[String/UUID change made/needed]: None

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8708112 [details]
MozReview Request: Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r?rstrong

Fix a future permafail, taking it

Comment 17

[Carsten Book [:Tomcat]]

has problems during uplift :

grafting 324416:9004cc522119 "Bug 1239810: UpdateUtils.jsm requires the update.locale file to be shipped regardless of whether the application update is included. r=nalexander,rstrong"
merging mobile/android/installer/package-manifest.in
warning: conflicts while merging mobile/android/installer/package-manifest.in! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

It's a trivial rebase (I've been including it in various Try pushes for awhile now). I'll push it to Aurora in a few.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/57bc24e6baeb