Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(472)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: build/android/pylib/local/device/local_device_instrumentation_test_run.py

Issue 2866103002: Add render test results to the results_details webpage. (Closed)
Patch Set: (Reland) Add failure screenshots and images to results detail. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | build/android/render_tests/process_render_test_results.py » ('j') | chrome/test/android/javatests/src/org/chromium/chrome/test/util/RenderUtils.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 c95fe6d6e9bf81d10465469fd425d253e45a56a7..42e4ec9815786d68b3423a129c132516ab342aad 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
@@ -6,6 +6,7 @@ import logging
import os
import posixpath
import re
+import tempfile
import time
from devil.android import device_errors
@@ -39,6 +40,31 @@ TIMEOUT_ANNOTATIONS = [
LOGCAT_FILTERS = ['*:e', 'chromium:v', 'cr_*:v']
+FEATURE_ANNOTATION = 'Feature'
+RENDER_TEST_FEATURE_ANNOTATION = 'RenderTest'
+
+RE_RENDER_IMAGE_NAME = re.compile(
PEConn 2017/05/09 08:52:39 Could you please put a comment here pointing to Re
jbudorick 2017/05/09 14:33:26 Might be good to add a few tests on both sides as
mikecase (-- gone --) 2017/05/10 01:05:00 Done
+ r'(?P<test_class>\w+)\.'
+ r'(?P<description>\w+)\.'
+ r'(?P<device_model>\w+)\.'
+ r'(?P<orientation>port|land)\.png')
+
+RENDER_TESTS_HTML_TEMPLATE = '''
PEConn 2017/05/09 08:52:39 Could you move this into a separate file (or Jinja
mikecase (-- gone --) 2017/05/10 01:05:00 Done. Moved into own file so it will be easier to
+ <html>
+ <table>
+ <tr>
+ <th>Failure</th>
+ <th>Golden</th>
+ <th>Diff</th>
+ </tr>
+ <tr>
+ <td><img src="%s"/></td>
jbudorick 2017/05/09 14:33:26 Are these ok with an empty URL? (It's been a while
mikecase (-- gone --) 2017/05/10 01:05:00 Yeah, I doubled checked, it just doesnt load anyth
+ <td><img src="%s"/></td>
+ <td><img src="%s"/></td>
+ </tr>
+ </table>
+ </html>
+ '''
# TODO(jbudorick): Make this private once the instrumentation test_runner is
# deprecated.
@@ -246,7 +272,8 @@ class LocalDeviceInstrumentationTestRun(
def _RunTest(self, device, test):
extras = {}
- flags = None
+ flags_to_add = []
jbudorick 2017/05/09 14:33:26 This change was in your other CL?
mikecase (-- gone --) 2017/05/10 01:05:00 Yeah, have to make the same change here as well. W
+ flags_to_remove = []
test_timeout_scale = None
if self._test_instance.coverage_directory:
coverage_basename = '%s.ec' % ('%s_group' % test[0]['method']
@@ -293,7 +320,8 @@ class LocalDeviceInstrumentationTestRun(
self._test_instance.test_package, self._test_instance.test_runner)
extras['class'] = test_name
if 'flags' in test:
- flags = test['flags']
+ flags_to_add.extend(test['flags'].add)
+ flags_to_remove.extend(test['flags'].remove)
timeout = self._GetTimeoutFromAnnotations(
test['annotations'], test_display_name)
@@ -305,10 +333,19 @@ class LocalDeviceInstrumentationTestRun(
logging.info('preparing to run %s: %s', test_display_name, test)
- if flags:
+ render_tests_device_output_dir = None
+ if _IsRenderTest(test):
+ # TODO(mikecase): Add DeviceTempDirectory class and use that instead.
+ render_tests_device_output_dir = posixpath.join(
+ device.GetExternalStoragePath(),
+ 'render_test_output_dir')
+ flags_to_add.append('--render-test-output-dir=%s' %
+ render_tests_device_output_dir)
+
+ if flags_to_add or flags_to_remove:
self._CreateFlagChangerIfNeeded(device)
self._flag_changers[str(device)].PushFlags(
- add=flags.add, remove=flags.remove)
+ add=flags_to_add, remove=flags_to_remove)
try:
device.RunShellCommand(
@@ -337,7 +374,7 @@ class LocalDeviceInstrumentationTestRun(
['log', '-p', 'i', '-t', _TAG, 'END %s' % test_name],
check_return=True)
duration_ms = time_ms() - start_ms
- if flags:
+ if flags_to_add or flags_to_remove:
self._flag_changers[str(device)].Restore()
if test_timeout_scale:
valgrind_tools.SetChromeTimeoutScale(
@@ -353,8 +390,15 @@ class LocalDeviceInstrumentationTestRun(
if logcat_url:
result.SetLink('logcat', logcat_url)
+ 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.
+ self._ProcessRenderTestResults(
+ device, render_tests_device_output_dir, results)
+
# Update the result name if the test used flags.
- if flags:
+ if flags_to_add or flags_to_remove:
for r in results:
if r.GetName() == test_name:
r.SetName(test_display_name)
@@ -442,6 +486,90 @@ class LocalDeviceInstrumentationTestRun(
result.SetLink('tombstones', tombstones_url)
return results, None
+ 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):
+ 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:
+ try:
+ device.PullFile(failure_images_device_dir, temp_dir)
+
+ if device.FileExists(diff_images_device_dir):
+ device.PullFile(diff_images_device_dir, temp_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)
+ else:
+ logging.error('Golden images not found on device.')
+ finally:
+ device.RemovePath(render_tests_device_output_dir, recursive=True)
jbudorick 2017/05/09 14:33:26 I think this should be the responsibility of _RunT
mikecase (-- gone --) 2017/05/10 01:05:00 Done
+
+ 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
+
+ failure_filepath = os.path.join(temp_dir, 'failures', failure_filename)
+ failure_link = google_storage_helper.upload(
+ google_storage_helper.unique_name(failure_filename, device=device),
+ 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(
+ google_storage_helper.unique_name(
+ failure_filename, device=device),
+ 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(
+ google_storage_helper.unique_name(
+ failure_filename, device=device),
+ diff_filepath,
+ bucket=render_tests_bucket)
+ else:
+ diff_link = ''
+
+ with tempfile.NamedTemporaryFile(suffix='.html') as temp_html:
+ temp_html.write(RENDER_TESTS_HTML_TEMPLATE %
+ (failure_link, golden_link, diff_link))
+ temp_html.flush()
+ html_results_link = google_storage_helper.upload(
+ google_storage_helper.unique_name('render_html', device=device),
+ 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', {}):
@@ -477,3 +605,10 @@ class LocalDeviceInstrumentationTestRun(
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, ())] for t in test)
« no previous file with comments | « no previous file | build/android/render_tests/process_render_test_results.py » ('j') | chrome/test/android/javatests/src/org/chromium/chrome/test/util/RenderUtils.java » ('J')

Powered by Google App Engine
This is Rietveld 408576698