diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py index 7531065a..006758ee 100644 --- a/mobly/controllers/android_device_lib/adb.py +++ b/mobly/controllers/android_device_lib/adb.py @@ -371,7 +371,7 @@ def connect(self, address) -> bytes: ) if PATTERN_ADB_CONNECT_SUCCESS.match(stdout.decode('utf-8')) is None: raise AdbError( - cmd=f'connect {address}', stdout=stdout, stderr='', ret_code=0 + cmd=f'connect {address}', stdout=stdout, stderr=b'', ret_code=0 ) return stdout diff --git a/mobly/controllers/android_device_lib/jsonrpc_shell_base.py b/mobly/controllers/android_device_lib/jsonrpc_shell_base.py index 1a2f304f..f7454eb5 100755 --- a/mobly/controllers/android_device_lib/jsonrpc_shell_base.py +++ b/mobly/controllers/android_device_lib/jsonrpc_shell_base.py @@ -66,7 +66,8 @@ def load_device(self, serial=None): if serial not in serials: raise Error('Device "%s" is not found by adb.' % serial) ads = android_device.get_instances([serial]) - assert len(ads) == 1 + if len(ads) != 1: + raise Error(f'Expected exactly one device for "{serial}", got {ads}.') self._ad = ads[0] def start_console(self): diff --git a/mobly/controllers/android_device_lib/service_manager.py b/mobly/controllers/android_device_lib/service_manager.py index 78d75762..8e6a1ad6 100644 --- a/mobly/controllers/android_device_lib/service_manager.py +++ b/mobly/controllers/android_device_lib/service_manager.py @@ -149,13 +149,11 @@ def list_live_services(self): Returns: list of strings, the aliases of the services that are running. """ - aliases = [] - self.for_each( - lambda service: aliases.append(service.alias) + return [ + service.alias + for service in self._service_objects.values() if service.is_alive - else None - ) - return aliases + ] def create_output_excerpts_all(self, test_info): """Creates output excerpts from all services. diff --git a/mobly/controllers/android_device_lib/services/snippet_management_service.py b/mobly/controllers/android_device_lib/services/snippet_management_service.py index 2e1a604b..9c1a9dea 100644 --- a/mobly/controllers/android_device_lib/services/snippet_management_service.py +++ b/mobly/controllers/android_device_lib/services/snippet_management_service.py @@ -35,7 +35,6 @@ class SnippetManagementService(base_service.BaseService): def __init__(self, device, configs=None): del configs # Unused param. self._device = device - self._is_alive = False self._snippet_clients = {} super().__init__(device) @@ -75,7 +74,7 @@ class for supported configurations. # Should not load snippet with the same name more than once. if name in self._snippet_clients: raise Error( - self, + self._device, f'Name "{name}" is already registered with package' f' "{self._snippet_clients[name].package}" for user ID' f' {self._snippet_clients[name].user_id}, the same name' @@ -89,9 +88,8 @@ class for supported configurations. ) for snippet_name, client in self._snippet_clients.items(): if new_client.identifier == client.identifier: - del new_client raise Error( - self, + self._device, f'Snippet "{client.package}" has already been registered for user' f' id {client.user_id} under name "{snippet_name}". The same' ' package cannot be registered again for the same user.', diff --git a/mobly/controllers/android_device_lib/snippet_client_v2.py b/mobly/controllers/android_device_lib/snippet_client_v2.py index e3ea26d8..054ecff0 100644 --- a/mobly/controllers/android_device_lib/snippet_client_v2.py +++ b/mobly/controllers/android_device_lib/snippet_client_v2.py @@ -428,7 +428,6 @@ def _read_protocol_line(self): errors.ServerStartError: If EOF is reached without any protocol lines being read. """ - self._server_start_stdout = [] while True: line = self._proc.stdout.readline().decode('utf-8') if not line: diff --git a/mobly/test_runner.py b/mobly/test_runner.py index e30bb246..4b556f23 100644 --- a/mobly/test_runner.py +++ b/mobly/test_runner.py @@ -329,9 +329,7 @@ def get_full_test_names(self): tests_set = set(tests) for test_name in test_run_info.tests: if test_name not in tests_set: - raise Error( - 'Unknown test method: %s in class %s', (test_name, test.TAG) - ) + raise Error(f'Unknown test method: {test_name} in class {test.TAG}') test_names.append(f'{test.TAG}.{test_name}') else: test_names.extend([f'{test.TAG}.{n}' for n in tests]) diff --git a/tests/mobly/controllers/android_device_lib/adb_test.py b/tests/mobly/controllers/android_device_lib/adb_test.py index 8aa9ca88..0f23e126 100755 --- a/tests/mobly/controllers/android_device_lib/adb_test.py +++ b/tests/mobly/controllers/android_device_lib/adb_test.py @@ -552,6 +552,14 @@ def test_connect_fail(self, mock_run_command): ): out = adb.AdbProxy().connect(mock_address) + @mock.patch('mobly.utils.run_command') + def test_connect_fail_error_fields_are_bytes(self, mock_run_command): + mock_run_command.return_value = (0, b'Connection refused\n', b'') + with self.assertRaises(adb.AdbError) as cm: + adb.AdbProxy().connect('localhost:1234') + self.assertIsInstance(cm.exception.stdout, bytes) + self.assertIsInstance(cm.exception.stderr, bytes) + def test_getprop(self): with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd: mock_exec_cmd.return_value = b'blah' diff --git a/tests/mobly/controllers/android_device_lib/jsonrpc_shell_base_test.py b/tests/mobly/controllers/android_device_lib/jsonrpc_shell_base_test.py index ffc8f77d..3588c6e6 100755 --- a/tests/mobly/controllers/android_device_lib/jsonrpc_shell_base_test.py +++ b/tests/mobly/controllers/android_device_lib/jsonrpc_shell_base_test.py @@ -47,6 +47,20 @@ def test_load_device_when_one_device( json_shell.load_device() self.assertEqual(json_shell._ad, mock_device) + @mock.patch.object(android_device, 'list_adb_devices') + @mock.patch.object(android_device, 'get_instances') + @mock.patch.object(os, 'environ', new={}) + def test_load_device_no_instance_raises_error( + self, mock_get_instances, mock_list_adb_devices + ): + mock_list_adb_devices.return_value = ['1234'] + mock_get_instances.return_value = [] + json_shell = jsonrpc_shell_base.JsonRpcShellBase() + with self.assertRaisesRegex( + jsonrpc_shell_base.Error, 'Expected exactly one device for "1234"' + ): + json_shell.load_device(serial='1234') + @mock.patch.object(android_device, 'list_adb_devices') @mock.patch.object(android_device, 'get_instances') @mock.patch.object(os, 'environ', new={'ANDROID_SERIAL': '1234'}) diff --git a/tests/mobly/controllers/android_device_lib/service_manager_test.py b/tests/mobly/controllers/android_device_lib/service_manager_test.py index 2d608d3d..cee4bf8b 100755 --- a/tests/mobly/controllers/android_device_lib/service_manager_test.py +++ b/tests/mobly/controllers/android_device_lib/service_manager_test.py @@ -432,6 +432,19 @@ def test_list_live_services(self): aliases = manager.list_live_services() self.assertEqual(aliases, []) + def test_list_live_services_does_not_record_expects_errors(self): + manager = service_manager.ServiceManager(mock.MagicMock()) + manager.register('mock_service1', MockService) + with mock.patch.object( + MockService, + 'is_alive', + new_callable=mock.PropertyMock, + side_effect=RuntimeError('boom'), + ): + with self.assertRaisesRegex(RuntimeError, 'boom'): + manager.list_live_services() + self.assertEqual(expects.recorder.error_count, 0) + def test_start_services(self): manager = service_manager.ServiceManager(mock.MagicMock()) manager.register('mock_service1', MockService, start_service=False) diff --git a/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py b/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py index f2af7325..cd0bd225 100755 --- a/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py +++ b/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py @@ -136,6 +136,19 @@ def test_add_snippet_client_with_config(self, mock_class): package=mock.ANY, ad=mock.ANY, config=snippet_config ) + @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH) + def test_add_snippet_client_dup_name_error_names_device(self, _): + device = mock.MagicMock() + device.__repr__ = lambda _: '[AndroidDevice|serial123]' + manager = snippet_management_service.SnippetManagementService(device) + manager.add_snippet_client('foo', MOCK_PACKAGE) + with self.assertRaisesRegex( + snippet_management_service.Error, + r'^\[AndroidDevice\|serial123\]::Service Name' + r' "foo" is already registered', + ): + manager.add_snippet_client('foo', MOCK_PACKAGE + 'ha') + @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH) def test_add_snippet_client_dup_name(self, _): manager = snippet_management_service.SnippetManagementService( diff --git a/tests/mobly/controllers/android_device_lib/snippet_client_v2_test.py b/tests/mobly/controllers/android_device_lib/snippet_client_v2_test.py index 75653911..cd8542a4 100644 --- a/tests/mobly/controllers/android_device_lib/snippet_client_v2_test.py +++ b/tests/mobly/controllers/android_device_lib/snippet_client_v2_test.py @@ -921,6 +921,30 @@ def test_start_server_error_message_include_discarded_output( ): self.client.start_server() + @mock.patch( + 'mobly.controllers.android_device_lib.snippet_client_v2.' + 'utils.start_standing_subprocess' + ) + def test_start_server_error_message_keeps_output_before_start_line( + self, mock_start_standing_subprocess + ): + """Output discarded before and after SNIPPET START is both reported.""" + self._make_client() + self._mock_server_process_starting_response( + mock_start_standing_subprocess, + resp_lines=[ + b'junk before start\n', + b'SNIPPET START, PROTOCOL 1 0\n', + b'junk after start\n', + b'INSTRUMENTATION_RESULT: shortMsg=Process crashed.', + ], + ) + with self.assertRaisesRegex( + errors.ServerStartProtocolError, + r'junk before start\njunk after start', + ): + self.client.start_server() + @mock.patch( 'mobly.controllers.android_device_lib.snippet_client_v2.' 'utils.start_standing_subprocess' diff --git a/tests/mobly/test_runner_test.py b/tests/mobly/test_runner_test.py index b1531453..95246945 100755 --- a/tests/mobly/test_runner_test.py +++ b/tests/mobly/test_runner_test.py @@ -433,6 +433,19 @@ def test_get_full_test_names_test_list(self): self.assertIn('IntegrationTest.test_hello_world', results) self.assertEqual(len(results), 1) + def test_get_full_test_names_unknown_test(self): + config = self.base_mock_test_config.copy() + tr = test_runner.TestRunner(self.log_dir, self.testbed_name) + with tr.mobly_logger(): + tr.add_test_class( + config, integration_test.IntegrationTest, tests=['test_nope'] + ) + with self.assertRaisesRegex( + test_runner.Error, + '^Unknown test method: test_nope in class IntegrationTest$', + ): + tr.get_full_test_names() + def test_get_full_test_names_test_list_empty(self): """Verifies that calling get_test_names with empty test list works properly.""" config = self.base_mock_test_config.copy()