Make TimeoutRetryThread's stoppable

build/android/pylib/device/device_utils_test.py

 #!/usr/bin/env python
 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """
 Unit tests for the contents of device_utils.py (mostly DeviceUtils).
 """
 
 # pylint: disable=C0321
@@ skipping 267 matching lines @@
 
 class _ShellError:
   def __init__(self, output=None, return_code=1):
     if output is None:
       self.output = 'Permission denied\r\n'
     else:
       self.output = output
     self.return_code = return_code
 
 
+class _CmdTimeout:
+  def __init__(self, msg=None):
+    if msg is None:
+      self.msg = 'Operation timed out'
+    else:
+      self.msg = msg
+
+
 class DeviceUtilsNewImplTest(unittest.TestCase):
 
   def setUp(self):
     test_serial = '0123456789abcdef'
     self.adb = mock.Mock(spec=adb_wrapper.AdbWrapper)
     self.adb.__str__ = mock.Mock(return_value=test_serial)
     self.adb.GetDeviceSerial.return_value = test_serial
     self.device = device_utils.DeviceUtils(
         self.adb, default_timeout=1, default_retries=0)
 
   def assertShellCallSequence(self, calls):
     '''Assert that we expect a sequence of calls to adb.Shell.
 
     Args:
       calls: a sequence of (cmd, return_value) pairs, where |cmd| is the
         expected shell command to run on the device (with any quoting already
         applied), and |return_value| is either a string to give as mock output
         or a _ShellError object to raise an AdbShellCommandFailedError.
     '''
     def mk_expected_call(cmd, return_value):
       expected_args = Args(cmd, expect_rc=0, timeout=1, retries=0)
       if isinstance(return_value, _ShellError):
         return_value = device_errors.AdbShellCommandFailedError(cmd,
             return_value.return_code, return_value.output, str(self.device))
+      elif isinstance(return_value, _CmdTimeout):
+        return_value = device_errors.CommandTimeoutError(return_value.msg,
+                                                         str(self.device))
       return (expected_args, return_value)
 
     expected_calls = (mk_expected_call(a, r) for a, r in calls)
     return MockCallSequence(self, self.adb, 'Shell', expected_calls)
 
   def assertShellCall(self, cmd, return_value=''):
     return self.assertShellCallSequence([(cmd, return_value)])
 
 
 class DeviceUtilsHybridImplTest(DeviceUtilsOldImplTest):
 
   def setUp(self):
     super(DeviceUtilsHybridImplTest, self).setUp()
     self.device.adb = self.adb = mock.Mock(spec=adb_wrapper.AdbWrapper)
 
 
-class DeviceUtilsIsOnlineTest(DeviceUtilsOldImplTest):
+class DeviceUtilsIsOnlineTest(DeviceUtilsNewImplTest):
 
   def testIsOnline_true(self):
-    with self.assertCalls('adb -s 0123456789abcdef devices',
-                                 '00123456789abcdef  device\r\n'):
-      self.assertTrue(self.device.IsOnline())
+    self.adb.GetState = mock.Mock(return_value='device')
+    self.assertTrue(self.device.IsOnline())
+    self.adb.GetState.assert_called_once_with()
 
   def testIsOnline_false(self):
-    with self.assertCalls('adb -s 0123456789abcdef devices', '\r\n'):
-      self.assertFalse(self.device.IsOnline())
+    self.adb.GetState = mock.Mock(return_value='offline')
+    self.assertFalse(self.device.IsOnline())
+    self.adb.GetState.assert_called_once_with()
 
 
 class DeviceUtilsHasRootTest(DeviceUtilsNewImplTest):
 
   def testHasRoot_true(self):
     with self.assertShellCall('ls /root', 'foo\r\n'):
       self.assertTrue(self.device.HasRoot())
 
   def testHasRoot_false(self):
     with self.assertShellCall('ls /root', _ShellError()):
@@ skipping 20 matching lines @@
   def testEnableRoot_rootFails(self):
     with self.assertCallsSequence([
         ('adb -s 0123456789abcdef shell getprop ro.build.type',
          'userdebug\r\n'),
         ('adb -s 0123456789abcdef root', 'no\r\n'),
         ('adb -s 0123456789abcdef wait-for-device', '')]):
       with self.assertRaises(device_errors.CommandFailedError):
         self.device.EnableRoot()
 
 
-class DeviceUtilsIsUserBuildTest(DeviceUtilsOldImplTest):
+class DeviceUtilsIsUserBuildTest(DeviceUtilsNewImplTest):
 
   def testIsUserBuild_yes(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell getprop ro.build.type',
-        'user\r\n'):
+    with self.assertShellCall('getprop ro.build.type', 'user\r\n'):
       self.assertTrue(self.device.IsUserBuild())
 
   def testIsUserBuild_no(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell getprop ro.build.type',
-        'userdebug\r\n'):
+    with self.assertShellCall('getprop ro.build.type', 'userdebug\r\n'):
       self.assertFalse(self.device.IsUserBuild())
 
 
 class DeviceUtilsGetExternalStoragePathTest(DeviceUtilsNewImplTest):
 
   def testGetExternalStoragePath_succeeds(self):
     fakeStoragePath = '/fake/storage/path'
     with self.assertShellCall('echo $EXTERNAL_STORAGE',
                               '%s\r\n' % fakeStoragePath):
       self.assertEquals(fakeStoragePath,
                         self.device.GetExternalStoragePath())
 
   def testGetExternalStoragePath_fails(self):
     with self.assertShellCall('echo $EXTERNAL_STORAGE', '\r\n'):
       with self.assertRaises(device_errors.CommandFailedError):
         self.device.GetExternalStoragePath()
 
 
-class DeviceUtilsWaitUntilFullyBootedTest(DeviceUtilsOldImplTest):
+class DeviceUtilsGetApplicationPathTest(DeviceUtilsNewImplTest):
+
+  def testGetApplicationPath_exists(self):
+    with self.assertShellCall('pm path android',
+                              'package:/path/to/android.apk\n'):
+      self.assertEquals('/path/to/android.apk',
+                        self.device.GetApplicationPath('android'))
+
+  def testGetApplicationPath_notExists(self):
+    with self.assertShellCall('pm path not.installed.app',
+                              ''):
+      self.assertEquals(None,
+                        self.device.GetApplicationPath('not.installed.app'))
+
+  def testGetApplicationPath_fails(self):
+    with self.assertShellCall('pm path android',
+                              'ERROR. Is package manager running?\n'):
+      with self.assertRaises(device_errors.CommandFailedError):
+        self.device.GetApplicationPath('android')
+
+
+class DeviceUtilsWaitUntilFullyBootedTest(DeviceUtilsNewImplTest):
 
   def testWaitUntilFullyBooted_succeedsNoWifi(self):
-    with self.assertCallsSequence([
-        # AndroidCommands.WaitForSystemBootCompleted
-        ('adb -s 0123456789abcdef wait-for-device', ''),
-        ('adb -s 0123456789abcdef shell getprop sys.boot_completed', '1\r\n'),
-        # AndroidCommands.WaitForDevicePm
-        ('adb -s 0123456789abcdef wait-for-device', ''),
-        ('adb -s 0123456789abcdef shell pm path android',
-         'package:this.is.a.test.package'),
-        # AndroidCommands.WaitForSdCardReady
-        ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'",
-         '/fake/storage/path'),
-        ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'",
-         'nothing\r\n')
-        ]):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'package:this.is.a.test.package\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '1\r\n')]):
       self.device.WaitUntilFullyBooted(wifi=False)
+      self.adb.WaitForDevice.assert_called_once_with()
 
   def testWaitUntilFullyBooted_succeedsWithWifi(self):
-    with self.assertCallsSequence([
-        # AndroidCommands.WaitForSystemBootCompleted
-        ('adb -s 0123456789abcdef wait-for-device', ''),
-        ('adb -s 0123456789abcdef shell getprop sys.boot_completed', '1\r\n'),
-        # AndroidCommands.WaitForDevicePm
-        ('adb -s 0123456789abcdef wait-for-device', ''),
-        ('adb -s 0123456789abcdef shell pm path android',
-         'package:this.is.a.test.package'),
-        # AndroidCommands.WaitForSdCardReady
-        ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'",
-         '/fake/storage/path'),
-        ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'",
-         'nothing\r\n'),
-        # wait for wifi
-        ("adb -s 0123456789abcdef shell 'dumpsys wifi'", 'Wi-Fi is enabled')]):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'package:this.is.a.test.package\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '1\r\n'),
+        # wifi_enabled
+        ('dumpsys wifi', 'stuff\r\nWi-Fi is enabled\r\nmore stuff\r\n')]):
       self.device.WaitUntilFullyBooted(wifi=True)
+      self.adb.WaitForDevice.assert_called_once_with()
+
+  def testWaitUntilFullyBooted_sdCardReadyFails_noPath(self):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '\r\n')]):
+      with self.assertRaises(device_errors.CommandFailedError):
+        self.device.WaitUntilFullyBooted(wifi=False)
+
+  def testWaitUntilFullyBooted_sdCardReadyFails_emptyPath(self):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '\r\n'),
+        # sc_card_ready
+        ('ls /fake/storage/path', '\r\n'),
+        # sc_card_ready
+        ('ls /fake/storage/path', _CmdTimeout())]):
+      with self.assertRaises(device_errors.CommandTimeoutError):
+        self.device.WaitUntilFullyBooted(wifi=False)
+
+  def testWaitUntilFullyBooted_devicePmFails(self):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'Error. Is package manager running?\r\n'),
+        # pm_ready
+        ('pm path android', 'Error. Is package manager running?\r\n'),
+        # pm_ready
+        ('pm path android', _CmdTimeout())]):
+      with self.assertRaises(device_errors.CommandTimeoutError):
+        self.device.WaitUntilFullyBooted(wifi=False)
 
   def testWaitUntilFullyBooted_bootFails(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-          # AndroidCommands.WaitForSystemBootCompleted
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell getprop sys.boot_completed',
-           '0\r\n')]):
-        with self.assertRaises(device_errors.CommandTimeoutError):
-          self.device.WaitUntilFullyBooted(wifi=False)
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'package:this.is.a.test.package\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '0\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '0\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', _CmdTimeout())]):
+      with self.assertRaises(device_errors.CommandTimeoutError):
+        self.device.WaitUntilFullyBooted(wifi=False)
 
-  def testWaitUntilFullyBooted_devicePmFails(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-          # AndroidCommands.WaitForSystemBootCompleted
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell getprop sys.boot_completed',
-           '1\r\n')]
-          # AndroidCommands.WaitForDevicePm
-        + 3 * ([('adb -s 0123456789abcdef wait-for-device', '')]
-             + 24 * [('adb -s 0123456789abcdef shell pm path android', '\r\n')]
-             + [("adb -s 0123456789abcdef shell 'stop'", '\r\n'),
-                ("adb -s 0123456789abcdef shell 'start'", '\r\n')])):
-        with self.assertRaises(device_errors.CommandTimeoutError):
-          self.device.WaitUntilFullyBooted(wifi=False)
-
-  def testWaitUntilFullyBooted_sdCardReadyFails_noPath(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-          # AndroidCommands.WaitForSystemBootCompleted
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell getprop sys.boot_completed',
-           '1\r\n'),
-          # AndroidCommands.WaitForDevicePm
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell pm path android',
-           'package:this.is.a.test.package'),
-          ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'", '\r\n')]):
-        with self.assertRaises(device_errors.CommandFailedError):
-          self.device.WaitUntilFullyBooted(wifi=False)
-
-  def testWaitUntilFullyBooted_sdCardReadyFails_emptyPath(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-          # AndroidCommands.WaitForSystemBootCompleted
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell getprop sys.boot_completed',
-           '1\r\n'),
-          # AndroidCommands.WaitForDevicePm
-          ('adb -s 0123456789abcdef wait-for-device', ''),
-          ('adb -s 0123456789abcdef shell pm path android',
-           'package:this.is.a.test.package'),
-          ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'",
-           '/fake/storage/path\r\n'),
-          ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'", '')]):
-        with self.assertRaises(device_errors.CommandTimeoutError):
-          self.device.WaitUntilFullyBooted(wifi=False)
+  def testWaitUntilFullyBooted_wifiFails(self):
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'package:this.is.a.test.package\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '1\r\n'),
+        # wifi_enabled
+        ('dumpsys wifi', 'stuff\r\nmore stuff\r\n'),
+        # wifi_enabled
+        ('dumpsys wifi', 'stuff\r\nmore stuff\r\n'),
+        # wifi_enabled
+        ('dumpsys wifi', _CmdTimeout())]):
+      with self.assertRaises(device_errors.CommandTimeoutError):
+        self.device.WaitUntilFullyBooted(wifi=True)
 
 
-class DeviceUtilsRebootTest(DeviceUtilsOldImplTest):
+class DeviceUtilsRebootTest(DeviceUtilsNewImplTest):
 
   def testReboot_nonBlocking(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-            ('adb -s 0123456789abcdef reboot', ''),
-            ('adb -s 0123456789abcdef devices', 'unknown\r\n'),
-            ('adb -s 0123456789abcdef wait-for-device', ''),
-            ('adb -s 0123456789abcdef shell pm path android',
-             'package:this.is.a.test.package'),
-            ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'",
-             '/fake/storage/path\r\n'),
-            ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'",
-             'nothing\r\n')]):
-        self.device.Reboot(block=False)
+    self.adb.Reboot = mock.Mock()
+    self.device.IsOnline = mock.Mock(return_value=False)
+    self.device.Reboot(block=False)
+    self.adb.Reboot.assert_called_once_with()
+    self.device.IsOnline.assert_called_once_with()
 
   def testReboot_blocking(self):
-    with mock.patch('time.sleep'):
-      with self.assertCallsSequence([
-            ('adb -s 0123456789abcdef reboot', ''),
-            ('adb -s 0123456789abcdef devices', 'unknown\r\n'),
-            ('adb -s 0123456789abcdef wait-for-device', ''),
-            ('adb -s 0123456789abcdef shell pm path android',
-             'package:this.is.a.test.package'),
-            ("adb -s 0123456789abcdef shell 'echo $EXTERNAL_STORAGE'",
-             '/fake/storage/path\r\n'),
-            ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'",
-             'nothing\r\n'),
-            ('adb -s 0123456789abcdef wait-for-device', ''),
-            ('adb -s 0123456789abcdef shell getprop sys.boot_completed',
-             '1\r\n'),
-            ('adb -s 0123456789abcdef wait-for-device', ''),
-            ('adb -s 0123456789abcdef shell pm path android',
-             'package:this.is.a.test.package'),
-            ("adb -s 0123456789abcdef shell 'ls /fake/storage/path'",
-             'nothing\r\n')]):
-        self.device.Reboot(block=True)
+    self.adb.Reboot = mock.Mock()
+    self.device.IsOnline = mock.Mock(return_value=False)
+    with self.assertShellCallSequence([
+        # sc_card_ready
+        ('echo $EXTERNAL_STORAGE', '/fake/storage/path\r\n'),
+        ('ls /fake/storage/path', '/fake/storage/path\r\n'),
+        # pm_ready
+        ('pm path android', 'package:this.is.a.test.package\r\n'),
+        # boot_completed
+        ('getprop sys.boot_completed', '1\r\n')]):
+      self.device.Reboot(block=True)
+      self.adb.Reboot.assert_called_once_with()
+      self.device.IsOnline.assert_called_once_with()
+      self.adb.WaitForDevice.assert_called_once_with()
 
 
 class DeviceUtilsInstallTest(DeviceUtilsOldImplTest):
 
   def testInstall_noPriorInstall(self):
     with mock.patch('os.path.isfile', return_value=True), (
          mock.patch('pylib.utils.apk_helper.GetPackageName',
                     return_value='this.is.a.test.package')):
       with self.assertCallsSequence([
           ("adb -s 0123456789abcdef shell 'pm path this.is.a.test.package'",
@@ skipping 141 matching lines @@
     with self.assertShellCall(cmd, '\r\n'):
       self.assertEquals('',
                         self.device.RunShellCommand(cmd, single_line=True))
 
   def testRunShellCommand_singleLine_successWithoutEndLine(self):
     cmd = 'echo -n $VALUE'
     with self.assertShellCall(cmd, 'some value'):
       self.assertEquals('some value',
                         self.device.RunShellCommand(cmd, single_line=True))
 
-  def testRunShellCommand_singleLine_failNoLines(self):
-    cmd = 'echo $VALUE'
+  def testRunShellCommand_singleLine_successNoOutput(self):
+    cmd = 'echo -n $VALUE'
     with self.assertShellCall(cmd, ''):
-      with self.assertRaises(device_errors.CommandFailedError):
-        self.device.RunShellCommand(cmd, single_line=True)
+      self.assertEquals('',
+                        self.device.RunShellCommand(cmd, single_line=True))
 
   def testRunShellCommand_singleLine_failTooManyLines(self):
     cmd = 'echo $VALUE'
     with self.assertShellCall(cmd, 'some value\r\nanother value\r\n'):
       with self.assertRaises(device_errors.CommandFailedError):
         self.device.RunShellCommand(cmd, single_line=True)
 
   def testRunShellCommand_checkReturn_success(self):
     cmd = 'echo $ANDROID_DATA'
     output = '/data\r\n'
@@ skipping 345 matching lines @@
   def testPushChangedFilesZipped_empty(self):
     test_files = []
     self.device._PushChangedFilesZipped(test_files)
     self.assertEqual(0, self.adb.Push.call_count)
 
   def testPushChangedFilesZipped_single(self):
     test_files = [('/test/host/path/file1', '/test/device/path/file1')]
 
     self.device._GetExternalStoragePathImpl = mock.Mock(
         return_value='/test/device/external_dir')
-    self.device._IsOnlineImpl = mock.Mock(return_value=True)
+    self.device.IsOnline = mock.Mock(return_value=True)
     self.device._RunShellCommandImpl = mock.Mock()
     mock_zip_temp = mock.mock_open()
     mock_zip_temp.return_value.name = '/test/temp/file/tmp.zip'
     with mock.patch('multiprocessing.Process') as mock_zip_proc, (
          mock.patch('tempfile.NamedTemporaryFile', mock_zip_temp)):
       self.device._PushChangedFilesZipped(test_files)
 
     mock_zip_proc.assert_called_once_with(
         target=device_utils.DeviceUtils._CreateDeviceZip,
         args=('/test/temp/file/tmp.zip', test_files))
     self.adb.Push.assert_called_once_with(
         '/test/temp/file/tmp.zip', '/test/device/external_dir/tmp.zip')
     self.assertEqual(2, self.device._RunShellCommandImpl.call_count)
     self.device._RunShellCommandImpl.assert_any_call(
         ['unzip', '/test/device/external_dir/tmp.zip'],
         as_root=True,
         env={'PATH': '$PATH:/data/local/tmp/bin'},
         check_return=True)
     self.device._RunShellCommandImpl.assert_any_call(
         ['rm', '/test/device/external_dir/tmp.zip'], check_return=True)
 
   def testPushChangedFilesZipped_multiple(self):
     test_files = [('/test/host/path/file1', '/test/device/path/file1'),
                   ('/test/host/path/file2', '/test/device/path/file2')]
 
     self.device._GetExternalStoragePathImpl = mock.Mock(
         return_value='/test/device/external_dir')
-    self.device._IsOnlineImpl = mock.Mock(return_value=True)
+    self.device.IsOnline = mock.Mock(return_value=True)
     self.device._RunShellCommandImpl = mock.Mock()
     mock_zip_temp = mock.mock_open()
     mock_zip_temp.return_value.name = '/test/temp/file/tmp.zip'
     with mock.patch('multiprocessing.Process') as mock_zip_proc, (
          mock.patch('tempfile.NamedTemporaryFile', mock_zip_temp)):
       self.device._PushChangedFilesZipped(test_files)
 
     mock_zip_proc.assert_called_once_with(
         target=device_utils.DeviceUtils._CreateDeviceZip,
         args=('/test/temp/file/tmp.zip', test_files))
@@ skipping 366 matching lines @@
             '%s\r\n' % constants.DEVICE_LOCAL_PROPERTIES_PATH),
            ('adb -s 0123456789abcdef pull %s %s' %
                 (constants.DEVICE_LOCAL_PROPERTIES_PATH, mock_file.name),
             '100 B/s (100 bytes in 1.000s)\r\n'),
            ('adb -s 0123456789abcdef shell '
                 'getprop dalvik.vm.enableassertions',
             'all\r\n')]):
         self.assertFalse(self.device.SetJavaAsserts(True))
 
 
-class DeviceUtilsGetPropTest(DeviceUtilsOldImplTest):
+class DeviceUtilsGetPropTest(DeviceUtilsNewImplTest):
 
   def testGetProp_exists(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell getprop this.is.a.test.property',
-        'test_property_value\r\n'):
+    with self.assertShellCall('getprop this.is.a.test.property',
+                              'test_property_value\r\n'):
       self.assertEqual('test_property_value',
                        self.device.GetProp('this.is.a.test.property'))
 
   def testGetProp_doesNotExist(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell '
-            'getprop this.property.does.not.exist', ''):
+    with self.assertShellCall('getprop this.property.does.not.exist',
+                              '\r\n'):
       self.assertEqual('', self.device.GetProp('this.property.does.not.exist'))
 
   def testGetProp_cachedRoProp(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell '
-            'getprop ro.build.type', 'userdebug'):
-      self.assertEqual('userdebug', self.device.GetProp('ro.build.type'))
-      self.assertEqual('userdebug', self.device.GetProp('ro.build.type'))
+    with self.assertShellCall('getprop ro.build.type',
+                              'userdebug\r\n'):
+      self.assertEqual('userdebug',
+                       self.device.GetProp('ro.build.type', cache=True))
+      self.assertEqual('userdebug',
+                       self.device.GetProp('ro.build.type', cache=True))
+
+  def testGetProp_retryAndCache(self):
+    with self.assertShellCallSequence([
+        ('getprop ro.build.type', _ShellError()),
+        ('getprop ro.build.type', _ShellError()),
+        ('getprop ro.build.type', 'userdebug\r\n')]):
+      self.assertEqual('userdebug',
+                       self.device.GetProp('ro.build.type',
+                                           cache=True, retries=3))
+      self.assertEqual('userdebug',
+                       self.device.GetProp('ro.build.type',
+                                           cache=True, retries=3))
 
 
-class DeviceUtilsSetPropTest(DeviceUtilsOldImplTest):
+class DeviceUtilsSetPropTest(DeviceUtilsNewImplTest):
 
   def testSetProp(self):
-    with self.assertCalls(
-        'adb -s 0123456789abcdef shell '
-            'setprop this.is.a.test.property "test_property_value"',
-        ''):
-      self.device.SetProp('this.is.a.test.property', 'test_property_value')
+    with self.assertShellCall(
+      "setprop this.is.a.test.property 'test property value'"):
+      self.device.SetProp('this.is.a.test.property', 'test property value')
 
 
 class DeviceUtilsGetPidsTest(DeviceUtilsNewImplTest):
 
   def testGetPids_noMatches(self):
     with self.assertShellCall(
         'ps',
         'USER   PID   PPID  VSIZE  RSS   WCHAN    PC       NAME\r\n'
         'user  1000    100   1024 1024   ffffffff 00000000 no.match\r\n'):
       self.assertEqual({}, self.device.GetPids('does.not.match'))
@@ skipping 119 matching lines @@
     self.device = device_utils.DeviceUtils(None)
     with self.assertCalls('adb  get-serialno', 'unknown'), (
          self.assertRaises(device_errors.NoDevicesError)):
       str(self.device)
 
 
 if __name__ == '__main__':
   logging.getLogger().setLevel(logging.DEBUG)
   unittest.main(verbosity=2)