Check Android devices exist before calling commands on them. (#743)
`AndroidDevice` instantiation calls `adb root` on devices if the device
is rootable. And today, we instantiate the device objects before we
check if the device exists at all, which is logically incorrect.
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py
index 0438cba..d7a7813 100644
--- a/mobly/controllers/android_device.py
+++ b/mobly/controllers/android_device.py
@@ -110,13 +110,6 @@
ads = get_instances(configs)
else:
raise Error('No valid config found in: %s' % configs)
- valid_ad_identifiers = list_adb_devices() + list_adb_devices_by_usb_id()
-
- for ad in ads:
- if ad.serial not in valid_ad_identifiers:
- raise DeviceError(
- ad, 'Android device is specified in config but is not '
- 'attached.')
_start_services_on_ads(ads)
return ads
@@ -146,6 +139,20 @@
return [ad.device_info for ad in ads]
+def _validate_device_existence(serials):
+ """Validate that all the devices specified by the configs can be reached.
+
+ Args:
+ serials: list of strings, the serials of all the devices that are expected
+ to exist.
+ """
+ valid_ad_identifiers = list_adb_devices() + list_adb_devices_by_usb_id()
+ for serial in serials:
+ if serial not in valid_ad_identifiers:
+ raise Error(f'Android device serial "{serial}" is specified in '
+ 'config but is not reachable.')
+
+
def _start_services_on_ads(ads):
"""Starts long running services on multiple AndroidDevice objects.
@@ -247,6 +254,8 @@
Returns:
A list of AndroidDevice objects.
"""
+ _validate_device_existence(serials)
+
results = []
for s in serials:
results.append(AndroidDevice(s))
@@ -265,13 +274,19 @@
Returns:
A list of AndroidDevice objects.
"""
- results = []
+ # First make sure each config contains a serial, and all the serials'
+ # corresponding devices exist.
+ serials = []
for c in configs:
try:
serial = c.pop('serial')
+ serials.append(serial)
except KeyError:
raise Error(
'Required value "serial" is missing in AndroidDevice config %s.' % c)
+ _validate_device_existence(serials)
+ results = []
+ for c in configs:
is_required = c.get(KEY_DEVICE_REQUIRED, True)
try:
ad = AndroidDevice(serial)
diff --git a/tests/mobly/controllers/android_device_test.py b/tests/mobly/controllers/android_device_test.py
index c64c826..1c4be14 100755
--- a/tests/mobly/controllers/android_device_test.py
+++ b/tests/mobly/controllers/android_device_test.py
@@ -27,6 +27,7 @@
from mobly import runtime_test_info
from mobly.controllers import android_device
from mobly.controllers.android_device_lib import adb
+from mobly.controllers.android_device_lib import errors
from mobly.controllers.android_device_lib import snippet_client
from mobly.controllers.android_device_lib.services import base_service
from mobly.controllers.android_device_lib.services import logcat
@@ -136,6 +137,61 @@
with self.assertRaisesRegex(android_device.Error, expected_msg):
android_device.create([1])
+ @mock.patch('mobly.controllers.android_device.list_adb_devices')
+ @mock.patch('mobly.controllers.android_device.list_adb_devices_by_usb_id')
+ @mock.patch('mobly.controllers.android_device.AndroidDevice')
+ def test_get_instances(self, mock_ad_class, mock_list_adb_usb, mock_list_adb):
+ mock_list_adb.return_value = ['1']
+ mock_list_adb_usb.return_value = []
+ android_device.get_instances(['1'])
+ mock_ad_class.assert_called_with('1')
+
+ @mock.patch('mobly.controllers.android_device.list_adb_devices')
+ @mock.patch('mobly.controllers.android_device.list_adb_devices_by_usb_id')
+ @mock.patch('mobly.controllers.android_device.AndroidDevice')
+ def test_get_instances_do_not_exist(self, mock_ad_class, mock_list_adb_usb,
+ mock_list_adb):
+ mock_list_adb.return_value = []
+ mock_list_adb_usb.return_value = []
+ with self.assertRaisesRegex(
+ errors.Error,
+ 'Android device serial "1" is specified in config but is not reachable'
+ ):
+ android_device.get_instances(['1'])
+
+ @mock.patch('mobly.controllers.android_device.list_adb_devices')
+ @mock.patch('mobly.controllers.android_device.list_adb_devices_by_usb_id')
+ @mock.patch('mobly.controllers.android_device.AndroidDevice')
+ def test_get_instances_with_configs(self, mock_ad_class, mock_list_adb_usb,
+ mock_list_adb):
+ mock_list_adb.return_value = ['1']
+ mock_list_adb_usb.return_value = []
+ config = {'serial': '1'}
+ android_device.get_instances_with_configs([config])
+ mock_ad_class.assert_called_with('1')
+
+ def test_get_instances_with_configs_invalid_config(self):
+ config = {'something': 'random'}
+ with self.assertRaisesRegex(
+ errors.Error,
+ f'Required value "serial" is missing in AndroidDevice config {config}'):
+ android_device.get_instances_with_configs([config])
+
+ @mock.patch('mobly.controllers.android_device.list_adb_devices')
+ @mock.patch('mobly.controllers.android_device.list_adb_devices_by_usb_id')
+ @mock.patch('mobly.controllers.android_device.AndroidDevice')
+ def test_get_instances_with_configsdo_not_exist(self, mock_ad_class,
+ mock_list_adb_usb,
+ mock_list_adb):
+ mock_list_adb.return_value = []
+ mock_list_adb_usb.return_value = []
+ config = {'serial': '1'}
+ with self.assertRaisesRegex(
+ errors.Error,
+ 'Android device serial "1" is specified in config but is not reachable'
+ ):
+ android_device.get_instances_with_configs([config])
+
def test_get_devices_success_with_extra_field(self):
ads = mock_android_device.get_mock_ads(5)
expected_label = 'selected'
