Fix miscellaneous minor bugs in android_device_lib and test_runner

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
 

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):

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.

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.',

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:

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])

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'

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'})

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)

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<SnippetManagementService> 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(

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'

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()