Add --render-results-dir arg to store render results locally.

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

 # Copyright 2015 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
 import os
 import posixpath
 import re
 import sys
 import tempfile
@@ skipping 495 matching lines @@
               google_storage_helper.unique_name(
                   'screenshot', device=device),
               screenshot_host_file,
               bucket=('%s/screenshots' %
                       self._test_instance.gs_results_bucket))
           for result in results:
             result.SetLink('post_test_screenshot', link)
 
   def _ProcessRenderTestResults(
       self, device, render_tests_device_output_dir, results):
-    # Will archive test images if we are given a GS bucket to store the results
-    # in and are given a results file to output the links to.
-    if not bool(self._test_instance.gs_results_bucket):
+    # 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')
-
     if not device.FileExists(failure_images_device_dir):
       return
 
     render_tests_bucket = (
         self._test_instance.gs_results_bucket + '/render_tests')
 
     diff_images_device_dir = posixpath.join(
         render_tests_device_output_dir, 'diffs')
 
     golden_images_device_dir = posixpath.join(
         render_tests_device_output_dir, 'goldens')
 
-    with tempfile_ext.NamedTemporaryDirectory() as temp_dir:
-      device.PullFile(failure_images_device_dir, temp_dir)
+    with contextlib_ext.Optional(
+        tempfile_ext.NamedTemporaryDirectory(),
+        self._test_instance.render_results_dir is None) as render_host_dir:
+      render_host_dir = (
+          self._test_instance.render_results_dir or render_host_dir)
+
+      # Pull all render test results from device.
+      device.PullFile(failure_images_device_dir, render_host_dir)
 
       if device.FileExists(diff_images_device_dir):
-        device.PullFile(diff_images_device_dir, temp_dir)
+        device.PullFile(diff_images_device_dir, render_host_dir)
       else:
         logging.error('Diff images not found on device.')
 
       if device.FileExists(golden_images_device_dir):
-        device.PullFile(golden_images_device_dir, temp_dir)
+        device.PullFile(golden_images_device_dir, render_host_dir)
       else:
         logging.error('Golden images not found on device.')
 
-      for failure_filename in os.listdir(os.path.join(temp_dir, 'failures')):
-        m = RE_RENDER_IMAGE_NAME.match(failure_filename)
-        if not m:
-          logging.warning('Unexpected file in render test failures: %s',
-                          failure_filename)
-          continue
+      # Upload results to Google Storage.
+      if self._test_instance.gs_results_bucket:

[PEConn, 2017/05/17 08:34:34]

Can you invert this if statement to reduce nesting?

if not self._test_instance.gs_results_bucket:
  return

for failure_filename in os.listdir(
...

[mikecase (-- gone --), 2017/05/17 21:10:29]

Done

+        for failure_filename in os.listdir(
+            os.path.join(render_host_dir, 'failures')):
+          m = RE_RENDER_IMAGE_NAME.match(failure_filename)
+          if not m:
+            logging.warning('Unexpected file in render test failures: %s',
+                            failure_filename)
+            continue
 
-        failure_filepath = os.path.join(temp_dir, 'failures', failure_filename)
-        failure_link = google_storage_helper.upload_content_addressed(
-            failure_filepath, bucket=render_tests_bucket)
+          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)
 
-        golden_filepath = os.path.join(temp_dir, 'goldens', failure_filename)
-        if os.path.exists(golden_filepath):
-          golden_link = google_storage_helper.upload_content_addressed(
-              golden_filepath, bucket=render_tests_bucket)
-        else:
-          golden_link = ''
+          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)
+          else:
+            golden_link = ''
 
-        diff_filepath = os.path.join(temp_dir, 'diffs', failure_filename)
-        if os.path.exists(diff_filepath):
-          diff_link = google_storage_helper.upload_content_addressed(
-              diff_filepath, bucket=render_tests_bucket)
-        else:
-          diff_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)
+          else:
+            diff_link = ''
 
-        with tempfile.NamedTemporaryFile(suffix='.html') as temp_html:
-          jinja2_env = jinja2.Environment(
-              loader=jinja2.FileSystemLoader(_JINJA_TEMPLATE_DIR),
-              trim_blocks=True)
-          template = jinja2_env.get_template(_JINJA_TEMPLATE_FILENAME)
-          # pylint: disable=no-member
-          processed_template_output = template.render(
-              test_name=failure_filename,
-              failure_link=failure_link,
-              golden_link=golden_link,
-              diff_link=diff_link)
+          with tempfile.NamedTemporaryFile(suffix='.html') as temp_html:
+            jinja2_env = jinja2.Environment(
+                loader=jinja2.FileSystemLoader(_JINJA_TEMPLATE_DIR),
+                trim_blocks=True)
+            template = jinja2_env.get_template(_JINJA_TEMPLATE_FILENAME)
+            # pylint: disable=no-member
+            processed_template_output = template.render(
+                test_name=failure_filename,
+                failure_link=failure_link,
+                golden_link=golden_link,
+                diff_link=diff_link)
 
-          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')
-          for result in results:
-            result.SetLink(failure_filename, html_results_link)
+            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')
+            for result in results:
+              result.SetLink(failure_filename, html_results_link)
 
   #override
   def _ShouldRetry(self, test):
     if 'RetryOnFailure' in test.get('annotations', {}):
       return True
 
     # TODO(jbudorick): Remove this log message once @RetryOnFailure has been
     # enabled for a while. See crbug.com/619055 for more details.
     logging.error('Default retries are being phased out. crbug.com/619055')
     return False
@@ skipping 23 matching lines @@
     timeout *= cls._GetTimeoutScaleFromAnnotations(annotations)
 
     return timeout
 
 def _IsRenderTest(test):
   """Determines if a test or list of tests has a RenderTest amongst them."""
   if not isinstance(test, list):
     test = [test]
   return any([RENDER_TEST_FEATURE_ANNOTATION in t['annotations'].get(
               FEATURE_ANNOTATION, {}).get('value', ()) for t in test])