Bug 1300442 - Housekeeping and QoL changes: code style, extra logging on VP. r=maja_zf
authorBryce Van Dyk <bvandyk@mozilla.com>
Mon, 05 Sep 2016 11:02:07 +1200
changeset 312994 60856a997b32a1866523c230e4a73509d78ea568
parent 312993 89b9735661b9f19ece9b6a2bb54b1c50d0556b34
child 312995 17f792799636c637c96229e877a2f1b93edcd147
push id81509
push usercbook@mozilla.com
push dateWed, 07 Sep 2016 15:23:10 +0000
treeherdermozilla-inbound@80dccdd8c94a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmaja_zf
bugs1300442
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1300442 - Housekeeping and QoL changes: code style, extra logging on VP. r=maja_zf - Use format() instead of old style formatting (%s, etc). - Remove unneeded positional args on format strings. - Break some long lines for pep8 conformance. - Use brackets instead of \ to continue long lines. - Log interval on video puppeteer. - Remove an unneeded media source check. We have explicit media source checks in tests, and the media source prefix has changed, rendering the check broken. MozReview-Commit-ID: 4FPVoOD0P5B
dom/media/test/external/external_media_harness/testcase.py
dom/media/test/external/external_media_tests/media_utils/video_puppeteer.py
dom/media/test/external/external_media_tests/media_utils/youtube_puppeteer.py
dom/media/test/external/external_media_tests/playback/test_eme_playback.py
dom/media/test/external/external_media_tests/playback/youtube/test_basic_playback.py
--- a/dom/media/test/external/external_media_harness/testcase.py
+++ b/dom/media/test/external/external_media_harness/testcase.py
@@ -8,18 +8,22 @@ import time
 
 from marionette import BrowserMobProxyTestCaseMixin, MarionetteTestCase, Marionette
 from marionette_driver import Wait
 from marionette_driver.errors import TimeoutException
 from marionette.marionette_test import SkipTest
 
 from firefox_puppeteer.testcases import BaseFirefoxTestCase
 from external_media_tests.utils import (timestamp_now, verbose_until)
-from external_media_tests.media_utils.video_puppeteer import (playback_done, playback_started,
-                                         VideoException, VideoPuppeteer as VP)
+from external_media_tests.media_utils.video_puppeteer import (
+    playback_done,
+    playback_started,
+    VideoException,
+    VideoPuppeteer as VP
+)
 
 
 class MediaTestCase(BaseFirefoxTestCase, MarionetteTestCase):
 
     """
     Necessary methods for MSE playback
 
     :param video_urls: Urls you are going to play as part of the tests.
@@ -42,17 +46,18 @@ class MediaTestCase(BaseFirefoxTestCase,
                             '.png'])
         path = os.path.join(screenshot_dir, filename)
         if not os.path.exists(screenshot_dir):
             os.makedirs(screenshot_dir)
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
             img_data = self.marionette.screenshot()
         with open(path, 'wb') as f:
             f.write(img_data.decode('base64'))
-        self.marionette.log('Screenshot saved in %s' % os.path.abspath(path))
+        self.marionette.log('Screenshot saved in {}'
+                            .format(os.path.abspath(path)))
 
     def log_video_debug_lines(self):
         """
         Log the debugging information that Firefox provides for video elements.
         """
         with self.marionette.using_context(Marionette.CONTEXT_CHROME):
             debug_lines = self.marionette.execute_script(VP._debug_script)
             if debug_lines:
@@ -155,21 +160,16 @@ class VideoPlaybackTestsMixin(object):
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
             for url in self.video_urls:
                 try:
                     video = VP(self.marionette, url, timeout=60)
                     # Second playback_started check in case video._start_time
                     # is not 0
                     self.check_playback_starts(video)
                     video.pause()
-                    src = video.video_src
-                    if not src.startswith('mediasource'):
-                        self.marionette.log('video is not '
-                                            'mediasource: %s' % src,
-                                            level='WARNING')
                 except TimeoutException as e:
                     raise self.failureException(e)
 
     def test_video_playback_partial(self):
         """
         Test to make sure that playback of 60 seconds works for each video.
         """
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
@@ -263,53 +263,55 @@ class EMESetupMixin(object):
                 adobe_result = self.marionette.execute_async_script(
                     reset_adobe_gmp_script,
                     script_timeout=60000)
                 widevine_result = self.marionette.execute_async_script(
                     reset_widevine_gmp_script,
                     script_timeout=60000)
                 if not adobe_result == 'success':
                     raise VideoException(
-                        'ERROR: Resetting Adobe GMP failed % s' % adobe_result)
+                        'ERROR: Resetting Adobe GMP failed {}'
+                        .format(adobe_result))
                 if not widevine_result == 'success':
                     raise VideoException(
-                        'ERROR: Resetting Widevine GMP failed % s'
-                        % widevine_result)
+                        'ERROR: Resetting Widevine GMP failed {}'
+                        .format(widevine_result))
 
             EMESetupMixin.version_needs_reset = False
 
     def check_and_log_boolean_pref(self, pref_name, expected_value):
         with self.marionette.using_context(Marionette.CONTEXT_CHROME):
             pref_value = self.prefs.get_pref(pref_name)
 
             if pref_value is None:
-                self.logger.info('Pref %s has no value.' % pref_name)
+                self.logger.info('Pref {} has no value.'.format(pref_name))
                 return False
             else:
-                self.logger.info('Pref %s = %s' % (pref_name, pref_value))
+                self.logger.info('Pref {} = {}'.format(pref_name, pref_value))
                 if pref_value != expected_value:
-                    self.logger.info('Pref %s has unexpected value.'
-                                     % pref_name)
+                    self.logger.info('Pref {} has unexpected value.'
+                                     .format(pref_name))
                     return False
 
         return True
 
     def check_and_log_integer_pref(self, pref_name, minimum_value=0):
         with self.marionette.using_context(Marionette.CONTEXT_CHROME):
             pref_value = self.prefs.get_pref(pref_name)
 
             if pref_value is None:
-                self.logger.info('Pref %s has no value.' % pref_name)
+                self.logger.info('Pref {} has no value.'.format(pref_name))
                 return False
             else:
-                self.logger.info('Pref %s = %s' % (pref_name, pref_value))
+                self.logger.info('Pref {} = {}'.format(pref_name, pref_value))
 
                 match = re.search('^\d+$', pref_value)
                 if not match:
-                    self.logger.info('Pref %s is not an integer' % pref_name)
+                    self.logger.info('Pref {} is not an integer'
+                                     .format(pref_name))
                     return False
 
             return pref_value >= minimum_value
 
     def chceck_and_log_version_string_pref(self, pref_name, minimum_value='0'):
         """
         Compare a pref made up of integers separated by stops .s, with a
         version string of the same format. The number of integers in each
@@ -318,25 +320,25 @@ class EMESetupMixin(object):
         must be made up of only integers, or this method will raise an
         unhandled exception of type ValueError when the conversion to int
         fails.
         """
         with self.marionette.using_context(Marionette.CONTEXT_CHROME):
             pref_value = self.prefs.get_pref(pref_name)
 
             if pref_value is None:
-                self.logger.info('Pref %s has no value.' % pref_name)
+                self.logger.info('Pref {} has no value.'.format(pref_name))
                 return False
             else:
-                self.logger.info('Pref %s = %s' % (pref_name, pref_value))
+                self.logger.info('Pref {} = {}'.format(pref_name, pref_value))
 
                 match = re.search('^\d(.\d+)*$', pref_value)
                 if not match:
-                    self.logger.info('Pref %s is not a version string'
-                                     % pref_name)
+                    self.logger.info('Pref {} is not a version string'
+                                     .format(pref_name))
                     return False
 
             pref_ints = [int(n) for n in pref_value.split('.')]
             minumum_ints = [int(n) for n in minimum_value.split('.')]
 
             return pref_ints >= minumum_ints
 
     def check_eme_prefs(self):
--- a/dom/media/test/external/external_media_tests/media_utils/video_puppeteer.py
+++ b/dom/media/test/external/external_media_tests/media_utils/video_puppeteer.py
@@ -269,25 +269,27 @@ class VideoPuppeteer(object):
         :param script: script to be executed
         :return: value returned by script
         """
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
             return self.marionette.execute_script(script,
                                                   script_args=[self.video])
 
     def __str__(self):
-        messages = ['%s - test url: %s: {' % (type(self).__name__,
-                                              self.test_url)]
+        messages = ['{} - test url: {}: '
+                    .format(type(self).__name__, self.test_url)]
+        messages += '{'
         if self.video:
             messages += [
                 '\t(video)',
                 '\tcurrent_time: {},'.format(self.current_time),
                 '\tduration: {},'.format(self.duration),
                 '\texpected_duration: {},'.format(self.expected_duration),
                 '\tplayed: {}'.format(self.played),
+                '\tinterval: {}'.format(self.interval),
                 '\tlag: {},'.format(self.lag),
                 '\turl: {}'.format(self.video_url),
                 '\tsrc: {}'.format(self.video_src),
                 '\tframes total: {}'.format(self.total_frames),
                 '\t - dropped: {}'.format(self.dropped_frames),
                 '\t - corrupted: {}'.format(self.corrupted_frames)
             ]
         else:
@@ -308,18 +310,20 @@ class TimeRanges:
     Class to represent the TimeRanges data returned by played(). Exposes a
     similar interface to the JavaScript TimeRanges object.
     """
     def __init__(self, length, ranges):
         self.length = length
         self.ranges = [(pair[0], pair[1]) for pair in ranges]
 
     def __repr__(self):
-        return 'TimeRanges: length: {}, ranges: {}'\
-               .format(self.length, self.ranges)
+        return (
+            'TimeRanges: length: {}, ranges: {}'
+            .format(self.length, self.ranges)
+        )
 
     def start(self, index):
         return self.ranges[index][0]
 
     def end(self, index):
         return self.ranges[index][1]
 
 
@@ -328,19 +332,21 @@ def playback_started(video):
     Determine if video has started
 
     :param video: The VideoPuppeteer instance that we are interested in.
 
     :return: True if is playing; False otherwise
     """
     try:
         played_ranges = video.played
-        return played_ranges.length > 0 and \
-               played_ranges.start(0) < played_ranges.end(0) and \
-               played_ranges.end(0) > 0.0
+        return (
+            played_ranges.length > 0 and
+            played_ranges.start(0) < played_ranges.end(0) and
+            played_ranges.end(0) > 0.0
+        )
     except Exception as e:
         print ('Got exception {}'.format(e))
         return False
 
 
 def playback_done(video):
     """
     If we are near the end and there is still a video element, then
@@ -353,13 +359,13 @@ def playback_done(video):
         otherwise.
     """
     if video.remaining_time < video.interval:
         return True
 
     # Check to see if the video has stalled. Accumulate the amount of lag
     # since the video started, and if it is too high, then raise.
     if video.stall_wait_time and (video.lag > video.stall_wait_time):
-        raise VideoException('Video %s stalled.\n%s' % (video.video_url,
-                                                        video))
+        raise VideoException('Video {} stalled.\n{}'
+                             .format(video.video_url, video))
 
     # We are cruising, so we are not done.
     return False
--- a/dom/media/test/external/external_media_tests/media_utils/youtube_puppeteer.py
+++ b/dom/media/test/external/external_media_tests/media_utils/youtube_puppeteer.py
@@ -335,18 +335,22 @@ class YouTubePuppeteer(VideoPuppeteer):
         """
         Try to skip this ad. If not, wait for this ad to finish.
         """
         if self.attempt_ad_skip() or self.ad_inactive:
             return
         ad_timeout = (self.search_ad_duration() or 30) + 5
         wait = Wait(self, timeout=ad_timeout, interval=1)
         try:
-            self.marionette.log('process_ad: waiting %s s for ad' % ad_timeout)
-            verbose_until(wait, self, lambda y: y.ad_ended, "Check if ad ended")
+            self.marionette.log('process_ad: waiting {} s for ad'
+                                .format(ad_timeout))
+            verbose_until(wait,
+                          self,
+                          lambda y: y.ad_ended,
+                          "Check if ad ended")
         except TimeoutException:
             self.marionette.log('Waiting for ad to end timed out',
                                 level='WARNING')
 
     def attempt_ad_skip(self):
         """
         Attempt to skip ad by clicking on skip-add button.
 
@@ -367,17 +371,17 @@ class YouTubePuppeteer(VideoPuppeteer):
                                                           selector))
                     ad_button = self.marionette.find_element(By.CSS_SELECTOR,
                                                              selector)
                     ad_button.click()
                     self.marionette.log('Skipped ad.')
                     return True
             except (TimeoutException, NoSuchElementException):
                 self.marionette.log('Could not obtain '
-                                    'element: %s' % selector,
+                                    'element: {}'.format(selector),
                                     level='WARNING')
         return False
 
     def search_ad_duration(self):
         """
 
         :return: ad duration in seconds, if currently displayed in player
         """
@@ -397,17 +401,17 @@ class YouTubePuppeteer(VideoPuppeteer):
                                                          selector)
                 ad_time = self._time_pattern.search(countdown.text)
                 if ad_time:
                     ad_minutes = int(ad_time.group('minute'))
                     ad_seconds = int(ad_time.group('second'))
                     return 60 * ad_minutes + ad_seconds
         except (TimeoutException, NoSuchElementException):
             self.marionette.log('Could not obtain '
-                                'element: %s' % selector,
+                                'element: {}'.format(selector),
                                 level='WARNING')
         return None
 
     @property
     def player_stalled(self):
         """
 
         :return: True if playback is not making progress for 4-9 seconds. This
@@ -457,35 +461,35 @@ class YouTubePuppeteer(VideoPuppeteer):
                 # the pref might get reset moments later.
                 sleep(1)
                 if get_status(checkbox):
                     mn.execute_script('return arguments[0].'
                                       'wrappedJSObject.click()',
                                       script_args=[checkbox])
                     self.marionette.log('Toggled autoplay.')
                 autoplay = get_status(checkbox)
-                self.marionette.log('Autoplay is %s' % autoplay)
+                self.marionette.log('Autoplay is {}'.format(autoplay))
                 return (autoplay is not None) and (not autoplay)
         except (NoSuchElementException, TimeoutException):
             return False
 
     def __str__(self):
         messages = [super(YouTubePuppeteer, self).__str__()]
         if self.player:
             player_state = self._yt_player_state_name[self.player_state]
             ad_state = self._yt_player_state_name[self.ad_state]
             messages += [
                 '.html5-media-player: {',
-                '\tvideo id: {0},'.format(self.movie_id),
-                '\tvideo_title: {0}'.format(self.movie_title),
-                '\tcurrent_state: {0},'.format(player_state),
-                '\tad_state: {0},'.format(ad_state),
-                '\tplayback_quality: {0},'.format(self.playback_quality),
-                '\tcurrent_time: {0},'.format(self.player_current_time),
-                '\tduration: {0},'.format(self.player_duration),
+                '\tvideo id: {},'.format(self.movie_id),
+                '\tvideo_title: {}'.format(self.movie_title),
+                '\tcurrent_state: {},'.format(player_state),
+                '\tad_state: {},'.format(ad_state),
+                '\tplayback_quality: {},'.format(self.playback_quality),
+                '\tcurrent_time: {},'.format(self.player_current_time),
+                '\tduration: {},'.format(self.player_duration),
                 '}'
             ]
         else:
             messages += ['\t.html5-media-player: None']
         return '\n'.join(messages)
 
 
 def playback_started(yt):
--- a/dom/media/test/external/external_media_tests/playback/test_eme_playback.py
+++ b/dom/media/test/external/external_media_tests/playback/test_eme_playback.py
@@ -1,13 +1,17 @@
 # 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/.
 
-from external_media_harness.testcase import MediaTestCase, VideoPlaybackTestsMixin, EMESetupMixin
+from external_media_harness.testcase import (
+    MediaTestCase,
+    VideoPlaybackTestsMixin,
+    EMESetupMixin
+)
 
 
 class TestEMEPlayback(MediaTestCase, VideoPlaybackTestsMixin, EMESetupMixin):
 
     def setUp(self):
         super(TestEMEPlayback, self).setUp()
         self.check_eme_system()
 
--- a/dom/media/test/external/external_media_tests/playback/youtube/test_basic_playback.py
+++ b/dom/media/test/external/external_media_tests/playback/youtube/test_basic_playback.py
@@ -4,18 +4,21 @@
 
 from marionette import Marionette
 from marionette_driver import Wait
 from marionette_driver.errors import TimeoutException
 
 from external_media_tests.utils import verbose_until
 from external_media_harness.testcase import MediaTestCase
 from external_media_tests.media_utils.video_puppeteer import VideoException
-from external_media_tests.media_utils.youtube_puppeteer import (YouTubePuppeteer, playback_done,
-                                           wait_for_almost_done)
+from external_media_tests.media_utils.youtube_puppeteer import (
+    YouTubePuppeteer,
+    playback_done,
+    wait_for_almost_done
+)
 
 
 class TestBasicYouTubePlayback(MediaTestCase):
     def test_mse_is_enabled_by_default(self):
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
             youtube = YouTubePuppeteer(self.marionette, self.video_urls[0],
                                        timeout=60)
             wait = Wait(youtube,
@@ -28,37 +31,39 @@ class TestBasicYouTubePlayback(MediaTest
             except TimeoutException as e:
                 raise self.failureException(e)
 
     def test_video_playing_in_one_tab(self):
         with self.marionette.using_context(Marionette.CONTEXT_CONTENT):
             for url in self.video_urls:
                 self.logger.info(url)
                 youtube = YouTubePuppeteer(self.marionette, url)
-                self.logger.info('Expected duration: %s' %
-                                 youtube.expected_duration)
+                self.logger.info('Expected duration: {}'
+                                 .format(youtube.expected_duration))
                 youtube.deactivate_autoplay()
 
                 final_piece = 60
                 try:
                     time_left = wait_for_almost_done(youtube,
                                                      final_piece=final_piece)
                 except VideoException as e:
                     raise self.failureException(e)
                 duration = abs(youtube.expected_duration) + 1
                 if duration > 1:
-                    self.logger.info('Almost done: %s - %s seconds left.' %
-                                     (youtube.movie_id, time_left))
+                    self.logger.info('Almost done: {} - {} seconds left.'
+                                     .format(youtube.movie_id, time_left))
                     if time_left > final_piece:
                         self.marionette.log('time_left greater than '
-                                            'final_piece - %s' % time_left,
+                                            'final_piece - {}'
+                                            .format(time_left),
                                             level='WARNING')
                         self.save_screenshot()
                 else:
-                    self.marionette.log('Duration close to 0 - %s' % youtube,
+                    self.marionette.log('Duration close to 0 - {}'
+                                        .format(youtube),
                                         level='WARNING')
                     self.save_screenshot()
                 try:
                     verbose_until(Wait(youtube,
                                        timeout=max(100, time_left) * 1.3,
                                        interval=1),
                                   youtube,
                                   playback_done)