Repository: google/mobly

Issue:
`utils._assert_subprocess_running` throws `ValueError`
If the process died, we try to call `p.communicate` to get info from the process.
Then `communicate` would sometimes fail with ValueError saying it's trying to do I/O operation on a closed file.

Discussion:

Patch:
diff --git a/mobly/controllers/android_device_lib/callback_handler.py b/mobly/controllers/android_device_lib/callback_handler.py
index 1ab67d8..1b0a461 100644
--- a/mobly/controllers/android_device_lib/callback_handler.py
+++ b/mobly/controllers/android_device_lib/callback_handler.py
@@ -60,6 +60,10 @@ class CallbackHandler(object):
         self.ret_value = ret_value
         self._method_name = method_name
 
+    @property
+    def callback_id(self):
+        return self._id
+
     def waitAndGet(self, event_name, timeout=DEFAULT_TIMEOUT):
         """Blocks until an event of the specified name has been received and
         return the event, or timeout.
diff --git a/mobly/controllers/android_device_lib/snippet_client.py b/mobly/controllers/android_device_lib/snippet_client.py
index 6d656a4..0a6d462 100644
--- a/mobly/controllers/android_device_lib/snippet_client.py
+++ b/mobly/controllers/android_device_lib/snippet_client.py
@@ -23,12 +23,21 @@ from mobly.controllers.android_device_lib import jsonrpc_client_base
 _INSTRUMENTATION_RUNNER_PACKAGE = (
     'com.google.android.mobly.snippet.SnippetRunner')
 
-# TODO(adorokhine): delete this in Mobly 1.6 when snippet v0 support is removed.
-_LAUNCH_CMD_V0 = ('%s am instrument -w -e action start -e port %s %s/' +
-                  _INSTRUMENTATION_RUNNER_PACKAGE)
-
-_LAUNCH_CMD_V1 = ('%s am instrument -w -e action start %s/' +
-                  _INSTRUMENTATION_RUNNER_PACKAGE)
+# Major version of the launch and communication protocol being used by this
+# client.
+# Incrementing this means that compatibility with clients using the older
+# version is broken. Avoid breaking compatibility unless there is no other
+# choice.
+_PROTOCOL_MAJOR_VERSION = 1
+
+# Minor version of the launch and communication protocol.
+# Increment this when new features are added to the launch and communication
+# protocol that are backwards compatible with the old protocol and don't break
+# existing clients.
+_PROTOCOL_MINOR_VERSION = 0
+
+_LAUNCH_CMD = ('%s am instrument -w -e action start %s/' +
+               _INSTRUMENTATION_RUNNER_PACKAGE)
 
 _STOP_CMD = (
     'am instrument -w -e action stop %s/' + _INSTRUMENTATION_RUNNER_PACKAGE)
@@ -45,10 +54,6 @@ _SETSID_COMMAND = 'setsid'
 
 _NOHUP_COMMAND = 'nohup'
 
-# Maximum time to wait for a v0 snippet to start on the device (10 minutes).
-# TODO(adorokhine): delete this in Mobly 1.6 when snippet v0 support is removed.
-_APP_START_WAIT_TIME_V0 = 10 * 60
-
 
 class Error(Exception):
     pass
@@ -63,9 +68,6 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
 
     See superclass documentation for a list of public attributes.
 
-    It currently supports both v0 and v1 snippet launch protocols, although
-    support for v0 will be removed in a future version.
-
     For a description of the launch protocols, see the documentation in
     mobly-snippet-lib, SnippetRunner.java.
     """
@@ -83,60 +85,42 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         self.package = package
         self._adb = adb_proxy
         self._proc = None
-        self._launch_version = 'v1'
 
     def start_app_and_connect(self):
         """Overrides superclass. Launches a snippet app and connects to it."""
         self._check_app_installed()
 
         persists_shell_cmd = self._get_persist_command()
-        # Try launching the app with the v1 protocol. If that fails, fall back
-        # to v0 for compatibility. Use info here so people know exactly what's
-        # happening here, which is helpful since they need to create their own
-        # instrumentations and manifest.
-        self.log.info('Launching snippet apk %s with protocol v1',
-                      self.package)
-        cmd = _LAUNCH_CMD_V1 % (persists_shell_cmd, self.package)
+        # Use info here so people can follow along with the snippet startup
+        # process. Starting snippets can be slow, especially if there are
+        # multiple, and this avoids the perception that the framework is hanging
+        # for a long time doing nothing.
+        self.log.info('Launching snippet apk %s with protocol %d.%d',
+                      self.package, _PROTOCOL_MAJOR_VERSION,
+                      _PROTOCOL_MINOR_VERSION)
+        cmd = _LAUNCH_CMD % (persists_shell_cmd, self.package)
         start_time = time.time()
         self._proc = self._do_start_app(cmd)
 
-        # "Instrumentation crashed" could be due to several reasons, eg
-        # exception thrown during startup or just a launch protocol 0 snippet
-        # dying because it needs the port flag. Sadly we have no way to tell so
-        # just warn and retry as v0.
-        # TODO(adorokhine): delete this in Mobly 1.6 when snippet v0 support is
-        # removed.
+        # Check protocol version and get the device port
         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)
+
+        line = self._read_protocol_line()
+        match = re.match('^SNIPPET SERVING, PORT ([0-9]+)$', line)
+        if not match:
+            raise ProtocolVersionError(line)
+        self.device_port = int(match.group(1))
+
         # Forward the device port to a new host port, and connect to that port
         self.host_port = utils.get_available_host_port()
-        if line in ('INSTRUMENTATION_RESULT: shortMsg=Process crashed.',
-                    'INSTRUMENTATION_RESULT: shortMsg='
-                    'java.lang.IllegalArgumentException'):
-            self.log.warning('Snippet %s crashed on startup. This might be an '
-                             'actual error or a snippet using deprecated v0 '
-                             'start protocol. Retrying as a v0 snippet.',
-                             self.package)
-            # Reuse the host port as the device port in v0 snippet. This isn't
-            # safe in general, but the protocol is deprecated.
-            self.device_port = self.host_port
-            cmd = _LAUNCH_CMD_V0 % (persists_shell_cmd, self.device_port,
-                                    self.package)
-            self._proc = self._do_start_app(cmd)
-            self._connect_to_v0()
-            self._launch_version = 'v0'
-        else:
-            # Check protocol version and get the device port
-            match = re.match('^SNIPPET START, PROTOCOL ([0-9]+) ([0-9]+)$',
-                             line)
-            if not match or match.group(1) != '1':
-                raise ProtocolVersionError(line)
-
-            line = self._read_protocol_line()
-            match = re.match('^SNIPPET SERVING, PORT ([0-9]+)$', line)
-            if not match:
-                raise ProtocolVersionError(line)
-            self.device_port = int(match.group(1))
-            self._connect_to_v1()
+        self._adb.forward(
+            ['tcp:%d' % self.host_port, 'tcp:%d' % self.device_port])
+        self.connect()
+
+        # 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)
 
@@ -157,11 +141,10 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
             AppRestoreConnectionError: When the app was not able to be started.
         """
         self.host_port = port or utils.get_available_host_port()
+        self._adb.forward(
+            ['tcp:%d' % self.host_port, 'tcp:%d' % self.device_port])
         try:
-            if self._launch_version == 'v0':
-                self._connect_to_v0()
-            else:
-                self._connect_to_v1()
+            self.connect()
         except:
             # Failed to connect to app, something went wrong.
             raise jsonrpc_client_base.AppRestoreConnectionError(
@@ -247,32 +230,6 @@ class SnippetClient(jsonrpc_client_base.JsonRpcClientBase):
         adb_cmd += ['shell', launch_cmd]
         return utils.start_standing_subprocess(adb_cmd, shell=False)
 
-    # TODO(adorokhine): delete this in Mobly 1.6 when snippet v0 support is
-    # removed.
-    def _connect_to_v0(self):
-        self._adb.forward(
-            ['tcp:%d' % self.host_port, 'tcp:%d' % self.device_port])
-        start_time = time.time()
-        expiration_time = start_time + _APP_START_WAIT_TIME_V0
-        while time.time() < expiration_time:
-            self.log.debug('Attempting to start %s.', self.package)
-            try:
-                self.connect()
-                return
-            except:
-                self.log.debug(
-                    'v0 snippet %s is not yet running, retrying',
-                    self.package,
-                    exc_info=True)
-            time.sleep(1)
-        raise jsonrpc_client_base.AppStartError(
-            '%s failed to start on %s.' % (self.package, self._adb.serial))
-
-    def _connect_to_v1(self):
-        self._adb.forward(
-            ['tcp:%d' % self.host_port, 'tcp:%d' % self.device_port])
-        self.connect()
-
     def _read_protocol_line(self):
         """Reads the next line of instrumentation output relevant to snippets.
 
diff --git a/mobly/utils.py b/mobly/utils.py
index 6960651..05371b2 100644
--- a/mobly/utils.py
+++ b/mobly/utils.py
@@ -273,29 +273,13 @@ def concurrent_exec(func, param_list):
             try:
                 return_vals.append(future.result())
             except Exception as exc:
-                print("{} generated an exception: {}".format(
+                logging.exception("{} generated an exception: {}".format(
                     params, traceback.format_exc()))
                 return_vals.append(exc)
         return return_vals
 
 
-def _assert_subprocess_running(proc):
-    """Checks if a subprocess has terminated on its own.
-
-    Args:
-        proc: A subprocess returned by subprocess.Popen.
-
-    Raises:
-        Error is raised if the subprocess has stopped.
-    """
-    ret = proc.poll()
-    if ret is not None:
-        out, err = proc.communicate()
-        raise Error("Process %d has terminated. ret: %d, stderr: %s,"
-                    " stdout: %s" % (proc.pid, ret, err, out))
-
-
-def start_standing_subprocess(cmd, check_health_delay=0, shell=False):
+def start_standing_subprocess(cmd, shell=False):
     """Starts a long-running subprocess.
 
     This is not a blocking call and the subprocess started by it should be
@@ -304,36 +288,27 @@ def start_standing_subprocess(cmd, check_health_delay=0, shell=False):
     For short-running commands, you should use subprocess.check_call, which
     blocks.
 
-    You can specify a health check after the subprocess is started to make sure
-    it did not stop prematurely.
-
     Args:
         cmd: string, the command to start the subprocess with.
-        check_health_delay: float, the number of seconds to wait after the
-                            subprocess starts to check its health. Default is 0,
-                            which means no check.
         shell: bool, True to run this command through the system shell,
             False to invoke it directly. See subprocess.Proc() docs.
 
     Returns:
         The subprocess that was started.
     """
-    logging.debug('Start standing subprocess with cmd: %s', cmd)
+    logging.debug('Starting standing subprocess with: %s', cmd)
     proc = subprocess.Popen(
         cmd,
         stdin=subprocess.PIPE,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         shell=shell)
-    logging.debug('Start standing subprocess with cmd: %s', cmd)
     # Leaving stdin open causes problems for input, e.g. breaking the
     # code.inspect() shell (http://stackoverflow.com/a/25512460/1612937), so
     # explicitly close it assuming it is not needed for standing subprocesses.
     proc.stdin.close()
     proc.stdin = None
-    if check_health_delay > 0:
-        time.sleep(check_health_delay)
-        _assert_subprocess_running(proc)
+    logging.debug('Started standing subprocess %d', proc.pid)
     return proc
 
 
@@ -352,10 +327,9 @@ def stop_standing_subprocess(proc, kill_signal=signal.SIGTERM):
         Error: if the subprocess could not be stopped.
     """
     pid = proc.pid
-    logging.debug('Stop standing subprocess %d', pid)
-    _assert_subprocess_running(proc)
+    logging.debug('Stopping standing subprocess %d', pid)
     process = psutil.Process(pid)
-    success = True
+    failed = []
     try:
         children = process.children(recursive=True)
     except AttributeError:
@@ -365,18 +339,25 @@ def stop_standing_subprocess(proc, kill_signal=signal.SIGTERM):
         try:
             child.kill()
             child.wait(timeout=10)
+        except psutil.NoSuchProcess:
+            # Ignore if the child process has already terminated.
+            pass
         except:
-            success = False
+            failed.append(child.pid)
             logging.exception('Failed to kill standing subprocess %d',
                               child.pid)
     try:
         process.kill()
         process.wait(timeout=10)
+    except psutil.NoSuchProcess:
+        # Ignore if the process has already terminated.
+        pass
     except:
-        success = False
+        failed.append(pid)
         logging.exception('Failed to kill standing subprocess %d', pid)
-    if not success:
-        raise Error('Some standing subprocess failed to die')
+    if failed:
+        raise Error('Failed to kill standing subprocesses: %s' % failed)
+    logging.debug('Stopped standing subprocess %d', pid)
 
 
 def wait_for_standing_subprocess(proc, timeout=None):


Test patch:
diff --git a/mobly/test_runner.py b/mobly/test_runner.py
index 8f9aba4..49e6b45 100644
--- a/mobly/test_runner.py
+++ b/mobly/test_runner.py
@@ -80,7 +80,7 @@ def main(argv=None):
         help='Specify which test beds to run tests on.')
     if not argv:
         argv = sys.argv[1:]
-    args = parser.parse_args(argv)
+    args = parser.parse_known_args(argv)[0]
     # Load test config file.
     test_configs = config_parser.load_test_config_file(args.config[0],
                                                        args.test_bed)
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 f288ef3..6a3e6d1 100755
--- a/tests/mobly/controllers/android_device_lib/callback_handler_test.py
+++ b/tests/mobly/controllers/android_device_lib/callback_handler_test.py
@@ -39,6 +39,17 @@ class CallbackHandlerTest(unittest.TestCase):
         self.assertGreaterEqual(jsonrpc_client_base._SOCKET_READ_TIMEOUT,
                                 callback_handler.MAX_TIMEOUT)
 
+    def test_callback_id_property(self):
+        mock_event_client = mock.Mock()
+        handler = callback_handler.CallbackHandler(
+            callback_id=MOCK_CALLBACK_ID,
+            event_client=mock_event_client,
+            ret_value=None,
+            method_name=None)
+        self.assertEqual(handler.callback_id, MOCK_CALLBACK_ID)
+        with self.assertRaisesRegex(AttributeError, "can't set attribute"):
+            handler.callback_id = 'ha'
+
     def test_event_dict_to_snippet_event(self):
         mock_event_client = mock.Mock()
         mock_event_client.eventWaitAndGet = mock.Mock(
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 7e127c9..589bc7f 100755
--- a/tests/mobly/controllers/android_device_lib/snippet_client_test.py
+++ b/tests/mobly/controllers/android_device_lib/snippet_client_test.py
@@ -70,6 +70,7 @@ class MockAdbProxy(object):
 class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
     """Unit tests for mobly.controllers.android_device_lib.snippet_client.
     """
+
     def test_check_app_installed_normal(self):
         sc = self._make_client()
         sc._check_app_installed()
@@ -78,23 +79,23 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
         sc = self._make_client(MockAdbProxy(apk_not_installed=True))
         expected_msg = '%s is not installed on .*' % MOCK_PACKAGE_NAME
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
-                                     expected_msg):
+                                    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.'
-                        % MOCK_PACKAGE_NAME)
+        expected_msg = ('%s is installed on .*, but it is not instrumented.' %
+                        MOCK_PACKAGE_NAME)
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
-                                     expected_msg):
+                                    expected_msg):
             sc._check_app_installed()
 
     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 .*'
-                        % MOCK_MISSING_PACKAGE_NAME)
+        expected_msg = ('Instrumentation target %s is not installed on .*' %
+                        MOCK_MISSING_PACKAGE_NAME)
         with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
-                                     expected_msg):
+                                    expected_msg):
             sc._check_app_installed()
 
     @mock.patch('socket.create_connection')
@@ -164,9 +165,9 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
                 'utils.start_standing_subprocess')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.get_available_host_port')
-    def test_snippet_start_app_and_connect_v1(self, mock_get_port,
-                                              mock_start_standing_subprocess,
-                                              mock_create_connection):
+    def test_snippet_start_app_and_connect(self, 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,
@@ -185,13 +186,12 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'SnippetClient._read_protocol_line')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
-                'SnippetClient._connect_to_v1')
+                'SnippetClient.connect')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.get_available_host_port')
-    def test_snippet_start_app_and_connect_v1_persistent_session(
-            self, mock_get_port, mock_connect_to_v1, mock_read_protocol_line,
+    def test_snippet_start_app_and_connect_persistent_session(
+            self, mock_get_port, mock_connect, mock_read_protocol_line,
             mock_check_app_installed, mock_do_start_app):
-
         def _mocked_shell(arg):
             if 'setsid' in arg:
                 raise adb.AdbError('cmd', 'stdout', 'stderr', 'ret_code')
@@ -213,8 +213,7 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
         client._adb.shell = mock.Mock(return_value=b'setsid')
         client.start_app_and_connect()
         cmd_setsid = '%s am instrument -w -e action start %s/%s' % (
-            snippet_client._SETSID_COMMAND,
-            MOCK_PACKAGE_NAME,
+            snippet_client._SETSID_COMMAND, MOCK_PACKAGE_NAME,
             snippet_client._INSTRUMENTATION_RUNNER_PACKAGE)
         mock_do_start_app.assert_has_calls(mock.call(cmd_setsid))
 
@@ -223,25 +222,20 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
         client._adb.shell = _mocked_shell
         client.start_app_and_connect()
         cmd_nohup = '%s am instrument -w -e action start %s/%s' % (
-            snippet_client._NOHUP_COMMAND,
-            MOCK_PACKAGE_NAME,
+            snippet_client._NOHUP_COMMAND, MOCK_PACKAGE_NAME,
             snippet_client._INSTRUMENTATION_RUNNER_PACKAGE)
-        mock_do_start_app.assert_has_calls([
-            mock.call(cmd_setsid),
-            mock.call(cmd_nohup)
-        ])
+        mock_do_start_app.assert_has_calls(
+            [mock.call(cmd_setsid), mock.call(cmd_nohup)])
 
         # Test both 'setsid' and 'nohup' do not exist
-        client._adb.shell = mock.Mock(
-            side_effect=adb.AdbError('cmd', 'stdout', 'stderr', 'ret_code'))
+        client._adb.shell = mock.Mock(side_effect=adb.AdbError(
+            'cmd', 'stdout', 'stderr', 'ret_code'))
         client = self._make_client()
         client.start_app_and_connect()
         cmd_not_persist = ' am instrument -w -e action start %s/%s' % (
-            MOCK_PACKAGE_NAME,
-            snippet_client._INSTRUMENTATION_RUNNER_PACKAGE)
+            MOCK_PACKAGE_NAME, snippet_client._INSTRUMENTATION_RUNNER_PACKAGE)
         mock_do_start_app.assert_has_calls([
-            mock.call(cmd_setsid),
-            mock.call(cmd_nohup),
+            mock.call(cmd_setsid), mock.call(cmd_nohup),
             mock.call(cmd_not_persist)
         ])
 
@@ -250,9 +244,9 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
                 'utils.start_standing_subprocess')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.get_available_host_port')
-    def test_snippet_start_app_and_connect_v0(self, mock_get_port,
-                                              mock_start_standing_subprocess,
-                                              mock_create_connection):
+    def test_snippet_start_app_crash(self, mock_get_port,
+                                     mock_start_standing_subprocess,
+                                     mock_create_connection):
         mock_get_port.return_value = 456
         self.setup_mock_socket_file(mock_create_connection)
         self._setup_mock_instrumentation_cmd(
@@ -261,8 +255,10 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
                 b'INSTRUMENTATION_RESULT: shortMsg=Process crashed.\n'
             ])
         client = self._make_client()
-        client.start_app_and_connect()
-        self.assertEqual(456, client.device_port)
+        with self.assertRaisesRegex(
+                snippet_client.ProtocolVersionError,
+                'INSTRUMENTATION_RESULT: shortMsg=Process crashed.'):
+            client.start_app_and_connect()
 
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.start_standing_subprocess')
@@ -275,7 +271,8 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
             mock_start_standing_subprocess,
             resp_lines=[b'SNIPPET START, PROTOCOL 99 0\n'])
         client = self._make_client()
-        with self.assertRaises(snippet_client.ProtocolVersionError):
+        with self.assertRaisesRegex(snippet_client.ProtocolVersionError,
+                                    'SNIPPET START, PROTOCOL 99 0'):
             client.start_app_and_connect()
 
     @mock.patch('socket.create_connection')
@@ -283,7 +280,7 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
                 'utils.start_standing_subprocess')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.get_available_host_port')
-    def test_snippet_start_app_and_connect_v1_header_junk(
+    def test_snippet_start_app_and_connect_header_junk(
             self, mock_get_port, mock_start_standing_subprocess,
             mock_create_connection):
         self.setup_mock_socket_file(mock_create_connection)
@@ -300,28 +297,6 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
         client.start_app_and_connect()
         self.assertEqual(123, client.device_port)
 
-    @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')
-    def test_snippet_start_app_and_connect_v0_header_junk(
-            self, mock_get_port, mock_start_standing_subprocess,
-            mock_create_connection):
-        mock_get_port.return_value = 456
-        self.setup_mock_socket_file(mock_create_connection)
-        self._setup_mock_instrumentation_cmd(
-            mock_start_standing_subprocess,
-            resp_lines=[
-                b'This is some header junk\n',
-                b'Some phones print arbitrary output\n',
-                b'\n',
-                b'INSTRUMENTATION_RESULT: shortMsg=Process crashed.\n',
-            ])
-        client = self._make_client()
-        client.start_app_and_connect()
-        self.assertEqual(456, client.device_port)
-
     @mock.patch('socket.create_connection')
     @mock.patch('mobly.controllers.android_device_lib.snippet_client.'
                 'utils.start_standing_subprocess')
@@ -340,12 +315,12 @@ class SnippetClientTest(jsonrpc_client_test_base.JsonRpcClientTestBase):
                 b'',  # readline uses '' to mark EOF
             ])
         client = self._make_client()
-        with self.assertRaisesRegex(
-                jsonrpc_client_base.AppStartError,
-                'Unexpected EOF waiting for app to start'):
+        with self.assertRaisesRegex(jsonrpc_client_base.AppStartError,
+                                    'Unexpected EOF waiting for app to start'):
             client.start_app_and_connect()
 
-    def _make_client(self, adb_proxy=MockAdbProxy()):
+    def _make_client(self, adb_proxy=None):
+        adb_proxy = adb_proxy or MockAdbProxy()
         return snippet_client.SnippetClient(
             package=MOCK_PACKAGE_NAME, adb_proxy=adb_proxy)
 
diff --git a/tests/mobly/test_runner_test.py b/tests/mobly/test_runner_test.py
index 7690c22..17d54c3 100755
--- a/tests/mobly/test_runner_test.py
+++ b/tests/mobly/test_runner_test.py
@@ -326,6 +326,18 @@ class TestRunnerTest(unittest.TestCase):
         finally:
             setattr(mock_controller, 'MOBLY_CONTROLLER_CONFIG_NAME', tmp)
 
+    @mock.patch(
+        'mobly.test_runner._find_test_class',
+        return_value=type('SampleTest', (), {}))
+    @mock.patch(
+        'mobly.test_runner.config_parser.load_test_config_file',
+        return_value=[config_parser.TestRunConfig()])
+    @mock.patch('mobly.test_runner.TestRunner', return_value=mock.MagicMock())
+    def test_main_parse_args(self, mock_test_runner, mock_config,
+                             mock_find_test):
+        test_runner.main(['-c', 'some/path/foo.yaml', '-b', 'hello'])
+        mock_config.assert_called_with('some/path/foo.yaml', None)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/mobly/utils_test.py b/tests/mobly/utils_test.py
index 1e2cdc1..cc24cbf 100755
--- a/tests/mobly/utils_test.py
+++ b/tests/mobly/utils_test.py
@@ -18,6 +18,7 @@ import time
 from future.tests.base import unittest
 
 import portpicker
+import psutil
 from mobly import utils
 
 MOCK_AVAILABLE_PORT = 5
@@ -27,21 +28,17 @@ class UtilsTest(unittest.TestCase):
     """This test class has unit tests for the implementation of everything
     under mobly.utils.
     """
+
     def test_start_standing_subproc(self):
-        with self.assertRaisesRegex(utils.Error, 'Process .* has terminated'):
-            utils.start_standing_subprocess(
-                ['sleep', '0'], check_health_delay=0.5)
+        p = utils.start_standing_subprocess(['sleep', '1'])
+        p1 = psutil.Process(p.pid)
+        self.assertTrue(p1.is_running())
 
     def test_stop_standing_subproc(self):
-        p = utils.start_standing_subprocess(['sleep', '5'])
+        p = utils.start_standing_subprocess(['sleep', '4'])
+        p1 = psutil.Process(p.pid)
         utils.stop_standing_subprocess(p)
-        self.assertIsNotNone(p.poll())
-
-    def test_stop_standing_subproc_already_dead(self):
-        p = utils.start_standing_subprocess(['sleep', '0'])
-        time.sleep(0.5)
-        with self.assertRaisesRegex(utils.Error, 'Process .* has terminated'):
-            utils.stop_standing_subprocess(p)
+        self.assertFalse(p1.is_running())
 
     @mock.patch(
         'mobly.controllers.android_device_lib.adb.list_occupied_adb_ports')