Closed Bug 1259249 Opened 4 years ago Closed 4 years ago

EME bandwidth limiting tests need to verify that the GMP plugin is installed like default EME tests do.

Categories

(Testing Graveyard :: external-media-tests, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: sydpolk, Assigned: sydpolk)

Details

Attachments

(1 file)

The network bandwidth limiting tests just run the netflix urls without verifying any preferences or verifying the GMP plugin.
EME/GMP system. Refactoring the EME tests so that they both call
this. r?SingingTree, r?maja_zf

Review commit: https://reviewboard.mozilla.org/r/42501/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42501/
Attachment #8734872 - Attachment description: MozReview Request: Bug 1259249 Make sure that EME network bandwidth limiting tests verify the → MozReview Request: Bug 1259249 Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf
Attachment #8734872 - Flags: review?(mjzffr)
Attachment #8734872 - Flags: review?(bvandyk)
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42501/diff/1-2/
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42501/#review39165

::: dom/media/test/external/external_media_harness/testcase.py:197
(Diff revision 2)
> +);
> +"""
> +
> +
> +
> +class ManageEMESetup(object):

Two things.

* This is shaping up to be a mixin, so let's call it something like EMESetupMixin
* If it's designed as a mixin for MediaTestCase, all those class methods should really be instance methods, because then we don't have to pass around `marionette`, `prefs` or `logger` -- they will be available to TestEMEPlayback (which uses the mixin) via `self`. (As you say, the mixin is not meant to be instantiated on its own anyway.)
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42501/diff/2-3/
Attachment #8734872 - Flags: review?(mjzffr)
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42501/#review39525

r+wc

::: dom/media/test/external/external_media_harness/testcase.py:219
(Diff revisions 2 - 3)
> +    reset to the current version, and all of the preferences will be
> +    verified.

I think this docstring is a little misleading: in order to reset the GMP plugin, one must call the `check_eme_system` method. Inheriting alone is not enough.
Attachment #8734872 - Flags: review?(mjzffr) → review+
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42501/diff/3-4/
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42501/#review39611

::: dom/media/test/external/external_media_tests/playback/test_eme_playback.py:5
(Diff revision 3)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  import re

I think we can remove this import since the re requiring functionality has been moved.
Attachment #8734872 - Flags: review?(bvandyk)
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42501/diff/4-5/
Attachment #8734872 - Flags: review?(bvandyk)
Comment on attachment 8734872 [details]
MozReview Request: Bug 1259249  Make sure that EME system is setup before running tests. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42501/#review39711
Attachment #8734872 - Flags: review?(bvandyk) → review+
Assignee: nobody → spolk
https://hg.mozilla.org/mozilla-central/rev/9aeae5b1f4a4
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.