1218993 - [mozdevice] do not repeat wait-for-device in adb commands

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: bug-1214530.patch

In bug 1214530 when attempting to work around issues on the Autophone mac mini host, I upgraded adb to the most recent version (still marked as 1.0.32) which caused syntax errors due to the way the adb.py shell commands are implemented.

The problem was in the shell method:
https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py#1013

where I automatically prepended a wait-for-device to the adb command. Other parts of adb.py used shell_command to issue a wait-for-device which called shell() indirectly and resulted in command lines of the form adb -s <serial> wait-for-device wait-for-device ....

I've known about this duplication for a while but since it wasn't causing problems, left it alone. But with the upgraded adb version, repeating wait-for-device on the command line resulted in an error.

The solution I used was to not issue wait-for-device in shell_command. To perform a simple wait, I just did a self.command_output([], timeout=timeout).

This patch is a version of the bug 1214530 patch updated to account for bug 1145680 moving the reboot method from adb_android.py to adb.py.

Comment 1

[Bob Clary [:bc] (inactive)]

Submitted to try
try: -b o -p android-api-9,android-api-11,android-x86,emulator,linux64_gecko -u all -t none
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c174b59b32bb

Comment 2

[Geoff Brown [:gbrown] (pto Apr 11-18)]

Comment on attachment 8679663 [details] [diff] [review]
bug-1214530.patch

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

command_output([]) looks funny and might be mistaken for a no-op -- maybe an in-line comment is warranted?

Comment 3

[Bob Clary [:bc] (inactive)]

try was broken due to leaving a file out of dave's patch.
new try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab2e418e2020


(In reply to Geoff Brown [:gbrown] from comment #2)
> 
> command_output([]) looks funny and might be mistaken for a no-op -- maybe an
> in-line comment is warranted?

Ok. Sounds good.

Comment 4

[Bob Clary [:bc] (inactive)]

Attachment: bug-1214530.patch v2 with comments

Do you think it would be better to just add a new method wait_for_device() ?

Comment 5

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I think it would be fine either way.

Comment 6

[William Lachance (:wlach)]

Comment on attachment 8679786 [details] [diff] [review]
bug-1214530.patch v2 with comments

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

::: testing/mozbase/mozdevice/mozdevice/adb_android.py
@@ +78,5 @@
>          :raises: * ADBTimeoutError
>                   * ADBError
>          """
> +        # Issue an empty command which effectively performs
> +        # a wait-for-device.

It would be good to explain *why* you're not doing wait-for-device here.

Comment 7

[Bob Clary [:bc] (inactive)]

Ok.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ae3cda0b5ff

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ae3cda0b5ff

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4ae3cda0b5ff

Comment 11

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion