Fix pause/resume behavior for snippet management service. (#513)
* Pause/Resume should not rely on is_alive
* Clean up adb forward port when pausing snippet clients.
diff --git a/mobly/controllers/android_device_lib/service_manager.py b/mobly/controllers/android_device_lib/service_manager.py
index ce652fb..9853395 100644
--- a/mobly/controllers/android_device_lib/service_manager.py
+++ b/mobly/controllers/android_device_lib/service_manager.py
@@ -106,18 +106,16 @@
def pause_all(self):
"""Pauses all active service instances."""
for alias, service in self._service_objects.items():
- if service.is_alive:
- with expects.expect_no_raises(
- 'Failed to pause service "%s".' % alias):
- service.pause()
+ with expects.expect_no_raises(
+ 'Failed to pause service "%s".' % alias):
+ service.pause()
def resume_all(self):
"""Resumes all paused service instances."""
for alias, service in self._service_objects.items():
- if not service.is_alive:
- with expects.expect_no_raises(
- 'Failed to pause service "%s".' % alias):
- service.resume()
+ with expects.expect_no_raises(
+ 'Failed to pause service "%s".' % alias):
+ service.resume()
def __getattr__(self, name):
"""Syntactic sugar to enable direct access of service objects by alias.
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 6e519a1..11358cc 100644
--- a/mobly/controllers/android_device_lib/services/snippet_management_service.py
+++ b/mobly/controllers/android_device_lib/services/snippet_management_service.py
@@ -104,27 +104,50 @@
"""Starts all the snippet clients under management."""
for client in self._snippet_clients.values():
if not client.is_alive:
+ self._device.log.debug('Starting SnippetClient<%s>.',
+ client.package)
client.start_app_and_connect()
+ else:
+ self._device.log.debug(
+ 'Not startng SnippetClient<%s> because it is already alive.',
+ client.package)
def stop(self):
"""Stops all the snippet clients under management."""
for client in self._snippet_clients.values():
if client.is_alive:
+ self._device.log.debug('Stopping SnippetClient<%s>.',
+ client.package)
client.stop_app()
+ else:
+ self._device.log.debug(
+ 'Not stopping SnippetClient<%s> because it is not alive.',
+ client.package)
def pause(self):
- """Intentionally no-op.
+ """Pauses all the snippet clients under management.
- No action on the client side is needed per current snippet clients
- design. Because snippet clients do not explicitly handle temporary
- disconnect (Issue #509).
+ This clears the host port of a client because a new port will be
+ 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()
def resume(self):
"""Resumes all paused snippet clients."""
for client in self._snippet_clients.values():
- if not client.is_alive:
+ # 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:
+ self._device.log.debug('Resuming SnippetClient<%s>.',
+ client.package)
client.restore_app_connection()
+ else:
+ self._device.log.debug('Not resuming SnippetClient<%s>.',
+ client.package)
def __getattr__(self, name):
client = self.get_snippet_client(name)
diff --git a/mobly/controllers/android_device_lib/snippet_client.py b/mobly/controllers/android_device_lib/snippet_client.py
index 9e6469c..7895715 100644
--- a/mobly/controllers/android_device_lib/snippet_client.py
+++ b/mobly/controllers/android_device_lib/snippet_client.py
@@ -236,8 +236,14 @@
'Failed to stop existing apk. Unexpected output: %s' % out)
finally:
# Always clean up the adb port
- if self.host_port:
- self._adb.forward(['--remove', 'tcp:%d' % self.host_port])
+ 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.host_port = None
def _start_event_client(self):
"""Overrides superclass."""
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 1d4c31c..66bb913 100755
--- a/tests/mobly/controllers/android_device_lib/service_manager_test.py
+++ b/tests/mobly/controllers/android_device_lib/service_manager_test.py
@@ -25,6 +25,8 @@
self._device = device
self._configs = configs
self._alive = False
+ self.is_pause_called = False
+ self.is_resume_called = False
@property
def is_alive(self):
@@ -38,15 +40,19 @@
self._alive = False
self._device.stop()
+ def pause(self):
+ self.is_pause_called = True
+
+ def resume(self):
+ self.is_resume_called = True
+
class ServiceManagerTest(unittest.TestCase):
def test_service_manager_instantiation(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
def test_register(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service', MockService)
service = manager.mock_service
self.assertTrue(service)
@@ -54,16 +60,14 @@
self.assertTrue(manager.is_any_alive)
def test_register_dup_alias(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service', MockService)
msg = '.* A service is already registered with alias "mock_service"'
with self.assertRaisesRegex(service_manager.Error, msg):
manager.register('mock_service', MockService)
def test_unregister(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service', MockService)
service = manager.mock_service
manager.unregister('mock_service')
@@ -71,8 +75,7 @@
self.assertFalse(service.is_alive)
def test_unregister_non_existent(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
with self.assertRaisesRegex(
service_manager.Error,
'.* No service is registered with alias "mock_service"'):
@@ -80,8 +83,7 @@
@mock.patch('mobly.expects.expect_no_raises')
def test_unregister_handle_error_from_stop(self, mock_expect_func):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service', MockService)
service = manager.mock_service
service._device.stop.side_deffect = Exception(
@@ -91,8 +93,7 @@
'Failed to stop service instance "mock_service".')
def test_unregister_all(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
@@ -103,8 +104,7 @@
self.assertFalse(service2.is_alive)
def test_unregister_all(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
@@ -115,8 +115,7 @@
self.assertFalse(service2.is_alive)
def test_unregister_all_with_some_failed(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
@@ -129,60 +128,54 @@
self.assertFalse(service2.is_alive)
def test_pause_all(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
service2 = manager.mock_service2
manager.pause_all()
- self.assertFalse(manager.is_any_alive)
- self.assertFalse(service1.is_alive)
- self.assertFalse(service2.is_alive)
+ self.assertTrue(service1.is_pause_called)
+ self.assertTrue(service2.is_pause_called)
+ self.assertFalse(service1.is_resume_called)
+ self.assertFalse(service2.is_resume_called)
def test_pause_all_with_some_failed(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
service1._device.pause.side_deffect = Exception(
- 'Something failed in stop.')
+ 'Something failed in pause.')
service2 = manager.mock_service2
manager.pause_all()
- self.assertFalse(manager.is_any_alive)
- # state of service1 is undefined
- # verify state of service2
- self.assertFalse(service2.is_alive)
+ self.assertTrue(service2.is_pause_called)
+ self.assertFalse(service1.is_resume_called)
+ self.assertFalse(service2.is_resume_called)
def test_resume_all(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
service2 = manager.mock_service2
manager.pause_all()
manager.resume_all()
- self.assertTrue(manager.is_any_alive)
- self.assertTrue(service1.is_alive)
- self.assertTrue(service2.is_alive)
+ self.assertTrue(service1.is_resume_called)
+ self.assertTrue(service1.is_pause_called)
+ self.assertTrue(service2.is_resume_called)
+ self.assertTrue(service2.is_pause_called)
def test_resume_all_with_some_failed(self):
- mock_device = mock.MagicMock()
- manager = service_manager.ServiceManager(mock_device)
+ manager = service_manager.ServiceManager(mock.MagicMock())
manager.register('mock_service1', MockService)
manager.register('mock_service2', MockService)
service1 = manager.mock_service1
service1._device.resume.side_deffect = Exception(
- 'Something failed in stop.')
+ 'Something failed in resume.')
service2 = manager.mock_service2
manager.pause_all()
manager.resume_all()
- self.assertTrue(manager.is_any_alive)
- # state of service1 is undefined
- # verify state of service2
- self.assertTrue(service2.is_alive)
+ self.assertTrue(service2.is_resume_called)
if __name__ == '__main__':
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 2e347e5..e542382 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
@@ -122,19 +122,48 @@
mock_client.start_app_and_connect.assert_called_once_with()
@mock.patch(SNIPPET_CLIENT_CLASS_PATH)
- def test_resume(self, mock_class):
+ def test_pause(self, mock_class):
+ mock_client = mock_class.return_value
+ manager = snippet_management_service.SnippetManagementService(
+ mock.MagicMock())
+ manager.add_snippet_client('foo', MOCK_PACKAGE)
+ manager.pause()
+ mock_client.clear_host_port.assert_called_once_with()
+
+ @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+ def test_resume_positive_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
- manager.resume()
- mock_client.restore_app_connection.assert_not_called()
- mock_client.is_alive = False
+ mock_client.host_port = None
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):
+ 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()
+
+ @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
def test_attribute_access(self, mock_class):
mock_client = mock.MagicMock()
mock_class.return_value = mock_client
