792084 - Add a method to mozdevice which runs a command and returns standard output

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[William Lachance (:wlach)]

The current shell function in devicemanager is great for when you want to read a bunch of data and write it directly to a file (or standard output), but it's unwieldly if we just want to run a simple command and operate directly on the output.

The most recent demand for something like this was in bug 792048, but it comes up quite often. I'd written a workaround for this myself in eideticker: 

https://github.com/mozilla/eideticker/blob/master/src/eideticker/eideticker/device.py#L62

Inspired by python's subprocess module, let's add a method called shellCheckOutput to devicemanager that works as above. For the cases that the command we're calling returns 0 (or otherwise fails), let's throw an exception. Yes, this amounts to a change in policy in devicemanager, but jmaher and I agreed it's for the better.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: patch v1.0

This patch adds the shellCheckOutput command to devicemanager.py. The implementation is the same regardless of environment, so it's not an abstract method.

I've tested this in an adb and sut environment and this works as expected.

Comment 2

[William Lachance (:wlach)]

Comment on attachment 664593 [details] [diff] [review]
patch v1.0

Wonderful! Thank you so much. Hopefully we can guide people to using this now as opposed to checkCmd and sendCmd.

>diff --git a/mozdevice/mozdevice/devicemanager.py b/mozdevice/mozdevice/devicemanager.py
>index 3150176..c0c72f9 100644

>+        if retval == None:
>+            raise DMError("Did not successfully run command %s (output: '%s', retval: 'None')" % (cmd, output))

PEP8 suggests that this be written as retval is None

"Comparisons to singletons like None should always be done with is or is not, never the equality operators.

Comment 3

[Malini Das [:mdas] - Away, not checking bugmail]

> PEP8 suggests that this be written as retval is None
> 
> "Comparisons to singletons like None should always be done with is or is
> not, never the equality operators.

Cool, thanks, I've changed it, tested it and pushed to mozilla-inbound:


https://hg.mozilla.org/integration/mozilla-inbound/rev/0450cb4ef66a

Comment 4

[Andrew Halberstadt [:ahal]]

Can you push to the github repo as well?

FYI (someone correct me if I'm wrong), we are now pushing to the github repo first, and then the changes will get mirrored in jhammel's weekly mozbase merge that he has started doing. If you need it right away you can mirror the mozbase repo immediately (see https://wiki.mozilla.org/Auto-tools/Projects/MozBase#Mirroring or ask jhammel)

Comment 5

[William Lachance (:wlach)]

(In reply to Andrew Halberstadt [:ahal] from comment #4)
> Can you push to the github repo as well?
> 
> FYI (someone correct me if I'm wrong), we are now pushing to the github repo
> first, and then the changes will get mirrored in jhammel's weekly mozbase
> merge that he has started doing. If you need it right away you can mirror
> the mozbase repo immediately (see
> https://wiki.mozilla.org/Auto-tools/Projects/MozBase#Mirroring or ask
> jhammel)

Yes, this is correct. The policy here is to commit to the github repo first and then use the mirroring script to move changes to m-c. For more details, see the wiki page that Andrew linked to.

That said, there is no particular harm done in this case. I think we can just commit to the github repository and be done with it: this would only be a problem if we did a mirror before that was done.

Comment 6

[Andrew Halberstadt [:ahal]]

Yeah, it won't cause problems, just for future reference.

Comment 7

[Malini Das [:mdas] - Away, not checking bugmail]

Agh, sorry, I forgot which way the mirror went. I'll do the commit to github now

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0450cb4ef66a