Verify adb root took effect and add skip_root option
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py index c0e4845..da95ce0 100644 --- a/mobly/controllers/android_device.py +++ b/mobly/controllers/android_device.py
@@ -71,6 +71,11 @@ # Default is False. KEY_SKIP_LOGCAT = 'skip_logcat' DEFAULT_VALUE_SKIP_LOGCAT = False +# If True, `adb root` will not be run automatically on rootable (debuggable) +# builds when the device is created and after a reboot handled via +# `handle_reboot`. Default is False. +KEY_SKIP_ROOT = 'skip_root' +DEFAULT_VALUE_SKIP_ROOT = False SERVICE_NAME_LOGCAT = 'logcat' # Default name for bug reports taken without a specified test name. @@ -326,8 +331,9 @@ for c in configs: serial = c.pop('serial') is_required = c.get(KEY_DEVICE_REQUIRED, True) + skip_root = c.pop(KEY_SKIP_ROOT, DEFAULT_VALUE_SKIP_ROOT) try: - ad = AndroidDevice(serial) + ad = AndroidDevice(serial, skip_root=skip_root) ad.load_config(c) except Exception: if is_required: @@ -523,9 +529,12 @@ via fastboot. services: ServiceManager, the manager of long-running services on the device. + skip_root: bool, if True, `adb root` is not run automatically on rootable + builds at creation time and after a reboot handled via `handle_reboot`. + Settable via the `skip_root` controller config key. """ - def __init__(self, serial=''): + def __init__(self, serial='', skip_root=DEFAULT_VALUE_SKIP_ROOT): self._serial = str(serial) # logging.log_path only exists when this is used in an Mobly test run. _log_path_base = utils.abs_path(getattr(logging, 'log_path', '/tmp/logs')) @@ -541,8 +550,8 @@ self._is_rebooting = False self.adb = adb.AdbProxy(serial) self.fastboot = fastboot.FastbootProxy(serial) - if self.is_rootable: - self.root_adb() + self.skip_root = skip_root + self._root_adb_if_allowed() self.services = service_manager.ServiceManager(self) self.services.register( SERVICE_NAME_LOGCAT, logcat.Logcat, start_service=False @@ -753,8 +762,7 @@ self._build_info = None self._is_rootable = None self._is_rebooting = False - if self.is_rootable: - self.root_adb() + self._root_adb_if_allowed() self.services.start_services(live_service_names) @contextlib.contextmanager @@ -932,11 +940,38 @@ If executed on a production build, adb will not be switched to root mode per security restrictions. + + Note that `adb root` exits with code 0 even when adbd refuses to restart + as root (e.g. "adbd cannot run as root in production builds"), so the + outcome is verified by checking whether adb is actually running as root + afterwards instead of relying on the command's output. + + Returns: + True if adb is running as root after this call, False otherwise. """ - self.adb.root() + out = self.adb.root() # `root` causes the device to temporarily disappear from adb. # So we need to wait for the device to come back before proceeding. self.adb.wait_for_device(timeout=DEFAULT_TIMEOUT_BOOT_COMPLETION_SECOND) + if self.is_adb_root: + return True + self.log.warning( + '`adb root` did not take effect, continuing without root. Output: %s', + out, + ) + return False + + def _root_adb_if_allowed(self): + """Runs `root_adb` on rootable builds unless `skip_root` is set. + + Called automatically at creation time and after a reboot handled via + `handle_reboot`. + """ + if self.skip_root: + self.log.debug('Skipping automatic adb root because skip_root is set.') + return + if self.is_rootable: + self.root_adb() def load_snippet(self, name, package, config=None): """Starts the snippet apk with the given package name and connects.
diff --git a/tests/lib/mock_android_device.py b/tests/lib/mock_android_device.py index 713286a..61926e9 100755 --- a/tests/lib/mock_android_device.py +++ b/tests/lib/mock_android_device.py
@@ -108,7 +108,7 @@ def shell(self, params, timeout=None): if params == 'id -u': - return b'root' + return b'0' elif params == 'bugreportz': if self.fail_br: return b'OMG I died!\n'
diff --git a/tests/mobly/controllers/android_device_lib/services/logcat_test.py b/tests/mobly/controllers/android_device_lib/services/logcat_test.py index 46def19..322e6f5 100755 --- a/tests/mobly/controllers/android_device_lib/services/logcat_test.py +++ b/tests/mobly/controllers/android_device_lib/services/logcat_test.py
@@ -829,7 +829,7 @@ 'ro.build.type': 'userdebug', 'ro.debuggable': '1', } - ad = android_device.AndroidDevice(serial=mock_serial) + ad = android_device.AndroidDevice(serial=mock_serial, skip_root=True) logcat_service = logcat.Logcat(ad) with self.assertNoLogs(level=logging.WARNING): logcat_service._enable_logpersist() @@ -871,7 +871,7 @@ 'ro.build.type': 'user', 'ro.debuggable': '0', } - ad = android_device.AndroidDevice(serial=mock_serial) + ad = android_device.AndroidDevice(serial=mock_serial, skip_root=True) logcat_service = logcat.Logcat(ad) logcat_service._enable_logpersist() mock_adb_proxy.shell.assert_not_called() @@ -898,7 +898,7 @@ 'ro.debuggable': '1', } mock_adb_proxy.shell.side_effect = MOCK_LOGPERSIST_START_MISSING_ADB_ERROR - ad = android_device.AndroidDevice(serial=mock_serial) + ad = android_device.AndroidDevice(serial=mock_serial, skip_root=True) logcat_service = logcat.Logcat(ad) with self.assertLogs(level=logging.WARNING) as cm: logcat_service._enable_logpersist() @@ -927,7 +927,7 @@ 'ro.debuggable': '1', } mock_adb_proxy.shell.side_effect = MOCK_LOGPERSIST_STOP_MISSING_ADB_ERROR - ad = android_device.AndroidDevice(serial=mock_serial) + ad = android_device.AndroidDevice(serial=mock_serial, skip_root=True) logcat_service = logcat.Logcat(ad) with self.assertLogs(level=logging.WARNING) as cm: logcat_service._enable_logpersist()
diff --git a/tests/mobly/controllers/android_device_test.py b/tests/mobly/controllers/android_device_test.py index 1486d59..624da29 100755 --- a/tests/mobly/controllers/android_device_test.py +++ b/tests/mobly/controllers/android_device_test.py
@@ -240,8 +240,26 @@ mock_list_adb_usb.return_value = [] configs = [{'serial': '1'}, {'serial': '2'}] android_device.get_instances_with_configs(configs) - mock_ad_class.assert_any_call('1') - mock_ad_class.assert_any_call('2') + mock_ad_class.assert_any_call('1', skip_root=False) + mock_ad_class.assert_any_call('2', skip_root=False) + + @mock.patch('mobly.controllers.android_device.list_fastboot_devices') + @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_skip_root( + self, mock_ad_class, mock_list_adb_usb, mock_list_adb, mock_list_fastboot + ): + mock_list_fastboot.return_value = [] + mock_list_adb.return_value = ['1'] + mock_list_adb_usb.return_value = [] + configs = [{'serial': '1', 'skip_root': True, 'foo': 'bar'}] + android_device.get_instances_with_configs(configs) + mock_ad_class.assert_called_once_with('1', skip_root=True) + # `skip_root` is consumed by the constructor, not `load_config`. + mock_ad_class.return_value.load_config.assert_called_once_with( + {'foo': 'bar'} + ) def test_get_instances_with_configs_invalid_config(self): config = {'something': 'random'} @@ -263,7 +281,7 @@ mock_list_adb_usb.return_value = [] mock_ad = mock.MagicMock() - def _make_ad(serial): + def _make_ad(serial, skip_root=False): if serial == '1': raise Exception('Something went wrong.') return mock_ad @@ -293,7 +311,7 @@ mock_list_adb_usb.return_value = [] mock_ad = mock.MagicMock() - def _make_ad(serial): + def _make_ad(serial, skip_root=False): if serial == '2': raise Exception('Something went wrong.') return mock_ad @@ -757,6 +775,87 @@ 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', return_value=mock_android_device.MockFastbootProxy('1'), ) + def test_AndroidDevice_init_roots_rootable_device( + self, MockFastboot, MockAdbProxy + ): + MockAdbProxy.return_value.root = mock.MagicMock(return_value=b'') + ad = android_device.AndroidDevice(serial='1') + self.assertFalse(ad.skip_root) + ad.adb.root.assert_called_once_with() + + @mock.patch( + 'mobly.controllers.android_device_lib.adb.AdbProxy', + return_value=mock_android_device.MockAdbProxy( + '1', + mock_properties={ + 'ro.build.id': 'AB42', + 'ro.build.type': 'user', + 'ro.debuggable': '0', + }, + ), + ) + @mock.patch( + 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', + return_value=mock_android_device.MockFastbootProxy('1'), + ) + def test_AndroidDevice_init_does_not_root_user_device( + self, MockFastboot, MockAdbProxy + ): + MockAdbProxy.return_value.root = mock.MagicMock() + ad = android_device.AndroidDevice(serial='1') + ad.adb.root.assert_not_called() + + @mock.patch( + 'mobly.controllers.android_device_lib.adb.AdbProxy', + return_value=mock_android_device.MockAdbProxy('1'), + ) + @mock.patch( + 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', + return_value=mock_android_device.MockFastbootProxy('1'), + ) + def test_AndroidDevice_init_honors_skip_root( + self, MockFastboot, MockAdbProxy + ): + MockAdbProxy.return_value.root = mock.MagicMock() + ad = android_device.AndroidDevice(serial='1', skip_root=True) + self.assertTrue(ad.skip_root) + ad.adb.root.assert_not_called() + # Explicit rooting still works when requested by the user. + self.assertTrue(ad.root_adb()) + ad.adb.root.assert_called_once_with() + + @mock.patch( + 'mobly.controllers.android_device_lib.adb.AdbProxy', + return_value=mock_android_device.MockAdbProxy('1'), + ) + @mock.patch( + 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', + return_value=mock_android_device.MockFastbootProxy('1'), + ) + def test_AndroidDevice_root_adb_warns_when_adbd_refuses_root( + self, MockFastboot, MockAdbProxy + ): + ad = android_device.AndroidDevice(serial='1', skip_root=True) + ad.adb.root = mock.MagicMock( + return_value=b'adbd cannot run as root in production builds' + ) + ad.adb.shell = mock.MagicMock(return_value=b'2000') + with mock.patch.object(ad, 'log') as mock_log: + self.assertFalse(ad.root_adb()) + ad.adb.root.assert_called_once_with() + mock_log.warning.assert_called_once() + self.assertIn( + 'adbd cannot run as root', mock_log.warning.call_args[0][-1].decode() + ) + + @mock.patch( + 'mobly.controllers.android_device_lib.adb.AdbProxy', + return_value=mock_android_device.MockAdbProxy('1'), + ) + @mock.patch( + 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', + return_value=mock_android_device.MockFastbootProxy('1'), + ) def test_AndroidDevice_device_info(self, MockFastboot, MockAdbProxy): ad = android_device.AndroidDevice(serial=1) device_info = ad.device_info @@ -2044,6 +2143,28 @@ 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', return_value=mock_android_device.MockFastbootProxy('1'), ) + def test_AndroidDevice_handle_reboot_roots_unless_skip_root( + self, FastbootProxy, MockAdbProxy + ): + ad = android_device.AndroidDevice(serial='1') + ad.adb.root = mock.MagicMock(return_value=b'') + with ad.handle_reboot(): + pass + ad.adb.root.assert_called_once_with() + ad.adb.root.reset_mock() + ad.skip_root = True + with ad.handle_reboot(): + pass + ad.adb.root.assert_not_called() + + @mock.patch( + 'mobly.controllers.android_device_lib.adb.AdbProxy', + return_value=mock_android_device.MockAdbProxy('1'), + ) + @mock.patch( + 'mobly.controllers.android_device_lib.fastboot.FastbootProxy', + return_value=mock_android_device.MockFastbootProxy('1'), + ) @mock.patch('mobly.utils.start_standing_subprocess', return_value='process') @mock.patch('mobly.utils.stop_standing_subprocess') @mock.patch.object(logcat.Logcat, '_open_logcat_file')