Reland of Migrate DeviceUtils.ReadFile to adb_wrapper (try 3)

tools/telemetry/telemetry/core/backends/chrome/android_browser_backend_unittest.py

Index: tools/telemetry/telemetry/core/backends/chrome/android_browser_backend_unittest.py
diff --git a/tools/telemetry/telemetry/core/backends/chrome/android_browser_backend_unittest.py b/tools/telemetry/telemetry/core/backends/chrome/android_browser_backend_unittest.py
new file mode 100644
index 0000000000000000000000000000000000000000..059b5ca70419f69c1b6764f24bd7606811dc0661
--- /dev/null
+++ b/tools/telemetry/telemetry/core/backends/chrome/android_browser_backend_unittest.py
@@ -0,0 +1,69 @@
+# Copyright 2013 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.
+
+import unittest
+
+from telemetry.core import browser_options
+from telemetry.core import util
+from telemetry.core.backends.chrome import android_browser_backend
+from telemetry.core.platform import android_device
+from telemetry.core.platform import android_platform_backend
+
+util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'build', 'android')
+from pylib.device import device_errors
+from pylib.device import device_utils
+
+util.AddDirToPythonPath(util.GetChromiumSrcDir(),
+                        'third_party', 'pymock')
+import mock # pylint: disable=import-error
+
+
+class MockBrowserBackend(android_browser_backend.AndroidBrowserBackend):
+  """Mock of an AndroidBrowserBackend with a custom device object"""
+
+  def __init__(self, device):
+    platform_backend = mock.Mock(
+        spec=android_platform_backend.AndroidPlatformBackend)
+    platform_backend.adb.device.return_value = device
+    browser_opts = browser_options.BrowserOptions()
+    browser_opts.browser_type = 'android-chrome-shell'
+    super(MockBrowserBackend, self).__init__(
+        android_platform_backend=platform_backend,
+        browser_options=browser_opts,
+        backend_settings=android_browser_backend.ChromeShellBackendSettings(
+            'test.package.name'),
+        use_rndis_forwarder=False,
+        output_profile_path=None,
+        extensions_to_load=[],
+        target_arch=None)
+
+class AndroidBrowserBackendTest(unittest.TestCase):
+  """Test Chrome command line set-up/restore."""
+
+  def testRestoreOldCommandLine_restoreExisting(self):
+    old_command_line = 'chrome_shell --with --some --old --options'
+    device = mock.Mock(spec=device_utils.DeviceUtils)
+    device.old_interface = mock.Mock()
+    device.ReadFile.return_value = old_command_line
+    browser_backend = MockBrowserBackend(device)
+    browser_backend._SetUpCommandLine()
+    device.WriteFile.assert_called_once_with(mock.ANY, mock.ANY, as_root=True)

[tonyg, 2015/01/05 17:28:43]

I think these mock-style unittests are just closely coupling telemetry to the
underlying implementation without providing actual functional coverage. In other
words, this test verifies that certain commands were called, but doesn't verify
that they worked as intended. At this level, we should be testing behavior. For
instance: start a browser with a given command line, then use `ps` to verify
that the process has that command line. Or start a browser with a given command
line, then inspect the file system afterwards to verify the file was deleted.

[jbudorick, 2015/01/05 17:50:42]

We've generally been switching to this style of test in our unit tests in
build/android/pylib/* to avoid having higher-level unit tests implicitly test
lower-level functionality (i.e., we verify the adb calls we make instead of the
results of those adb calls on an actual device). Our goals may be different,
though -- are telemetry's Android unit tests generally designed to run using a
real device? Should these be closer to functional tests than unit tests?

+    device.WriteFile.reset_mock()
+    browser_backend._RestoreCommandLine()
+    device.WriteFile.assert_called_once_with(mock.ANY, old_command_line,
+                                             as_root=True)
+
+  def testRestoreOldCommandLine_didNotExist(self):
+    device = mock.Mock(spec=device_utils.DeviceUtils)
+    device.old_interface = mock.Mock()
+    device.ReadFile.side_effect = device_errors.CommandFailedError(
+      'File not found', str(device))
+    browser_backend = MockBrowserBackend(device)
+    browser_backend._SetUpCommandLine()
+    device.WriteFile.assert_called_once_with(mock.ANY, mock.ANY, as_root=True)
+    device.WriteFile.reset_mock()
+    browser_backend._RestoreCommandLine()
+    self.assertFalse(device.WriteFile.called)
+    device.RunShellCommand.assert_called_once_with(['rm', mock.ANY],
+                                                   as_root=True)