Bug 1550828: String fixes and minor cleanup for power.py and Raptor's mach_commands.py. r=bc
authorStephen Donner <stephen.donner@gmail.com>
Sat, 11 May 2019 08:13:29 +0000
changeset 532454 71d1c36b3e1891c6876ab6273fec63b7bc7e3f90
parent 532453 a811c910cfd3527d20556d09fca7dafc4003bc4d
child 532455 06e37cc6a55c2d06f9164a1f2078b8f8a4d9e3a2
push id11268
push usercsabou@mozilla.com
push dateTue, 14 May 2019 15:24:22 +0000
treeherdermozilla-beta@5fb7fcd568d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbc
bugs1550828
milestone68.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 1550828: String fixes and minor cleanup for power.py and Raptor's mach_commands.py. r=bc Differential Revision: https://phabricator.services.mozilla.com/D30775
testing/raptor/mach_commands.py
testing/raptor/raptor/power.py
--- a/testing/raptor/mach_commands.py
+++ b/testing/raptor/mach_commands.py
@@ -24,19 +24,19 @@ from mozbuild.base import MachCommandCon
 HERE = os.path.dirname(os.path.realpath(__file__))
 BENCHMARK_REPOSITORY = 'https://github.com/mozilla/perf-automation'
 BENCHMARK_REVISION = '2720cdc790828952964524bb44ce8b4c14670e90'
 
 
 class RaptorRunner(MozbuildObject):
     def run_test(self, raptor_args, kwargs):
         """
-        We want to do couple of things before running raptor
+        We want to do a few things before running Raptor:
         1. Clone mozharness
-        2. Make config for raptor mozharness
+        2. Make the config for Raptor mozharness
         3. Run mozharness
         """
 
         self.init_variables(raptor_args, kwargs)
         self.setup_benchmarks()
         self.make_config()
         self.write_config()
         self.make_args()
@@ -199,28 +199,28 @@ class MachRaptor(MachCommandBase):
         raptor = self._spawn(RaptorRunner)
 
         try:
             if kwargs['app'] in firefox_android_browsers and kwargs['power_test']:
                 device = ADBAndroid(verbose=True)
                 adbhost = ADBHost(verbose=True)
                 device_serial = "%s:5555" % device.get_ip_address()
                 device.command_output(["tcpip", "5555"])
-                raw_input("Please disconnect your device from USB then press ENTER...")
+                raw_input("Please disconnect your device from USB then press Enter/return...")
                 adbhost.command_output(["connect", device_serial])
                 while len(adbhost.devices()) > 1:
                     raw_input("You must disconnect your device from USB before continuing.")
                 # must reset the environment DEVICE_SERIAL which was set during
                 # verify_android_device to match our new tcpip value.
                 os.environ["DEVICE_SERIAL"] = device_serial
             return raptor.run_test(sys.argv[2:], kwargs)
         except Exception as e:
             print(repr(e))
             return 1
         finally:
             try:
                 if kwargs['app'] in firefox_android_browsers and kwargs['power_test']:
-                    raw_input("Connect device via usb and press ENTER...")
+                    raw_input("Connect device via USB and press Enter/return...")
                     device = ADBAndroid(device=device_serial, verbose=True)
                     device.command_output(["usb"])
                     adbhost.command_output(["disconnect", device_serial])
             except Exception:
                 adbhost.command_output(["kill-server"])
--- a/testing/raptor/raptor/power.py
+++ b/testing/raptor/raptor/power.py
@@ -1,33 +1,37 @@
 # 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 __future__ import absolute_import
 
 import os
 import re
 
 
 def init_android_power_test(raptor):
-    upload_dir = os.getenv('MOZ_UPLOAD_DIR')
+    upload_dir = os.getenv("MOZ_UPLOAD_DIR")
     if not upload_dir:
-        raptor.log.critical("% power test ignored; MOZ_UPLOAD_DIR unset" % raptor.config['app'])
+        raptor.log.critical(
+            "% power test ignored; MOZ_UPLOAD_DIR unset" % raptor.config["app"]
+        )
         return
-    # Set the screen off timeout to 2 hours since the device will be running
-    # disconnected and would otherwise turn off the screen thereby halting
+    # Set the screen-off timeout to two (2) hours, since the device will be running
+    # disconnected, and would otherwise turn off the screen, thereby halting
     # execution of the test. Save the current value so we can restore it later
     # since it is a persistent change.
     raptor.screen_off_timeout = raptor.device.shell_output(
-        "settings get system screen_off_timeout").strip()
+        "settings get system screen_off_timeout"
+    ).strip()
     raptor.device.shell_output("settings put system screen_off_timeout 7200000")
     raptor.device.shell_output("dumpsys batterystats --reset")
     raptor.device.shell_output("dumpsys batterystats --enable full-wake-history")
-    filepath = os.path.join(upload_dir, 'battery-before.txt')
-    with open(filepath, 'w') as output:
+    filepath = os.path.join(upload_dir, "battery-before.txt")
+    with open(filepath, "w") as output:
         output.write(raptor.device.shell_output("dumpsys battery"))
 
 
 # The batterystats output for Estimated power use differs
 # for Android 7 and Android 8 and later.
 #
 # Android 7
 # Estimated power use (mAh):
@@ -44,78 +48,83 @@ def init_android_power_test(raptor):
 #   Capacity: 2700, Computed drain: 145, actual drain: 135-162
 #   Screen: 68.2 Excluded from smearing
 #   Uid u0a208: 61.7 ( cpu=60.5 wifi=1.28 ) Including smearing: 141 ( screen=67.7 proportional=9. )
 #   Cell standby: 2.49 ( radio=2.49 ) Excluded from smearing
 #   Idle: 1.63 Excluded from smearing
 #   Bluetooth: 0.527 ( cpu=0.00319 bt=0.524 ) Including smearing: 0.574 ( proportional=... )
 #   Wifi: 0.423 ( cpu=0.343 wifi=0.0800 ) Including smearing: 0.461 ( proportional=0.0375 )
 #
-# For Android 8 the cpu, wifi, screen and proportional values from the
-# Uid line for the app. If the test does not run long enough it
-# appears that the screen value from the Uid will be missing but the
+# For Android 8, the cpu, wifi, screen, and proportional values are available from
+# the Uid line for the app. If the test does not run long enough, it
+# appears that the screen value from the Uid will be missing, but the
 # standalone Screen value is available.
 #
-# For Android 7 only the cpu value is available from the Uid line. We
+# For Android 7, only the cpu value is available from the Uid line. We
 # can use the Screen and Wifi values for Android 7 from the Screen
-# and Wifi lines which may include contributions from the system or
-# other apps however it should be useful for spotting changes in power
+# and Wifi lines, which might include contributions from the system or
+# other apps; however, it should still be useful for spotting changes in power
 # usage.
 #
-# If the energy values from Uid line for Android 8 are available they
-# will be used. If for any reason that the screen or wifi power is
+# If the energy values from the Uid line for Android 8 are available, they
+# will be used. If for any reason either/both screen or wifi power is
 # missing, the values from the Screen and Wifi lines will be used.
 #
-# If only the cpu energy value is available, then it will be used
+# If only the cpu energy value is available, it will be used
 # along with the values from the Screen and Wifi lines.
 
+
 def finish_android_power_test(raptor, test_name):
-    upload_dir = os.getenv('MOZ_UPLOAD_DIR')
+    upload_dir = os.getenv("MOZ_UPLOAD_DIR")
     if not upload_dir:
-        raptor.log.critical("% power test ignored because MOZ_UPLOAD_DIR was not set" % test_name)
+        raptor.log.critical(
+            "% power test ignored because MOZ_UPLOAD_DIR was not set" % test_name
+        )
         return
-    # Restore the screen off timeout.
+    # Restore screen_off_timeout.
     raptor.device.shell_output(
-        "settings put system screen_off_timeout %s" % raptor.screen_off_timeout)
-    filepath = os.path.join(upload_dir, 'battery-after.txt')
-    with open(filepath, 'w') as output:
+        "settings put system screen_off_timeout %s" % raptor.screen_off_timeout
+    )
+    filepath = os.path.join(upload_dir, "battery-after.txt")
+    with open(filepath, "w") as output:
         output.write(raptor.device.shell_output("dumpsys battery"))
     verbose = raptor.device._verbose
     raptor.device._verbose = False
-    filepath = os.path.join(upload_dir, 'batterystats.csv')
-    with open(filepath, 'w') as output:
+    filepath = os.path.join(upload_dir, "batterystats.csv")
+    with open(filepath, "w") as output:
         output.write(raptor.device.shell_output("dumpsys batterystats --checkin"))
-    filepath = os.path.join(upload_dir, 'batterystats.txt')
-    with open(filepath, 'w') as output:
+    filepath = os.path.join(upload_dir, "batterystats.txt")
+    with open(filepath, "w") as output:
         batterystats = raptor.device.shell_output("dumpsys batterystats")
         output.write(batterystats)
     raptor.device._verbose = verbose
     estimated_power = False
     uid = None
     total = cpu = wifi = smearing = screen = proportional = 0
     full_screen = 0
     full_wifi = 0
-    re_uid = re.compile(r'proc=([^:]+):"%s"' % raptor.config['binary'])
-    re_estimated_power = re.compile(r'\s+Estimated power use [(]mAh[)]')
-    re_proportional = re.compile(r'proportional=([\d.]+)')
-    re_screen = re.compile(r'screen=([\d.]+)')
-    re_full_screen = re.compile(r'\s+Screen:\s+([\d.]+)')
-    re_full_wifi = re.compile(r'\s+Wifi:\s+([\d.]+)')
+    re_uid = re.compile(r'proc=([^:]+):"%s"' % raptor.config["binary"])
+    re_estimated_power = re.compile(r"\s+Estimated power use [(]mAh[)]")
+    re_proportional = re.compile(r"proportional=([\d.]+)")
+    re_screen = re.compile(r"screen=([\d.]+)")
+    re_full_screen = re.compile(r"\s+Screen:\s+([\d.]+)")
+    re_full_wifi = re.compile(r"\s+Wifi:\s+([\d.]+)")
     re_power = None
-    batterystats = batterystats.split('\n')
+    batterystats = batterystats.split("\n")
     for line in batterystats:
         if uid is None:
-            # The proc line containing the uid and app name appears
+            # The proc line containing the Uid and app name appears
             # before the Estimated power line.
             match = re_uid.search(line)
             if match:
                 uid = match.group(1)
                 re_power = re.compile(
-                    r'\s+Uid %s:\s+([\d.]+) ([(] cpu=([\d.]+) wifi=([\d.]+) [)] '
-                    r'Including smearing: ([\d.]+))?' % uid)
+                    r"\s+Uid %s:\s+([\d.]+) ([(] cpu=([\d.]+) wifi=([\d.]+) [)] "
+                    r"Including smearing: ([\d.]+))?" % uid
+                )
                 continue
         if not estimated_power:
             # Do not attempt to parse data until we have seen
             # Estimated Power in the output.
             match = re_estimated_power.match(line)
             if match:
                 estimated_power = True
             continue
@@ -130,45 +139,50 @@ def finish_android_power_test(raptor, te
                 full_wifi = match.group(1)
                 continue
         if re_power:
             match = re_power.match(line)
             if match:
                 (total, android8, cpu, wifi, smearing) = match.groups()
                 if android8:
                     # android8 is not None only if the Uid line
-                    # contained values for cpu and wifi which is
+                    # contained values for cpu and wifi, which is
                     # true only for Android 8+.
                     match = re_screen.search(line)
                     if match:
                         screen = match.group(1)
                     match = re_proportional.search(line)
                     if match:
                         proportional = match.group(1)
         if full_screen and full_wifi and (cpu and wifi and smearing or total):
             # Stop parsing batterystats once we have a full set of data.
             break
 
     cpu = total if cpu is None else cpu
     screen = full_screen if screen == 0 else screen
     wifi = full_wifi if wifi is None else wifi
 
-    raptor.log.info('power data for uid: %s, cpu: %s, wifi: %s, screen: %s, proportional: %s' %
-                    (uid, cpu, wifi, screen, proportional))
+    raptor.log.info(
+        "power data for uid: %s, cpu: %s, wifi: %s, screen: %s, proportional: %s"
+        % (uid, cpu, wifi, screen, proportional)
+    )
 
-    # send power data directly to the control server results handler;
-    # so it can be formatted and output for perfherder ingestion
+    # send power data directly to the control-server results handler,
+    # so it can be formatted and out-put for perfherder ingestion
 
-    power_data = {'type': 'power',
-                  'test': test_name,
-                  'unit': 'mAh',
-                  'values': {
-                      'cpu': float(cpu),
-                      'wifi': float(wifi),
-                      'screen': float(screen),
-                      'proportional': float(proportional)}}
+    power_data = {
+        "type": "power",
+        "test": test_name,
+        "unit": "mAh",
+        "values": {
+            "cpu": float(cpu),
+            "wifi": float(wifi),
+            "screen": float(screen),
+            "proportional": float(proportional),
+        },
+    }
 
     raptor.log.info("submitting power data via control server directly")
     raptor.control_server.submit_supporting_data(power_data)
 
     # generate power bugreport zip
     raptor.log.info("generating power bugreport zip")
     raptor.device.command_output(["bugreport", upload_dir])