Skip to content

Commit 9c4f09b

Browse files
douglas-raillard-armmarcbonnici
authored andcommitted
utils/android: Fix AdbConnection.execute(check_exit_code=False) default
Switch to have check_exit_code=True just like any other connection. The current behavior will not raise any exception if the command returns a non-zero exit code. This leads to failed attempt at parsing the output, which is now an error message rather than the expected data. Worse, the caller may never realize the command failed. This is especially bad as that behavior will only manifest itself when things go wrong, which is not the majority of the time, leading to code that seems to work ok, but does not handle failure properly (like a shell script). Lastly, since this is at odds with all the other connection types, generic code will typically assume check_exit_code=True by default and end up being buggy when used in conjunction of the AdbConnection.
1 parent f71b6a7 commit 9c4f09b

File tree

2 files changed

+3
-4
lines changed

2 files changed

+3
-4
lines changed

devlib/utils/android.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def _push_pull(self, action, sources, dest, timeout):
359359
popen.communicate()
360360

361361
# pylint: disable=unused-argument
362-
def execute(self, command, timeout=None, check_exit_code=False,
362+
def execute(self, command, timeout=None, check_exit_code=True,
363363
as_root=False, strip_colors=True, will_succeed=False):
364364
if as_root and self.connected_as_root:
365365
as_root = False
@@ -483,8 +483,7 @@ def _setup_su(self):
483483
return
484484
try:
485485
# Try the new style of invoking `su`
486-
self.execute('ls', timeout=self.timeout, as_root=True,
487-
check_exit_code=True)
486+
self.execute('ls', timeout=self.timeout, as_root=True)
488487
# If failure assume either old style or unrooted. Here we will assume
489488
# old style and root status will be verified later.
490489
except (TargetStableError, TargetTransientError, TimeoutError):

doc/connection.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class that implements the following methods.
4141
transfer does not complete within this period, an exception will be
4242
raised.
4343

44-
.. method:: execute(self, command, timeout=None, check_exit_code=False, as_root=False, strip_colors=True, will_succeed=False)
44+
.. method:: execute(self, command, timeout=None, check_exit_code=True, as_root=False, strip_colors=True, will_succeed=False)
4545

4646
Execute the specified command on the connected device and return its output.
4747

0 commit comments

Comments
 (0)