From 9c4f09b5f3c45e77c3f9fe760460732a0031a9ac Mon Sep 17 00:00:00 2001
From: Douglas Raillard <douglas.raillard@arm.com>
Date: Tue, 20 May 2025 16:49:58 +0100
Subject: [PATCH] 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.
---
 devlib/utils/android.py | 5 ++---
 doc/connection.rst      | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/devlib/utils/android.py b/devlib/utils/android.py
index 001cb93..2be37cc 100755
--- a/devlib/utils/android.py
+++ b/devlib/utils/android.py
@@ -359,7 +359,7 @@ class AdbConnection(ConnectionBase):
                 popen.communicate()
 
     # pylint: disable=unused-argument
-    def execute(self, command, timeout=None, check_exit_code=False,
+    def execute(self, command, timeout=None, check_exit_code=True,
                 as_root=False, strip_colors=True, will_succeed=False):
         if as_root and self.connected_as_root:
             as_root = False
@@ -483,8 +483,7 @@ class AdbConnection(ConnectionBase):
             return
         try:
             # Try the new style of invoking `su`
-            self.execute('ls', timeout=self.timeout, as_root=True,
-                         check_exit_code=True)
+            self.execute('ls', timeout=self.timeout, as_root=True)
         # If failure assume either old style or unrooted. Here we will assume
         # old style and root status will be verified later.
         except (TargetStableError, TargetTransientError, TimeoutError):
diff --git a/doc/connection.rst b/doc/connection.rst
index 71ce155..7f641d9 100644
--- a/doc/connection.rst
+++ b/doc/connection.rst
@@ -41,7 +41,7 @@ class that implements the following methods.
        transfer does not complete within this period, an exception will be
        raised.
 
-.. method:: execute(self, command, timeout=None, check_exit_code=False, as_root=False, strip_colors=True, will_succeed=False)
+.. method:: execute(self, command, timeout=None, check_exit_code=True, as_root=False, strip_colors=True, will_succeed=False)
 
    Execute the specified command on the connected device and return its output.