diff options
Diffstat (limited to 'catapult/devil/devil/android')
31 files changed, 342 insertions, 150 deletions
diff --git a/catapult/devil/devil/android/apk_helper.py b/catapult/devil/devil/android/apk_helper.py index fdece072..4d723a56 100644 --- a/catapult/devil/devil/android/apk_helper.py +++ b/catapult/devil/devil/android/apk_helper.py @@ -11,6 +11,8 @@ import shutil import tempfile import zipfile +import six + from devil import base_error from devil.android.ndk import abis from devil.android.sdk import aapt @@ -69,7 +71,7 @@ def GetInstrumentationName(apk_path): def ToHelper(path_or_helper): """Creates an ApkHelper unless one is already given.""" - if not isinstance(path_or_helper, basestring): + if not isinstance(path_or_helper, six.string_types): return path_or_helper elif path_or_helper.endswith('.apk'): return ApkHelper(path_or_helper) @@ -86,7 +88,7 @@ def ToSplitHelper(path_or_helper, split_apks): if sorted(path_or_helper.split_apk_paths) != sorted(split_apks): raise ApkHelperError('Helper has different split APKs') return path_or_helper - elif (isinstance(path_or_helper, basestring) + elif (isinstance(path_or_helper, six.string_types) and path_or_helper.endswith('.apk')): return SplitApkHelper(path_or_helper, split_apks) diff --git a/catapult/devil/devil/android/app_ui.py b/catapult/devil/devil/android/app_ui.py index 399c2ee3..4f7af1d7 100644 --- a/catapult/devil/devil/android/app_ui.py +++ b/catapult/devil/devil/android/app_ui.py @@ -51,7 +51,7 @@ class _UiNode(object): A geometry.Rectangle instance. """ d = _RE_BOUNDS.match(self._GetAttribute('bounds')).groupdict() - return geometry.Rectangle.FromDict({k: int(v) for k, v in d.iteritems()}) + return geometry.Rectangle.FromDict({k: int(v) for k, v in d.items()}) def Tap(self, point=None, dp_units=False): """Send a tap event to the UI node. @@ -150,7 +150,7 @@ class _UiNode(object): and ':id/' not in resource_id): kwargs['resource_id'] = '%s:id/%s' % (self._package, resource_id) - criteria = [(k.replace('_', '-'), v) for k, v in kwargs.iteritems() + criteria = [(k.replace('_', '-'), v) for k, v in kwargs.items() if v is not None] if not criteria: raise TypeError('At least one search criteria should be specified') @@ -193,8 +193,12 @@ class AppUi(object): A UI node instance pointing to the root of the xml screenshot. """ with device_temp_file.DeviceTempFile(self._device.adb) as dtemp: - self._device.RunShellCommand(['uiautomator', 'dump', dtemp.name], - check_return=True) + output = self._device.RunShellCommand( + ['uiautomator', 'dump', dtemp.name], single_line=True, + check_return=True) + if output.startswith('ERROR:'): + raise RuntimeError( + 'uiautomator dump command returned error: {}'.format(output)) xml_node = element_tree.fromstring( self._device.ReadFile(dtemp.name, force_pull=True)) return _UiNode(self._device, xml_node, package=self._package) diff --git a/catapult/devil/devil/android/app_ui_test.py b/catapult/devil/devil/android/app_ui_test.py index 938fd408..50f00ca1 100644 --- a/catapult/devil/devil/android/app_ui_test.py +++ b/catapult/devil/devil/android/app_ui_test.py @@ -80,7 +80,7 @@ class UiAppTest(unittest.TestCase): def assertNodeHasAttribs(self, node, attr): # pylint: disable=protected-access - for key, value in attr.iteritems(): + for key, value in attr.items(): self.assertEquals(node._GetAttribute(key), value) def assertTappedOnceAt(self, x, y): diff --git a/catapult/devil/devil/android/battery_utils.py b/catapult/devil/devil/android/battery_utils.py index e8134d2b..d680f03f 100644 --- a/catapult/devil/devil/android/battery_utils.py +++ b/catapult/devil/devil/android/battery_utils.py @@ -300,7 +300,7 @@ class BatteryUtils(object): 'uid': uid, 'data': pwi_entries[uid] } - for p, uid in self._cache['uids'].iteritems() + for p, uid in self._cache['uids'].items() } return {'system_total': system_total, 'per_package': per_package} diff --git a/catapult/devil/devil/android/cpu_temperature_test.py b/catapult/devil/devil/android/cpu_temperature_test.py index 8d082bb9..47cc28a6 100644 --- a/catapult/devil/devil/android/cpu_temperature_test.py +++ b/catapult/devil/devil/android/cpu_temperature_test.py @@ -69,9 +69,11 @@ class CpuTemperatureGetThermalDeviceInformationTest(CpuTemperatureTest): 'cpu6': '/sys/class/thermal/thermal_zone17/temp', 'cpu7': '/sys/class/thermal/thermal_zone18/temp' } - self.assertEqual( - cmp(correct_information, - self.cpu_temp.GetDeviceInfoForTesting().get('cpu_temps')), 0) + + self.assertDictEqual( + correct_information, + self.cpu_temp.GetDeviceInfoForTesting().get('cpu_temps') + ) class CpuTemperatureIsSupportedTest(CpuTemperatureTest): diff --git a/catapult/devil/devil/android/decorators.py b/catapult/devil/devil/android/decorators.py index 0b3778aa..11d2494b 100644 --- a/catapult/devil/devil/android/decorators.py +++ b/catapult/devil/devil/android/decorators.py @@ -9,6 +9,8 @@ import functools import itertools import sys +import six + from devil.android import device_errors from devil.utils import cmd_helper from devil.utils import reraiser_thread @@ -56,14 +58,19 @@ def _TimeoutRetryWrapper(f, desc = '%s(%s)' % (f.__name__, ', '.join( itertools.chain( (str(a) for a in args), - ('%s=%s' % (k, str(v)) for k, v in kwargs.iteritems())))) + ('%s=%s' % (k, str(v)) for k, v in six.iteritems(kwargs))))) return timeout_retry.Run( impl, timeout, retries, desc=desc, retry_if_func=retry_if_func) except reraiser_thread.TimeoutError as e: - raise device_errors.CommandTimeoutError(str(e)), None, (sys.exc_info()[2]) + six.reraise( + device_errors.CommandTimeoutError, + device_errors.CommandTimeoutError(str(e)), + sys.exc_info()[2]) except cmd_helper.TimeoutError as e: - raise device_errors.CommandTimeoutError( - str(e), output=e.output), None, (sys.exc_info()[2]) + six.reraise( + device_errors.CommandTimeoutError, + device_errors.CommandTimeoutError(str(e), output=e.output), + sys.exc_info()[2]) return timeout_retry_wrapper diff --git a/catapult/devil/devil/android/device_errors.py b/catapult/devil/devil/android/device_errors.py index 6e710876..75bf7e3f 100644 --- a/catapult/devil/devil/android/device_errors.py +++ b/catapult/devil/devil/android/device_errors.py @@ -23,6 +23,8 @@ The class hierarchy for device exceptions is: """ +import six + from devil import base_error from devil.utils import cmd_helper from devil.utils import parallelizer @@ -158,7 +160,7 @@ class AdbShellCommandFailedError(AdbCommandFailedError): segments.append(' exit status: %s\n' % status) if output: segments.append(' output:\n') - if isinstance(output, basestring): + if isinstance(output, six.string_types): output_lines = output.splitlines() else: output_lines = output diff --git a/catapult/devil/devil/android/device_list.py b/catapult/devil/devil/android/device_list.py index cd631dbd..5fb586f6 100644 --- a/catapult/devil/devil/android/device_list.py +++ b/catapult/devil/devil/android/device_list.py @@ -7,6 +7,8 @@ import json import logging import os +import six + logger = logging.getLogger(__name__) @@ -26,7 +28,7 @@ def GetPersistentDeviceList(file_name): with open(file_name) as f: devices = json.load(f) if not isinstance(devices, list) or not all( - isinstance(d, basestring) for d in devices): + isinstance(d, six.string_types) for d in devices): logger.warning('Unrecognized device file format: %s', devices) return [] return [d for d in devices if d != '(error)'] diff --git a/catapult/devil/devil/android/device_utils.py b/catapult/devil/devil/android/device_utils.py index 7b7dad24..093bfc71 100644 --- a/catapult/devil/devil/android/device_utils.py +++ b/catapult/devil/devil/android/device_utils.py @@ -24,6 +24,8 @@ import time import threading import uuid +import six + from devil import base_error from devil import devil_env from devil.utils import cmd_helper @@ -105,6 +107,13 @@ _RESTART_ADBD_SCRIPT = """ restart & """ +_UNZIP_AND_CHMOD_SCRIPT = """ + {bin_dir}/unzip {zip_file} && (for dir in {dirs} + do + chmod -R 777 "$dir" || exit 1 + done) +""" + # Not all permissions can be set. _PERMISSIONS_DENYLIST_RE = re.compile('|'.join( fnmatch.translate(p) for p in [ @@ -131,6 +140,7 @@ _PERMISSIONS_DENYLIST_RE = re.compile('|'.join( 'android.permission.INTERNET', 'android.permission.KILL_BACKGROUND_PROCESSES', 'android.permission.MANAGE_ACCOUNTS', + 'android.permission.MANAGE_EXTERNAL_STORAGE', 'android.permission.MODIFY_AUDIO_SETTINGS', 'android.permission.NFC', 'android.permission.QUERY_ALL_PACKAGES', @@ -314,30 +324,6 @@ def GetAVDs(): return avds -@decorators.WithExplicitTimeoutAndRetries(_DEFAULT_TIMEOUT, _DEFAULT_RETRIES) -def RestartServer(): - """Restarts the adb server. - - Raises: - CommandFailedError if we fail to kill or restart the server. - """ - - def adb_killed(): - return not adb_wrapper.AdbWrapper.IsServerOnline() - - def adb_started(): - return adb_wrapper.AdbWrapper.IsServerOnline() - - adb_wrapper.AdbWrapper.KillServer() - if not timeout_retry.WaitFor(adb_killed, wait_period=1, max_tries=5): - # TODO(crbug.com/442319): Switch this to raise an exception if we - # figure out why sometimes not all adb servers on bots get killed. - logger.warning('Failed to kill adb server') - adb_wrapper.AdbWrapper.StartServer() - if not timeout_retry.WaitFor(adb_started, wait_period=1, max_tries=5): - raise device_errors.CommandFailedError('Failed to start adb server') - - def _ParseModeString(mode_str): """Parse a mode string, e.g. 'drwxrwxrwx', into a st_mode value. @@ -376,7 +362,7 @@ def _CreateAdbWrapper(device): def _FormatPartialOutputError(output): - lines = output.splitlines() if isinstance(output, basestring) else output + lines = output.splitlines() if isinstance(output, six.string_types) else output message = ['Partial output found:'] if len(lines) > 11: message.extend('- %s' % line for line in lines[:5]) @@ -468,7 +454,7 @@ class DeviceUtils(object): operation should be retried on failure if no explicit value is provided. """ self.adb = None - if isinstance(device, basestring): + if isinstance(device, six.string_types): self.adb = _CreateAdbWrapper(device) elif isinstance(device, adb_wrapper.AdbWrapper): self.adb = device @@ -1533,7 +1519,7 @@ class DeviceUtils(object): else: raise - if isinstance(cmd, basestring): + if isinstance(cmd, six.string_types): if not shell: # TODO(crbug.com/1029769): Make this an error instead. logger.warning( @@ -1543,7 +1529,7 @@ class DeviceUtils(object): else: cmd = ' '.join(cmd_helper.SingleQuote(s) for s in cmd) if env: - env = ' '.join(env_quote(k, v) for k, v in env.iteritems()) + env = ' '.join(env_quote(k, v) for k, v in env.items()) cmd = '%s %s' % (env, cmd) if cwd: cmd = 'cd %s && %s' % (cmd_helper.SingleQuote(cwd), cmd) @@ -1740,7 +1726,7 @@ class DeviceUtils(object): cmd.append('-w') if raw: cmd.append('-r') - for k, v in extras.iteritems(): + for k, v in extras.items(): cmd.extend(['-e', str(k), str(v)]) cmd.append(component) @@ -2223,13 +2209,19 @@ class DeviceUtils(object): self.adb, suffix='.zip') as device_temp: self.adb.Push(zip_path, device_temp.name) - quoted_dirs = ' '.join(cmd_helper.SingleQuote(d) for d in dirs) - self.RunShellCommand( - 'unzip %s&&chmod -R 777 %s' % (device_temp.name, quoted_dirs), - shell=True, - as_root=True, - env={'PATH': '%s:$PATH' % install_commands.BIN_DIR}, - check_return=True) + with device_temp_file.DeviceTempFile(self.adb, suffix='.sh') as script: + # Read dirs from temp file to avoid potential errors like + # "Argument list too long" (crbug.com/1174331) when the list + # is too long. + self.WriteFile( + script.name, + _UNZIP_AND_CHMOD_SCRIPT.format(bin_dir=install_commands.BIN_DIR, + zip_file=device_temp.name, + dirs=' '.join(dirs))) + + self.RunShellCommand(['source', script.name], + check_return=True, + as_root=True) return True @@ -2263,7 +2255,7 @@ class DeviceUtils(object): DeviceUnreachableError on missing device. """ paths = device_paths - if isinstance(paths, basestring): + if isinstance(paths, six.string_types): paths = (paths, ) if not paths: return True @@ -2322,7 +2314,7 @@ class DeviceUtils(object): args.append('-f') if recursive: args.append('-r') - if isinstance(device_path, basestring): + if isinstance(device_path, six.string_types): args.append(device_path if not rename else _RenamePath(device_path)) else: args.extend( @@ -2596,7 +2588,7 @@ class DeviceUtils(object): """ entries = self._ParseLongLsOutput(device_path, as_root=as_root, **kwargs) for d in entries: - for key, value in d.items(): + for key, value in list(d.items()): if value is None: del d[key] # Remove missing fields. d['st_mode'] = _ParseModeString(d['st_mode']) @@ -2960,7 +2952,7 @@ class DeviceUtils(object): """ assert isinstance( property_name, - basestring), ("property_name is not a string: %r" % property_name) + six.string_types), ("property_name is not a string: %r" % property_name) if cache: # It takes ~120ms to query a single property, and ~130ms to query all @@ -3004,8 +2996,8 @@ class DeviceUtils(object): """ assert isinstance( property_name, - basestring), ("property_name is not a string: %r" % property_name) - assert isinstance(value, basestring), "value is not a string: %r" % value + six.string_types), ("property_name is not a string: %r" % property_name) + assert isinstance(value, six.string_types), "value is not a string: %r" % value self.RunShellCommand(['setprop', property_name, value], check_return=True) prop_cache = self._cache['getprop'] @@ -3084,18 +3076,19 @@ class DeviceUtils(object): Returns: A list of ProcessInfo tuples with |name|, |pid|, and |ppid| fields. """ + # pylint: disable=broad-except process_name = process_name or '' processes = [] for line in self._GetPsOutput(process_name): row = line.split() try: - row = {k: row[i] for k, i in _PS_COLUMNS.iteritems()} + row = {k: row[i] for k, i in _PS_COLUMNS.items()} if row['pid'] == 'PID' or process_name not in row['name']: # Skip over header and non-matching processes. continue row['pid'] = int(row['pid']) row['ppid'] = int(row['ppid']) - except StandardError: # e.g. IndexError, TypeError, ValueError. + except Exception: # e.g. IndexError, TypeError, ValueError. logging.warning('failed to parse ps line: %r', line) continue processes.append(ProcessInfo(**row)) @@ -3550,10 +3543,10 @@ class DeviceUtils(object): # When using a cache across script invokations, verify that apps have # not been uninstalled. self._cache['package_apk_paths_to_verify'] = set( - self._cache['package_apk_paths'].iterkeys()) + self._cache['package_apk_paths']) package_apk_checksums = obj.get('package_apk_checksums', {}) - for k, v in package_apk_checksums.iteritems(): + for k, v in package_apk_checksums.items(): package_apk_checksums[k] = set(v) self._cache['package_apk_checksums'] = package_apk_checksums device_path_checksums = obj.get('device_path_checksums', {}) @@ -3577,27 +3570,27 @@ class DeviceUtils(object): obj['package_apk_paths'] = self._cache['package_apk_paths'] obj['package_apk_checksums'] = self._cache['package_apk_checksums'] # JSON can't handle sets. - for k, v in obj['package_apk_checksums'].iteritems(): + for k, v in obj['package_apk_checksums'].items(): obj['package_apk_checksums'][k] = list(v) obj['device_path_checksums'] = self._cache['device_path_checksums'] return json.dumps(obj, separators=(',', ':')) @classmethod - def parallel(cls, devices, async=False): + def parallel(cls, devices, asyn=False): """Creates a Parallelizer to operate over the provided list of devices. Args: devices: A list of either DeviceUtils instances or objects from from which DeviceUtils instances can be constructed. If None, all attached devices will be used. - async: If true, returns a Parallelizer that runs operations + asyn: If true, returns a Parallelizer that runs operations asynchronously. Returns: A Parallelizer operating over |devices|. """ devices = [d if isinstance(d, cls) else cls(d) for d in devices] - if async: + if asyn: return parallelizer.Parallelizer(devices) else: return parallelizer.SyncParallelizer(devices) @@ -3710,7 +3703,7 @@ class DeviceUtils(object): else: reset_usb.reset_all_android_devices() - for attempt in xrange(retries + 1): + for attempt in range(retries + 1): try: return _get_devices() except device_errors.NoDevicesError: @@ -3728,7 +3721,7 @@ class DeviceUtils(object): 'No devices found. Will try again after restarting adb server ' 'and a short nap of %d s.', sleep_s) time.sleep(sleep_s) - RestartServer() + adb_wrapper.RestartServer() @decorators.WithTimeoutAndRetriesFromInstance() def RestartAdbd(self, timeout=None, retries=None): @@ -3745,6 +3738,17 @@ class DeviceUtils(object): if not permissions: return + # For Andorid-11(R), enable MANAGE_EXTERNAL_STORAGE for testing. + # See https://bit.ly/2MBjBIM for details. + if ('android.permission.MANAGE_EXTERNAL_STORAGE' in permissions + and self.build_version_sdk >= version_codes.R): + script_manage_ext_storage = [ + 'appops set {package} MANAGE_EXTERNAL_STORAGE allow', + 'echo "{sep}MANAGE_EXTERNAL_STORAGE{sep}$?{sep}"', + ] + else: + script_manage_ext_storage = [] + permissions = set(p for p in permissions if not _PERMISSIONS_DENYLIST_RE.match(p)) @@ -3752,10 +3756,15 @@ class DeviceUtils(object): and 'android.permission.READ_EXTERNAL_STORAGE' not in permissions): permissions.add('android.permission.READ_EXTERNAL_STORAGE') - script = ';'.join([ - 'p={package}', 'for q in {permissions}', 'do pm grant "$p" "$q"', - 'echo "{sep}$q{sep}$?{sep}"', 'done' - ]).format( + script_raw = [ + 'p={package}', + 'for q in {permissions}', + 'do pm grant "$p" "$q"', + 'echo "{sep}$q{sep}$?{sep}"', + 'done', + ] + script_manage_ext_storage + + script = ';'.join(script_raw).format( package=cmd_helper.SingleQuote(package), permissions=' '.join( cmd_helper.SingleQuote(p) for p in sorted(permissions)), diff --git a/catapult/devil/devil/android/device_utils_test.py b/catapult/devil/devil/android/device_utils_test.py index 62313c5b..8e583f01 100755 --- a/catapult/devil/devil/android/device_utils_test.py +++ b/catapult/devil/devil/android/device_utils_test.py @@ -11,6 +11,7 @@ Unit tests for the contents of device_utils.py (mostly DeviceUtils). import collections import contextlib +import io import json import logging import os @@ -19,6 +20,8 @@ import stat import sys import unittest +import six + from devil import devil_env from devil.android import device_errors from devil.android import device_signal @@ -117,9 +120,10 @@ class DeviceUtilsInitTest(unittest.TestCase): self.assertEqual(serial_as_str, d.adb.GetDeviceSerial()) def testInitWithUnicode(self): - serial_as_unicode = unicode('fedcba9876543210') - d = device_utils.DeviceUtils(serial_as_unicode) - self.assertEqual(serial_as_unicode, d.adb.GetDeviceSerial()) + if six.PY2: + serial_as_unicode = unicode('fedcba9876543210') + d = device_utils.DeviceUtils(serial_as_unicode) + self.assertEqual(serial_as_unicode, d.adb.GetDeviceSerial()) def testInitWithAdbWrapper(self): serial = '123456789abcdef0' @@ -162,12 +166,12 @@ class DeviceUtilsRestartServerTest(mock_calls.TestCase): ['pgrep', 'adb']), (1, '')), (mock.call.devil.utils.cmd_helper.GetCmdStatusAndOutput( ['pgrep', 'adb']), (0, '123\n'))): - device_utils.RestartServer() + adb_wrapper.RestartServer() class MockTempFile(object): def __init__(self, name='/tmp/some/file'): - self.file = mock.MagicMock(spec=file) + self.file = mock.MagicMock(spec=io.BufferedIOBase) self.file.name = name self.file.name_quoted = cmd_helper.SingleQuote(name) @@ -217,6 +221,12 @@ class DeviceUtilsTest(mock_calls.TestCase): self.adb, default_timeout=10, default_retries=0) self.watchMethodCalls(self.call.adb, ignore=['GetDeviceSerial']) + def safeAssertItemsEqual(self, expected, actual): + if six.PY2: + self.assertItemsEqual(expected, actual) + else: + self.assertCountEqual(expected, actual) # pylint: disable=no-member + def AdbCommandError(self, args=None, output=None, status=None, msg=None): if args is None: args = ['[unspecified]'] @@ -2194,11 +2204,16 @@ class DeviceUtilsPushChangedFilesZippedTest(DeviceUtilsTest): self.assertFalse( self.device._PushChangedFilesZipped(test_files, ['/test/dir'])) - def _testPushChangedFilesZipped_spec(self, test_files): + def _testPushChangedFilesZipped_spec(self, test_files, test_dirs): @contextlib.contextmanager def mock_zip_temp_dir(): yield '/test/temp/dir' + expected_cmd = ''.join([ + '\n /data/local/tmp/bin/unzip %s &&', + ' (for dir in %s\n do\n chmod -R 777 "$dir" || exit 1\n', + ' done)\n' + ]) % ('/sdcard/foo123.zip', ' '.join(test_dirs)) with self.assertCalls( (self.call.device._MaybeInstallCommands(), True), (mock.call.py_utils.tempfile_ext.NamedTemporaryDirectory(), @@ -2207,25 +2222,27 @@ class DeviceUtilsPushChangedFilesZippedTest(DeviceUtilsTest): (mock.call.os.path.getsize('/test/temp/dir/tmp.zip'), 123), (self.call.device.NeedsSU(), True), (mock.call.devil.android.device_temp_file.DeviceTempFile( - self.adb, suffix='.zip'), MockTempFile('/test/sdcard/foo123.zip')), - self.call.adb.Push('/test/temp/dir/tmp.zip', '/test/sdcard/foo123.zip'), - self.call.device.RunShellCommand( - 'unzip /test/sdcard/foo123.zip&&chmod -R 777 /test/dir', - shell=True, - as_root=True, - env={'PATH': '/data/local/tmp/bin:$PATH'}, - check_return=True)): + self.adb, suffix='.zip'), MockTempFile('/sdcard/foo123.zip')), + self.call.adb.Push('/test/temp/dir/tmp.zip', '/sdcard/foo123.zip'), + (mock.call.devil.android.device_temp_file.DeviceTempFile( + self.adb, suffix='.sh'), MockTempFile('/sdcard/temp-123.sh')), + self.call.device.WriteFile('/sdcard/temp-123.sh', expected_cmd), + (self.call.device.RunShellCommand(['source', '/sdcard/temp-123.sh'], + check_return=True, + as_root=True))): self.assertTrue( - self.device._PushChangedFilesZipped(test_files, ['/test/dir'])) + self.device._PushChangedFilesZipped(test_files, test_dirs)) def testPushChangedFilesZipped_single(self): - self._testPushChangedFilesZipped_spec([('/test/host/path/file1', - '/test/device/path/file1')]) + self._testPushChangedFilesZipped_spec( + [('/test/host/path/file1', '/test/device/path/file1')], + ['/test/dir1']) def testPushChangedFilesZipped_multiple(self): self._testPushChangedFilesZipped_spec( [('/test/host/path/file1', '/test/device/path/file1'), - ('/test/host/path/file2', '/test/device/path/file2')]) + ('/test/host/path/file2', '/test/device/path/file2')], + ['/test/dir1', '/test/dir2']) class DeviceUtilsPathExistsTest(DeviceUtilsTest): @@ -2374,7 +2391,8 @@ class DeviceUtilsReadFileTest(DeviceUtilsTest): with self.assertCalls( (mock.call.tempfile.mkdtemp(), tmp_host_dir), (self.call.adb.Pull('/path/to/device/file', mock.ANY)), - (mock.call.__builtin__.open(mock.ANY, 'r'), tmp_host), + (mock.call.__builtin__.open(mock.ANY, 'r'), tmp_host) if six.PY2 else \ + (mock.call.builtins.open(mock.ANY, 'r'), tmp_host), (mock.call.os.path.exists(tmp_host_dir), True), (mock.call.shutil.rmtree(tmp_host_dir), None)): self.assertEquals('some interesting contents', @@ -2587,8 +2605,8 @@ class DeviceUtilsStatDirectoryTest(DeviceUtilsTest): self.getStatEntries(path_given='/foo/bar', path_listed='/foo/bar/') def testStatDirectory_fileList(self): - self.assertItemsEqual(self.getStatEntries().keys(), self.FILENAMES) - self.assertItemsEqual(self.getListEntries(), self.FILENAMES) + self.safeAssertItemsEqual(self.getStatEntries().keys(), self.FILENAMES) + self.safeAssertItemsEqual(self.getListEntries(), self.FILENAMES) def testStatDirectory_fileModes(self): expected_modes = ( @@ -2656,7 +2674,7 @@ class DeviceUtilsStatDirectoryTest(DeviceUtilsTest): def testStatDirectory_symbolicLinks(self): entries = self.getStatEntries() self.assertEqual(entries['lnk']['symbolic_link_to'], '/a/path') - for d in entries.itervalues(): + for d in entries.values(): self.assertEqual('symbolic_link_to' in d, stat.S_ISLNK(d['st_mode'])) @@ -3463,7 +3481,7 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): device_utils.DeviceUtils.HealthyDevices(device_arg=[], retries=0) @mock.patch('time.sleep') - @mock.patch('devil.android.device_utils.RestartServer') + @mock.patch('devil.android.sdk.adb_wrapper.RestartServer') def testHealthyDevices_EmptyListDeviceArg_no_attached_with_retry( self, mock_restart, mock_sleep): with self.assertCalls( @@ -3481,7 +3499,7 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): mock.call(8), mock.call(16)]) @mock.patch('time.sleep') - @mock.patch('devil.android.device_utils.RestartServer') + @mock.patch('devil.android.sdk.adb_wrapper.RestartServer') def testHealthyDevices_EmptyListDeviceArg_no_attached_with_resets( self, mock_restart, mock_sleep): # The reset_usb import fails on windows. Mock the full import here so it can @@ -3557,7 +3575,7 @@ class DeviceUtilsGrantPermissionsTest(DeviceUtilsTest): def _PmGrantShellCall(self, package, permissions): fragment = 'p=%s;for q in %s;' % (package, ' '.join(sorted(permissions))) results = [] - for permission, result in sorted(permissions.iteritems()): + for permission, result in sorted(permissions.items()): if result: output, status = result + '\n', 1 else: @@ -3610,6 +3628,23 @@ class DeviceUtilsGrantPermissionsTest(DeviceUtilsTest): self.device.GrantPermissions('package', [WRITE]) self.assertEqual(logger.warnings, []) + def testGrantPermissions_ManageExtrnalStorage(self): + with PatchLogger() as logger: + with self.patch_call(self.call.device.build_version_sdk, + return_value=version_codes.R): + with self.assertCalls( + (self.call.device.RunShellCommand( + AnyStringWith('appops set pkg MANAGE_EXTERNAL_STORAGE allow'), + shell=True, + raw_output=True, + large_output=True, + check_return=True), + '{sep}MANAGE_EXTERNAL_STORAGE{sep}0{sep}\n'.format( + sep=device_utils._SHELL_OUTPUT_SEPARATOR))): + self.device.GrantPermissions( + 'pkg', ['android.permission.MANAGE_EXTERNAL_STORAGE']) + self.assertEqual(logger.warnings, []) + def testGrantPermissions_DenyList(self): with PatchLogger() as logger: with self.patch_call( @@ -3866,6 +3901,12 @@ class IterPushableComponentsTest(unittest.TestCase): yield Layout(layout_root, basic_file, symlink, symlink_dir, dir1, dir2) + def safeAssertItemsEqual(self, expected, actual): + if six.PY2: + self.assertItemsEqual(expected, actual) + else: + self.assertCountEqual(expected, actual) # pylint: disable=no-member + def testFile(self): with self.sampleLayout() as layout: device_path = '/sdcard/basic_file' @@ -3873,7 +3914,7 @@ class IterPushableComponentsTest(unittest.TestCase): expected = [(layout.basic_file, device_path, True)] actual = list( device_utils._IterPushableComponents(layout.basic_file, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) def testSymlinkFile(self): with self.sampleLayout() as layout: @@ -3883,7 +3924,7 @@ class IterPushableComponentsTest(unittest.TestCase): actual = list( device_utils._IterPushableComponents(layout.symlink_file, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) def testDirectoryWithNoSymlink(self): with self.sampleLayout() as layout: @@ -3893,7 +3934,7 @@ class IterPushableComponentsTest(unittest.TestCase): actual = list( device_utils._IterPushableComponents(layout.dir_without_symlinks, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) def testDirectoryWithSymlink(self): with self.sampleLayout() as layout: @@ -3910,7 +3951,7 @@ class IterPushableComponentsTest(unittest.TestCase): actual = list( device_utils._IterPushableComponents(layout.dir_with_symlinks, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) def testSymlinkDirectory(self): with self.sampleLayout() as layout: @@ -3919,7 +3960,7 @@ class IterPushableComponentsTest(unittest.TestCase): expected = [(os.path.realpath(layout.symlink_dir), device_path, False)] actual = list( device_utils._IterPushableComponents(layout.symlink_dir, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) def testDirectoryWithNestedSymlink(self): with self.sampleLayout() as layout: @@ -3947,7 +3988,7 @@ class IterPushableComponentsTest(unittest.TestCase): ] actual = list( device_utils._IterPushableComponents(layout.root, device_path)) - self.assertItemsEqual(expected, actual) + self.safeAssertItemsEqual(expected, actual) class DeviceUtilsGetTracingPathTest(DeviceUtilsTest): diff --git a/catapult/devil/devil/android/fastboot_utils_test.py b/catapult/devil/devil/android/fastboot_utils_test.py index 1ad73190..ed891393 100755 --- a/catapult/devil/devil/android/fastboot_utils_test.py +++ b/catapult/devil/devil/android/fastboot_utils_test.py @@ -13,6 +13,8 @@ import io import logging import unittest +import six + from devil import devil_env from devil.android import device_errors from devil.android import device_utils @@ -347,16 +349,22 @@ class FastbootUtilsFastbootMode(FastbootUtilsTest): pass +if six.PY2: + _BUILTIN_OPEN = '__builtin__.open' +else: + _BUILTIN_OPEN = 'builtins.open' + + class FastbootUtilsVerifyBoard(FastbootUtilsTest): def testVerifyBoard_bothValid(self): mock_file = io.StringIO(u'require board=%s\n' % _BOARD) - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_VALID_FILES): self.assertTrue(self.fastboot._VerifyBoard('test')) def testVerifyBoard_BothNotValid(self): mock_file = io.StringIO(u'abc') - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_INVALID_FILES): self.assertFalse(self.assertFalse(self.fastboot._VerifyBoard('test'))) @@ -366,31 +374,31 @@ class FastbootUtilsVerifyBoard(FastbootUtilsTest): def testVerifyBoard_ZipNotFoundFileValid(self): mock_file = io.StringIO(u'require board=%s\n' % _BOARD) - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=['android-info.txt']): self.assertTrue(self.fastboot._VerifyBoard('test')) def testVerifyBoard_zipNotValidFileIs(self): mock_file = io.StringIO(u'require board=%s\n' % _BOARD) - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_INVALID_FILES): self.assertTrue(self.fastboot._VerifyBoard('test')) def testVerifyBoard_fileNotValidZipIs(self): mock_file = io.StringIO(u'require board=WrongBoard') - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_VALID_FILES): self.assertFalse(self.fastboot._VerifyBoard('test')) def testVerifyBoard_noBoardInFileValidZip(self): mock_file = io.StringIO(u'Regex wont match') - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_VALID_FILES): self.assertTrue(self.fastboot._VerifyBoard('test')) def testVerifyBoard_noBoardInFileInvalidZip(self): mock_file = io.StringIO(u'Regex wont match') - with mock.patch('__builtin__.open', return_value=mock_file, create=True): + with mock.patch(_BUILTIN_OPEN, return_value=mock_file, create=True): with mock.patch('os.listdir', return_value=_INVALID_FILES): self.assertFalse(self.fastboot._VerifyBoard('test')) @@ -419,7 +427,7 @@ class FastbootUtilsFindAndVerifyPartitionsAndImages(FastbootUtilsTest): ] with mock.patch('os.listdir', return_value=files): imgs = self.fastboot._FindAndVerifyPartitionsAndImages(PARTITIONS, 'test') - parts = imgs.keys() + parts = list(imgs.keys()) self.assertDictEqual(imgs, img_check) self.assertListEqual(parts, parts_check) @@ -451,7 +459,7 @@ class FastbootUtilsFindAndVerifyPartitionsAndImages(FastbootUtilsTest): with self.patch_call(self.call.fastboot.supports_ab, return_value=False): imgs = self.fastboot._FindAndVerifyPartitionsAndImages( PARTITIONS, 'test') - parts = imgs.keys() + parts = list(imgs.keys()) self.assertDictEqual(imgs, img_check) self.assertListEqual(parts, parts_check) diff --git a/catapult/devil/devil/android/flag_changer_test.py b/catapult/devil/devil/android/flag_changer_test.py index 564ead6e..9c155f13 100755 --- a/catapult/devil/devil/android/flag_changer_test.py +++ b/catapult/devil/devil/android/flag_changer_test.py @@ -6,6 +6,8 @@ import posixpath import unittest +import six + from devil.android import flag_changer _CMDLINE_FILE = 'chrome-command-line' @@ -109,15 +111,22 @@ class ParseSerializeFlagsTest(unittest.TestCase): def _testParseCmdLine(self, command_line, expected_flags): # Start with a command line, check that flags are parsed as expected. # pylint: disable=protected-access + # pylint: disable=no-member flags = flag_changer._ParseFlags(command_line) - self.assertItemsEqual(flags, expected_flags) + if six.PY2: + self.assertItemsEqual(flags, expected_flags) + else: + self.assertCountEqual(flags, expected_flags) # Check that flags survive a round-trip. # Note: Although new_command_line and command_line may not match, they # should describe the same set of flags. new_command_line = flag_changer._SerializeFlags(flags) new_flags = flag_changer._ParseFlags(new_command_line) - self.assertItemsEqual(new_flags, expected_flags) + if six.PY2: + self.assertItemsEqual(new_flags, expected_flags) + else: + self.assertCountEqual(new_flags, expected_flags) def testParseCmdLine_simple(self): self._testParseCmdLine('chrome --foo --bar="a b" --baz=true --fine="ok"', diff --git a/catapult/devil/devil/android/logcat_monitor.py b/catapult/devil/devil/android/logcat_monitor.py index bec74440..df306b0a 100644 --- a/catapult/devil/devil/android/logcat_monitor.py +++ b/catapult/devil/devil/android/logcat_monitor.py @@ -13,6 +13,8 @@ import tempfile import threading import time +import six + from devil.android import decorators from devil.android import device_errors from devil.android.sdk import adb_wrapper @@ -102,9 +104,9 @@ class LogcatMonitor(object): raise LogcatMonitorCommandError( 'Must be recording logcat when calling |WaitFor|', device_serial=str(self._adb)) - if isinstance(success_regex, basestring): + if isinstance(success_regex, six.string_types): success_regex = re.compile(success_regex) - if isinstance(failure_regex, basestring): + if isinstance(failure_regex, six.string_types): failure_regex = re.compile(failure_regex) logger.debug('Waiting %d seconds for "%s"', timeout, success_regex.pattern) @@ -220,10 +222,14 @@ class LogcatMonitor(object): Clears the logcat if |clear| was set in |__init__|. """ + # pylint: disable=unexpected-keyword-arg if self._clear: self._adb.Logcat(clear=True) if not self._record_file: - self._record_file = tempfile.NamedTemporaryFile(mode='a', bufsize=1) + if six.PY2: + self._record_file = tempfile.NamedTemporaryFile(mode='a', bufsize=1) + else: + self._record_file = tempfile.NamedTemporaryFile(mode='a', buffering=1) self._StartRecording() def Stop(self): diff --git a/catapult/devil/devil/android/logcat_monitor_test.py b/catapult/devil/devil/android/logcat_monitor_test.py index 7f2f10a6..356fe041 100755 --- a/catapult/devil/devil/android/logcat_monitor_test.py +++ b/catapult/devil/devil/android/logcat_monitor_test.py @@ -9,6 +9,8 @@ import itertools import threading import unittest +import six + from devil import devil_env from devil.android import logcat_monitor from devil.android.sdk import adb_wrapper @@ -24,6 +26,13 @@ def _CreateTestLog(raw_logcat=None): return test_log +def zip_longest(expected, actual): + # pylint: disable=no-member + if six.PY2: + return itertools.izip_longest(expected, actual) + else: + return itertools.zip_longest(expected, actual) + class LogcatMonitorTest(unittest.TestCase): _TEST_THREADTIME_LOGCAT_DATA = [ @@ -44,7 +53,7 @@ class LogcatMonitorTest(unittest.TestCase): ] def assertIterEqual(self, expected_iter, actual_iter): - for expected, actual in itertools.izip_longest(expected_iter, actual_iter): + for expected, actual in zip_longest(expected_iter, actual_iter): self.assertIsNotNone( expected, msg='actual has unexpected elements starting with %s' % str(actual)) diff --git a/catapult/devil/devil/android/md5sum.py b/catapult/devil/devil/android/md5sum.py index 8adf4ef7..e67f3f60 100644 --- a/catapult/devil/devil/android/md5sum.py +++ b/catapult/devil/devil/android/md5sum.py @@ -3,10 +3,12 @@ # found in the LICENSE file. import base64 +import io import gzip import os import re -import StringIO + +import six from devil import devil_env from devil.android import device_errors @@ -38,7 +40,7 @@ def CalculateHostMd5Sums(paths): Returns: A dict mapping file paths to their respective md5sum checksums. """ - if isinstance(paths, basestring): + if isinstance(paths, six.string_types): paths = [paths] paths = list(paths) @@ -47,13 +49,17 @@ def CalculateHostMd5Sums(paths): raise IOError('File not built: %s' % md5sum_bin_host_path) out = "" for i in range(0, len(paths), _MAX_PATHS_PER_INVOCATION): - mem_file = StringIO.StringIO() + mem_file = io.BytesIO() compressed = gzip.GzipFile(fileobj=mem_file, mode="wb") - compressed.write(";".join( - [os.path.realpath(p) for p in paths[i:i+_MAX_PATHS_PER_INVOCATION]])) + data = ";".join( + [os.path.realpath(p) for p in paths[i:i+_MAX_PATHS_PER_INVOCATION]]) + if six.PY3: + data = data.encode('utf-8') + compressed.write(data) compressed.close() compressed_paths = base64.b64encode(mem_file.getvalue()) - out += cmd_helper.GetCmdOutput([md5sum_bin_host_path, "-gz", compressed_paths]) + out += cmd_helper.GetCmdOutput( + [md5sum_bin_host_path, "-gz", compressed_paths]) return dict(zip(paths, out.splitlines())) @@ -72,7 +78,7 @@ def CalculateDeviceMd5Sums(paths, device): if not paths: return {} - if isinstance(paths, basestring): + if isinstance(paths, six.string_types): paths = [paths] paths = list(paths) @@ -97,9 +103,12 @@ def CalculateDeviceMd5Sums(paths, device): # Make sure it can find libbase.so md5sum_script += 'export LD_LIBRARY_PATH=%s;' % MD5SUM_DEVICE_LIB_PATH for i in range(0, len(paths), _MAX_PATHS_PER_INVOCATION): - mem_file = StringIO.StringIO() + mem_file = io.BytesIO() compressed = gzip.GzipFile(fileobj=mem_file, mode="wb") - compressed.write(";".join(paths[i:i+_MAX_PATHS_PER_INVOCATION])) + data = ";".join(paths[i:i+_MAX_PATHS_PER_INVOCATION]) + if six.PY3: + data = data.encode('utf-8') + compressed.write(data) compressed.close() compressed_paths = base64.b64encode(mem_file.getvalue()) md5sum_script += '$a -gz %s;' % compressed_paths diff --git a/catapult/devil/devil/android/md5sum_test.py b/catapult/devil/devil/android/md5sum_test.py index 548b2d02..9a51313e 100755 --- a/catapult/devil/devil/android/md5sum_test.py +++ b/catapult/devil/devil/android/md5sum_test.py @@ -112,7 +112,7 @@ class Md5SumTest(unittest.TestCase): def testCalculateDeviceMd5Sums_generator(self): test_path = ('/storage/emulated/legacy/test/file%d.dat' % n - for n in xrange(0, 2)) + for n in range(0, 2)) device = mock.NonCallableMock() device_md5sum_output = [ diff --git a/catapult/devil/devil/android/perf/perf_control.py b/catapult/devil/devil/android/perf/perf_control.py index 59485e0e..398b27fa 100644 --- a/catapult/devil/devil/android/perf/perf_control.py +++ b/catapult/devil/devil/android/perf/perf_control.py @@ -203,7 +203,7 @@ class PerfControl(object): if not isinstance(cpu_max_freq, dict): self._SetScalingMaxFreqForCpus(cpu_max_freq, self._cpu_file_list) else: - for key, max_frequency in cpu_max_freq.iteritems(): + for key, max_frequency in cpu_max_freq.items(): # Convert 'X' to 'cpuX' and 'X..Y' to 'cpuX cpu<X+1> .. cpuY'. if '..' in key: range_min, range_max = key.split('..') @@ -211,7 +211,7 @@ class PerfControl(object): else: range_min = range_max = int(key) cpu_files = [ - 'cpu%d' % number for number in xrange(range_min, range_max + 1) + 'cpu%d' % number for number in range(range_min, range_max + 1) ] # Set the |max_frequency| on requested subset of the cores. self._SetScalingMaxFreqForCpus(max_frequency, ' '.join(cpu_files)) diff --git a/catapult/devil/devil/android/perf/perf_control_test.py b/catapult/devil/devil/android/perf/perf_control_test.py index bde54d5a..a841a0e2 100644 --- a/catapult/devil/devil/android/perf/perf_control_test.py +++ b/catapult/devil/devil/android/perf/perf_control_test.py @@ -47,7 +47,7 @@ class PerfControlTest(unittest.TestCase): # pylint: disable=no-self-use def testNexus5HighPerfMode(self): # Mock out the device state for PerfControl. - cpu_list = ['cpu%d' % cpu for cpu in xrange(4)] + cpu_list = ['cpu%d' % cpu for cpu in range(4)] mock_device = mock.Mock(spec=device_utils.DeviceUtils) mock_device.product_model = 'Nexus 5' mock_device.adb = mock.Mock(spec=adb_wrapper.AdbWrapper) @@ -70,7 +70,7 @@ class PerfControlTest(unittest.TestCase): def testNexus5XHighPerfMode(self): # Mock out the device state for PerfControl. - cpu_list = ['cpu%d' % cpu for cpu in xrange(6)] + cpu_list = ['cpu%d' % cpu for cpu in range(6)] mock_device = mock.Mock(spec=device_utils.DeviceUtils) mock_device.product_model = 'Nexus 5X' mock_device.adb = mock.Mock(spec=adb_wrapper.AdbWrapper) @@ -93,7 +93,7 @@ class PerfControlTest(unittest.TestCase): def testNexus5XDefaultPerfMode(self): # Mock out the device state for PerfControl. - cpu_list = ['cpu%d' % cpu for cpu in xrange(6)] + cpu_list = ['cpu%d' % cpu for cpu in range(6)] mock_device = mock.Mock(spec=device_utils.DeviceUtils) mock_device.product_model = 'Nexus 5X' mock_device.adb = mock.Mock(spec=adb_wrapper.AdbWrapper) diff --git a/catapult/devil/devil/android/perf/surface_stats_collector.py b/catapult/devil/devil/android/perf/surface_stats_collector.py index 6240624f..4ddc6f5d 100644 --- a/catapult/devil/devil/android/perf/surface_stats_collector.py +++ b/catapult/devil/devil/android/perf/surface_stats_collector.py @@ -3,10 +3,16 @@ # found in the LICENSE file. import logging -import Queue import re import threading +import six + +if six.PY3: + import queue # pylint: disable=wrong-import-order +else: + import Queue as queue # pylint: disable=wrong-import-order + # Log marker containing SurfaceTexture timestamps. _SURFACE_TEXTURE_TIMESTAMPS_MESSAGE = 'SurfaceTexture update timestamps' _SURFACE_TEXTURE_TIMESTAMP_RE = r'\d+' @@ -37,7 +43,7 @@ class SurfaceStatsCollector(object): if self._ClearSurfaceFlingerLatencyData(): self._get_data_event = threading.Event() self._stop_event = threading.Event() - self._data_queue = Queue.Queue() + self._data_queue = queue.Queue() self._collector_thread = threading.Thread(target=self._CollectorThread) self._collector_thread.start() else: @@ -148,6 +154,10 @@ class SurfaceStatsCollector(object): return ParseFrameData(output, parse_timestamps=bool(window_name)) +def to_long_int(val): + """Cast val to a long int type.""" + return long(val) if six.PY2 else int(val) + def ParseFrameData(lines, parse_timestamps): # adb shell dumpsys SurfaceFlinger --latency <window name> # prints some information about the last 128 frames displayed in @@ -174,6 +184,7 @@ def ParseFrameData(lines, parse_timestamps): # (each time the number above changes, we have a "jank"). # If this happens a lot during an animation, the animation appears # janky, even if it runs at 60 fps in average. + # pylint: disable=redefined-variable-type results = [] for line in lines: # Skip over lines with anything other than digits and whitespace. @@ -186,7 +197,8 @@ def ParseFrameData(lines, parse_timestamps): timestamps = [] nanoseconds_per_millisecond = 1e6 - refresh_period = long(results[0]) / nanoseconds_per_millisecond + refresh_period = to_long_int(results[0]) / nanoseconds_per_millisecond + if not parse_timestamps: return refresh_period, timestamps @@ -201,7 +213,8 @@ def ParseFrameData(lines, parse_timestamps): if len(fields) != 3: logging.warning('Unexpected line: %s', line) continue - timestamp = long(fields[1]) + timestamp = to_long_int(fields[1]) + if timestamp == pending_fence_timestamp: continue timestamp /= nanoseconds_per_millisecond diff --git a/catapult/devil/devil/android/ports.py b/catapult/devil/devil/android/ports.py index f25c8bfd..4a7c2945 100644 --- a/catapult/devil/devil/android/ports.py +++ b/catapult/devil/devil/android/ports.py @@ -159,7 +159,7 @@ def IsHttpServerConnectable(host, message the server returns when connect status is false. """ assert tries >= 1 - for i in xrange(0, tries): + for i in range(0, tries): client_error = None try: with contextlib.closing( diff --git a/catapult/devil/devil/android/sdk/aapt.py b/catapult/devil/devil/android/sdk/aapt.py index 76c7ef68..fd354079 100644 --- a/catapult/devil/devil/android/sdk/aapt.py +++ b/catapult/devil/devil/android/sdk/aapt.py @@ -3,6 +3,8 @@ # found in the LICENSE file. """This module wraps the Android Asset Packaging Tool.""" +import six + from devil.android.sdk import build_tools from devil.utils import cmd_helper from devil.utils import lazy @@ -36,6 +38,6 @@ def Dump(what, apk, assets=None): assets: List of assets in apk you want to dump information for. """ assets = assets or [] - if isinstance(assets, basestring): + if isinstance(assets, six.string_types): assets = [assets] return _RunAaptCmd(['dump', what, apk] + assets).splitlines() diff --git a/catapult/devil/devil/android/sdk/adb_wrapper.py b/catapult/devil/devil/android/sdk/adb_wrapper.py index 71928d58..d8992242 100644 --- a/catapult/devil/devil/android/sdk/adb_wrapper.py +++ b/catapult/devil/devil/android/sdk/adb_wrapper.py @@ -18,6 +18,8 @@ import posixpath import re import subprocess +import six + from devil import base_error from devil import devil_env from devil.android import decorators @@ -127,6 +129,30 @@ def _IsExtraneousLine(line, send_cmd): return send_cmd.rstrip() in line +@decorators.WithExplicitTimeoutAndRetries(timeout=60, retries=3) +def RestartServer(): + """Restarts the adb server. + + Raises: + CommandFailedError if we fail to kill or restart the server. + """ + + def adb_killed(): + return not AdbWrapper.IsServerOnline() + + def adb_started(): + return AdbWrapper.IsServerOnline() + + AdbWrapper.KillServer() + if not timeout_retry.WaitFor(adb_killed, wait_period=1, max_tries=5): + # TODO(crbug.com/442319): Switch this to raise an exception if we + # figure out why sometimes not all adb servers on bots get killed. + logger.warning('Failed to kill adb server') + AdbWrapper.StartServer() + if not timeout_retry.WaitFor(adb_started, wait_period=1, max_tries=5): + raise device_errors.CommandFailedError('Failed to start adb server') + + class AdbWrapper(object): """A wrapper around a local Android Debug Bridge executable.""" @@ -1105,7 +1131,7 @@ class AdbWrapper(object): Returns: The output of the emulator console command. """ - if isinstance(cmd, basestring): + if isinstance(cmd, six.string_types): cmd = [cmd] return self._RunDeviceAdbCmd(['emu'] + cmd, timeout, retries) diff --git a/catapult/devil/devil/android/sdk/dexdump.py b/catapult/devil/devil/android/sdk/dexdump.py index 2a59e9bf..c71442ce 100644 --- a/catapult/devil/devil/android/sdk/dexdump.py +++ b/catapult/devil/devil/android/sdk/dexdump.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import six + from devil.android.sdk import build_tools from devil.utils import cmd_helper from devil.utils import lazy @@ -20,7 +22,7 @@ def DexDump(dexfiles, file_summary=False): An iterable over the output lines. """ # TODO(jbudorick): Add support for more options as necessary. - if isinstance(dexfiles, basestring): + if isinstance(dexfiles, six.string_types): dexfiles = [dexfiles] args = [_dexdump_path.read()] + dexfiles if file_summary: diff --git a/catapult/devil/devil/android/sdk/intent.py b/catapult/devil/devil/android/sdk/intent.py index 69c7d900..2ea38c33 100644 --- a/catapult/devil/devil/android/sdk/intent.py +++ b/catapult/devil/devil/android/sdk/intent.py @@ -116,7 +116,7 @@ class Intent(object): if self.flags: args.extend(['-f', self.flags]) if self.extras: - for key, value in self.extras.iteritems(): + for key, value in self.extras.items(): if value is None: args.extend(['--esn', key]) elif isinstance(value, str): diff --git a/catapult/devil/devil/android/sdk/shared_prefs.py b/catapult/devil/devil/android/sdk/shared_prefs.py index 7b12bf54..32b5bc4d 100644 --- a/catapult/devil/devil/android/sdk/shared_prefs.py +++ b/catapult/devil/devil/android/sdk/shared_prefs.py @@ -11,6 +11,8 @@ import logging import posixpath from xml.etree import ElementTree +import six + from devil.android import device_errors from devil.android.sdk import version_codes @@ -43,7 +45,10 @@ class BasePref(object): def __str__(self): """Get the underlying xml element as a string.""" - return ElementTree.tostring(self._elem) + if six.PY2: + return ElementTree.tostring(self._elem) + else: + return ElementTree.tostring(self._elem, encoding="unicode") def get(self): """Get the value of this preference.""" @@ -231,7 +236,11 @@ class SharedPrefs(object): def __str__(self): """Get the underlying xml document as a string.""" - return _XML_DECLARATION + ElementTree.tostring(self.xml) + if six.PY2: + return _XML_DECLARATION + ElementTree.tostring(self.xml) + else: + return _XML_DECLARATION + \ + ElementTree.tostring(self.xml, encoding="unicode") @property def package(self): diff --git a/catapult/devil/devil/android/sdk/shared_prefs_test.py b/catapult/devil/devil/android/sdk/shared_prefs_test.py index 7374c892..b50d9e0b 100755 --- a/catapult/devil/devil/android/sdk/shared_prefs_test.py +++ b/catapult/devil/devil/android/sdk/shared_prefs_test.py @@ -132,6 +132,8 @@ class SharedPrefsTest(unittest.TestCase): }) # data survived roundtrip def testForceCommit(self): + type(self.device).build_version_sdk = mock.PropertyMock( + return_value=version_codes.LOLLIPOP_MR1) prefs = shared_prefs.SharedPrefs(self.device, 'com.some.package', 'prefs.xml') prefs.Load() diff --git a/catapult/devil/devil/android/sdk/version_codes.py b/catapult/devil/devil/android/sdk/version_codes.py index 943b9d3f..564f30da 100644 --- a/catapult/devil/devil/android/sdk/version_codes.py +++ b/catapult/devil/devil/android/sdk/version_codes.py @@ -20,3 +20,4 @@ OREO = 26 OREO_MR1 = 27 PIE = 28 Q = 29 +R = 30 diff --git a/catapult/devil/devil/android/tools/device_monitor.py b/catapult/devil/devil/android/tools/device_monitor.py index 26e89a24..730df141 100755 --- a/catapult/devil/devil/android/tools/device_monitor.py +++ b/catapult/devil/devil/android/tools/device_monitor.py @@ -184,7 +184,7 @@ def get_all_status(denylist): } if denylist: - for device, reason in denylist.Read().iteritems(): + for device, reason in denylist.Read().items(): status_dict['devices'][device] = { 'state': reason.get('reason', 'denylisted') } diff --git a/catapult/devil/devil/android/tools/device_monitor_test.py b/catapult/devil/devil/android/tools/device_monitor_test.py index 8082d26e..1bb5680a 100755 --- a/catapult/devil/devil/android/tools/device_monitor_test.py +++ b/catapult/devil/devil/android/tools/device_monitor_test.py @@ -7,6 +7,8 @@ import os import sys import unittest +import six + if __name__ == '__main__': sys.path.append( os.path.abspath( @@ -53,7 +55,7 @@ class DeviceMonitorTest(unittest.TestCase): } def mock_run_shell(cmd, **_kwargs): - args = cmd.split() if isinstance(cmd, basestring) else cmd + args = cmd.split() if isinstance(cmd, six.string_types) else cmd try: return self.cmd_outputs[args[0]] except KeyError: diff --git a/catapult/devil/devil/android/tools/system_app.py b/catapult/devil/devil/android/tools/system_app.py index 62bf5a59..50d85595 100755 --- a/catapult/devil/devil/android/tools/system_app.py +++ b/catapult/devil/devil/android/tools/system_app.py @@ -22,6 +22,7 @@ from devil.android import decorators from devil.android import device_errors from devil.android import device_temp_file from devil.android.sdk import version_codes +from devil.android.sdk import adb_wrapper from devil.android.tools import script_common from devil.utils import cmd_helper from devil.utils import parallelizer @@ -128,7 +129,15 @@ _ENABLE_MODIFICATION_PROP = 'devil.modify_sys_apps' def _ShouldRetryModification(exc): - return not isinstance(exc, device_errors.CommandTimeoutError) + try: + if isinstance(exc, device_errors.CommandTimeoutError): + logger.info('Restarting the adb server') + adb_wrapper.RestartServer() + return True + except Exception: # pylint: disable=broad-except + logger.exception(('Caught an exception when deciding' + ' to retry system modification')) + return False # timeout and retries are both required by the decorator, but neither @@ -172,7 +181,7 @@ def _SetUpSystemAppModification(device, timeout=None, retries=None): # Point the user to documentation, since there's a good chance they can # workaround this on an emulator. docs_url = ('https://chromium.googlesource.com/chromium/src/+/' - 'master/docs/android_emulator.md#writable-system-partition') + 'HEAD/docs/android_emulator.md#writable-system-partition') logger.error( 'Did you start the emulator with "-writable-system?"\n' 'See %s\n', docs_url) @@ -187,6 +196,22 @@ def _TearDownSystemAppModification(device, timeout=None, retries=None): try: + # The function may be re-entered after the the device loses root + # privilege. For instance if the adb server is restarted before + # re-entering the function then the device may lose root privilege. + # Therefore we need to do a sanity check for root privilege + # on the device and then re-enable root privilege if the device + # does not have it. + if not device.HasRoot(): + logger.warning('Need to re-enable root.') + device.EnableRoot() + + if not device.HasRoot(): + raise device_errors.CommandFailedError( + ('Failed to tear down modification of ' + 'system apps on non-rooted device.'), + str(device)) + device.SetProp(_ENABLE_MODIFICATION_PROP, '0') device.Reboot() device.WaitUntilFullyBooted() diff --git a/catapult/devil/devil/android/tools/video_recorder.py b/catapult/devil/devil/android/tools/video_recorder.py index 984931f3..4004c467 100755 --- a/catapult/devil/devil/android/tools/video_recorder.py +++ b/catapult/devil/devil/android/tools/video_recorder.py @@ -164,7 +164,7 @@ def main(): parallel_devices = device_utils.DeviceUtils.parallel(script_common.GetDevices( args.devices, args.denylist_file), - async=True) + asyn=True) stop_recording = threading.Event() running_recording = parallel_devices.pMap(record_video, stop_recording) print 'Recording. Press Enter to stop.', |