Closed Bug 1232798 Opened 9 years ago Closed 7 years ago

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"

Categories

(Release Engineering :: General, defect)

Product:
Release Engineering
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: nalexander, Unassigned)

References

Details

Bug 1234268 has partial fixes for beta CI Android bustage, but there appear to be legitimate xpcshell test failures. See https://treeherder.mozilla.org/logviewer.html#?job_id=684733&repo=mozilla-beta and others.
Cracking open some package omnijars shows there's no update.locale file in root of the omni.ja in the 44 beta builds, while there is in the 43 beta builds. I inspected: 43 beta - http://archive.mozilla.org/pub/mobile/releases/43.0b8/android-api-11/en-US/fennec-43.0b8.en-US.android-arm.apk 44 beta - http://archive.mozilla.org/pub/mobile/tinderbox-builds/mozilla-beta-android-x86/1450155073/fennec-44.0.en-US.android-i386.apk Digging in to understand what's relevant.
Code inspection doesn't reveal something obvious here. In fact, I'm surprised that this worked in 43 beta! For reasons I don't understand, there's a difference between release/beta and aurora/nightly configs: MOZ_UPDATER= https://dxr.mozilla.org/mozilla-central/source/mobile/android/branding/beta/configure.sh#7 vs. MOZ_UPDATER=1 https://dxr.mozilla.org/mozilla-central/source/mobile/android/branding/aurora/configure.sh#7 I imagine mfinkle (the committer) wanted to disable the updater, but didn't actually achieve that. Since this is all based on #ifdef, these *both* should enable the updater; but subsequent moz.build things differentiate. See https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/moz.build#114; I think the CONFIG['MOZ_UPDATER'] value will be an empty string, which evaluates to False in Python. All this suggests this should have been disabled earlier, but something has changed to actually disable it. In any case, I'm re-enabling now. (This matches expected behaviour: see https://bugzil.la/1192279#c4.)
http://archive.mozilla.org/pub/mobile/tinderbox-builds/mozilla-beta-android-x86/1450217938/fennec-44.0.en-US.android-i386.apk has an update.locale in the root of the omni.ja, suggesting we'll get green tests. Let's see!
mfinkle: so the very first commits to mobile/android/branding had the MOZ_UPDATER= and MOZ_UPDATER=1 lines in https://bugzilla.mozilla.org/show_bug.cgi?id=1232798#c3. Did you intend to disable the updater? (The code does not, but it's an easy mistake to make.) I can't explain what changed between Aurora and Beta, but something did. I "fixed" it by making all the lines be MOZ_UPDATER=1 (so clearly enabled on all channels); see https://bugzilla.mozilla.org/show_bug.cgi?id=1232798#c2. I think we should land this on Aurora and Nightly. Do you concur?
Flags: needinfo?(mark.finkle)
Yes. the initial intent was to disable the updater on beta and release. I remember being bitten by MOZ_UPDATER=0, but I could have sworn MOZ_UPDATER= was the "correct" way to disable for #ifdefs.
Flags: needinfo?(mark.finkle)
Regarding MOZ_UPDATER, bug 1152634 was filed to get Android to use its own define instead of repurposing MOZ_UPDATER so I can get rid of one off code to prevent Android from compiling application update which that define was created for.
Component: General Automation → General
I think this bug outlived its usefulness. Closing this, feel free to reopen if I'm wrong.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.