Apply service mechanism for snippet client management. (#507)

* Use service mechaism to manager snippet clients.
* SnippetClient improvements
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py
index e5cde1c..d8468f3 100644
--- a/mobly/controllers/android_device.py
+++ b/mobly/controllers/android_device.py
@@ -22,14 +22,14 @@
 import time
 
 from mobly import logger as mobly_logger
-from mobly import signals
 from mobly import utils
 from mobly.controllers.android_device_lib import adb
+from mobly.controllers.android_device_lib import errors
 from mobly.controllers.android_device_lib import fastboot
 from mobly.controllers.android_device_lib import service_manager
 from mobly.controllers.android_device_lib import sl4a_client
-from mobly.controllers.android_device_lib import snippet_client
 from mobly.controllers.android_device_lib.services import logcat
+from mobly.controllers.android_device_lib.services import snippet_management_service
 
 # Convenience constant for the package of Mobly Bundled Snippets
 # (http://github.com/google/mobly-bundled-snippets).
@@ -58,21 +58,10 @@
 # Default Timeout to wait for boot completion
 DEFAULT_TIMEOUT_BOOT_COMPLETION_SECOND = 15 * 60
 
-
-class Error(signals.ControllerError):
-    pass
-
-
-class DeviceError(Error):
-    """Raised for errors from within an AndroidDevice object."""
-
-    def __init__(self, ad, msg):
-        new_msg = '%s %s' % (repr(ad), msg)
-        super(DeviceError, self).__init__(new_msg)
-
-
-class SnippetError(DeviceError):
-    """Raised for errors related to Mobly snippet."""
+# Aliases of error types for backward compatibility.
+Error = errors.Error
+DeviceError = errors.DeviceError
+SnippetError = snippet_management_service.Error
 
 
 def create(configs):
@@ -447,9 +436,8 @@
         if not self.is_bootloader and self.is_rootable:
             self.root_adb()
         self.services = service_manager.ServiceManager(self)
-        # A dict for tracking snippet clients. Keys are clients' attribute
-        # names, values are the clients: {<attr name string>: <client object>}.
-        self._snippet_clients = {}
+        self.services.register(
+            'snippets', snippet_management_service.SnippetManagementService)
         # Device info cache.
         self._user_added_device_info = {}
 
@@ -538,8 +526,7 @@
 
         A service can be a snippet or logcat collection.
         """
-        return any(
-            [self._snippet_clients, self.services.is_any_alive, self.sl4a])
+        return any([self.services.is_any_alive, self.sl4a])
 
     @property
     def log_path(self):
@@ -617,6 +604,7 @@
         """
         configs = logcat.Config(clear_log=clear_log)
         self.services.register('logcat', logcat.Logcat, configs)
+        self.services.start_all()
 
     def start_adb_logcat(self, clear_log=True):
         """.. deprecated:: 1.8
@@ -644,15 +632,10 @@
             are torn down. This can be used to restore these services, which
             includes snippets and sl4a.
         """
+        self.services.stop_all()
         service_info = {}
-        service_info['snippet_info'] = self._get_active_snippet_info()
         service_info['use_sl4a'] = self.sl4a is not None
         self._terminate_sl4a()
-        for attr_name, client in self._snippet_clients.items():
-            client.stop_app()
-            delattr(self, attr_name)
-        self._snippet_clients = {}
-        self.services.unregister_all()
         return service_info
 
     @contextlib.contextmanager
@@ -670,8 +653,24 @@
         try:
             yield
         finally:
+            self.wait_for_boot_completion()
+            if self.is_rootable:
+                self.root_adb()
             self._restore_services(service_info)
 
+    def _restore_services(self, service_info):
+        """Restores services after a device has come back from temporary
+        being offline.
+
+        Args:
+            service_info: A dict containing information on the services to
+                          restore, which could include snippet and sl4a.
+        """
+        self.services.start_all()
+        # Restore sl4a if needed.
+        if service_info['use_sl4a']:
+            self.load_sl4a()
+
     @contextlib.contextmanager
     def handle_usb_disconnect(self):
         """Properly manage the service life cycle when USB is disconnected.
@@ -729,32 +728,9 @@
         finally:
             self._reconnect_to_services()
 
-    def _restore_services(self, service_info):
-        """Restores services after a device has come back from temporary
-        being offline.
-
-        Args:
-            service_info: A dict containing information on the services to
-                          restore, which could include snippet and sl4a.
-        """
-        self.wait_for_boot_completion()
-        if self.is_rootable:
-            self.root_adb()
-        self.start_services()
-        # Restore snippets.
-        snippet_info = service_info['snippet_info']
-        for attr_name, package_name in snippet_info:
-            self.load_snippet(attr_name, package_name)
-        # Restore sl4a if needed.
-        if service_info['use_sl4a']:
-            self.load_sl4a()
-
     def _reconnect_to_services(self):
         """Reconnects to services after USB reconnected."""
         self.services.resume_all()
-        # Restore snippets.
-        for attr_name, client in self._snippet_clients.items():
-            client.restore_app_connection()
         # Restore sl4a if needed.
         if self.sl4a:
             self.sl4a.restore_app_connection()
@@ -861,54 +837,21 @@
             ad.maps.activateZoom('3')
 
         Args:
-            name: The attribute name to which to attach the snippet server.
-                e.g. name='maps' will attach the snippet server to ad.maps.
-            package: The package name defined in AndroidManifest.xml of the
-                snippet apk.
+            name: string, the attribute name to which to attach the snippet
+                client. E.g. `name='maps'` attaches the snippet client to
+                `ad.maps`.
+            package: string, the package name of the snippet apk to connect to.
 
         Raises:
             SnippetError: Illegal load operations are attempted.
         """
-        # Should not load snippet with the same attribute more than once.
-        if name in self._snippet_clients:
-            raise SnippetError(
-                self,
-                'Attribute "%s" is already registered with package "%s", it '
-                'cannot be used again.' %
-                (name, self._snippet_clients[name].package))
         # Should not load snippet with an existing attribute.
         if hasattr(self, name):
             raise SnippetError(
                 self,
                 'Attribute "%s" already exists, please use a different name.' %
                 name)
-        # Should not load the same snippet package more than once.
-        for client_name, client in self._snippet_clients.items():
-            if package == client.package:
-                raise SnippetError(
-                    self,
-                    'Snippet package "%s" has already been loaded under name'
-                    ' "%s".' % (package, client_name))
-        client = snippet_client.SnippetClient(package=package, ad=self)
-        try:
-            client.start_app_and_connect()
-        except snippet_client.AppStartPreCheckError:
-            # Precheck errors don't need cleanup, directly raise.
-            raise
-        except Exception as e:
-            # Log the stacktrace of `e` as re-raising doesn't preserve trace.
-            self.log.exception('Failed to start app and connect.')
-            # If errors happen, make sure we clean up before raising.
-            try:
-                client.stop_app()
-            except:
-                self.log.exception(
-                    'Failed to stop app after failure to start app and connect.'
-                )
-            # Explicitly raise the error from start app failure.
-            raise e
-        self._snippet_clients[name] = client
-        setattr(self, name, client)
+        self.services.snippets.add_snippet_client(name, package)
 
     def unload_snippet(self, name):
         """Stops a snippet apk.
@@ -919,12 +862,7 @@
         Raises:
             SnippetError: The given snippet name is not registered.
         """
-        if name not in self._snippet_clients:
-            raise SnippetError(self,
-                               'No snippet registered with name "%s"' % name)
-        client = self._snippet_clients.pop(name)
-        delattr(self, name)
-        client.stop_app()
+        self.services.snippets.remove_snippet_client(name)
 
     def load_sl4a(self):
         """Start sl4a service on the Android device.
@@ -1064,20 +1002,6 @@
             return True
         return False
 
-    def _get_active_snippet_info(self):
-        """Collects information on currently active snippet clients.
-        The info is used for restoring the snippet clients after rebooting the
-        device.
-        Returns:
-            A list of tuples, each tuple's first element is the name of the
-            snippet client's attribute, the second element is the package name
-            of the snippet.
-        """
-        snippet_info = []
-        for attr_name, client in self._snippet_clients.items():
-            snippet_info.append((attr_name, client.package))
-        return snippet_info
-
     def reboot(self):
         """Reboots the device.
 
@@ -1098,6 +1022,16 @@
         with self.handle_reboot():
             self.adb.reboot()
 
+    def __getattr__(self, name):
+        """Tries to return a snippet client registered with `name`.
+
+        This is for backward compatibility of direct accessing snippet clients.
+        """
+        client = self.services.snippets.get_snippet_client(name)
+        if client:
+            return client
+        return self.__getattribute__(name)
+
 
 class AndroidDeviceLoggerAdapter(logging.LoggerAdapter):
     """A wrapper class that adds a prefix to each log line.
diff --git a/mobly/controllers/android_device_lib/service_manager.py b/mobly/controllers/android_device_lib/service_manager.py
index f0a96c4..ce652fb 100644
--- a/mobly/controllers/android_device_lib/service_manager.py
+++ b/mobly/controllers/android_device_lib/service_manager.py
@@ -87,21 +87,37 @@
         for alias in aliases:
             self.unregister(alias)
 
+    def start_all(self):
+        """Pauses all active service instances."""
+        for alias, service in self._service_objects.items():
+            if not service.is_alive:
+                with expects.expect_no_raises(
+                        'Failed to start service "%s".' % alias):
+                    service.start()
+
+    def stop_all(self):
+        """Resumes all paused service instances."""
+        for alias, service in self._service_objects.items():
+            if service.is_alive:
+                with expects.expect_no_raises(
+                        'Failed to stop service "%s".' % alias):
+                    service.stop()
+
     def pause_all(self):
         """Pauses all active service instances."""
-        for alias, obj in self._service_objects.items():
-            if obj.is_alive:
+        for alias, service in self._service_objects.items():
+            if service.is_alive:
                 with expects.expect_no_raises(
                         'Failed to pause service "%s".' % alias):
-                    obj.pause()
+                    service.pause()
 
     def resume_all(self):
         """Resumes all paused service instances."""
-        for alias, obj in self._service_objects.items():
-            if not obj.is_alive:
+        for alias, service in self._service_objects.items():
+            if not service.is_alive:
                 with expects.expect_no_raises(
                         'Failed to pause service "%s".' % alias):
-                    obj.resume()
+                    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
new file mode 100644
index 0000000..6e519a1
--- /dev/null
+++ b/mobly/controllers/android_device_lib/services/snippet_management_service.py
@@ -0,0 +1,133 @@
+# Copyright 2018 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Module for the snippet management service."""
+from mobly.controllers.android_device_lib import errors
+from mobly.controllers.android_device_lib import snippet_client
+from mobly.controllers.android_device_lib.services import base_service
+
+MISSING_SNIPPET_CLIENT_MSG = 'No snippet client is registered with name "%s".'
+
+
+class Error(errors.ServiceError):
+    """Root error type for snippet management service."""
+    SERVICE_TYPE = 'SnippetManagementService'
+
+
+class SnippetManagementService(base_service.BaseService):
+    """Management service of snippet clients.
+
+    This service manages all the snippet clients associated with an Android
+    device.
+    """
+
+    def __init__(self, device, configs=None):
+        del configs  # Unused param.
+        self._device = device
+        self._is_alive = False
+        self._snippet_clients = {}
+        super(SnippetManagementService, self).__init__(device)
+
+    @property
+    def is_alive(self):
+        """True if any client is running, False otherwise."""
+        return any(
+            [client.is_alive for client in self._snippet_clients.values()])
+
+    def get_snippet_client(self, name):
+        """Gets the snippet client managed under a given name.
+
+        Args:
+            name: string, the name of the snippet client under management.
+
+        Returns:
+            SnippetClient.
+        """
+        if name in self._snippet_clients:
+            return self._snippet_clients[name]
+
+    def add_snippet_client(self, name, package):
+        """Adds a snippet client to the management.
+
+        Args:
+            name: string, the attribute name to which to attach the snippet
+                client. E.g. `name='maps'` attaches the snippet client to
+                `ad.maps`.
+            package: string, the package name of the snippet apk to connect to.
+
+        Raises:
+            Error, if a duplicated name or package is passed in.
+        """
+        # Should not load snippet with the same name more than once.
+        if name in self._snippet_clients:
+            raise Error(
+                self,
+                'Name "%s" is already registered with package "%s", it cannot '
+                'be used again.' %
+                (name, self._snippet_clients[name].client.package))
+        # Should not load the same snippet package more than once.
+        for snippet_name, client in self._snippet_clients.items():
+            if package == client.package:
+                raise Error(
+                    self,
+                    'Snippet package "%s" has already been loaded under name'
+                    ' "%s".' % (package, snippet_name))
+        client = snippet_client.SnippetClient(package=package, ad=self._device)
+        client.start_app_and_connect()
+        self._snippet_clients[name] = client
+
+    def remove_snippet_client(self, name):
+        """Removes a snippet client from management.
+
+        Args:
+            name: string, the name of the snippet client to remove.
+
+        Raises:
+            Error: if no snippet client is managed under the specified name.
+        """
+        if name not in self._snippet_clients:
+            raise Error(self._device, MISSING_SNIPPET_CLIENT_MSG % name)
+        client = self._snippet_clients.pop(name)
+        client.stop_app()
+
+    def start(self):
+        """Starts all the snippet clients under management."""
+        for client in self._snippet_clients.values():
+            if not client.is_alive:
+                client.start_app_and_connect()
+
+    def stop(self):
+        """Stops all the snippet clients under management."""
+        for client in self._snippet_clients.values():
+            if client.is_alive:
+                client.stop_app()
+
+    def pause(self):
+        """Intentionally no-op.
+
+        No action on the client side is needed per current snippet clients
+        design. Because snippet clients do not explicitly handle temporary
+        disconnect (Issue #509).
+        """
+
+    def resume(self):
+        """Resumes all paused snippet clients."""
+        for client in self._snippet_clients.values():
+            if not client.is_alive:
+                client.restore_app_connection()
+
+    def __getattr__(self, name):
+        client = self.get_snippet_client(name)
+        if client:
+            return client
+        return self.__getattribute__(name)
diff --git a/mobly/controllers/android_device_lib/snippet_client.py b/mobly/controllers/android_device_lib/snippet_client.py
index 03674ff..9e6469c 100644
--- a/mobly/controllers/android_device_lib/snippet_client.py
+++ b/mobly/controllers/android_device_lib/snippet_client.py
@@ -87,8 +87,61 @@
         self._adb = ad.adb
         self._proc = None
 
+    @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.
+        """
+        return self._conn is not None
+
     def start_app_and_connect(self):
-        """Overrides superclass. Launches a snippet app and connects to it."""
+        """Starts snippet apk on the device and connects to it.
+
+        This wraps the main logic with safe handling
+
+        Raises:
+            AppStartPreCheckError, when pre-launch checks fail.
+        """
+        try:
+            self._start_app_and_connect()
+        except AppStartPreCheckError:
+            # Precheck errors don't need cleanup, directly raise.
+            raise
+        except Exception as e:
+            # Log the stacktrace of `e` as re-raising doesn't preserve trace.
+            self._ad.log.exception('Failed to start app and connect.')
+            # If errors happen, make sure we clean up before raising.
+            try:
+                self.stop_app()
+            except:
+                self._ad.log.exception(
+                    'Failed to stop app after failure to start and connect.')
+            # Explicitly raise the original error from starting app.
+            raise e
+
+    def _start_app_and_connect(self):
+        """Starts snippet apk on the device and connects to it.
+
+        After prechecks, this launches the snippet apk with an adb cmd in a
+        standing subprocess, checks the cmd response from the apk for protocol
+        version, then sets up the socket connection over adb port-forwarding.
+
+        Args:
+            ProtocolVersionError, if protocol info or port info cannot be
+                retrieved from the snippet apk.
+        """
         self._check_app_installed()
         self.disable_hidden_api_blacklist()
 
@@ -125,7 +178,8 @@
 
         # Yaaay! We're done!
         self.log.debug('Snippet %s started after %.1fs on host port %s',
-                       self.package, time.time() - start_time, self.host_port)
+                       self.package,
+                       time.time() - start_time, self.host_port)
 
     def restore_app_connection(self, port=None):
         """Restores the app after device got reconnected.
@@ -174,6 +228,7 @@
             self.disconnect()
             if self._proc:
                 utils.stop_standing_subprocess(self._proc)
+            self._proc = None
             out = self._adb.shell(_STOP_CMD % self.package).decode('utf-8')
             if 'OK (0 tests)' not in out:
                 raise errors.DeviceError(
@@ -217,7 +272,7 @@
             raise AppStartPreCheckError(
                 self._ad,
                 '%s is installed, but it is not instrumented.' % self.package)
-        match = re.search('^instrumentation:(.*)\/(.*) \(target=(.*)\)$',
+        match = re.search(r'^instrumentation:(.*)\/(.*) \(target=(.*)\)$',
                           matched_out[0])
         target_name = match.group(3)
         # Check that the instrumentation target is installed if it's not the
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
new file mode 100755
index 0000000..2e347e5
--- /dev/null
+++ b/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py
@@ -0,0 +1,149 @@
+# Copyright 2018 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import mock
+from future.tests.base import unittest
+
+from mobly.controllers.android_device_lib.services import snippet_management_service
+
+MOCK_PACKAGE = 'com.mock.package'
+SNIPPET_CLIENT_CLASS_PATH = 'mobly.controllers.android_device_lib.snippet_client.SnippetClient'
+
+
+class SnippetManagementServiceTest(unittest.TestCase):
+    """Tests for the snippet management service."""
+
+    def test_empty_manager_start_stop(self):
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.start()
+        # When no client is registered, manager is never alive.
+        self.assertFalse(manager.is_alive)
+        manager.stop()
+        self.assertFalse(manager.is_alive)
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_get_snippet_client(self, mock_class):
+        mock_client = mock_class.return_value
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.add_snippet_client('foo', MOCK_PACKAGE)
+        self.assertEqual(manager.get_snippet_client('foo'), mock_client)
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_get_snippet_client_fail(self, _):
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        self.assertIsNone(manager.get_snippet_client('foo'))
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_stop_with_live_client(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.start_app_and_connect.assert_called_once_with()
+        manager.stop()
+        mock_client.stop_app.assert_called_once_with()
+        mock_client.stop_app.reset_mock()
+        mock_client.is_alive = False
+        self.assertFalse(manager.is_alive)
+        manager.stop()
+        mock_client.stop_app.assert_not_called()
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_add_snippet_client_dup_name(self, _):
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.add_snippet_client('foo', MOCK_PACKAGE)
+        msg = ('.* Name "foo" is already registered with package ".*", it '
+               'cannot be used again.')
+        with self.assertRaisesRegex(snippet_management_service.Error, msg):
+            manager.add_snippet_client('foo', MOCK_PACKAGE + 'ha')
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_add_snippet_client_dup_package(self, mock_class):
+        mock_client = mock_class.return_value
+        mock_client.package = MOCK_PACKAGE
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.add_snippet_client('foo', MOCK_PACKAGE)
+        msg = ('Snippet package "com.mock.package" has already been loaded '
+               'under name "foo".')
+        with self.assertRaisesRegex(snippet_management_service.Error, msg):
+            manager.add_snippet_client('bar', MOCK_PACKAGE)
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_remove_snippet_client(self, mock_class):
+        mock_client = mock.MagicMock()
+        mock_class.return_value = mock_client
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.add_snippet_client('foo', MOCK_PACKAGE)
+        manager.remove_snippet_client('foo')
+        msg = 'No snippet client is registered with name "foo".'
+        with self.assertRaisesRegex(snippet_management_service.Error, msg):
+            manager.foo.do_something()
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_remove_snippet_client(self, mock_class):
+        mock_client = mock.MagicMock()
+        mock_class.return_value = mock_client
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        with self.assertRaisesRegex(
+                snippet_management_service.Error,
+                'No snippet client is registered with name "foo".'):
+            manager.remove_snippet_client('foo')
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_start_with_live_service(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.start_app_and_connect.reset_mock()
+        mock_client.is_alive = True
+        manager.start()
+        mock_client.start_app_and_connect.assert_not_called()
+        self.assertTrue(manager.is_alive)
+        mock_client.is_alive = False
+        manager.start()
+        mock_client.start_app_and_connect.assert_called_once_with()
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_resume(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
+        manager.resume()
+        mock_client.restore_app_connection.assert_called_once_with()
+
+    @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+    def test_attribute_access(self, mock_class):
+        mock_client = mock.MagicMock()
+        mock_class.return_value = mock_client
+        manager = snippet_management_service.SnippetManagementService(
+            mock.MagicMock())
+        manager.add_snippet_client('foo', MOCK_PACKAGE)
+        manager.foo.ha('param')
+        mock_client.ha.assert_called_once_with('param')
+
+
+if __name__ == '__main__':
+    unittest.main()
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 61765e7..b0f28c7 100755
--- a/tests/mobly/controllers/android_device_lib/snippet_client_test.py
+++ b/tests/mobly/controllers/android_device_lib/snippet_client_test.py
@@ -193,6 +193,119 @@
         client = self._make_client()
         client.start_app_and_connect()
         self.assertEqual(123, client.device_port)
+        self.assertTrue(client.is_alive)
+
+    @mock.patch('socket.create_connection')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.stop_standing_subprocess')
+    def test_snippet_stop_app(self, mock_stop_standing_subprocess,
+                              mock_create_connection):
+        adb_proxy = mock.MagicMock()
+        adb_proxy.shell.return_value = b'OK (0 tests)'
+        client = self._make_client(adb_proxy)
+        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_disconect,
+                                     mock_create_connection):
+        mock_disconect.side_effect = Exception('ha')
+        adb_proxy = mock.MagicMock()
+        adb_proxy.shell.return_value = b'OK (0 tests)'
+        client = self._make_client(adb_proxy)
+        client.host_port = 1
+        with self.assertRaisesRegex(Exception, 'ha'):
+            client.stop_app()
+        adb_proxy.forward.assert_called_once_with(['--remove', 'tcp:1'])
+
+    @mock.patch('socket.create_connection')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.start_standing_subprocess')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.get_available_host_port')
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient.'
+        'disable_hidden_api_blacklist')
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient.'
+        'stop_app')
+    def test_start_app_and_connect_precheck_fail(
+            self, mock_stop, mock_precheck, mock_get_port,
+            mock_start_standing_subprocess, mock_create_connection):
+        self.setup_mock_socket_file(mock_create_connection)
+        self._setup_mock_instrumentation_cmd(
+            mock_start_standing_subprocess,
+            resp_lines=[
+                b'SNIPPET START, PROTOCOL 1 0\n',
+                b'SNIPPET SERVING, PORT 123\n',
+            ])
+        client = self._make_client()
+        mock_precheck.side_effect = snippet_client.AppStartPreCheckError(
+            client.ad, 'ha')
+        with self.assertRaisesRegex(snippet_client.AppStartPreCheckError,
+                                    'ha'):
+            client.start_app_and_connect()
+        mock_stop.assert_not_called()
+        self.assertFalse(client.is_alive)
+
+    @mock.patch('socket.create_connection')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.start_standing_subprocess')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.get_available_host_port')
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient._start_app_and_connect'
+    )
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient.stop_app'
+    )
+    def test_start_app_and_connect_generic_error(
+            self, mock_stop, mock_start, mock_get_port,
+            mock_start_standing_subprocess, mock_create_connection):
+        self.setup_mock_socket_file(mock_create_connection)
+        self._setup_mock_instrumentation_cmd(
+            mock_start_standing_subprocess,
+            resp_lines=[
+                b'SNIPPET START, PROTOCOL 1 0\n',
+                b'SNIPPET SERVING, PORT 123\n',
+            ])
+        client = self._make_client()
+        mock_start.side_effect = Exception('ha')
+        with self.assertRaisesRegex(Exception, 'ha'):
+            client.start_app_and_connect()
+        mock_stop.assert_called_once_with()
+        self.assertFalse(client.is_alive)
+
+    @mock.patch('socket.create_connection')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.start_standing_subprocess')
+    @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
+                'utils.get_available_host_port')
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient._start_app_and_connect'
+    )
+    @mock.patch(
+        'mobly.controllers.android_device_lib.snippet_client.SnippetClient.stop_app'
+    )
+    def test_start_app_and_connect_fail_stop_also_fail(
+            self, mock_stop, mock_start, mock_get_port,
+            mock_start_standing_subprocess, mock_create_connection):
+        self.setup_mock_socket_file(mock_create_connection)
+        self._setup_mock_instrumentation_cmd(
+            mock_start_standing_subprocess,
+            resp_lines=[
+                b'SNIPPET START, PROTOCOL 1 0\n',
+                b'SNIPPET SERVING, PORT 123\n',
+            ])
+        client = self._make_client()
+        mock_start.side_effect = Exception('Some error')
+        mock_stop.side_effect = Exception('Another error')
+        with self.assertRaisesRegex(Exception, 'Some error'):
+            client.start_app_and_connect()
+        mock_stop.assert_called_once_with()
+        self.assertFalse(client.is_alive)
 
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'SnippetClient._do_start_app')
diff --git a/tests/mobly/controllers/android_device_test.py b/tests/mobly/controllers/android_device_test.py
index ffb6a94..a79c119 100755
--- a/tests/mobly/controllers/android_device_test.py
+++ b/tests/mobly/controllers/android_device_test.py
@@ -529,59 +529,13 @@
     @mock.patch(
         'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
     @mock.patch('mobly.utils.get_available_host_port')
-    def test_AndroidDevice_load_snippet_failure(
-            self, MockGetPort, MockSnippetClient, MockFastboot, MockAdbProxy):
+    def test_AndroidDevice_getattr(self, MockGetPort, MockSnippetClient,
+                                   MockFastboot, MockAdbProxy):
         ad = android_device.AndroidDevice(serial='1')
-        client = mock.MagicMock()
-        client.start_app_and_connect.side_effect = Exception(
-            'Something went wrong.')
-        MockSnippetClient.return_value = client
-        with self.assertRaisesRegex(Exception, 'Something went wrong.'):
-            ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
-        client.stop_app.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'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
-    @mock.patch('mobly.utils.get_available_host_port')
-    def test_AndroidDevice_load_snippet_precheck_failure(
-            self, MockGetPort, MockSnippetClient, MockFastboot, MockAdbProxy):
-        ad = android_device.AndroidDevice(serial='1')
-        client = mock.MagicMock()
-        client.start_app_and_connect.side_effect = snippet_client.AppStartPreCheckError(
-            mock.MagicMock, 'Something went wrong in precheck.')
-        MockSnippetClient.return_value = client
-        with self.assertRaisesRegex(snippet_client.AppStartPreCheckError,
-                                    'Something went wrong in precheck.'):
-            ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
-        client.stop_app.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.controllers.android_device_lib.snippet_client.SnippetClient')
-    @mock.patch('mobly.utils.get_available_host_port')
-    def test_AndroidDevice_load_snippet_fail_cleanup_also_fail(
-            self, MockGetPort, MockSnippetClient, MockFastboot, MockAdbProxy):
-        ad = android_device.AndroidDevice(serial='1')
-        client = mock.MagicMock()
-        client.start_app_and_connect.side_effect = Exception(
-            'Something went wrong in start app.')
-        client.stop_app.side_effect = Exception('Stop app also failed.')
-        MockSnippetClient.return_value = client
-        with self.assertRaisesRegex(Exception,
-                                    'Something went wrong in start app.'):
-            ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
-        client.stop_app.assert_called_once_with()
+        ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
+        value = {'value': 42}
+        actual_value = getattr(ad, 'some_attr', value)
+        self.assertEqual(actual_value, value)
 
     @mock.patch(
         'mobly.controllers.android_device_lib.adb.AdbProxy',
@@ -616,9 +570,7 @@
             self, MockGetPort, MockSnippetClient, MockFastboot, MockAdbProxy):
         ad = android_device.AndroidDevice(serial='1')
         ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
-        expected_msg = ('Attribute "%s" is already registered with package '
-                        '"%s", it cannot be used again.') % (
-                            'snippet', MOCK_SNIPPET_PACKAGE_NAME)
+        expected_msg = '.* Attribute "snippet" already exists, please use a different name.'
         with self.assertRaisesRegex(android_device.Error, expected_msg):
             ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME + 'haha')
 
@@ -704,7 +656,7 @@
         ad = android_device.AndroidDevice(serial='1')
         ad.start_services()
         ad.load_snippet('snippet', MOCK_SNIPPET_PACKAGE_NAME)
-        ad.stop_services()
+        ad.unload_snippet('snippet')
         self.assertFalse(hasattr(ad, 'snippet'))
 
     @mock.patch(