From f7f323e319d8ef66eb7ddf8ee7f5cb17d395b481 Mon Sep 17 00:00:00 2001
From: Ang Li <angli@google.com>
Date: Fri, 13 Jan 2023 14:54:08 -0800
Subject: Adjust timeout strategy for `AdbProxy#getprop` (#863)

Because this command can take surprisingly long, like when the
device used is a virtual device over network.

* Extend the default timeout.
* Allow custom timeout value.
---
 mobly/controllers/android_device_lib/adb.py        | 23 ++++++++++++++--------
 .../controllers/android_device_lib/adb_test.py     | 11 +++++++++++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py
index 84051cd..b3994c9 100644
--- a/mobly/controllers/android_device_lib/adb.py
+++ b/mobly/controllers/android_device_lib/adb.py
@@ -35,8 +35,9 @@ ADB_ROOT_RETRY_ATTEMPT_INTERVAL_SEC = 10
 # Qualified class name of the default instrumentation test runner.
 DEFAULT_INSTRUMENTATION_RUNNER = 'com.android.common.support.test.runner.AndroidJUnitRunner'
 
-# Adb getprop call should never take too long.
-DEFAULT_GETPROP_TIMEOUT_SEC = 5
+# `adb shell getprop` can take surprisingly long, when the device is a
+# networked virtual device.
+DEFAULT_GETPROP_TIMEOUT_SEC = 10
 DEFAULT_GETPROPS_ATTEMPTS = 3
 DEFAULT_GETPROPS_RETRY_SLEEP_SEC = 1
 
@@ -290,7 +291,8 @@ class AdbProxy:
     out = self._exec_cmd(adb_cmd, shell=shell, timeout=timeout, stderr=stderr)
     return out
 
-  def _execute_adb_and_process_stdout(self, name, args, shell, handler) -> bytes:
+  def _execute_adb_and_process_stdout(self, name, args, shell,
+                                      handler) -> bytes:
     adb_cmd = self._construct_adb_cmd(name, args, shell=shell)
     err = self._execute_and_process_stdout(adb_cmd,
                                            shell=shell,
@@ -367,21 +369,22 @@ class AdbProxy:
                      ret_code=0)
     return stdout
 
-  def getprop(self, prop_name):
+  def getprop(self, prop_name, timeout=DEFAULT_GETPROP_TIMEOUT_SEC):
     """Get a property of the device.
 
     This is a convenience wrapper for `adb shell getprop xxx`.
 
     Args:
       prop_name: A string that is the name of the property to get.
+      timeout: float, the number of seconds to wait before timing out.
+          If not specified, the DEFAULT_GETPROP_TIMEOUT_SEC is used.
 
     Returns:
       A string that is the value of the property, or None if the property
       doesn't exist.
     """
-    return self.shell(
-        ['getprop', prop_name],
-        timeout=DEFAULT_GETPROP_TIMEOUT_SEC).decode('utf-8').strip()
+    return self.shell(['getprop', prop_name],
+                      timeout=timeout).decode('utf-8').strip()
 
   def getprops(self, prop_names):
     """Get multiple properties of the device.
@@ -439,7 +442,11 @@ class AdbProxy:
                                 timeout=None,
                                 stderr=None)
 
-  def instrument(self, package, options=None, runner=None, handler=None) -> bytes:
+  def instrument(self,
+                 package,
+                 options=None,
+                 runner=None,
+                 handler=None) -> bytes:
     """Runs an instrumentation command on the device.
 
     This is a convenience wrapper to avoid parameter formatting.
diff --git a/tests/mobly/controllers/android_device_lib/adb_test.py b/tests/mobly/controllers/android_device_lib/adb_test.py
index 94e3975..ecbd813 100755
--- a/tests/mobly/controllers/android_device_lib/adb_test.py
+++ b/tests/mobly/controllers/android_device_lib/adb_test.py
@@ -505,6 +505,17 @@ class AdbTest(unittest.TestCase):
           stderr=None,
           timeout=adb.DEFAULT_GETPROP_TIMEOUT_SEC)
 
+  def test_getprop_custom_timeout(self):
+    timeout_s = adb.DEFAULT_GETPROP_TIMEOUT_SEC * 2
+    with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd:
+      mock_exec_cmd.return_value = b'blah'
+      self.assertEqual(adb.AdbProxy().getprop('haha', timeout=timeout_s),
+                       'blah')
+      mock_exec_cmd.assert_called_once_with(['adb', 'shell', 'getprop', 'haha'],
+                                            shell=False,
+                                            stderr=None,
+                                            timeout=timeout_s)
+
   def test__parse_getprop_output_special_values(self):
     mock_adb_output = (
         b'[selinux.restorecon_recursive]: [/data/misc_ce/10]\n'
-- 
cgit v1.2.3