1135242 - Various GMP tests are going to permafail when Gecko 38 merges to Beta

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Attachment: xpcshell failure log

[Tracking Requested - why for this release]: Test permafail when Gecko 38 merges to Beta.

Rumor has it that bug 1089867 is wanted for Fx37 too, so requesting tracking there as well.

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

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: mochitest-bc failure log

Comment 2

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/try/raw-rev/a116b0b86186 is the raw diff for the patch if you want to test locally.

Comment 3

[Stephen A Pohl [:spohl] (OOO until 3/13)]

I'm able to reproduce these failures locally. For some reason, when we run into these failures we're disabling Adobe EME after we enabled it via the pref... Still trying to make sense of it all.

Comment 4

[Stephen A Pohl [:spohl] (OOO until 3/13)]

Attachment: Patch

Ok, looks like the media.eme.enabled pref is set to false by default when we're run as beta. The tests failed to force this pref to true during initialization and hence the tests failed. To be clear: the mistake was in our tests and the client code appears to be working as intended.

Comment 5

[Stephen A Pohl [:spohl] (OOO until 3/13)]

With this patch, all tests succeed with local builds on OSX and Linux.

Comment 6

[Stephen A Pohl [:spohl] (OOO until 3/13)]

Thanks, Georg!

https://hg.mozilla.org/integration/mozilla-inbound/rev/c09d4b852a36

Comment 7

[Stephen A Pohl [:spohl] (OOO until 3/13)]

Comment on attachment 8567438 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Adobe EME
[User impact if declined]: When bug 1089867 merges to beta, GMP tests will start failing. We may therefore not be able to land bug 1089867 for Firefox 37 unless this patch lands at the same time.
[Describe test coverage new/current, TreeHerder]: Local testing showed that all issues discovered by RyanVM are fixed with this patch.
[Risks and why]: None that I'm aware of.
[String/UUID change made/needed]: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c09d4b852a36

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Verified on Try too :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8567438 [details] [diff] [review]
Patch

Test-only changes don't need approval. Personally, I'd just include these fixes in the roll-up patch in the other bug.

Comment 11

[Stephen A Pohl [:spohl] (OOO until 3/13)]

Thanks, Ryan! I've rolled it up into the patch for uplift in bug 1089867, but I'm unable to set the approval flag back to a+. Are you able to do so?

Comment 12

[Ryan VanderMeulen [:RyanVM]]

In this case, I think it's fine to just use the original approval since the only change was the test-only addition.