931078 - mozdevice should allow passing in arbitrary signals to killProcess()

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

This is needed to send SIGABRT to the b2g process. It replaces forceKill, but forceKill functionality can still be achieved with signal.SIGKILL

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 - Add signal parameter to dm.killProcess()

Comment 2

[William Lachance (:wlach)]

Comment on attachment 822425 [details] [diff] [review]
Patch 1.0 - Add signal parameter to dm.killProcess()

Review of attachment 822425 [details] [diff] [review]:
-----------------------------------------------------------------

This is good. I was never very happy with the forceKill parameter (which I believe was only ever used in a now-scrapped piece of automation for android crashtesting) as its meaning was somewhat nebulous.

Have some minor requests for improvement. r+ with them addressed.

::: mozdevice/mozdevice/devicemanager.py
@@ +419,5 @@
>          return pid
>  
>  
>      @abstractmethod
> +    def killProcess(self, processName, sig=None):

I would prefer the parameter name "signal" here.

@@ +424,3 @@
>          """
> +        Kills the process named processName. If sig is not None, process is
> +        killed with the specified signal.

While you're here, could you document the parameters using the :param directive as we do elsewhere in this file?

Comment 3

[Andrew Halberstadt [:ahal]]

As per irc discussion we agreed to keep sig as signal could interfere with the signal module, and the os module uses 'sig' anyway. Fixed docstrings though:

https://github.com/mozilla/mozbase/commit/c27774f8ee4bdb37caa1eb5c94d5a17d36ea6b1f

Comment 4

[Andrew Halberstadt [:ahal]]

Cherry picked:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3796ea602411

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3796ea602411