Add retries for adb getprops. (#597)
diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py
index 81e8a46..e6658fd 100644
--- a/mobly/controllers/android_device_lib/adb.py
+++ b/mobly/controllers/android_device_lib/adb.py
@@ -18,6 +18,7 @@
import logging
import psutil
import subprocess
+import time
import threading
from mobly import utils
@@ -34,6 +35,8 @@
# Adb getprop call should never take too long.
DEFAULT_GETPROP_TIMEOUT_SEC = 5
+DEFAULT_GETPROPS_ATTEMPTS = 3
+DEFAULT_GETPROPS_RETRY_SLEEP_SEC = 1
class Error(Exception):
@@ -332,13 +335,23 @@
A dict containing name-value pairs of the properties requested, if
they exist.
"""
- raw_output = self.shell(
- ['getprop'], timeout=DEFAULT_GETPROP_TIMEOUT_SEC)
- properties = self._parse_getprop_output(raw_output)
+ attempts = DEFAULT_GETPROPS_ATTEMPTS
results = {}
- for name in prop_names:
- if name in properties:
- results[name] = properties[name]
+ for attempt in range(attempts):
+ # The ADB getprop command can randomly return empty string, so try
+ # multiple times. This value should always be non-empty if the device
+ # in a working state.
+ raw_output = self.shell(
+ ['getprop'], timeout=DEFAULT_GETPROP_TIMEOUT_SEC)
+ properties = self._parse_getprop_output(raw_output)
+ if properties:
+ for name in prop_names:
+ if name in properties:
+ results[name] = properties[name]
+ break
+ # Don't call sleep on the last attempt.
+ if attempt < attempts - 1:
+ time.sleep(DEFAULT_GETPROPS_RETRY_SLEEP_SEC)
return results
def has_shell_command(self, command):
diff --git a/tests/mobly/controllers/android_device_lib/adb_test.py b/tests/mobly/controllers/android_device_lib/adb_test.py
index 25ebde9..4a0dc34 100755
--- a/tests/mobly/controllers/android_device_lib/adb_test.py
+++ b/tests/mobly/controllers/android_device_lib/adb_test.py
@@ -174,8 +174,8 @@
self._mock_execute_and_process_stdout_process(mock_popen)
mock_handler = mock.MagicMock()
mock_popen.return_value.communicate = mock.Mock(
- return_value=(unexpected_stdout,
- MOCK_DEFAULT_STDERR.encode('utf-8')))
+ return_value=(unexpected_stdout, MOCK_DEFAULT_STDERR.encode(
+ 'utf-8')))
err = adb.AdbProxy()._execute_and_process_stdout(
['fake_cmd'], shell=False, handler=mock_handler)
@@ -508,7 +508,8 @@
}
self.assertEqual(parsed_props, expected_output)
- def test_getprops(self):
+ @mock.patch('time.sleep', return_value=mock.MagicMock())
+ def test_getprops(self, mock_sleep):
with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd:
mock_exec_cmd.return_value = (
b'\n[sendbug.preferred.domain]: [google.com]\n'
@@ -521,17 +522,52 @@
'sendbug.preferred.domain', # string value
'nonExistentProp'
])
- self.assertEqual(
- actual_output, {
- 'sys.wifitracing.started': '1',
- 'sys.uidcpupower': '',
- 'sendbug.preferred.domain': 'google.com'
- })
+ self.assertEqual(actual_output, {
+ 'sys.wifitracing.started': '1',
+ 'sys.uidcpupower': '',
+ 'sendbug.preferred.domain': 'google.com'
+ })
mock_exec_cmd.assert_called_once_with(
['adb', 'shell', 'getprop'],
shell=False,
stderr=None,
timeout=adb.DEFAULT_GETPROP_TIMEOUT_SEC)
+ mock_sleep.assert_not_called()
+
+ @mock.patch('time.sleep', return_value=mock.MagicMock())
+ def test_getprops_when_empty_string_randomly_returned(self, mock_sleep):
+ with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd:
+ mock_exec_cmd.side_effect = [
+ b'', (b'\n[ro.build.id]: [AB42]\n'
+ b'[ro.build.type]: [userdebug]\n\n')
+ ]
+ actual_output = adb.AdbProxy().getprops(['ro.build.id'])
+ self.assertEqual(actual_output, {
+ 'ro.build.id': 'AB42',
+ })
+ self.assertEqual(mock_exec_cmd.call_count, 2)
+ mock_exec_cmd.assert_called_with(
+ ['adb', 'shell', 'getprop'],
+ shell=False,
+ stderr=None,
+ timeout=adb.DEFAULT_GETPROP_TIMEOUT_SEC)
+ self.assertEqual(mock_sleep.call_count, 1)
+ mock_sleep.assert_called_with(1)
+
+ @mock.patch('time.sleep', return_value=mock.MagicMock())
+ def test_getprops_when_empty_string_always_returned(self, mock_sleep):
+ with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd:
+ mock_exec_cmd.return_value = b''
+ actual_output = adb.AdbProxy().getprops(['ro.build.id'])
+ self.assertEqual(actual_output, {})
+ self.assertEqual(mock_exec_cmd.call_count, 3)
+ mock_exec_cmd.assert_called_with(
+ ['adb', 'shell', 'getprop'],
+ shell=False,
+ stderr=None,
+ timeout=adb.DEFAULT_GETPROP_TIMEOUT_SEC)
+ self.assertEqual(mock_sleep.call_count, 2)
+ mock_sleep.assert_called_with(1)
def test_forward(self):
with mock.patch.object(adb.AdbProxy, '_exec_cmd') as mock_exec_cmd:
