Create a top level ui screenshot description

Merge the screenshot descriptions produced by the instrumentation tests
into a single description. If the tests are run locally print a link
to this description, if the are run remotely add a link to the
description to the json file produced by test_results_presentation

Change-Id: Ia9dd491460633148283bed66949cec47a410dea3
Reviewed-on: https://chromium-review.googlesource.com/952972Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Anthony Berent <aberent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543393}

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

@@ -39,6 +39,7 @@ from py_utils import contextlib_ext
 from py_utils import tempfile_ext
 import tombstones
 
+
 with host_paths.SysPath(
     os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 0):
   import jinja2  # pylint: disable=import-error
@@ -75,8 +76,6 @@ EXTRA_TRACE_FILE = ('org.chromium.base.test.BaseJUnit4ClassRunner.TraceFile')
 _EXTRA_TEST_LIST = (
     'org.chromium.base.test.BaseChromiumAndroidJUnitRunner.TestList')
 
-UI_CAPTURE_DIRS = ['chromium_tests_root', 'UiCapture']
-
 FEATURE_ANNOTATION = 'Feature'
 RENDER_TEST_FEATURE_ANNOTATION = 'RenderTest'
 
@@ -126,7 +125,6 @@ class LocalDeviceInstrumentationTestRun(
     super(LocalDeviceInstrumentationTestRun, self).__init__(
         env, test_instance)
     self._flag_changers = {}
-    self._ui_capture_dir = dict()
     self._replace_package_contextmanager = None
 
   #override
@@ -253,21 +251,8 @@ class LocalDeviceInstrumentationTestRun(
         valgrind_tools.SetChromeTimeoutScale(
             dev, self._test_instance.timeout_scale)
 
-      @trace_event.traced
-      def setup_ui_capture_dir(dev):
-        # Make sure the UI capture directory exists and is empty by deleting
-        # and recreating it.
-        # TODO (aberent) once DeviceTempDir exists use it here.
-        self._ui_capture_dir[dev] = posixpath.join(
-            dev.GetExternalStoragePath(),
-            *UI_CAPTURE_DIRS)
-
-        if dev.PathExists(self._ui_capture_dir[dev]):
-          dev.RunShellCommand(['rm', '-rf', self._ui_capture_dir[dev]])
-        dev.RunShellCommand(['mkdir', self._ui_capture_dir[dev]])
-
       steps += [set_debug_app, edit_shared_prefs, push_test_data,
-                create_flag_changer, setup_ui_capture_dir]
+                create_flag_changer]
 
       def bind_crash_handler(step, dev):
         return lambda: crash_handler.RetryOnSystemCrash(step, dev)
@@ -372,7 +357,11 @@ class LocalDeviceInstrumentationTestRun(
         device.adb, suffix='.png', dir=device.GetExternalStoragePath())
     extras[EXTRA_SCREENSHOT_FILE] = screenshot_device_file.name
 
-    extras[EXTRA_UI_CAPTURE_DIR] = self._ui_capture_dir[device]
+    # Set up the screenshot directory. This needs to be done for each test so
+    # that we only get screenshots created by that test.
+    ui_capture_dir = device_temp_file.NamedDeviceTemporaryDirectory(
+        device.adb)
+    extras[EXTRA_UI_CAPTURE_DIR] = ui_capture_dir.name
 
     if self._env.trace_output:
       trace_device_file = device_temp_file.DeviceTempFile(
@@ -450,6 +439,7 @@ class LocalDeviceInstrumentationTestRun(
         time.strftime('%Y%m%dT%H%M%S-UTC', time.gmtime()),
         device.serial)
 
+    with ui_capture_dir:
       with self._env.output_manager.ArchivedTempfile(
           stream_name, 'logcat') as logcat_file:
         try:
@@ -457,7 +447,8 @@ class LocalDeviceInstrumentationTestRun(
               device.adb,
               filter_specs=local_device_environment.LOGCAT_FILTERS,
               output_file=logcat_file.name,
-            transform_func=self._test_instance.MaybeDeobfuscateLines) as logmon:
+              transform_func=self._test_instance.MaybeDeobfuscateLines
+              ) as logmon:
             with _LogTestEndpoints(device, test_name):
               with contextlib_ext.Optional(
                   trace_event.trace(test_name),
@@ -506,9 +497,9 @@ class LocalDeviceInstrumentationTestRun(
 
       def handle_render_test_data():
         if _IsRenderTest(test):
-        # Render tests do not cause test failure by default. So we have to check
-        # to see if any failure images were generated even if the test does not
-        # fail.
+          # Render tests do not cause test failure by default. So we have to
+          # check to see if any failure images were generated even if the test
+          # does not fail.
           try:
             self._ProcessRenderTestResults(
                 device, render_tests_device_output_dir, results)
@@ -518,7 +509,7 @@ class LocalDeviceInstrumentationTestRun(
 
       def pull_ui_screen_captures():
         screenshots = []
-      for filename in device.ListDirectory(self._ui_capture_dir[device]):
+        for filename in device.ListDirectory(ui_capture_dir.name):
           if filename.endswith('.json'):
             screenshots.append(pull_ui_screenshot(filename))
         if screenshots:
@@ -533,7 +524,7 @@ class LocalDeviceInstrumentationTestRun(
             result.SetLink('ui screenshot', json_archive.Link())
 
       def pull_ui_screenshot(filename):
-      source_dir = self._ui_capture_dir[device]
+        source_dir = ui_capture_dir.name
         json_path = posixpath.join(source_dir, filename)
         json_data = json.loads(device.ReadFile(json_path))
         image_file_path = posixpath.join(source_dir, json_data['location'])

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

@@ -6,7 +6,9 @@
 
 import argparse
 import collections
+import contextlib
 import json
+import logging
 import tempfile
 import os
 import sys
@@ -361,6 +363,56 @@ def upload_to_google_bucket(html, bucket, dest):
         authenticated_link=True)
 
 
+def ui_screenshot_set(json_path):
+  with open(json_path) as json_file:
+    json_object = json.loads(json_file.read())
+  if not 'per_iteration_data' in json_object:
+    # This will be reported as an error by result_details, no need to duplicate.
+    return None
+  ui_screenshots = []
+  for testsuite_run in json_object['per_iteration_data']:
+    for _, test_runs in testsuite_run.iteritems():
+      for test_run in test_runs:
+        if 'ui screenshot' in test_run['links']:
+          screenshot_link = test_run['links']['ui screenshot']
+          if screenshot_link.startswith('file:'):
+            with contextlib.closing(urllib.urlopen(screenshot_link)) as f:
+              test_screenshots = json.load(f)
+          else:
+            # Assume anything that isn't a file link is a google storage link
+            screenshot_string = google_storage_helper.read_from_link(
+                screenshot_link)
+            if not screenshot_string:
+              logging.error('Bad screenshot link %s', screenshot_link)
+              continue
+            test_screenshots = json.loads(
+                screenshot_string)
+          ui_screenshots.extend(test_screenshots)
+
+  if ui_screenshots:
+    return json.dumps(ui_screenshots)
+  return None
+
+
+def upload_screenshot_set(json_path, test_name, bucket, builder_name,
+                          build_number):
+  screenshot_set = ui_screenshot_set(json_path)
+  if not screenshot_set:
+    return None
+  dest = google_storage_helper.unique_name(
+    'screenshots_%s_%s_%s' % (test_name, builder_name, build_number),
+    suffix='.json')
+  with tempfile.NamedTemporaryFile(suffix='.json') as temp_file:
+    temp_file.write(screenshot_set)
+    temp_file.flush()
+    return google_storage_helper.upload(
+        name=dest,
+        filepath=temp_file.name,
+        bucket='%s/json' % bucket,
+        content_type='application/json',
+        authenticated_link=True)
+
+
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--json-file', help='Path of json file.')
@@ -455,16 +507,28 @@ def main():
       'Result details link do not match. The link returned by get_url_link'
       ' should be the same as that returned by upload.')
 
+  ui_screenshot_link = upload_screenshot_set(json_file, args.test_name,
+      args.bucket, builder_name, build_number)
+
+
   if args.output_json:
     with open(json_file) as original_json_file:
       json_object = json.load(original_json_file)
       json_object['links'] = {
           'result_details (logcats, flakiness links)': result_details_link
       }
+
+      if ui_screenshot_link:
+        json_object['links']['ui screenshots'] = ui_screenshot_link
+
       with open(args.output_json, 'w') as f:
         json.dump(json_object, f)
   else:
-    print result_details_link
+    print 'Result Details: %s' % result_details_link
+
+    if ui_screenshot_link:
+      print 'UI Screenshots %s' % ui_screenshot_link
+
 
 if __name__ == '__main__':
   sys.exit(main())

build/android/pylib/utils/google_storage_helper.py

@@ -13,6 +13,7 @@ import logging
 import os
 import sys
 import time
+import urlparse
 
 from pylib.constants import host_paths
 from pylib.utils import decorators
@@ -62,6 +63,15 @@ def upload(name, filepath, bucket, gs_args=None, command_args=None,
   return get_url_link(name, bucket, authenticated_link)
 
 
+@decorators.NoRaiseException(default_return_value='')
+def read_from_link(link):
+  # Note that urlparse returns the path with an initial '/', so we only need to
+  # add one more after the 'gs;'
+  gs_path = 'gs:/%s' % urlparse.urlparse(link).path
+  cmd = [_GSUTIL_PATH, '-q', 'cat', gs_path]
+  return cmd_helper.GetCmdOutput(cmd)
+
+
 @decorators.NoRaiseException(default_return_value=False)
 def exists(name, bucket):
   bucket = _format_bucket_name(bucket)

build/android/test_runner.py

@@ -937,6 +937,16 @@ def RunTestsInPlatformMode(args):
         results_detail_file.flush()
       logging.critical('TEST RESULTS: %s', results_detail_file.Link())
 
+      ui_screenshots = test_results_presentation.ui_screenshot_set(
+          json_file.name)
+      if ui_screenshots:
+        with out_manager.ArchivedTempfile(
+            'ui_screenshots.json',
+            'ui_capture',
+            output_manager.Datatype.JSON) as ui_screenshot_file:
+          ui_screenshot_file.write(ui_screenshots)
+        logging.critical('UI Screenshots: %s', ui_screenshot_file.Link())
+
   if args.command == 'perf' and (args.steps or args.single_step):
     return 0
 

chrome/android/javatests/src/org/chromium/chrome/browser/test/ScreenShooter.java

@@ -10,6 +10,7 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import android.annotation.SuppressLint;
 import android.app.Instrumentation;
 import android.content.res.Configuration;
 import android.graphics.Point;
@@ -93,6 +94,7 @@ import java.util.Map;
  * }
  * </pre>
  */
+@SuppressLint("SetWorldReadable")
 public class ScreenShooter extends TestWatcher {
     private static final String SCREENSHOT_DIR =
             "org.chromium.base.test.util.Screenshooter.ScreenshotDir";
@@ -204,6 +206,9 @@ public class ScreenShooter extends TestWatcher {
             File shotFile = File.createTempFile(shotName, IMAGE_SUFFIX, new File(mBaseDir));
             assertTrue("Screenshot " + shotName, mDevice.takeScreenshot(shotFile));
             writeImageDescription(shotFile, filters, tags, metadata);
+            // Set as world readable so that the test runner can read it from /data/local/tmp
+            // without having to run as root
+            shotFile.setReadable(true, false);
         } catch (IOException e) {
             fail("Cannot create shot files " + e.toString());
         }
@@ -231,8 +236,12 @@ public class ScreenShooter extends TestWatcher {
         String jsonFileName =
                 shotFileName.substring(0, shotFileName.length() - IMAGE_SUFFIX.length())
                 + JSON_SUFFIX;
-        try (FileWriter fileWriter = new FileWriter(new File(mBaseDir, jsonFileName));) {
+        File descriptionFile = new File(mBaseDir, jsonFileName);
+        try (FileWriter fileWriter = new FileWriter(descriptionFile)) {
             fileWriter.write(imageDescription.toString());
         }
+        // Set as world readable so that the test runner can read it from /data/local/tmp without
+        // having to run as root
+        descriptionFile.setReadable(true, false);
     }
 }