xpconanfan: 



Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

*[tests/mobly/utils_test.py, line 33 at r1](https://reviewable.io:443/reviews/google/mobly/164#-Kft4IlohqXOZQbkgvwN:-Kft4IlpLhgfHHd6AJ8z:b-a1kill) ([raw file](https://github.com/google/mobly/blob/7b01a61aa1a22614e0e70eae04eb10370a4c8f4f/tests/mobly/utils_test.py#L33)):*
> ```Python
>                                      "Process .* has terminated"):
>             utils.start_standing_subprocess(
>                 ['sleep','0'], check_health_delay=0.5)
> ```

why the change from 0.1 to 0.5 and str to list?
this does not seem to be related to `exe_cmd`?

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164)*
<!-- Sent from Reviewable.io -->

adorokhine: 



Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

*[tests/mobly/utils_test.py, line 33 at r1](https://reviewable.io:443/reviews/google/mobly/164#-Kft4IlohqXOZQbkgvwN:-Kft95spFobm0QYGR4uD:b-896fix) ([raw file](https://github.com/google/mobly/blob/454b556b53133176e6e1f02ae6e0b3f41739bcb1/tests/mobly/utils_test.py#L33)):*
<details><summary><i>Previously, xpconanfan (Ang Li) wrote…</i></summary><blockquote>

why the change from 0.1 to 0.5 and str to list?
this does not seem to be related to `exe_cmd`?
</blockquote></details>

Done.

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164)*
<!-- Sent from Reviewable.io -->

dthkao: <img class="emoji" title=":lgtm:" alt=":lgtm:" align="absmiddle" src="https://reviewable.io/lgtm.png" height="20" width="61"/>

---

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---



*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164#-:-KftHytyvUUIumc_97NM:bnfp4nl)*
<!-- Sent from Reviewable.io -->

xpconanfan: 



Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

*[tests/mobly/utils_test.py, line 33 at r1](https://reviewable.io:443/reviews/google/mobly/164#-Kft4IlohqXOZQbkgvwN:-Kg0s0bj_r8a4fdh3Uew:b-yz7gkv) ([raw file](https://github.com/google/mobly/blob/454b556b53133176e6e1f02ae6e0b3f41739bcb1/tests/mobly/utils_test.py#L33)):*
<details><summary><i>Previously, adorokhine (Alexander Dorokhine) wrote…</i></summary><blockquote>

Done.
</blockquote></details>

I still see this being changed to a list.
Is this a leak from the start standing subprocess change?

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164)*
<!-- Sent from Reviewable.io -->

adorokhine: 



Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

*[tests/mobly/utils_test.py, line 33 at r1](https://reviewable.io:443/reviews/google/mobly/164#-Kft4IlohqXOZQbkgvwN:-Kg1Ig0bDteHmFq-Yszy:brssb8g) ([raw file](https://github.com/google/mobly/blob/454b556b53133176e6e1f02ae6e0b3f41739bcb1/tests/mobly/utils_test.py#L33)):*
<details><summary><i>Previously, xpconanfan (Ang Li) wrote…</i></summary><blockquote>

I still see this being changed to a list.
Is this a leak from the start standing subprocess change?
</blockquote></details>

Thanks, good catch. Done.

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164)*
<!-- Sent from Reviewable.io -->

xpconanfan: <img class="emoji" title=":lgtm:" alt=":lgtm:" align="absmiddle" src="https://reviewable.io/lgtm.png" height="20" width="61"/>

---

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---



*Comments from [Reviewable](https://reviewable.io:443/reviews/google/mobly/164#-:-KgFrMky1iYJR0yzvtbz:bnfp4nl)*
<!-- Sent from Reviewable.io -->

diff --git a/mobly/controllers/sniffer_lib/local/local_base.py b/mobly/controllers/sniffer_lib/local/local_base.py
index 859c242..781e4b6 100644
--- a/mobly/controllers/sniffer_lib/local/local_base.py
+++ b/mobly/controllers/sniffer_lib/local/local_base.py
@@ -54,9 +54,10 @@ class SnifferLocalBase(sniffer.Sniffer):
         self._base_configs = base_configs
 
         try:
-            utils.exe_cmd("ifconfig", self._interface, "down")
-            utils.exe_cmd("iwconfig", self._interface, "mode", "monitor")
-            utils.exe_cmd("ifconfig", self._interface, "up")
+            subprocess.check_call(['ifconfig', self._interface, 'down'])
+            subprocess.check_call(
+                ['iwconfig', self._interface, 'mode', 'monitor'])
+            subprocess.check_call(['ifconfig', self._interface, 'up'])
         except Exception as err:
             raise sniffer.ExecutionError(err)
 
@@ -87,8 +88,11 @@ class SnifferLocalBase(sniffer.Sniffer):
 
         if sniffer.Sniffer.CONFIG_KEY_CHANNEL in final_configs:
             try:
-                utils.exe_cmd("iwconfig", self._interface, "channel",
-                        str(final_configs[sniffer.Sniffer.CONFIG_KEY_CHANNEL]))
+                subprocess.check_call([
+                    'iwconfig',
+                    self._interface,
+                    'channel',
+                    str(final_configs[sniffer.Sniffer.CONFIG_KEY_CHANNEL])])
             except Exception as err:
                 raise sniffer.ExecutionError(err)
 
diff --git a/mobly/utils.py b/mobly/utils.py
index ba47c60..c9be01e 100644
--- a/mobly/utils.py
+++ b/mobly/utils.py
@@ -280,27 +280,6 @@ def concurrent_exec(func, param_list):
         return return_vals
 
 
-def exe_cmd(*cmds):
-    """Executes commands in a new shell.
-
-    Args:
-        cmds: A sequence of commands and arguments.
-
-    Returns:
-        The output of the command run.
-
-    Raises:
-        OSError is raised if an error occurred during the command execution.
-    """
-    cmd = ' '.join(cmds)
-    proc = subprocess.Popen(
-        cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
-    (out, err) = proc.communicate()
-    if not err:
-        return out
-    raise OSError(err)
-
-
 def _assert_subprocess_running(proc):
     """Checks if a subprocess has terminated on its own.
 
@@ -368,14 +347,21 @@ def stop_standing_subprocess(proc, kill_signal=signal.SIGTERM):
     _assert_subprocess_running(proc)
     process = psutil.Process(pid)
     success = True
-    for child in process.children(recursive=True):
+    try:
+        children = process.children(recursive=True)
+    except AttributeError:
+        # Handle versions <3.0.0 of psutil.
+        children = process.get_children(recursive=True)
+    for child in children:
         try:
             child.kill()
+            child.wait(timeout=10)
         except:
             success = False
             logging.exception('Failed to kill standing subprocess %d', child.pid)
     try:
         process.kill()
+        process.wait(timeout=10)
     except:
         success = False
         logging.exception('Failed to kill standing subprocess %d', pid)

Flake in unit tests
Reported in #87 
https://travis-ci.org/google/mobly/jobs/197104959

=================================== FAILURES ===================================
____________________ UtilsTest.test_start_standing_subproc _____________________
self = <tests.mobly.utils_test.UtilsTest testMethod=test_start_standing_subproc>
    def test_start_standing_subproc(self):
        with self.assertRaisesRegexp(utils.Error,
                                     "Process .* has terminated"):
\>           utils.start_standing_subprocess("sleep 0", check_health_delay=0.1)
E           AssertionError: Error not raised
tests/mobly/utils_test.py:32: AssertionErro
diff --git a/tests/mobly/controllers/android_device_test.py b/tests/mobly/controllers/android_device_test.py
index c5b3733..3a0940e 100755
--- a/tests/mobly/controllers/android_device_test.py
+++ b/tests/mobly/controllers/android_device_test.py
@@ -209,8 +209,7 @@ class AndroidDeviceTest(unittest.TestCase):
     @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
                 return_value=mock_android_device.MockFastbootProxy(1))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch('mobly.utils.exe_cmd')
-    def test_AndroidDevice_take_bug_report(self, exe_mock, create_dir_mock,
+    def test_AndroidDevice_take_bug_report(self, create_dir_mock,
                                            FastbootProxy, MockAdbProxy):
         """Verifies AndroidDevice.take_bug_report calls the correct adb command
         and writes the bugreport file to the correct path.
@@ -227,8 +226,7 @@ class AndroidDeviceTest(unittest.TestCase):
     @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
                 return_value=mock_android_device.MockFastbootProxy(1))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch('mobly.utils.exe_cmd')
-    def test_AndroidDevice_take_bug_report_fail(self, exe_mock, create_dir_mock,
+    def test_AndroidDevice_take_bug_report_fail(self, create_dir_mock,
                                                 FastbootProxy, MockAdbProxy):
         """Verifies AndroidDevice.take_bug_report writes out the correct message
         when taking bugreport fails.
@@ -245,8 +243,7 @@ class AndroidDeviceTest(unittest.TestCase):
     @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
                 return_value=mock_android_device.MockFastbootProxy(1))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch('mobly.utils.exe_cmd')
-    def test_AndroidDevice_take_bug_report_fallback(self, exe_mock,
+    def test_AndroidDevice_take_bug_report_fallback(self,
         create_dir_mock, FastbootProxy, MockAdbProxy):
         """Verifies AndroidDevice.take_bug_report falls back to traditional
         bugreport on builds that do not have bugreportz.
diff --git a/tests/mobly/utils_test.py b/tests/mobly/utils_test.py
index fb2f519..de618a5 100755
--- a/tests/mobly/utils_test.py
+++ b/tests/mobly/utils_test.py
@@ -32,6 +32,11 @@ class UtilsTest(unittest.TestCase):
             utils.start_standing_subprocess("sleep 0", check_health_delay=0.1)
 
     def test_stop_standing_subproc(self):
+        p = utils.start_standing_subprocess('sleep 5')
+        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.1)
         with self.assertRaisesRegexp(utils.Error,

pip install -e .[dev]
parse_log_pytest
requirements.txt
pytest
apt-get update apt-get install -y gcc
3.9
requirements/base.txt
pytest --no-header -rA --tb=line --color=no -p no:cacheprovider -W ignore::DeprecationWarning
exceptiongroup==1.2.2
future==1.0.0
iniconfig==2.1.0
-e git+https://github.com/google/mobly.git@464a16aee403100025085846f0b6a5e29620e5b6#egg=mobly
mock==1.0.1
packaging==24.2
pluggy==1.5.0
psutil==7.0.0
pytest==8.3.5
pytz==2025.2
PyYAML==6.0.2
tomli==2.2.1

name: mobly
channels:
  - defaults
  - https://repo.anaconda.com/pkgs/main
  - https://repo.anaconda.com/pkgs/r
  - conda-forge
dependencies:
  - _libgcc_mutex=0.1=main
  - _openmp_mutex=5.1=1_gnu
  - ca-certificates=2025.2.25=h06a4308_0
  - ld_impl_linux-64=2.40=h12ee557_0
  - libffi=3.4.4=h6a678d5_1
  - libgcc-ng=11.2.0=h1234567_1
  - libgomp=11.2.0=h1234567_1
  - libstdcxx-ng=11.2.0=h1234567_1
  - ncurses=6.4=h6a678d5_0
  - openssl=3.0.16=h5eee18b_0
  - pip=25.0=py39h06a4308_0
  - python=3.9.21=he870216_1
  - readline=8.2=h5eee18b_0
  - setuptools=75.8.0=py39h06a4308_0
  - sqlite=3.45.3=h5eee18b_0
  - tk=8.6.14=h39e8969_0
  - tzdata=2025a=h04d1e81_0
  - wheel=0.45.1=py39h06a4308_0
  - xz=5.6.4=h5eee18b_1
  - zlib=1.2.13=h5eee18b_1
  - pip:
      - exceptiongroup==1.2.2
      - future==1.0.0
      - iniconfig==2.1.0
      - mock==1.0.1
      - packaging==24.2
      - pluggy==1.5.0
      - psutil==7.0.0
      - pytest==8.3.5
      - pytz==2025.2
      - pyyaml==6.0.2
      - tomli==2.2.1
prefix: /opt/conda/envs/mobly

tests/mobly/utils_test.py::UtilsTest::test_stop_standing_subproc
tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_build_info tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_cat_adb_log tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_debug_tag tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_instantiation tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_load_snippet tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_load_snippet_dup_attribute_name tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_load_snippet_dup_package tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_load_snippet_dup_snippet_name tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_snippet_cleanup tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_take_bug_report tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_take_bug_report_fail tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_take_bug_report_fallback tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_take_logcat tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_AndroidDevice_take_logcat_with_user_param tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_dict_list tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_empty_config tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_no_valid_config tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_not_list_config tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_pickup_all tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_create_with_string_list tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_get_device_no_match tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_get_device_success_with_serial tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_get_device_success_with_serial_and_extra_field tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_get_device_too_many_matches tests/mobly/controllers/android_device_test.py::AndroidDeviceTest::test_start_services_on_ads tests/mobly/utils_test.py::UtilsTest::test_is_port_available_negative tests/mobly/utils_test.py::UtilsTest::test_is_port_available_positive tests/mobly/utils_test.py::UtilsTest::test_start_standing_subproc tests/mobly/utils_test.py::UtilsTest::test_stop_standing_subproc_already_dead
mobly/controllers/sniffer_lib/local/local_base.py mobly/utils.py
mobly/controllers/sniffer_lib/local/local_base.py mobly/utils.py