Closed Bug 1276720 Opened 4 years ago Closed 4 years ago

browser chrome tests fail on ESR45 due to unexpected Firefox version

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- fixed
firefox-esr45 --- fixed

People

(Reporter: rail, Assigned: rail)

Details

Attachments

(1 file)

See https://treeherder.mozilla.org/logviewer.html#?job_id=45883&repo=mozilla-esr45#L4264

19:02:55 INFO - 829 INFO TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_bug590347.js | Warning message should be correct - Got Test add-on is incompatible with Firefox 45.1.2., expected Test add-on is known to cause security or stability issues. 

I landed https://hg.mozilla.org/releases/mozilla-esr45/rev/6f20e58674c6 to start using real version numbers on ESR, so we can promote CI builds to release builds. This also matches what we had in release builds before: https://hg.mozilla.org/releases/mozilla-esr45/rev/8b70c093909e

Sounds like this breaks some assumptions in the tests (they expect "esrpre" or something?)
Dave, maybe you have ideas how to fix this?
Flags: needinfo?(dtownsend)
Assignee: nobody → rail
To clarify the issue: in bug 1271832 we switched to mozharness based builds and in bug 1253757 we changed the update channel to "esr" for all opt CI builds.

PREF_CHECK_COMPATIBILITY set in https://hg.mozilla.org/releases/mozilla-esr45/file/tip/toolkit/mozapps/extensions/test/browser/head.js#l57 and uses Firefox version in the key name:

PREF_CHECK_COMPATIBILITY = "extensions.checkCompatibility." + version;

Fore release channel (aurora/beta/release) version value is based on app version, otherwise it's set to "nighlty": https://hg.mozilla.org/releases/mozilla-esr45/file/tip/toolkit/mozapps/extensions/test/browser/head.js#l53

The test uses PREF_CHECK_COMPATIBILITY in https://hg.mozilla.org/releases/mozilla-esr45/file/tip/toolkit/mozapps/extensions/test/browser/browser_bug590347.js#l98, but fails to set it properly if the update channel is set to "esr".
Comment on attachment 8758017 [details]
MozReview Request: Bug 1276720 - Add esr to checkCompatibility list; r=Gijs

Looks trivial enough, r+ for landing, but leaving existing request open so a proper peer can give it a once-over.
Attachment #8758017 - Flags: review+
Comment on attachment 8758017 [details]
MozReview Request: Bug 1276720 - Add esr to checkCompatibility list; r=Gijs

https://reviewboard.mozilla.org/r/56378/#review52958

rs=me for the trivial fix here, please also land on m-c so we don't break this next ESR.


However... have we used this update channel in the past? Like for esr38? If not, I wonder if other code expects it...
Attachment #8758017 - Flags: review?(gijskruitbosch+bugs) → review+
https://reviewboard.mozilla.org/r/56378/#review52958

not on esr38, this is something new I landed yesterday (the channel switch from default to esr)
Comment on attachment 8758017 [details]
MozReview Request: Bug 1276720 - Add esr to checkCompatibility list; r=Gijs

This is to fix some browser chrome tests, behaving differently after we started using "esr" update channel in CI builds. The fix is trivial and the risk is very low.

User impact if declined: None
Fix Landed on Version: 49
Risk to taking this patch (and alternatives if risky): this is a test-only patch, the risk is minimal
String or UUID changes made by this patch: None
Attachment #8758017 - Flags: approval-mozilla-esr45?
NI is not required anymore I think.
Flags: needinfo?(dtownsend)
https://hg.mozilla.org/mozilla-central/rev/09d0d424f914
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8758017 [details]
MozReview Request: Bug 1276720 - Add esr to checkCompatibility list; r=Gijs

Test only fix, ok to uplift to esr45
Attachment #8758017 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
Comment on attachment 8758017 [details]
MozReview Request: Bug 1276720 - Add esr to checkCompatibility list; r=Gijs

https://hg.mozilla.org/releases/mozilla-esr45/rev/4f4d27edc880
You need to log in before you can comment on or make changes to this bug.