Repository: google/mobly

Issue:
Properly identify exceptions thrown from service modules within AndroidDevice
Similar to #105, it'll be good if all the exceptions from service modules like `SnippetClient` is also tagged with the device identity prefix.

Discussion:

Patch:
diff --git a/docs/tutorial.md b/docs/tutorial.md
index 035f634..59ea651 100644
--- a/docs/tutorial.md
+++ b/docs/tutorial.md
@@ -56,12 +56,14 @@ if __name__ == '__main__':
   test_runner.main()
 ```
  
-*To execute:*
- 
-    $ python hello_world_test.py -c sample_config.yml
- 
-*Expect*
- 
+To execute:
+
+```
+$ python hello_world_test.py -c sample_config.yml
+```
+
+*Expect*:
+
 A "Hello World!" toast notification appears on your device's screen.
  
 Within SampleTestBed's `Controllers` section, we used `AndroidDevice: '*'` to tell
@@ -106,21 +108,24 @@ if __name__ == '__main__':
 ```
  
 *To execute:*
+
+```
+$ python hello_world_test.py -c sample_config.yml --test_case test_bye
+```
  
-    $ python hello_world_test.py -c sample_config.yml --test_case test_bye
- 
- 
-*Expect*
- 
+*Expect*:
+
 A "Goodbye!" toast notification appears on your device's screen.
  
 You can dictate what test cases to execute within a test script and their
-execution order, shown below:
- 
-    $ python hello_world_test.py -c sample_config.yml --test_case test_bye test_hello test_bye
- 
-*Expect*
- 
+execution order, for example:
+
+```
+$ python hello_world_test.py -c sample_config.yml --test_case test_bye test_hello test_bye
+```
+
+*Expect*:
+
 Toast notifications appear on your device's screen in the following order:
 "Goodbye!", "Hello World!", "Goodbye!".
  
@@ -184,11 +189,13 @@ TestBeds:
  
 You can choose which one to execute on with the command line argument
 `--test_bed`:
- 
-    $ python hello_world_test.py -c sample_config.yml --test_bed AbcTestBed
- 
-*Expect*
- 
+
+```
+$ python hello_world_test.py -c sample_config.yml --test_bed AbcTestBed
+```
+
+*Expect*:
+
 A "Hello World!" and a "Goodbye!" toast notification appear on your device's
 screen.
  
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py
index e52e5fd..f1f6e9d 100644
--- a/mobly/controllers/android_device.py
+++ b/mobly/controllers/android_device.py
@@ -723,17 +723,18 @@ class AndroidDevice(object):
                     self,
                     'Snippet package "%s" has already been loaded under name'
                     ' "%s".' % (package, client_name))
-        client = snippet_client.SnippetClient(
-            package=package, adb_proxy=self.adb, log=self.log)
+        client = snippet_client.SnippetClient(package=package, ad=self)
         try:
             client.start_app_and_connect()
         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 launch.')
+                    'Failed to stop app after failure to start app and connect.')
             # Raise the error from start app failure.
             raise e
         self._snippet_clients[name] = client
@@ -836,8 +837,10 @@ class AndroidDevice(object):
             extra_params = self.adb_logcat_param
         except AttributeError:
             extra_params = ''
-        cmd = '"%s" -s %s logcat -v threadtime %s >> "%s"' % (
-            adb.ADB, self.serial, extra_params, logcat_file_path)
+        cmd = '"%s" -s %s logcat -v threadtime %s >> "%s"' % (adb.ADB,
+                                                              self.serial,
+                                                              extra_params,
+                                                              logcat_file_path)
         process = utils.start_standing_subprocess(cmd, shell=True)
         self._adb_logcat_process = process
         self.adb_logcat_file_path = logcat_file_path
@@ -853,12 +856,14 @@ class AndroidDevice(object):
         utils.stop_standing_subprocess(self._adb_logcat_process)
         self._adb_logcat_process = None
 
-    def take_bug_report(self, test_name, begin_time):
+    def take_bug_report(self, test_name, begin_time, timetout=300):
         """Takes a bug report on the device and stores it in a file.
 
         Args:
             test_name: Name of the test method that triggered this bug report.
             begin_time: Timestamp of when the test started.
+            timeout: float, the number of seconds to wait for bugreport to
+                complete, default is 5min.
         """
         new_br = True
         try:
@@ -881,7 +886,8 @@ class AndroidDevice(object):
         self.wait_for_boot_completion()
         self.log.info('Taking bugreport for %s.', test_name)
         if new_br:
-            out = self.adb.shell('bugreportz').decode('utf-8')
+            out = self.adb.shell(
+                'bugreportz', timeout=timetout).decode('utf-8')
             if not out.startswith('OK'):
                 raise DeviceError(self, 'Failed to take bugreport: %s' % out)
             br_out_path = out.split(':')[1].strip()
@@ -889,7 +895,8 @@ class AndroidDevice(object):
         else:
             # shell=True as this command redirects the stdout to a local file
             # using shell redirection.
-            self.adb.bugreport(' > %s' % full_out_path, shell=True)
+            self.adb.bugreport(
+                ' > %s' % full_out_path, shell=True, timeout=timetout)
         self.log.info('Bugreport for %s taken at %s.', test_name,
                       full_out_path)
 
diff --git a/mobly/controllers/android_device_lib/callback_handler.py b/mobly/controllers/android_device_lib/callback_handler.py
index 5ef611b..b36cf8e 100644
--- a/mobly/controllers/android_device_lib/callback_handler.py
+++ b/mobly/controllers/android_device_lib/callback_handler.py
@@ -14,6 +14,7 @@
 
 import time
 
+from mobly.controllers.android_device_lib import errors
 from mobly.controllers.android_device_lib import snippet_event
 
 # The max timeout cannot be larger than the max time the socket waits for a
@@ -23,7 +24,7 @@ MAX_TIMEOUT = 60 * 10
 DEFAULT_TIMEOUT = 120  # two minutes
 
 
-class Error(Exception):
+class Error(errors.DeviceError):
     pass
 
 
@@ -54,11 +55,12 @@ class CallbackHandler(object):
         ret_value: The direct return value of the async Rpc call.
     """
 
-    def __init__(self, callback_id, event_client, ret_value, method_name):
+    def __init__(self, callback_id, event_client, ret_value, method_name, ad):
         self._id = callback_id
         self._event_client = event_client
         self.ret_value = ret_value
         self._method_name = method_name
+        self._ad = ad
 
     @property
     def callback_id(self):
@@ -82,7 +84,7 @@ class CallbackHandler(object):
         """
         if timeout:
             if timeout > MAX_TIMEOUT:
-                raise Error(
+                raise Error(self._ad,
                     'Specified timeout %s is longer than max timeout %s.' %
                     (timeout, MAX_TIMEOUT))
         # Convert to milliseconds for java side.
@@ -92,7 +94,7 @@ class CallbackHandler(object):
                 self._id, event_name, timeout_ms)
         except Exception as e:
             if 'EventSnippetException: timeout.' in str(e):
-                raise TimeoutError(
+                raise TimeoutError(self._ad,
                     'Timed out after waiting %ss for event "%s" triggered by'
                     ' %s (%s).' % (timeout, event_name, self._method_name,
                                    self._id))
@@ -140,7 +142,7 @@ class CallbackHandler(object):
                 break
             if predicate(event):
                 return event
-        raise TimeoutError(
+        raise TimeoutError(self._ad,
             'Timed out after %ss waiting for an "%s" event that satisfies the '
             'predicate "%s".' % (timeout, event_name, predicate.__name__))
 
diff --git a/mobly/controllers/android_device_lib/errors.py b/mobly/controllers/android_device_lib/errors.py
new file mode 100644
index 0000000..c5d889a
--- /dev/null
+++ b/mobly/controllers/android_device_lib/errors.py
@@ -0,0 +1,28 @@
+# Copyright 2017 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 errors thrown from AndroidDevice object.
+
+from mobly import signals
+
+
+class Error(signals.ControllerError):
+    pass
+
+
+class DeviceError(Error):
+    """Raised for errors specific to an AndroidDevice object."""
+    def __init__(self, ad, msg):
+        new_msg = '%s %s' % (repr(ad), msg)
+        super(DeviceError, self).__init__(new_msg)
diff --git a/mobly/controllers/android_device_lib/jsonrpc_client_base.py b/mobly/controllers/android_device_lib/jsonrpc_client_base.py
index efbcdcc..db19f36 100644
--- a/mobly/controllers/android_device_lib/jsonrpc_client_base.py
+++ b/mobly/controllers/android_device_lib/jsonrpc_client_base.py
@@ -42,6 +42,7 @@ import socket
 import threading
 
 from mobly.controllers.android_device_lib import callback_handler
+from mobly.controllers.android_device_lib import errors
 
 # UID of the 'unknown' jsonrpc session. Will cause creation of a new session.
 UNKNOWN_UID = -1
@@ -53,7 +54,7 @@ _SOCKET_CONNECTION_TIMEOUT = 60
 _SOCKET_READ_TIMEOUT = callback_handler.MAX_TIMEOUT
 
 
-class Error(Exception):
+class Error(errors.DeviceError):
     pass
 
 
@@ -102,17 +103,18 @@ class JsonRpcClientBase(object):
         uid: (int) The uid of this session.
     """
 
-    def __init__(self, app_name, log=logging.getLogger()):
+    def __init__(self, app_name, ad):
         """
         Args:
             app_name: (str) The user-visible name of the app being communicated
                 with.
-            log: (logging.Logger) logger to which to send log messages.
+            ad: (AndroidDevice) The device object associated with a client.
         """
         self.host_port = None
         self.device_port = None
         self.app_name = app_name
-        self.log = log
+        self._ad = ad
+        self.log = self._ad.log
         self.uid = None
         self._client = None  # prevent close errors on connect failure
         self._conn = None
@@ -207,7 +209,8 @@ class JsonRpcClientBase(object):
 
         resp = self._cmd(cmd, uid)
         if not resp:
-            raise ProtocolError(ProtocolError.NO_RESPONSE_FROM_HANDSHAKE)
+            raise ProtocolError(
+                self._ad, ProtocolError.NO_RESPONSE_FROM_HANDSHAKE)
         result = json.loads(str(resp, encoding='utf8'))
         if result['status']:
             self.uid = result['uid']
@@ -262,12 +265,13 @@ class JsonRpcClientBase(object):
             self._client.flush()
             response = self._client.readline()
         if not response:
-            raise ProtocolError(ProtocolError.NO_RESPONSE_FROM_SERVER)
+            raise ProtocolError(self._ad,
+                                ProtocolError.NO_RESPONSE_FROM_SERVER)
         result = json.loads(str(response, encoding="utf8"))
         if result['error']:
-            raise ApiError(result['error'])
+            raise ApiError(self._ad, result['error'])
         if result['id'] != apiid:
-            raise ProtocolError(ProtocolError.MISMATCHED_API_ID)
+            raise ProtocolError(self._ad, ProtocolError.MISMATCHED_API_ID)
         if result.get('callback') is not None:
             if self._event_client is None:
                 self._event_client = self._start_event_client()
@@ -275,7 +279,8 @@ class JsonRpcClientBase(object):
                 callback_id=result['callback'],
                 event_client=self._event_client,
                 ret_value=result['result'],
-                method_name=method)
+                method_name=method,
+                ad=self._ad)
         return result['result']
 
     def __getattr__(self, name):
diff --git a/mobly/controllers/android_device_lib/snippet_client.py b/mobly/controllers/android_device_lib/snippet_client.py
index 46477f4..6395b66 100644
--- a/mobly/controllers/android_device_lib/snippet_client.py
+++ b/mobly/controllers/android_device_lib/snippet_client.py
@@ -18,6 +18,7 @@ import time
 
 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 jsonrpc_client_base
 
 _INSTRUMENTATION_RUNNER_PACKAGE = (
@@ -55,10 +56,6 @@ _SETSID_COMMAND = 'setsid'
 _NOHUP_COMMAND = 'nohup'
 
 
-class Error(Exception):
-    pass
-
-
 class ProtocolVersionError(jsonrpc_client_base.AppStartError):
     """Raised when the protocol reported by the snippet is unknown."""
 
@@ -72,18 +69,18 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
     mobly-snippet-lib, SnippetRunner.java.
     """
 
-    def __init__(self, package, adb_proxy, log=logging.getLogger()):
+    def __init__(self, package, ad):
         """Initializes a SnippetClient.
 
         Args:
             package: (str) The package name of the apk where the snippets are
                 defined.
-            adb_proxy: (adb.AdbProxy) Adb proxy for running adb commands.
-            log: (logging.Logger) logger to which to send log messages.
+            ad: (AndroidDevice) the device object associated with this client.
         """
-        super(SnippetClient, self).__init__(app_name=package, log=log)
+        super(SnippetClient, self).__init__(app_name=package, ad=ad)
         self.package = package
-        self._adb = adb_proxy
+        self._ad = ad
+        self._adb = ad.adb
         self._proc = None
 
     def start_app_and_connect(self):
@@ -106,12 +103,12 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         line = self._read_protocol_line()
         match = re.match('^SNIPPET START, PROTOCOL ([0-9]+) ([0-9]+)$', line)
         if not match or match.group(1) != '1':
-            raise ProtocolVersionError(line)
+            raise ProtocolVersionError(self._ad, line)
 
         line = self._read_protocol_line()
         match = re.match('^SNIPPET SERVING, PORT ([0-9]+)$', line)
         if not match:
-            raise ProtocolVersionError(line)
+            raise ProtocolVersionError(self._ad, line)
         self.device_port = int(match.group(1))
 
         # Forward the device port to a new host port, and connect to that port
@@ -147,7 +144,7 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
             self.connect()
         except:
             # Failed to connect to app, something went wrong.
-            raise jsonrpc_client_base.AppRestoreConnectionError(
+            raise jsonrpc_client_base.AppRestoreConnectionError(self._ad
                 ('Failed to restore app connection for %s at host port %s, '
                  'device port %s'), self.package, self.host_port,
                 self.device_port)
@@ -170,8 +167,8 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
                 utils.stop_standing_subprocess(self._proc)
             out = self._adb.shell(_STOP_CMD % self.package).decode('utf-8')
             if 'OK (0 tests)' not in out:
-                raise Error('Failed to stop existing apk. Unexpected '
-                            'output: %s' % out)
+                raise errors.DeviceError(self._ad,
+                    'Failed to stop existing apk. Unexpected output: %s' % out)
         finally:
             # Always clean up the adb port
             if self.host_port:
@@ -179,8 +176,7 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
 
     def _start_event_client(self):
         """Overrides superclass."""
-        event_client = SnippetClient(
-            package=self.package, adb_proxy=self._adb, log=self.log)
+        event_client = SnippetClient(package=self.package, ad=self)
         event_client.host_port = self.host_port
         event_client.device_port = self.device_port
         event_client.connect(self.uid,
@@ -201,16 +197,15 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         out = self._adb.shell('pm list package')
         if not utils.grep('^package:%s$' % self.package, out):
             raise jsonrpc_client_base.AppStartError(
-                '%s is not installed on %s' % (self.package, self._adb.serial))
+                self._ad, '%s is not installed.' % self.package)
         # Check that the app is instrumented.
         out = self._adb.shell('pm list instrumentation')
         matched_out = utils.grep('^instrumentation:%s/%s' %
                                  (self.package,
                                   _INSTRUMENTATION_RUNNER_PACKAGE), out)
         if not matched_out:
-            raise jsonrpc_client_base.AppStartError(
-                '%s is installed on %s, but it is not instrumented.' %
-                (self.package, self._adb.serial))
+            raise jsonrpc_client_base.AppStartError(self._ad,
+                '%s is installed, but it is not instrumented.' % self.package)
         match = re.search('^instrumentation:(.*)\/(.*) \(target=(.*)\)$',
                           matched_out[0])
         target_name = match.group(3)
@@ -219,9 +214,9 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         if target_name != self.package:
             out = self._adb.shell('pm list package')
             if not utils.grep('^package:%s$' % target_name, out):
-                raise jsonrpc_client_base.AppStartError(
-                    'Instrumentation target %s is not installed on %s' %
-                    (target_name, self._adb.serial))
+                raise jsonrpc_client_base.AppStartError(self._ad,
+                    'Instrumentation target %s is not installed.' %
+                    target_name)
 
     def _do_start_app(self, launch_cmd):
         adb_cmd = [adb.ADB]
@@ -246,7 +241,7 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         while True:
             line = self._proc.stdout.readline().decode('utf-8')
             if not line:
-                raise jsonrpc_client_base.AppStartError(
+                raise jsonrpc_client_base.AppStartError(self._ad,
                     'Unexpected EOF waiting for app to start')
             # readline() uses an empty string to mark EOF, and a single newline
             # to mark regular empty lines in the output. Don't move the strip()
diff --git a/mobly/utils.py b/mobly/utils.py
index 79bf9e2..ac3b1aa 100644
--- a/mobly/utils.py
+++ b/mobly/utils.py
@@ -357,6 +357,11 @@ def stop_standing_subprocess(proc, kill_signal=signal.SIGTERM):
         logging.exception('Failed to kill standing subprocess %d', pid)
     if failed:
         raise Error('Failed to kill standing subprocesses: %s' % failed)
+    # Call wait and close pipes on the original Python object so we don't get
+    # runtime warnings.
+    proc.stdout.close()
+    proc.stderr.close()
+    proc.wait()
     logging.debug('Stopped standing subprocess %d', pid)
 
 


Test patch:
diff --git a/tests/lib/mock_android_device.py b/tests/lib/mock_android_device.py
index caa587e..567160e 100755
--- a/tests/lib/mock_android_device.py
+++ b/tests/lib/mock_android_device.py
@@ -68,7 +68,7 @@ class MockAdbProxy(object):
         self.fail_br = fail_br
         self.fail_br_before_N = fail_br_before_N
 
-    def shell(self, params):
+    def shell(self, params, timeout=None):
         if params == "id -u":
             return b"root"
         elif params == "bugreportz":
@@ -90,7 +90,7 @@ class MockAdbProxy(object):
         elif params == "sys.boot_completed":
             return "1"
 
-    def bugreport(self, args, shell=False):
+    def bugreport(self, args, shell=False, timeout=None):
         expected = os.path.join(logging.log_path,
                                 'AndroidDevice%s' % self.serial, 'BugReports',
                                 'test_something,sometime,%s' % self.serial)
diff --git a/tests/mobly/controllers/android_device_lib/callback_handler_test.py b/tests/mobly/controllers/android_device_lib/callback_handler_test.py
index 6a3e6d1..5aeacae 100755
--- a/tests/mobly/controllers/android_device_lib/callback_handler_test.py
+++ b/tests/mobly/controllers/android_device_lib/callback_handler_test.py
@@ -45,7 +45,8 @@ class CallbackHandlerTest(unittest.TestCase):
             callback_id=MOCK_CALLBACK_ID,
             event_client=mock_event_client,
             ret_value=None,
-            method_name=None)
+            method_name=None,
+            ad=mock.Mock())
         self.assertEqual(handler.callback_id, MOCK_CALLBACK_ID)
         with self.assertRaisesRegex(AttributeError, "can't set attribute"):
             handler.callback_id = 'ha'
@@ -58,7 +59,8 @@ class CallbackHandlerTest(unittest.TestCase):
             callback_id=MOCK_CALLBACK_ID,
             event_client=mock_event_client,
             ret_value=None,
-            method_name=None)
+            method_name=None,
+            ad=mock.Mock())
         event = handler.waitAndGet('ha')
         self.assertEqual(event.name, MOCK_RAW_EVENT['name'])
         self.assertEqual(event.creation_time, MOCK_RAW_EVENT['time'])
@@ -70,12 +72,14 @@ class CallbackHandlerTest(unittest.TestCase):
         java_timeout_msg = ('com.google.android.mobly.snippet.event.'
                             'EventSnippet$EventSnippetException: timeout.')
         mock_event_client.eventWaitAndGet = mock.Mock(
-            side_effect=jsonrpc_client_base.ApiError(java_timeout_msg))
+            side_effect=jsonrpc_client_base.ApiError(mock.Mock(),
+                java_timeout_msg))
         handler = callback_handler.CallbackHandler(
             callback_id=MOCK_CALLBACK_ID,
             event_client=mock_event_client,
             ret_value=None,
-            method_name=None)
+            method_name=None,
+            ad=mock.Mock())
         expected_msg = 'Timed out after waiting .*s for event "ha" .*'
         with self.assertRaisesRegex(callback_handler.TimeoutError,
                                     expected_msg):
@@ -89,7 +93,8 @@ class CallbackHandlerTest(unittest.TestCase):
             callback_id=MOCK_CALLBACK_ID,
             event_client=mock_event_client,
             ret_value=None,
-            method_name=None)
+            method_name=None,
+            ad=mock.Mock())
 
         def some_condition(event):
             return event.data['successful']
@@ -104,7 +109,8 @@ class CallbackHandlerTest(unittest.TestCase):
             callback_id=MOCK_CALLBACK_ID,
             event_client=mock_event_client,
             ret_value=None,
-            method_name=None)
+            method_name=None,
+            ad=mock.Mock())
         expected_msg = (
             'Timed out after 0.01s waiting for an "AsyncTaskResult" event that'
             ' satisfies the predicate "some_condition".')
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 aea7771..a696d88 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
@@ -25,7 +25,8 @@ from tests.lib import jsonrpc_client_test_base
 
 class FakeRpcClient(jsonrpc_client_base.JsonRpcClientBase):
     def __init__(self):
-        super(FakeRpcClient, self).__init__(app_name='FakeRpcClient')
+        super(FakeRpcClient, self).__init__(app_name='FakeRpcClient',
+            ad=mock.Mock())
 
 
 class JsonRpcClientBaseTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
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 589bc7f..47a25c6 100755
--- a/tests/mobly/controllers/android_device_lib/snippet_client_test.py
+++ b/tests/mobly/controllers/android_device_lib/snippet_client_test.py
@@ -77,14 +77,14 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
 
     def test_check_app_installed_fail_app_not_installed(self):
         sc = self._make_client(MockAdbProxy(apk_not_installed=True))
-        expected_msg = '%s is not installed on .*' % MOCK_PACKAGE_NAME
+        expected_msg = '.* %s is not installed.' % MOCK_PACKAGE_NAME
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
                                     expected_msg):
             sc._check_app_installed()
 
     def test_check_app_installed_fail_not_instrumented(self):
         sc = self._make_client(MockAdbProxy(apk_not_instrumented=True))
-        expected_msg = ('%s is installed on .*, but it is not instrumented.' %
+        expected_msg = ('.* %s is installed, but it is not instrumented.' %
                         MOCK_PACKAGE_NAME)
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
                                     expected_msg):
@@ -92,7 +92,7 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
 
     def test_check_app_installed_fail_target_not_installed(self):
         sc = self._make_client(MockAdbProxy(target_not_installed=True))
-        expected_msg = ('Instrumentation target %s is not installed on .*' %
+        expected_msg = ('.* Instrumentation target %s is not installed.' %
                         MOCK_MISSING_PACKAGE_NAME)
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
                                     expected_msg):
@@ -321,8 +321,10 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
 
     def _make_client(self, adb_proxy=None):
         adb_proxy = adb_proxy or MockAdbProxy()
+        ad = mock.Mock()
+        ad.adb = adb_proxy
         return snippet_client.SnippetClient(
-            package=MOCK_PACKAGE_NAME, adb_proxy=adb_proxy)
+            package=MOCK_PACKAGE_NAME, ad=ad)
 
     def _setup_mock_instrumentation_cmd(self, mock_start_standing_subprocess,
                                         resp_lines):
diff --git a/tests/mobly/utils_test.py b/tests/mobly/utils_test.py
index cc24cbf..ce36c96 100755
--- a/tests/mobly/utils_test.py
+++ b/tests/mobly/utils_test.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import mock
+import platform
 import socket
 import time
 from future.tests.base import unittest
@@ -29,13 +30,22 @@ class UtilsTest(unittest.TestCase):
     under mobly.utils.
     """
 
+    def setUp(self):
+        system = platform.system()
+        self.sleep_cmd = 'timeout' if system == 'Windows' else 'sleep'
+
     def test_start_standing_subproc(self):
-        p = utils.start_standing_subprocess(['sleep', '1'])
-        p1 = psutil.Process(p.pid)
-        self.assertTrue(p1.is_running())
+        try:
+            p = utils.start_standing_subprocess([self.sleep_cmd, '0.1'])
+            p1 = psutil.Process(p.pid)
+            self.assertTrue(p1.is_running())
+        finally:
+            p.stdout.close()
+            p.stderr.close()
+            p.wait()
 
     def test_stop_standing_subproc(self):
-        p = utils.start_standing_subprocess(['sleep', '4'])
+        p = utils.start_standing_subprocess([self.sleep_cmd, '4'])
         p1 = psutil.Process(p.pid)
         utils.stop_standing_subprocess(p)
         self.assertFalse(p1.is_running())