search

google / mobly

E2E test framework for tests with complex environment requirements.
https://github.com/google/mobly
Apache License 2.0
621 stars 174 forks source link

Fix ps check of child pids on BSD-based MacOS. Use pgrep instead. #916

Closed stuartmls closed 1 month ago

stuartmls commented 2 months ago

Issue:

Unregistering a service results in the following error:

ps: illegal option -- -
usage: ps [-AaCcEefhjlMmrSTvwXx] [-O fmt | -o fmt] [-G gid[,gid...]]
          [-g grp[,grp...]] [-u [uid,uid...]]
          [-p pid[,pid...]] [-t tty[,tty...]] [-U user[,user...]]
       ps [-L]

Call-stack:

_collect_process_tree (lib/python3.12/site-packages/mobly/utils.py:271) _kill_process_tree (lib/python3.12/site-packages/mobly/utils.py:316) stop_standing_subprocess (lib/python3.12/site-packages/mobly/utils.py:555) _stop_server (lib/python3.12/site-packages/mobly/controllers/android_device_lib/snippet_client_v2.py:699) stop (python3.12/site-packages/mobly/controllers/android_device_lib/snippet_client_v2.py:660) remove_snippet_client (lib/python3.12/site-packages/mobly/controllers/android_device_lib/services/snippet_management_service.py:110) unload_snippet (lib/python3.12/site-packages/mobly/controllers/android_device.py:963) stop (lib/python3.12/site-packages/snippet_uiautomator/uiautomator.py:227) unregister (lib/python3.12/site-packages/mobly/controllers/android_device_lib/service_manager.py:109)

Sample code:

from mobly.controllers import android_device
from snippet_uiautomator import uiautomator

ad = android_device.AndroidDevice(sn)
ui = uiautomator.UiAutomatorService
ad.services.register(uiautomator.ANDROID_SERVICE_NAME, ui)
...    
ad.services.unregister(uiautomator.ANDROID_SERVICE_NAME)

This change is Reviewable

Co-authored-by: stuartmiles@google.com

google-cla[bot] commented 2 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

mhaoli commented 2 months ago

Thanks for noticing this bug and creating the fix. Could you help add unit tests for it?

mhaoli commented 1 month ago

Closing this as I already fixed this error in #920