Updating Maps pixel test for stability

content/test/gpu/gpu_tests/maps.py

Index: content/test/gpu/gpu_tests/maps.py
diff --git a/content/test/gpu/gpu_tests/maps.py b/content/test/gpu/gpu_tests/maps.py
index f4b4ef99938efb4250cc2740cc4cc093bb9babc4..d80807dc1d005115c015e5eb4a9ecd620996312b 100644
--- a/content/test/gpu/gpu_tests/maps.py
+++ b/content/test/gpu/gpu_tests/maps.py
@@ -7,15 +7,23 @@ Performs several common navigation actions on the map (pan, zoom, rotate) then
 captures a screenshot and compares selected pixels against expected values"""
 
 import json
+import optparse
 import os
 import re
 
+import maps_expectations
+
 from telemetry import test
 from telemetry.core.backends import png_bitmap
 from telemetry.core import util
 from telemetry.page import page_test
 from telemetry.page import page_set
 
+test_data_dir = os.path.abspath(os.path.join(
+    os.path.dirname(__file__), '..', '..', 'data', 'gpu'))
+
+default_generated_data_dir = os.path.join(test_data_dir, 'generated')
+
 class MapsValidator(page_test.PageTest):
   def __init__(self):
     super(MapsValidator, self).__init__('ValidatePage')
@@ -24,14 +32,47 @@ class MapsValidator(page_test.PageTest):
     options.AppendExtraBrowserArgs('--enable-gpu-benchmarking')
 
   def ValidatePage(self, page, tab, results):
+    MapsValidator.SpinWaitOnRAF(tab, 3)

[Ken Russell (switch to Gerrit), 2013/11/19 01:22:30]

Please add a TODO about removing this, since theoretically it shouldn't be
needed.

[bajones, 2013/11/19 18:27:31]

Done.

+
     if not tab.screenshot_supported:
       raise page_test.Failure('Browser does not support screenshot capture')
     screenshot = tab.Screenshot(5)
     if not screenshot:
       raise page_test.Failure('Could not capture screenshot')
 
+    dpr = tab.EvaluateJavaScript('window.devicePixelRatio')
     expected = MapsValidator.ReadPixelExpectations(page)
-    MapsValidator.CompareToExpectations(screenshot, expected)
+
+    try:
+      MapsValidator.CompareToExpectations(screenshot, expected, dpr)
+    except page_test.Failure:
+      image_name = MapsValidator.UrlToImageName(page.display_name)
+      MapsValidator.WriteErrorImage(self.options.generated_dir,
+          image_name, self.options.build_revision, screenshot)
+      raise
+
+  @staticmethod
+  def SpinWaitOnRAF(tab, iterations, timeout = 60):
+    waitScript = r"""
+      window.__spinWaitOnRAFDone = false;
+      var iterationsLeft = %d;
+
+      function spin() {
+        iterationsLeft--;
+        if (iterationsLeft == 0) {
+          window.__spinWaitOnRAFDone = true;
+          return;
+        }
+        window.requestAnimationFrame(spin);
+      }
+      window.requestAnimationFrame(spin);
+    """ % iterations
+
+    def IsWaitComplete():
+      return tab.EvaluateJavaScript('window.__spinWaitOnRAFDone')
+
+    tab.ExecuteJavaScript(waitScript)
+    util.WaitFor(IsWaitComplete, timeout)
 
   @staticmethod
   def ReadPixelExpectations(page):
@@ -41,10 +82,12 @@ class MapsValidator(page_test.PageTest):
     return json_contents
 
   @staticmethod
-  def CompareToExpectations(screenshot, expectations):
+  def CompareToExpectations(screenshot, expectations, devicePixelRatio):
     for expectation in expectations:
       location = expectation["location"]
-      pixel_color = screenshot.GetPixelColor(location[0], location[1])
+      pixel_color = screenshot.GetPixelColor(
+        location[0] * devicePixelRatio,
+        location[1] * devicePixelRatio)
       expect_color = png_bitmap.PngColor(
           expectation["color"][0],
           expectation["color"][1],
@@ -58,10 +101,49 @@ class MapsValidator(page_test.PageTest):
             str(pixel_color.g) + ", " +
             str(pixel_color.b) + "]")
 
+  @staticmethod
+  def UrlToImageName(url):
+    image_name = re.sub(r'^(http|https|file)://(/*)', '', url)
+    image_name = re.sub(r'\.\./', '', image_name)
+    image_name = re.sub(r'(\.|/|-)', '_', image_name)
+    return image_name
+
+  @staticmethod
+  def WriteErrorImage(img_dir, img_name, build_revision, screenshot):
+    full_image_name = img_name + '_' + str(build_revision)
+    full_image_name = full_image_name + '.png'
+
+    # This is a nasty and temporary hack: The pixel test archive step will copy
+    # DIFF images directly, but for FAIL images it also requires a ref. This
+    # allows us to archive the erronous image while the archiving step is being
+    # refactored
+    image_path = os.path.join(img_dir, 'DIFF_' + full_image_name)
+
+    output_dir = os.path.dirname(image_path)
+    if not os.path.exists(output_dir):
+      os.makedirs(output_dir)
+
+    screenshot.WriteFile(image_path)
+
 class Maps(test.Test):
   """Google Maps pixel tests."""
   test = MapsValidator
 
+  @staticmethod
+  def AddTestCommandLineOptions(parser):
+    group = optparse.OptionGroup(parser, 'Maps test options')
+    group.add_option('--generated-dir',
+        help='Overrides the default location for generated test images that '
+        'fail expectations checks',
+        default=default_generated_data_dir)
+    group.add_option('--build-revision',
+        help='Chrome revision being tested.',
+        default="unknownrev")

[Ken Russell (switch to Gerrit), 2013/11/19 01:22:30]

Presumably this command line argument will be added to the GPU recipe afterward?

[bajones, 2013/11/19 18:27:31]

The maps test has been removed from the recipe at the moment, but yes, these
arguments will be passed in when the test is added. It will essentially be a
copy/paste of the code being used for pixel_test.

+    parser.add_option_group(group)
+
+  def CreateExpectations(self, page_set):
+    return maps_expectations.MapsExpectations()
+
   def CreatePageSet(self, options):
     page_set_path = os.path.join(
         util.GetChromiumSrcDir(), 'content', 'test', 'gpu', 'page_sets')
@@ -70,6 +152,7 @@ class Maps(test.Test):
       'make_javascript_deterministic': False,
       'pages': [
         {
+          'name': 'Maps.maps_001',
           'url': 'http://localhost:10020/tracker.html',
           'navigate_steps': [
             { 'action': 'navigate' },