Revert "[android] Add local results details pages."

This reverts commit 425294bf.

Reason for revert: Broke chrome_public_test_apk

Original change's description:
> [android] Add local results details pages.
> 
> Bug: 772032
> Change-Id: Ieca8a24e1f2d4bfbae1bef4a1a4ebb0a67365710
> Reviewed-on: https://chromium-review.googlesource.com/702682
> Commit-Queue: John Budorick <jbudorick@chromium.org>
> Reviewed-by: Zhiling Huang <hzl@chromium.org>
> Reviewed-by: Yoland Yan <yolandyan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#509116}

TBR=yolandyan@chromium.org,hzl@chromium.org,jbudorick@chromium.org

Change-Id: I9c20ed803802cae8db453282f43008019103618f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 772032,775448
Reviewed-on: https://chromium-review.googlesource.com/722642Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509346}

build/android/pylib/android/logdog_logcat_monitor.py

+# Copyright 2016 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 logging
+
+from devil.android import logcat_monitor
+from devil.utils import reraiser_thread
+from pylib.utils import logdog_helper
+
+class LogdogLogcatMonitor(logcat_monitor.LogcatMonitor):
+  """Logcat monitor that writes logcat to a logdog stream.
+
+  The logdog stream client will return a url which contains the logcat.
+  """
+  def __init__(self, adb, stream_name, clear=True, filter_specs=None,
+               deobfuscate_func=None):
+    super(LogdogLogcatMonitor, self).__init__(adb, clear, filter_specs)
+    self._logcat_url = ''
+    self._logdog_stream = None
+    self._stream_client = None
+    self._stream_name = stream_name
+    self._deobfuscate_func = deobfuscate_func or (lambda lines: lines)
+
+  def GetLogcatURL(self):
+    """Return logcat url.
+
+    The default logcat url is '', if failed to create stream_client.
+    """
+    return self._logcat_url
+
+  def Stop(self):
+    """Stops the logcat monitor.
+
+    Close the logdog stream as well.
+    """
+    try:
+      super(LogdogLogcatMonitor, self)._StopRecording()
+      if self._logdog_stream:
+        self._logdog_stream.close()
+    except Exception as e: # pylint: disable=broad-except
+      logging.exception('Unknown Error: %s.', e)
+
+  def Start(self):
+    """Starts the logdog logcat monitor.
+
+    Clears the logcat if |clear| was set in |__init__|.
+    """
+    if self._clear:
+      self._adb.Logcat(clear=True)
+
+    self._logdog_stream = logdog_helper.open_text(self._stream_name)
+    self._logcat_url = logdog_helper.get_viewer_url(self._stream_name)
+    logging.info('Logcat will be saved to %s', self._logcat_url)
+    self._StartRecording()
+
+  def _StartRecording(self):
+    """Starts recording logcat to file.
+
+    Write logcat to stream at the same time.
+    """
+    def record_to_stream():
+      if self._logdog_stream:
+        for data in self._adb.Logcat(filter_specs=self._filter_specs,
+                                     logcat_format='threadtime',
+                                     iter_timeout=0.08):
+          if self._stop_recording_event.isSet():
+            return
+          if data:
+            data = '\n'.join(self._deobfuscate_func([data]))
+            self._logdog_stream.write(data + '\n')
+          if self._stop_recording_event.isSet():
+            return
+
+    self._stop_recording_event.clear()
+    if not self._record_thread:
+      self._record_thread = reraiser_thread.ReraiserThread(record_to_stream)
+      self._record_thread.start()
+
+  def Close(self):
+    """Override parent's close method."""
+    pass
+
+  def __del__(self):
+    """Override parent's delete method."""
+    pass

build/android/pylib/base/environment.py

@@ -16,14 +16,8 @@ class Environment(object):
       machine.
   """
 
-  def __init__(self, output_manager):
-    """Environment constructor.
-
-    Args:
-      output_manager: Instance of |output_manager.OutputManager| used to
-          save test output.
-    """
-    self._output_manager = output_manager
+  def __init__(self):
+    pass
 
   def SetUp(self):
     raise NotImplementedError
@@ -38,6 +32,3 @@ class Environment(object):
   def __exit__(self, _exc_type, _exc_val, _exc_tb):
     self.TearDown()
 
-  @property
-  def output_manager(self):
-    return self._output_manager

build/android/pylib/base/environment_factory.py

@@ -6,14 +6,12 @@ from pylib import constants
 from pylib.local.device import local_device_environment
 from pylib.local.machine import local_machine_environment
 
-def CreateEnvironment(args, output_manager, error_func):
+def CreateEnvironment(args, error_func):
 
   if args.environment == 'local':
     if args.command not in constants.LOCAL_MACHINE_TESTS:
-      return local_device_environment.LocalDeviceEnvironment(
-          args, output_manager, error_func)
+      return local_device_environment.LocalDeviceEnvironment(args, error_func)
     else:
-      return local_machine_environment.LocalMachineEnvironment(
-          args, output_manager, error_func)
+      return local_machine_environment.LocalMachineEnvironment(args, error_func)
 
   error_func('Unable to create %s environment.' % args.environment)

build/android/pylib/base/output_manager.py

-# Copyright 2017 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 contextlib
-import logging
-import os
-import tempfile
-
-from devil.utils import reraiser_thread
-
-
-class Datatype(object):
-  HTML = 'html'
-  IMAGE = 'image'
-  TEXT = 'text'
-
-
-class OutputManager(object):
-
-  def __init__(self):
-    """OutputManager Constructor.
-
-    This class provides a simple interface to save test output. Subclasses
-    of this will allow users to save test results in the cloud or locally.
-    """
-    self._allow_upload = False
-    self._thread_group = None
-
-  @contextlib.contextmanager
-  def ArchivedTempfile(
-      self, out_filename, out_subdir, datatype=Datatype.TEXT):
-    """Archive file contents asynchonously and then deletes file.
-
-    Args:
-      out_filename: Name for saved file.
-      out_subdir: Directory to save |out_filename| to.
-      datatype: Datatype of file.
-
-    Returns:
-      An ArchivedFile file. This file will be uploaded async when the context
-      manager exits. AFTER the context manager exits, you can get the link to
-      where the file will be stored using the Link() API. You can use typical
-      file APIs to write and flish the ArchivedFile. You can also use file.name
-      to get the local filepath to where the underlying file exists. If you do
-      this, you are responsible of flushing the file before exiting the context
-      manager.
-    """
-    if not self._allow_upload:
-      raise Exception('Must run |SetUp| before attempting to upload!')
-
-    f = self._CreateArchivedFile(out_filename, out_subdir, datatype)
-    try:
-      yield f
-    finally:
-      f.PrepareArchive()
-
-      def archive():
-        try:
-          f.Archive()
-        finally:
-          f.Delete()
-
-      thread = reraiser_thread.ReraiserThread(func=archive)
-      thread.start()
-      self._thread_group.Add(thread)
-
-  def _CreateArchivedFile(self, out_filename, out_subdir, datatype):
-    """Returns an instance of ArchivedFile."""
-    raise NotImplementedError
-
-  def SetUp(self):
-    self._allow_upload = True
-    self._thread_group = reraiser_thread.ReraiserThreadGroup()
-
-  def TearDown(self):
-    self._allow_upload = False
-    logging.info('Finishing archiving output.')
-    self._thread_group.JoinAll()
-
-  def __enter__(self):
-    self.SetUp()
-    return self
-
-  def __exit__(self, _exc_type, _exc_val, _exc_tb):
-    self.TearDown()
-
-
-class ArchivedFile(object):
-
-  def __init__(self, out_filename, out_subdir, datatype):
-    self._out_filename = out_filename
-    self._out_subdir = out_subdir
-    self._datatype = datatype
-
-    self._f = tempfile.NamedTemporaryFile(delete=False)
-    self._ready_to_archive = False
-
-  @property
-  def name(self):
-    return self._f.name
-
-  def write(self, *args, **kwargs):
-    if self._ready_to_archive:
-      raise Exception('Cannot write to file after archiving has begun!')
-    self._f.write(*args, **kwargs)
-
-  def flush(self, *args, **kwargs):
-    if self._ready_to_archive:
-      raise Exception('Cannot flush file after archiving has begun!')
-    self._f.flush(*args, **kwargs)
-
-  def Link(self):
-    """Returns location of archived file."""
-    if not self._ready_to_archive:
-      raise Exception('Cannot get link to archived file before archiving '
-                      'has begun')
-    return self._Link()
-
-  def _Link(self):
-    """Note for when overriding this function.
-
-    This function will certainly be called before the file
-    has finished being archived. Therefore, this needs to be able to know the
-    exact location of the archived file before it is finished being archived.
-    """
-    raise NotImplementedError
-
-  def PrepareArchive(self):
-    """Meant to be called synchronously to prepare file for async archiving."""
-    self.flush()
-    self._ready_to_archive = True
-    self._PrepareArchive()
-
-  def _PrepareArchive(self):
-    """Note for when overriding this function.
-
-    This function is needed for things such as computing the location of
-    content addressed files. This is called after the file is written but
-    before archiving has begun.
-    """
-    pass
-
-  def Archive(self):
-    """Archives file."""
-    if not self._ready_to_archive:
-      raise Exception('File is not ready to archive. Be sure you are not '
-                      'writing to the file and PrepareArchive has been called')
-    self._Archive()
-
-  def _Archive(self):
-    raise NotImplementedError
-
-  def Delete(self):
-    """Deletes the backing file."""
-    os.remove(self.name)

build/android/pylib/base/output_manager_factory.py

-# Copyright 2017 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.
-
-from pylib import constants
-from pylib.output import local_output_manager
-from pylib.output import remote_output_manager
-
-
-def CreateOutputManager(args):
-  if args.local_output:
-    return local_output_manager.LocalOutputManager(
-        output_dir=constants.GetOutDirectory())
-  else:
-    return remote_output_manager.RemoteOutputManager(
-        bucket=args.gs_results_bucket)

build/android/pylib/gtest/gtest_test_instance.py

@@ -287,6 +287,7 @@ class GtestTestInstance(test_instance.TestInstance):
     self._extract_test_list_from_filter = args.extract_test_list_from_filter
     self._filter_tests_lock = threading.Lock()
     self._shard_timeout = args.shard_timeout
+    self._should_save_logcat = bool(args.json_results_file)
     self._store_tombstones = args.store_tombstones
     self._total_external_shards = args.test_launcher_total_shards
     self._suite = args.suite_name[0]
@@ -445,6 +446,12 @@ class GtestTestInstance(test_instance.TestInstance):
   def shard_timeout(self):
     return self._shard_timeout
 
+  # TODO(jbudorick): Remove this once mikecase lands
+  # https://codereview.chromium.org/2933993002/
+  @property
+  def should_save_logcat(self):
+    return self._should_save_logcat
+
   @property
   def store_tombstones(self):
     return self._store_tombstones

build/android/pylib/instrumentation/instrumentation_test_instance.py

@@ -504,6 +504,8 @@ class InstrumentationTestInstance(test_instance.TestInstance):
     self._symbolizer = None
     self._enable_java_deobfuscation = False
     self._deobfuscator = None
+    self._gs_results_bucket = None
+    self._should_save_logcat = None
     self._initializeLogAttributes(args)
 
     self._edit_shared_prefs = []
@@ -699,6 +701,9 @@ class InstrumentationTestInstance(test_instance.TestInstance):
         self.apk_under_test.path if self.apk_under_test else None,
         args.enable_relocation_packing)
 
+    self._gs_results_bucket = args.gs_results_bucket
+    self._should_save_logcat = bool(args.json_results_file)
+
   def _initializeEditPrefsAttributes(self, args):
     if not hasattr(args, 'shared_prefs_file') or not args.shared_prefs_file:
       return
@@ -754,6 +759,10 @@ class InstrumentationTestInstance(test_instance.TestInstance):
   def flags(self):
     return self._flags
 
+  @property
+  def gs_results_bucket(self):
+    return self._gs_results_bucket
+
   @property
   def junit3_runner_class(self):
     return self._junit3_runner_class
@@ -766,6 +775,10 @@ class InstrumentationTestInstance(test_instance.TestInstance):
   def junit4_runner_supports_listing(self):
     return self._junit4_runner_supports_listing
 
+  @property
+  def should_save_logcat(self):
+    return self._should_save_logcat
+
   @property
   def package_info(self):
     return self._package_info

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

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import contextlib
 import datetime
 import functools
 import logging
@@ -9,6 +10,7 @@ import os
 import shutil
 import tempfile
 import threading
+import time
 
 import devil_chromium
 from devil import base_error
@@ -20,9 +22,11 @@ from devil.android.sdk import adb_wrapper
 from devil.utils import file_utils
 from devil.utils import parallelizer
 from pylib import constants
+from pylib.android import logdog_logcat_monitor
 from pylib.base import environment
 from pylib.utils import instrumentation_tracing
 from py_trace_event import trace_event
+from py_utils import contextlib_ext
 
 
 LOGCAT_FILTERS = [
@@ -82,10 +86,43 @@ def handle_shard_failures_with(on_failure):
   return decorator
 
 
+# TODO(jbudorick): Reconcile this with the output manager logic in
+# https://codereview.chromium.org/2933993002/ once that lands.
+@contextlib.contextmanager
+def OptionalPerTestLogcat(
+    device, test_name, condition, additional_filter_specs=None,
+    deobfuscate_func=None):
+  """Conditionally capture logcat and stream it to logdog.
+
+  Args:
+    device: (DeviceUtils) the device from which logcat should be captured.
+    test_name: (str) the test name to use in the stream name.
+    condition: (bool) whether or not to capture the logcat.
+    additional_filter_specs: (list) additional logcat filters.
+    deobfuscate_func: (callable) an optional unary function that
+      deobfuscates logcat lines. The callable should take an iterable
+      of logcat lines and return a list of deobfuscated logcat lines.
+  Yields:
+    A LogdogLogcatMonitor instance whether condition is true or not,
+    though it may not be active.
+  """
+  stream_name = 'logcat_%s_%s_%s' % (
+      test_name,
+      time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
+      device.serial)
+  filter_specs = LOGCAT_FILTERS + (additional_filter_specs or [])
+  logmon = logdog_logcat_monitor.LogdogLogcatMonitor(
+      device.adb, stream_name, filter_specs=filter_specs,
+      deobfuscate_func=deobfuscate_func)
+
+  with contextlib_ext.Optional(logmon, condition):
+    yield logmon
+
+
 class LocalDeviceEnvironment(environment.Environment):
 
-  def __init__(self, args, output_manager, _error_func):
-    super(LocalDeviceEnvironment, self).__init__(output_manager)
+  def __init__(self, args, _error_func):
+    super(LocalDeviceEnvironment, self).__init__()
     self._blacklist = (device_blacklist.Blacklist(args.blacklist_file)
                        if args.blacklist_file
                        else None)

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

@@ -13,7 +13,6 @@ import time
 from devil.android import crash_handler
 from devil.android import device_errors
 from devil.android import device_temp_file
-from devil.android import logcat_monitor
 from devil.android import ports
 from devil.utils import reraiser_thread
 from incremental_install import installer
@@ -454,24 +453,15 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun):
         for f in flags:
           logging.info('  %s', f)
 
-        stream_name = 'logcat_%s_%s_%s' % (
-            hash(tuple(test)),
-            time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
-            device.serial)
-
-        with self._env.output_manager.ArchivedTempfile(
-            stream_name, 'logcat') as logcat_file:
-          with logcat_monitor.LogcatMonitor(
-              device.adb,
-              filter_specs=local_device_environment.LOGCAT_FILTERS,
-              output_file=logcat_file.name) as logmon:
-            with contextlib_ext.Optional(
-                trace_event.trace(str(test)),
-                self._env.trace_output):
-              output = self._delegate.Run(
-                  test, device, flags=' '.join(flags),
-                  timeout=timeout, retries=0)
-          logmon.Close()
+        with local_device_environment.OptionalPerTestLogcat(
+            device, hash(tuple(test)),
+            self._test_instance.should_save_logcat) as logmon:
+          with contextlib_ext.Optional(
+              trace_event.trace(str(test)),
+              self._env.trace_output):
+            output = self._delegate.Run(
+                test, device, flags=' '.join(flags),
+                timeout=timeout, retries=0)
 
         if self._test_instance.enable_xml_result_parsing:
           try:
@@ -485,6 +475,7 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun):
                 str(e))
             gtest_xml = None
 
+        logcat_url = logmon.GetLogcatURL()
         test_artifacts_url = self._UploadTestArtifacts(device,
                                                        test_artifacts_dir)
 
@@ -509,8 +500,8 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun):
 
     tombstones_url = None
     for r in results:
-      if logcat_file:
-        r.SetLink('logcat', logcat_file.Link())
+      if self._test_instance.should_save_logcat:
+        r.SetLink('logcat', logcat_url)
 
       if self._test_instance.gs_test_artifacts_bucket:
         r.SetLink('test_artifacts', test_artifacts_url)

build/android/pylib/local/machine/local_machine_environment.py

@@ -9,8 +9,8 @@ from pylib.base import environment
 
 class LocalMachineEnvironment(environment.Environment):
 
-  def __init__(self, _args, output_manager, _error_func):
-    super(LocalMachineEnvironment, self).__init__(output_manager)
+  def __init__(self, _args, _error_func):
+    super(LocalMachineEnvironment, self).__init__()
 
     devil_chromium.Initialize(
         output_directory=constants.GetOutDirectory())

build/android/pylib/output/__init__.py

-# Copyright 2017 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.

build/android/pylib/output/local_output_manager.py

-# Copyright 2017 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 time
-import os
-import shutil
-import urllib
-
-from pylib.base import output_manager
-
-
-class LocalOutputManager(output_manager.OutputManager):
-  """Saves and manages test output files locally in output directory.
-
-  Location files will be saved in {output_dir}/TEST_RESULTS_{timestamp}.
-  """
-
-  def __init__(self, output_dir):
-    super(LocalOutputManager, self).__init__()
-    timestamp = time.strftime(
-        '%Y_%m_%dT%H_%M_%S', time.localtime())
-    self._output_root = os.path.abspath(os.path.join(
-        output_dir, 'TEST_RESULTS_%s' % timestamp))
-
-  #override
-  def _CreateArchivedFile(self, out_filename, out_subdir, datatype):
-    return LocalArchivedFile(
-        out_filename, out_subdir, datatype, self._output_root)
-
-
-class LocalArchivedFile(output_manager.ArchivedFile):
-
-  def __init__(self, out_filename, out_subdir, datatype, out_root):
-    super(LocalArchivedFile, self).__init__(
-        out_filename, out_subdir, datatype)
-    self._output_path = os.path.join(out_root, out_subdir, out_filename)
-
-  def _Link(self):
-    return 'file://%s' % urllib.quote(self._output_path)
-
-  def _Archive(self):
-    if not os.path.exists(os.path.dirname(self._output_path)):
-      os.makedirs(os.path.dirname(self._output_path))
-    shutil.copy(self.name, self._output_path)

build/android/pylib/output/noop_output_manager.py

-# Copyright 2017 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.
-
-from pylib.base import output_manager
-
-# TODO(jbudorick): This class is currently mostly unused.
-# Add a --bot-mode argument that all bots pass. If --bot-mode and
-# --local-output args are both not passed to test runner then use this
-# as the output manager impl.
-
-# pylint: disable=abstract-method
-# pylint: disable=no-self-use
-# pylint: disable=super-init-not-called
-# pylint: disable=unused-argument
-
-class NoopOutputManager(output_manager.OutputManager):
-
-  def __init__(self):
-    super(NoopOutputManager, self).__init__()
-
-
-class NoopArchivedFile(output_manager.ArchivedFile):
-
-  def __init__(self):
-    pass
-
-  @property
-  def name(self):
-    return ''
-
-  def write(self, *args, **kwargs):
-    pass
-
-  def flush(self, *args, **kwargs):
-    pass
-
-  def Link(self):
-    return ''
-
-  def Archive(self):
-    pass
-
-  def Delete(self):
-    pass

build/android/pylib/output/remote_output_manager.py

-# Copyright 2017 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 hashlib
-import os
-
-from pylib.base import output_manager
-from pylib.output import noop_output_manager
-from pylib.utils import logdog_helper
-from pylib.utils import google_storage_helper
-
-
-class RemoteOutputManager(output_manager.OutputManager):
-
-  def __init__(self, bucket):
-    """Uploads output files to Google Storage or LogDog.
-
-    Files will either be uploaded directly to Google Storage or LogDog
-    depending on the datatype.
-
-    Args
-      bucket: Bucket to use when saving to Google Storage.
-    """
-    super(RemoteOutputManager, self).__init__()
-    self._bucket = bucket
-
-  #override
-  def _CreateArchivedFile(self, out_filename, out_subdir, datatype):
-    if datatype == output_manager.Datatype.TEXT:
-      try:
-        logdog_helper.get_logdog_client()
-        return LogdogArchivedFile(out_filename, out_subdir, datatype)
-      except RuntimeError:
-        return noop_output_manager.NoopArchivedFile()
-    else:
-      if self._bucket is None:
-        return noop_output_manager.NoopArchivedFile()
-      return GoogleStorageArchivedFile(
-          out_filename, out_subdir, datatype, self._bucket)
-
-
-class LogdogArchivedFile(output_manager.ArchivedFile):
-
-  def __init__(self, out_filename, out_subdir, datatype):
-    super(LogdogArchivedFile, self).__init__(out_filename, out_subdir, datatype)
-    self._stream_name = '%s_%s' % (out_subdir, out_filename)
-
-  def _Link(self):
-    return logdog_helper.get_viewer_url(self._stream_name)
-
-  def _Archive(self):
-    with open(self.name, 'r') as f:
-      logdog_helper.text(self._stream_name, f.read())
-
-
-class GoogleStorageArchivedFile(output_manager.ArchivedFile):
-
-  def __init__(self, out_filename, out_subdir, datatype, bucket):
-    super(GoogleStorageArchivedFile, self).__init__(
-        out_filename, out_subdir, datatype)
-    self._bucket = bucket
-    self._upload_path = None
-    self._content_addressed = None
-
-  def _PrepareArchive(self):
-    self._content_addressed = (self._datatype in (
-        output_manager.Datatype.HTML,
-        output_manager.Datatype.IMAGE))
-    if self._content_addressed:
-      sha1 = hashlib.sha1()
-      with open(self.name, 'rb') as f:
-        sha1.update(f.read())
-      self._upload_path = sha1.hexdigest()
-    else:
-      self._upload_path = os.path.join(self._out_subdir, self._out_filename)
-
-  def _Link(self):
-    return google_storage_helper.get_url_link(
-        self._upload_path, self._bucket)
-
-  def _Archive(self):
-    if (self._content_addressed and
-        google_storage_helper.exists(self._upload_path, self._bucket)):
-      return
-
-    content_type = None
-    if self._datatype == output_manager.Datatype.HTML:
-      content_type = 'text/html'
-    google_storage_helper.upload(
-        self._upload_path, self.name, self._bucket, content_type)

build/android/pylib/results/presentation/template/main.html

@@ -75,11 +75,9 @@
         {% include 'template/table.html' %}
       {% endfor %}
     </div>
-  {% if feedback_url %}
-    </br>
-      <a href="{{feedback_url}}" target="_blank"><b>Feedback</b></a>
-    </body>
-  {%- endif %}
+  </br>
+    <a href="{{feedback_url}}" target="_blank"><b>Feedback</b></a>
+  </body>
   <script>
     sortSuiteTableByFailedTestCases();
     showSuiteTableOnlyWithReplaceState();

build/android/pylib/results/presentation/test_results_presentation.py

@@ -266,27 +266,20 @@ def create_suite_table(results_dict):
 
 
 def feedback_url(result_details_link):
-  url_args = [
+  url_args = urllib.urlencode([
       ('labels', 'Pri-2,Type-Bug,Restrict-View-Google'),
       ('summary', 'Result Details Feedback:'),
       ('components', 'Test>Android'),
-  ]
-  if result_details_link:
-    url_args.append(('comment', 'Please check out: %s' % result_details_link))
-  url_args = urllib.urlencode(url_args)
+      ('comment', 'Please check out: %s' % result_details_link)])
   return 'https://bugs.chromium.org/p/chromium/issues/entry?%s' % url_args
 
 
 def results_to_html(results_dict, cs_base_url, bucket, test_name,
-                    builder_name, build_number, local_output):
-  """Convert list of test results into html format.
+                    builder_name, build_number):
+  """Convert list of test results into html format."""
 
-  Args:
-    local_output: Whether this results file is uploaded to Google Storage or
-        just a local file.
-  """
-  test_rows_header, test_rows = create_test_table(
-      results_dict, cs_base_url, test_name)
+  test_rows_header, test_rows = create_test_table(results_dict, cs_base_url,
+                                                  test_name)
   suite_rows_header, suite_rows, suite_row_footer = create_suite_table(
       results_dict)
 
@@ -305,35 +298,21 @@ def results_to_html(results_dict, cs_base_url, bucket, test_name,
 
   main_template = JINJA_ENVIRONMENT.get_template(
       os.path.join('template', 'main.html'))
+  dest = google_storage_helper.unique_name(
+      '%s_%s_%s' % (test_name, builder_name, build_number))
 
-  if local_output:
-    html_render = main_template.render(  #  pylint: disable=no-member
-        {
-          'tb_values': [suite_table_values, test_table_values],
-          'feedback_url': feedback_url(None),
-        })
-    return (html_render, None, None)
-  else:
-    dest = google_storage_helper.unique_name(
-        '%s_%s_%s' % (test_name, builder_name, build_number))
-    result_details_link = google_storage_helper.get_url_link(
-        dest, '%s/html' % bucket)
-    html_render = main_template.render(  #  pylint: disable=no-member
-        {
-          'tb_values': [suite_table_values, test_table_values],
-          'feedback_url': feedback_url(result_details_link),
-        })
-    return (html_render, dest, result_details_link)
-
-
-def result_details(json_path, test_name, cs_base_url, bucket=None,
-                   builder_name=None, build_number=None, local_output=False):
-  """Get result details from json path and then convert results to html.
+  result_details_link = google_storage_helper.get_url_link(
+      dest, '%s/html' % bucket)
 
-  Args:
-    local_output: Whether this results file is uploaded to Google Storage or
-        just a local file.
-  """
+  return (main_template.render(  #  pylint: disable=no-member
+      {'tb_values': [suite_table_values, test_table_values],
+       'feedback_url': feedback_url(result_details_link)
+      }), dest, result_details_link)
+
+
+def result_details(json_path, cs_base_url, bucket, test_name,
+                   builder_name, build_number):
+  """Get result details from json path and then convert results to html."""
 
   with open(json_path) as json_file:
     json_object = json.loads(json_file.read())
@@ -345,8 +324,8 @@ def result_details(json_path, test_name, cs_base_url, bucket=None,
   for testsuite_run in json_object['per_iteration_data']:
     for test, test_runs in testsuite_run.iteritems():
       results_dict[test].extend(test_runs)
-  return results_to_html(results_dict, cs_base_url, bucket, test_name,
-                         builder_name, build_number, local_output)
+  return results_to_html(results_dict, cs_base_url, bucket,
+                         test_name, builder_name, build_number)
 
 
 def upload_to_google_bucket(html, bucket, dest):
@@ -373,18 +352,18 @@ def main():
                       required=True)
   parser.add_argument(
       '-o', '--output-json',
-      help='(Swarming Merge Script API) '
-           'Output JSON file to create.')
+      help='(Swarming Merge Script API)'
+           ' Output JSON file to create.')
   parser.add_argument(
       '--build-properties',
       help='(Swarming Merge Script API) '
            'Build property JSON file provided by recipes.')
   parser.add_argument(
       '--summary-json',
-      help='(Swarming Merge Script API) '
-           'Summary of shard state running on swarming. '
-           '(Output of the swarming.py collect '
-           '--task-summary-json=XXX command.)')
+      help='(Swarming Merge Script API)'
+           ' Summary of shard state running on swarming.'
+           ' (Output of the swarming.py collect'
+           ' --task-summary-json=XXX command.)')
   parser.add_argument(
       'positional', nargs='*',
       help='output.json from shards.')
@@ -441,12 +420,13 @@ def main():
 
   # Link to result details presentation page is a part of the page.
   result_html_string, dest, result_details_link = result_details(
-      json_file, args.test_name, args.cs_base_url, args.bucket,
-      builder_name, build_number)
+      json_file, args.cs_base_url, args.bucket,
+      args.test_name, builder_name, build_number)
 
   result_details_link_2 = upload_to_google_bucket(
       result_html_string.encode('UTF-8'),
       args.bucket, dest)
+
   assert result_details_link == result_details_link_2, (
       'Result details link do not match. The link returned by get_url_link'
       ' should be the same as that returned by upload.')

build/android/pylib/utils/google_storage_helper.py

@@ -9,6 +9,7 @@ Due to logdog not having image or HTML viewer, those instead should be uploaded
 to Google Storage directly using this module.
 """
 
+import hashlib
 import logging
 import os
 import sys
@@ -60,6 +61,21 @@ def upload(name, filepath, bucket, content_type=None, authenticated_link=True):
   return get_url_link(name, bucket, authenticated_link)
 
 
+def upload_content_addressed(
+    filepath, bucket, content_type=None, authenticated_link=True):
+  """Uploads data to Google Storage with filename as sha1 hash.
+
+  If file already exists in bucket with hash name, nothing is uploaded.
+  """
+  sha1 = hashlib.sha1()
+  with open(filepath, 'rb') as f:
+    sha1.update(f.read())
+  sha1_hash = sha1.hexdigest()
+  if not exists(sha1_hash, bucket):
+    upload(sha1_hash, filepath, bucket, content_type, authenticated_link)
+  return get_url_link(sha1_hash, bucket, authenticated_link)
+
+
 @decorators.NoRaiseException(default_return_value=False)
 def exists(name, bucket):
   bucket = _format_bucket_name(bucket)
@@ -73,7 +89,6 @@ def exists(name, bucket):
     return False
 
 
-# TODO(jbudorick): Delete this function. Only one user of it.
 def unique_name(basename, suffix='', timestamp=True, device=None):
   """Helper function for creating a unique name for a file to store in GS.
 

build/android/test_runner.py

@@ -36,13 +36,10 @@ from devil.utils import run_tests_helper
 from pylib import constants
 from pylib.base import base_test_result
 from pylib.base import environment_factory
-from pylib.base import output_manager
-from pylib.base import output_manager_factory
 from pylib.base import test_instance_factory
 from pylib.base import test_run_factory
 from pylib.results import json_results
 from pylib.results import report_results
-from pylib.results.presentation import test_results_presentation
 from pylib.utils import logdog_helper
 from pylib.utils import logging_utils
 
@@ -157,12 +154,6 @@ def AddCommonOptions(parser):
       default='local', choices=constants.VALID_ENVIRONMENTS,
       help='Test environment to run in (default: %(default)s).')
 
-  parser.add_argument(
-      '--local-output',
-      action='store_true',
-      help='Whether to archive test output locally and generate '
-           'a local results detail page.')
-
   class FastLocalDevAction(argparse.Action):
     def __call__(self, parser, namespace, values, option_string=None):
       namespace.verbose_count = max(namespace.verbose_count, 1)
@@ -837,85 +828,69 @@ def RunTestsInPlatformMode(args):
 
   ### Set up test objects.
 
-  out_manager = output_manager_factory.CreateOutputManager(args)
-  env = environment_factory.CreateEnvironment(
-      args, out_manager, infra_error)
+  env = environment_factory.CreateEnvironment(args, infra_error)
   test_instance = test_instance_factory.CreateTestInstance(args, infra_error)
   test_run = test_run_factory.CreateTestRun(
       args, env, test_instance, infra_error)
 
   ### Run.
-  with out_manager:
-    with json_writer, logcats_uploader, env, test_instance, test_run:
-
-      repetitions = (xrange(args.repeat + 1) if args.repeat >= 0
-                     else itertools.count())
-      result_counts = collections.defaultdict(
-          lambda: collections.defaultdict(int))
-      iteration_count = 0
-      for _ in repetitions:
-        raw_results = test_run.RunTests()
-        if not raw_results:
-          continue
-
-        all_raw_results.append(raw_results)
-
-        iteration_results = base_test_result.TestRunResults()
-        for r in reversed(raw_results):
-          iteration_results.AddTestRunResults(r)
-        all_iteration_results.append(iteration_results)
-
-        iteration_count += 1
-        for r in iteration_results.GetAll():
-          result_counts[r.GetName()][r.GetType()] += 1
-        report_results.LogFull(
-            results=iteration_results,
-            test_type=test_instance.TestType(),
-            test_package=test_run.TestPackage(),
-            annotation=getattr(args, 'annotations', None),
-            flakiness_server=getattr(args, 'flakiness_dashboard_server',
-                                     None))
-        if args.break_on_failure and not iteration_results.DidRunPass():
-          break
-
-      if iteration_count > 1:
-        # display summary results
-        # only display results for a test if at least one test did not pass
-        all_pass = 0
-        tot_tests = 0
-        for test_name in result_counts:
-          tot_tests += 1
-          if any(result_counts[test_name][x] for x in (
-              base_test_result.ResultType.FAIL,
-              base_test_result.ResultType.CRASH,
-              base_test_result.ResultType.TIMEOUT,
-              base_test_result.ResultType.UNKNOWN)):
-            logging.critical(
-                '%s: %s',
-                test_name,
-                ', '.join('%s %s' % (str(result_counts[test_name][i]), i)
-                          for i in base_test_result.ResultType.GetTypes()))
-          else:
-            all_pass += 1
-
-        logging.critical('%s of %s tests passed in all %s runs',
-                         str(all_pass),
-                         str(tot_tests),
-                         str(iteration_count))
-
-    if args.local_output and args.json_results_file:
-      with out_manager.ArchivedTempfile(
-          'test_results_presentation.html',
-          'test_results_presentation',
-          output_manager.Datatype.HTML) as results_detail_file:
-        result_html_string, _, _ = test_results_presentation.result_details(
-            json_path=args.json_results_file,
-            test_name=args.command,
-            cs_base_url='http://cs.chromium.org',
-            local_output=True)
-        results_detail_file.write(result_html_string)
-        results_detail_file.flush()
-      logging.critical('TEST RESULTS: %s', results_detail_file.Link())
+
+  with json_writer, logcats_uploader, env, test_instance, test_run:
+
+    repetitions = (xrange(args.repeat + 1) if args.repeat >= 0
+                   else itertools.count())
+    result_counts = collections.defaultdict(
+        lambda: collections.defaultdict(int))
+    iteration_count = 0
+    for _ in repetitions:
+      raw_results = test_run.RunTests()
+      if not raw_results:
+        continue
+
+      all_raw_results.append(raw_results)
+
+      iteration_results = base_test_result.TestRunResults()
+      for r in reversed(raw_results):
+        iteration_results.AddTestRunResults(r)
+      all_iteration_results.append(iteration_results)
+
+      iteration_count += 1
+      for r in iteration_results.GetAll():
+        result_counts[r.GetName()][r.GetType()] += 1
+      report_results.LogFull(
+          results=iteration_results,
+          test_type=test_instance.TestType(),
+          test_package=test_run.TestPackage(),
+          annotation=getattr(args, 'annotations', None),
+          flakiness_server=getattr(args, 'flakiness_dashboard_server',
+                                   None))
+      if args.break_on_failure and not iteration_results.DidRunPass():
+        break
+
+    if iteration_count > 1:
+      # display summary results
+      # only display results for a test if at least one test did not pass
+      all_pass = 0
+      tot_tests = 0
+      for test_name in result_counts:
+        tot_tests += 1
+        if any(result_counts[test_name][x] for x in (
+            base_test_result.ResultType.FAIL,
+            base_test_result.ResultType.CRASH,
+            base_test_result.ResultType.TIMEOUT,
+            base_test_result.ResultType.UNKNOWN)):
+          logging.critical(
+              '%s: %s',
+              test_name,
+              ', '.join('%s %s' % (str(result_counts[test_name][i]), i)
+                        for i in base_test_result.ResultType.GetTypes()))
+        else:
+          all_pass += 1
+
+      logging.critical('%s of %s tests passed in all %s runs',
+                       str(all_pass),
+                       str(tot_tests),
+                       str(iteration_count))
 
   if args.command == 'perf' and (args.steps or args.single_step):
     return 0

build/android/test_runner.pydeps

@@ -124,12 +124,12 @@ gyp/util/md5_check.py
 incremental_install/__init__.py
 incremental_install/installer.py
 pylib/__init__.py
+pylib/android/__init__.py
+pylib/android/logdog_logcat_monitor.py
 pylib/base/__init__.py
 pylib/base/base_test_result.py
 pylib/base/environment.py
 pylib/base/environment_factory.py
-pylib/base/output_manager.py
-pylib/base/output_manager_factory.py
 pylib/base/test_collection.py
 pylib/base/test_exception.py
 pylib/base/test_instance.py
@@ -165,10 +165,6 @@ pylib/local/machine/local_machine_environment.py
 pylib/local/machine/local_machine_junit_test_run.py
 pylib/monkey/__init__.py
 pylib/monkey/monkey_test_instance.py
-pylib/output/__init__.py
-pylib/output/local_output_manager.py
-pylib/output/noop_output_manager.py
-pylib/output/remote_output_manager.py
 pylib/perf/__init__.py
 pylib/perf/perf_test_instance.py
 pylib/results/__init__.py
@@ -176,9 +172,6 @@ pylib/results/flakiness_dashboard/__init__.py
 pylib/results/flakiness_dashboard/json_results_generator.py
 pylib/results/flakiness_dashboard/results_uploader.py
 pylib/results/json_results.py
-pylib/results/presentation/__init__.py
-pylib/results/presentation/standard_gtest_merge.py
-pylib/results/presentation/test_results_presentation.py
 pylib/results/report_results.py
 pylib/symbols/__init__.py
 pylib/symbols/deobfuscator.py