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

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

Issue 2701473003: Add failure screenshots and render test images to results detail. (Closed)
Patch Set: Add failure screenshots and render test images to results detail. Created 3 years, 10 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
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 4b3f353ba6feefcda60f4d5335ed094223f4b551..b38728f90a57b062b7eb441824cb3dafa5c99ff3 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,8 @@ import logging
import os
import posixpath
import re
+import shutil
+import tempfile
import time
from devil.android import device_errors
@@ -13,6 +15,7 @@ from devil.android import flag_changer
from devil.utils import reraiser_thread
from pylib import valgrind_tools
from pylib.android import logdog_logcat_monitor
+from pylib.constants import host_paths
from pylib.base import base_test_result
from pylib.instrumentation import instrumentation_test_instance
from pylib.local.device import local_device_environment
@@ -34,6 +37,15 @@ TIMEOUT_ANNOTATIONS = [
('SmallTest', 1 * 60),
]
+_RE_RENDER_IMAGE_NAME = re.compile(
+ r'(?P<test_class>\w+)\.'
+ r'(?P<description>\w+)\.'
+ r'(?P<device_model>\w+)\.'
+ r'(?P<orientation>port|land)\.png')
+
+RENDER_TESTS_RESULTS_DIR = {
+ 'chrome_public_test_apk': 'chrome/test/data/android/render_tests'
+}
# TODO(jbudorick): Make this private once the instrumentation test_runner is
# deprecated.
@@ -295,6 +307,56 @@ class LocalDeviceInstrumentationTestRun(
if logcat_url:
result.SetLink('logcat', logcat_url)
+ # Save render test results.
+ if self._test_instance.should_save_images:
jbudorick 2017/02/21 18:37:12 Also, would it make sense to extract this into its
mikecase (-- gone --) 2017/02/23 00:21:26 Done
+ if self._test_instance.suite in RENDER_TESTS_RESULTS_DIR:
+ render_results_dir = RENDER_TESTS_RESULTS_DIR[self._test_instance.suite]
+
+ temp_dir = None
jbudorick 2017/02/21 18:32:40 https://chromium.googlesource.com/external/github.
mikecase (-- gone --) 2017/02/23 00:21:26 Done
+ try:
+ temp_dir = tempfile.mkdtemp()
+
+ failure_images_device_dir = posixpath.join(
+ device.GetExternalStoragePath(),
+ 'chromium_tests_root', render_results_dir, 'failures')
+ device.PullFile(failure_images_device_dir, temp_dir)
+ device.RemovePath(failure_images_device_dir)
+
+ for failure_filename in os.listdir(
jbudorick 2017/02/21 18:37:12 Of note, though: process_render_test_results attem
mikecase (-- gone --) 2017/02/23 00:21:26 Added diffing back. Makes this a bit more complex.
+ os.path.join(temp_dir, 'failures')):
+
+ m = _RE_RENDER_IMAGE_NAME.match(failure_filename)
jbudorick 2017/02/21 18:32:39 Do you do anything with m other than check it here
mikecase (-- gone --) 2017/02/23 00:21:26 no
+ if not m:
+ logging.warning('Unexpected file in render test failures, %s',
jbudorick 2017/02/21 18:32:39 nit: "... failures: %s" ^
mikecase (-- gone --) 2017/02/23 00:21:26 Done
+ failure_filename)
+ continue
+
+ failure_filepath = os.path.join(
+ temp_dir, 'failures', failure_filename)
+
+ link = logdog_helper.image(
+ logdog_helper.unique_name(failure_filename, device=device),
+ failure_filepath)
+ for result in results:
+ result.SetLink('%s_FAIL' % failure_filename, link)
jbudorick 2017/02/21 18:32:39 nit: name the link something else. Not immediately
mikecase (-- gone --) 2017/02/23 00:21:26 I changed this since I didnt want three separate l
+
+ golden_filepath = os.path.join(
+ host_paths.DIR_SOURCE_ROOT, render_results_dir,
+ failure_filename)
+ if not os.path.exists(golden_filepath):
+ logging.error('Cannot find golden image for %s', failure_filename)
+ continue
+
+ link = logdog_helper.image(
+ logdog_helper.unique_name(failure_filename, device=device),
+ golden_filepath)
+ for result in results:
+ result.SetLink('%s_GOLDEN' % failure_filename, link)
jbudorick 2017/02/21 18:32:39 nit: same
mikecase (-- gone --) 2017/02/23 00:21:26 See above.
+
+ finally:
+ if temp_dir:
+ shutil.rmtree(temp_dir)
+
# Update the result name if the test used flags.
if flags:
for r in results:
@@ -326,11 +388,18 @@ class LocalDeviceInstrumentationTestRun(
file_name = '%s-%s.png' % (
test_display_name,
time.strftime('%Y%m%dT%H%M%S', time.localtime()))
- saved_dir = device.TakeScreenshot(
+ screenshot_file = device.TakeScreenshot(
os.path.join(self._test_instance.screenshot_dir, file_name))
logging.info(
'Saved screenshot for %s to %s.',
- test_display_name, saved_dir)
+ test_display_name, screenshot_file)
+ if self._test_instance.should_save_images:
+ link = logdog_helper.image(
+ logdog_helper.unique_name('screenshot', device=device),
+ screenshot_file)
+ for result in results:
+ result.SetLink('failure_screenshot', link)
+
logging.info('detected failure in %s. raw output:', test_display_name)
for l in output:
logging.info(' %s', l)
@@ -342,7 +411,6 @@ class LocalDeviceInstrumentationTestRun(
else None)
device.ClearApplicationState(self._test_instance.package_info.package,
permissions=permissions)
-
else:
logging.debug('raw output from %s:', test_display_name)
for l in output:

Powered by Google App Engine
This is Rietveld 408576698