Add local results details pages.

build/android/pylib/local/device/local_device_instrumentation_test_run.py

Index: build/android/pylib/local/device/local_device_instrumentation_test_run.py
diff --git a/build/android/pylib/local/device/local_device_instrumentation_test_run.py b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
index 8f2b2e74327a4f78cfde413a380d7b09f873c449..c48fa45d235383a72eb36a3c9e99cc263979fe1c 100644
--- a/build/android/pylib/local/device/local_device_instrumentation_test_run.py
+++ b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
@@ -13,17 +13,16 @@ import time
 from devil.android import device_errors
 from devil.android import device_temp_file
 from devil.android import flag_changer
+from devil.android import logcat_monitor
 from devil.android.sdk import shared_prefs
 from devil.utils import reraiser_thread
 from pylib import valgrind_tools
-from pylib.android import logdog_logcat_monitor
 from pylib.base import base_test_result
 from pylib.constants import host_paths
 from pylib.instrumentation import instrumentation_test_instance
 from pylib.local.device import local_device_environment
 from pylib.local.device import local_device_test_run
-from pylib.utils import google_storage_helper
-from pylib.utils import logdog_helper
+from pylib.utils import test_output_saver_factory
 from py_trace_event import trace_event
 from py_utils import contextlib_ext
 from py_utils import tempfile_ext
@@ -108,8 +107,9 @@ _CURRENT_FOCUS_CRASH_RE = re.compile(
 
 class LocalDeviceInstrumentationTestRun(
     local_device_test_run.LocalDeviceTestRun):
-  def __init__(self, env, test_instance):
-    super(LocalDeviceInstrumentationTestRun, self).__init__(env, test_instance)
+  def __init__(self, env, test_instance, test_output_saver):
+    super(LocalDeviceInstrumentationTestRun, self).__init__(
+        env, test_instance, test_output_saver)
     self._flag_changers = {}
     self._ui_capture_dir = dict()
 
@@ -409,19 +409,18 @@ class LocalDeviceInstrumentationTestRun(
         test_name.replace('#', '.'),
         time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
         device.serial)
-    logmon = logdog_logcat_monitor.LogdogLogcatMonitor(
-        device.adb, stream_name, filter_specs=LOGCAT_FILTERS)
-
-    with contextlib_ext.Optional(
-        logmon, self._test_instance.should_save_logcat):

[jbudorick, 2017/06/20 14:12:54]

Does this option do anything after your change?

[mikecase (-- gone --), 2017/07/10 17:11:10]

Probably not. Would need to get rid of it I suppose

-      with _LogTestEndpoints(device, test_name):
-        with contextlib_ext.Optional(
-            trace_event.trace(test_name),
-            self._env.trace_output):
-          output = device.StartInstrumentation(
-              target, raw=True, extras=extras, timeout=timeout, retries=0)
-
-    logcat_url = logmon.GetLogcatURL()
+    with tempfile.NamedTemporaryFile() as logcat_tmp:
+      with logcat_monitor.LogcatMonitor(

[jbudorick, 2017/06/20 14:12:54]

I'm wondering if there are situations in which we wouldn't want to save the
logcat.

[mikecase (-- gone --), 2017/07/10 17:11:11]

Well, the default is the NoopOutputSaver, which doesnt archive the logcat. I
guess the logcat is always saved to a tmp file in my current impl (even if you
have the NoopOutputSaver)

+          device.adb, filter_specs=LOGCAT_FILTERS, output_file=logcat_tmp.name):
+        with _LogTestEndpoints(device, test_name):
+          with contextlib_ext.Optional(
+              trace_event.trace(test_name),
+              self._env.trace_output):
+            output = device.StartInstrumentation(
+                target, raw=True, extras=extras, timeout=timeout, retries=0)
+      logcat_url = self._test_output_saver.Save(
+          logcat_tmp.name, stream_name, 'logcat',
+          test_output_saver_factory.Datatype.TEXT)
     duration_ms = time_ms() - start_ms
     if flags_to_add or flags_to_remove:
       self._flag_changers[str(device)].Restore()
@@ -521,8 +520,12 @@ class LocalDeviceInstrumentationTestRun(
             stream_name = 'tombstones_%s_%s' % (
                 time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
                 device.serial)
-            tombstones_url = logdog_helper.text(
-                stream_name, '\n'.join(resolved_tombstones))
+            with tempfile.NamedTemporaryFile() as tombstone_tmp:
+              tombstone_tmp.write('\n'.join(resolved_tombstones))
+              tombstones_url = self._test_output_saver.Save(
+                  tombstone_tmp.name,
+                  stream_name, 'tombstones',
+                  test_output_saver_factory.Datatype.TEXT)
           result.SetLink('tombstones', tombstones_url)
     return results, None
 
@@ -544,23 +547,14 @@ class LocalDeviceInstrumentationTestRun(
             'Saved screenshot for %s to %s.',
             test_name, screenshot_host_file)
         if self._test_instance.gs_results_bucket:
-          link = google_storage_helper.upload(
-              google_storage_helper.unique_name(
-                  'screenshot', device=device),
-              screenshot_host_file,
-              bucket=('%s/screenshots' %
-                      self._test_instance.gs_results_bucket))
+          link = self._test_output_saver.Save(

[jbudorick, 2017/06/20 14:12:54]

This only saves the screenshot if we've set a results bucket. Should we be
saving it regardless & let _test_output_saver wrangle the bucket?

[mikecase (-- gone --), 2017/07/10 17:11:11]

Thanks for this catch. I had fixed this for the render tests, but not here.

+              screenshot_host_file, os.path.join('screenshots', test_name),
+              test_output_saver_factory.Datatype.IMAGE)
           for result in results:
             result.SetLink('post_test_screenshot', link)
 
   def _ProcessRenderTestResults(
       self, device, render_tests_device_output_dir, results):
-    # If GS results bucket is specified, will archive render result images.
-    # If render image dir is specified, will pull the render result image from
-    # the device and leave in the directory.
-    if not (bool(self._test_instance.gs_results_bucket) or
-            bool(self._test_instance.render_results_dir)):
-      return
 
     failure_images_device_dir = posixpath.join(
         render_tests_device_output_dir, 'failures')
@@ -596,12 +590,9 @@ class LocalDeviceInstrumentationTestRun(
         logging.error('Golden images not found on device.')
 
       # Upload results to Google Storage.
-      if self._test_instance.gs_results_bucket:
-        self._UploadRenderTestResults(render_host_dir, results)
+      self._SaveRenderTestResults(render_host_dir, results)
 
-  def _UploadRenderTestResults(self, render_host_dir, results):
-    render_tests_bucket = (
-        self._test_instance.gs_results_bucket + '/render_tests')
+  def _SaveRenderTestResults(self, render_host_dir, results):
 
     for failure_filename in os.listdir(
         os.path.join(render_host_dir, 'failures')):
@@ -613,22 +604,26 @@ class LocalDeviceInstrumentationTestRun(
 
       failure_filepath = os.path.join(
           render_host_dir, 'failures', failure_filename)
-      failure_link = google_storage_helper.upload_content_addressed(
-          failure_filepath, bucket=render_tests_bucket)
+      failure_link = self._test_output_saver.Save(
+          failure_filepath, failure_filename, 'render_tests',
+          test_output_saver_factory.Datatype.IMAGE)
+
 
       golden_filepath = os.path.join(
           render_host_dir, 'goldens', failure_filename)
       if os.path.exists(golden_filepath):
-        golden_link = google_storage_helper.upload_content_addressed(
-            golden_filepath, bucket=render_tests_bucket)
+        golden_link = self._test_output_saver.Save(
+            golden_filepath, 'golden_%s' % failure_filename, 'render_tests',
+            test_output_saver_factory.Datatype.IMAGE)
       else:
         golden_link = ''
 
       diff_filepath = os.path.join(
           render_host_dir, 'diffs', failure_filename)
       if os.path.exists(diff_filepath):
-        diff_link = google_storage_helper.upload_content_addressed(
-            diff_filepath, bucket=render_tests_bucket)
+        diff_link = self._test_output_saver.Save(
+            diff_filepath, 'diff_%s' % failure_filename, 'render_tests',
+            test_output_saver_factory.Datatype.IMAGE)
       else:
         diff_link = ''
 
@@ -646,10 +641,9 @@ class LocalDeviceInstrumentationTestRun(
 
         temp_html.write(processed_template_output)
         temp_html.flush()
-        html_results_link = google_storage_helper.upload_content_addressed(
-            temp_html.name,
-            bucket=render_tests_bucket,
-            content_type='text/html')
+        html_results_link = self._test_output_saver.Save(
+            temp_html.name, 'render.html', 'render_tests',
+            test_output_saver_factory.Datatype.HTML)
         for result in results:
           result.SetLink(failure_filename, html_results_link)