Fix the behavior of `SnippetClient#is_alive`. (#732)
diff --git a/mobly/controllers/android_device_lib/jsonrpc_client_base.py b/mobly/controllers/android_device_lib/jsonrpc_client_base.py
index ebe0b67..e28f5a1 100644
--- a/mobly/controllers/android_device_lib/jsonrpc_client_base.py
+++ b/mobly/controllers/android_device_lib/jsonrpc_client_base.py
@@ -245,16 +245,27 @@
self.uid = UNKNOWN_UID
def disconnect(self):
- """Close the connection to the remote client."""
- if self._conn:
- self._conn.close()
- self._conn = None
+ """Close the connection to the snippet server on the device.
+
+ This is a unilateral disconnect from the client side, without tearing down
+ the snippet server running on the device.
+
+ The connection to the snippet server can be re-established by calling
+ `SnippetClient.restore_app_connection`.
+ """
+ try:
+ if self._conn:
+ self._conn.close()
+ self._conn = None
+ finally:
+ # Always clear the host port as part of the disconnect step.
+ self.clear_host_port()
def clear_host_port(self):
"""Stops the adb port forwarding of the host port used by this client.
"""
if self.host_port:
- self._adb.forward(['--remove', 'tcp:%d' % self.host_port])
+ self._ad.adb.forward(['--remove', 'tcp:%d' % self.host_port])
self.host_port = None
def _client_send(self, msg):
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 fa9967e..1cd0587 100644
--- a/mobly/controllers/android_device_lib/services/snippet_management_service.py
+++ b/mobly/controllers/android_device_lib/services/snippet_management_service.py
@@ -125,16 +125,13 @@
allocated in `resume`.
"""
for client in self._snippet_clients.values():
- self._device.log.debug('Clearing host port %d of SnippetClient<%s>.',
- client.host_port, client.package)
- client.clear_host_port()
+ self._device.log.debug('Pausing SnippetClient<%s>.', client.package)
+ client.disconnect()
def resume(self):
"""Resumes all paused snippet clients."""
for client in self._snippet_clients.values():
- # Resume is only applicable if a client is alive and does not have
- # a host port.
- if client.is_alive and client.host_port is None:
+ if not client.is_alive:
self._device.log.debug('Resuming SnippetClient<%s>.', client.package)
client.restore_app_connection()
else:
diff --git a/mobly/controllers/android_device_lib/snippet_client.py b/mobly/controllers/android_device_lib/snippet_client.py
index b8260db..25adb85 100644
--- a/mobly/controllers/android_device_lib/snippet_client.py
+++ b/mobly/controllers/android_device_lib/snippet_client.py
@@ -90,21 +90,7 @@
@property
def is_alive(self):
- """Is the client alive.
-
- The client is considered alive if there is a connection object held for
- it. This is an approximation due to the following scenario:
-
- In the USB disconnect case, the host subprocess that kicked off the
- snippet apk would die, but the snippet apk itself would continue
- running on the device.
-
- The best approximation we can make is, the connection object has not
- been explicitly torn down, so the client should be considered alive.
-
- Returns:
- True if the client is considered alive, False otherwise.
- """
+ """Does the client have an active connection to the snippet server."""
return self._conn is not None
def _get_user_command_string(self):
@@ -237,23 +223,17 @@
# print the failure stack trace if there was any, and reap it from the
# process table.
self.log.debug('Stopping snippet apk %s', self.package)
- try:
- # Close the socket connection.
- self.disconnect()
- if self._proc:
- utils.stop_standing_subprocess(self._proc)
+ # Close the socket connection.
+ self.disconnect()
+ if self._proc:
+ utils.stop_standing_subprocess(self._proc)
self._proc = None
- out = self._adb.shell(
- _STOP_CMD.format(
- snippet_package=self.package,
- user=self._get_user_command_string())).decode('utf-8')
- if 'OK (0 tests)' not in out:
- raise errors.DeviceError(
- self._ad,
- 'Failed to stop existing apk. Unexpected output: %s' % out)
- finally:
- # Always clean up the adb port
- self.clear_host_port()
+ out = self._adb.shell(
+ _STOP_CMD.format(snippet_package=self.package,
+ user=self._get_user_command_string())).decode('utf-8')
+ if 'OK (0 tests)' not in out:
+ raise errors.DeviceError(
+ self._ad, 'Failed to stop existing apk. Unexpected output: %s' % out)
def _start_event_client(self):
"""Overrides superclass."""
diff --git a/tests/mobly/controllers/android_device_lib/jsonrpc_client_base_test.py b/tests/mobly/controllers/android_device_lib/jsonrpc_client_base_test.py
index 2b3d40d..60f5105 100755
--- a/tests/mobly/controllers/android_device_lib/jsonrpc_client_base_test.py
+++ b/tests/mobly/controllers/android_device_lib/jsonrpc_client_base_test.py
@@ -71,6 +71,42 @@
jsonrpc_client_base.ProtocolError.NO_RESPONSE_FROM_HANDSHAKE):
client.connect()
+ def test_disconnect(self):
+ client = FakeRpcClient()
+ mock_conn = mock.MagicMock()
+ client.clear_host_port = mock.MagicMock()
+ client._conn = mock_conn
+ client.disconnect()
+ self.assertIsNone(client._conn)
+ mock_conn.close.assert_called_once_with()
+ client.clear_host_port.assert_called_once_with()
+
+ def test_disconnect_raises(self):
+ client = FakeRpcClient()
+ mock_conn = mock.MagicMock()
+ client.clear_host_port = mock.MagicMock()
+ client._conn = mock_conn
+ # Explicitly making the second side_effect noop to avoid uncaught exception
+ # when `__del__` is called after the test is done, which triggers
+ # `disconnect`.
+ mock_conn.close.side_effect = [Exception('ha'), None]
+ with self.assertRaisesRegex(Exception, 'ha'):
+ client.disconnect()
+ client.clear_host_port.assert_called_once_with()
+
+ def test_clear_host_port_positive(self):
+ client = FakeRpcClient()
+ client.host_port = 1
+ client.clear_host_port()
+ client._ad.adb.forward.assert_called_once_with(['--remove', 'tcp:1'])
+ self.assertIsNone(client.host_port)
+
+ def test_clear_host_port_negative(self):
+ client = FakeRpcClient()
+ client.host_port = None
+ client.clear_host_port()
+ client._ad.adb.forward.assert_not_called()
+
@mock.patch('socket.create_connection')
def test_connect_handshake(self, mock_create_connection):
"""Test client handshake
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 d086b60..7557513 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
@@ -128,7 +128,7 @@
mock.MagicMock())
manager.add_snippet_client('foo', MOCK_PACKAGE)
manager.pause()
- mock_client.clear_host_port.assert_called_once_with()
+ mock_client.disconnect.assert_called_once_with()
@mock.patch(SNIPPET_CLIENT_CLASS_PATH)
def test_resume_positive_case(self, mock_class):
@@ -136,30 +136,17 @@
manager = snippet_management_service.SnippetManagementService(
mock.MagicMock())
manager.add_snippet_client('foo', MOCK_PACKAGE)
- mock_client.is_alive = True
- mock_client.host_port = None
+ mock_client.is_alive = False
manager.resume()
mock_client.restore_app_connection.assert_called_once_with()
@mock.patch(SNIPPET_CLIENT_CLASS_PATH)
- def test_resume_alive_with_host_port(self, mock_class):
+ def test_resume_negative_case(self, mock_class):
mock_client = mock_class.return_value
manager = snippet_management_service.SnippetManagementService(
mock.MagicMock())
manager.add_snippet_client('foo', MOCK_PACKAGE)
mock_client.is_alive = True
- mock_client.host_port = 1
- manager.resume()
- mock_client.restore_app_connection.assert_not_called()
-
- @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
- def test_resume_not_alive_no_host_port(self, mock_class):
- mock_client = mock_class.return_value
- manager = snippet_management_service.SnippetManagementService(
- mock.MagicMock())
- manager.add_snippet_client('foo', MOCK_PACKAGE)
- mock_client.is_alive = False
- mock_client.host_port = None
manager.resume()
mock_client.restore_app_connection.assert_not_called()
diff --git a/tests/mobly/controllers/android_device_lib/snippet_client_test.py b/tests/mobly/controllers/android_device_lib/snippet_client_test.py
index 6dd16f0..4843a0d 100755
--- a/tests/mobly/controllers/android_device_lib/snippet_client_test.py
+++ b/tests/mobly/controllers/android_device_lib/snippet_client_test.py
@@ -157,8 +157,7 @@
self.assertTrue(client.is_alive)
@mock.patch('socket.create_connection')
- @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
- 'utils.stop_standing_subprocess')
+ @mock.patch('mobly.utils.stop_standing_subprocess')
def test_snippet_stop_app(self, mock_stop_standing_subprocess,
mock_create_connection):
adb_proxy = mock.MagicMock()
@@ -167,19 +166,16 @@
client.stop_app()
self.assertFalse(client.is_alive)
- @mock.patch('socket.create_connection')
- @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
- 'SnippetClient.disconnect')
- def test_snippet_stop_app_raises(self, mock_disconnect,
- mock_create_connection):
- # Explicitly making the second side_effect noop to avoid uncaught exception
- # when `__del__` is called after the test is done, which triggers
- # `disconnect`.
- mock_disconnect.side_effect = [Exception('ha'), None]
+ def test_snippet_stop_app_raises(self):
adb_proxy = mock.MagicMock()
adb_proxy.shell.return_value = b'OK (0 tests)'
client = self._make_client(adb_proxy)
client.host_port = 1
+ client._conn = mock.MagicMock()
+ # Explicitly making the second side_effect noop to avoid uncaught exception
+ # when `__del__` is called after the test is done, which triggers
+ # `disconnect`.
+ client._conn.close.side_effect = [Exception('ha'), None]
with self.assertRaisesRegex(Exception, 'ha'):
client.stop_app()
adb_proxy.forward.assert_called_once_with(['--remove', 'tcp:1'])
