rebaseline_server: add ImagePair class, a step towards new intermediate JSON schema

See https://goto.google.com/ChangingRbsJson and bug 1919 for additional context

BUG=skia:1919
NOTRY=True
R=rmistry@google.com

Author: epoger@google.com

Review URL: https://codereview.chromium.org/157593006

git-svn-id: http://skia.googlecode.com/svn/trunk@13385 2bbb7eff-a529-9590-31e7-b0007b416f81

gm/rebaseline_server/imagediffdb.py

@@ -12,6 +12,7 @@ Calulate differences between image pairs, and store them in a database.
 import contextlib
 import logging
 import os
+import re
 import shutil
 import urllib
 try:
@@ -23,6 +24,8 @@ except ImportError:
 DEFAULT_IMAGE_SUFFIX = '.png'
 DEFAULT_IMAGES_SUBDIR = 'images'
 
+DISALLOWED_FILEPATH_CHAR_REGEX = re.compile('[^\w\-]')
+
 DIFFS_SUBDIR = 'diffs'
 WHITEDIFFS_SUBDIR = 'whitediffs'
 
@@ -61,6 +64,9 @@ class DiffRecord(object):
       actual_images_subdir: the subdirectory actual images are stored in.
       image_suffix: the suffix of images.
     """
+    expected_image_locator = _sanitize_locator(expected_image_locator)
+    actual_image_locator = _sanitize_locator(actual_image_locator)
+
     # Download the expected/actual images, if we don't have them already.
     # TODO(rmistry): Add a parameter that makes _download_and_open_image raise
     # an exception if images are not found locally (instead of trying to
@@ -132,6 +138,16 @@ class DiffRecord(object):
     for each R/G/B channel, as a list."""
     return self._max_diff_per_channel
 
+  def as_dict(self):
+    """Returns a dictionary representation of this DiffRecord, as needed when
+    constructing the JSON representation."""
+    return {
+        'numDifferingPixels': self._num_pixels_differing,
+        'percentDifferingPixels': self.get_percent_pixels_differing(),
+        'weightedDiffMeasure': self.get_weighted_diff_measure(),
+        'maxDiffPerChannel': self._max_diff_per_channel,
+    }
+
 
 class ImageDiffDB(object):
   """ Calculates differences between image pairs, maintaining a database of
@@ -174,6 +190,8 @@ class ImageDiffDB(object):
           actual image within storage_root (probably including a checksum to
           guarantee uniqueness)
     """
+    expected_image_locator = _sanitize_locator(expected_image_locator)
+    actual_image_locator = _sanitize_locator(actual_image_locator)
     key = (expected_image_locator, actual_image_locator)
     if not key in self._diff_dict:
       try:
@@ -193,7 +211,8 @@ class ImageDiffDB(object):
 
     Raises a KeyError if we don't have a DiffRecord for this image pair.
     """
-    key = (expected_image_locator, actual_image_locator)
+    key = (_sanitize_locator(expected_image_locator),
+           _sanitize_locator(actual_image_locator))
     return self._diff_dict[key]
 
 
@@ -322,6 +341,15 @@ def _mkdir_unless_exists(path):
   if not os.path.isdir(path):
     os.makedirs(path)
 
+def _sanitize_locator(locator):
+  """Returns a sanitized version of a locator (one in which we know none of the
+  characters will have special meaning in filenames).
+
+  Args:
+    locator: string, or something that can be represented as a string
+  """
+  return DISALLOWED_FILEPATH_CHAR_REGEX.sub('_', str(locator))
+
 def _get_difference_locator(expected_image_locator, actual_image_locator):
   """Returns the locator string used to look up the diffs between expected_image
   and actual_image.
@@ -330,7 +358,8 @@ def _get_difference_locator(expected_image_locator, actual_image_locator):
     expected_image_locator: locator string pointing at expected image
     actual_image_locator: locator string pointing at actual image
 
-  Returns: locator where the diffs between expected and actual images can be
+  Returns: already-sanitized locator where the diffs between expected and
-      found
+      actual images can be found
   """
-  return "%s-vs-%s" % (expected_image_locator, actual_image_locator)
+  return "%s-vs-%s" % (_sanitize_locator(expected_image_locator),
+                       _sanitize_locator(actual_image_locator))

gm/rebaseline_server/imagediffdb_test.py

@@ -26,6 +26,7 @@ class ImageDiffDbTest(unittest.TestCase):
 
   def setUp(self):
     self._temp_dir = tempfile.mkdtemp()
+    self.maxDiff = None
 
   def tearDown(self):
     shutil.rmtree(self._temp_dir)
@@ -34,7 +35,20 @@ class ImageDiffDbTest(unittest.TestCase):
     """Tell unittest framework to not print docstrings for test cases."""
     return None
 
+  def test_sanitize_locator(self):
+    """Test _sanitize_locator()."""
+    self.assertEqual(imagediffdb._sanitize_locator('simple'), 'simple')
+    self.assertEqual(imagediffdb._sanitize_locator(1234), '1234')
+    self.assertEqual(imagediffdb._sanitize_locator('one/two'),  'one_two')
+    self.assertEqual(imagediffdb._sanitize_locator('one\\two'), 'one_two')
+    self.assertEqual(imagediffdb._sanitize_locator('one_two'),  'one_two')
+
   def test_simple(self):
+    """Test ImageDiffDB, downloading real known images from Google Storage.
+
+    TODO(epoger): Instead of hitting Google Storage, we should read image
+    files from local disk using a file:// IMG_URL_BASE.
+    """
     # params for each self-test:
     # 0. expected image locator
     # 1. expected image URL
@@ -45,16 +59,16 @@ class ImageDiffDbTest(unittest.TestCase):
     # 6. expected max_diff_per_channel
     selftests = [
         [
-            '16206093933823793653',
+            'arcofzorro/16206093933823793653',
             IMG_URL_BASE + 'arcofzorro/16206093933823793653.png',
-            '13786535001616823825',
+            'arcofzorro/13786535001616823825',
             IMG_URL_BASE + 'arcofzorro/13786535001616823825.png',
             '0.0662', '0.0113', [255, 255, 247],
         ],
         [
-            '10552995703607727960',
+            'gradients_degenerate_2pt/10552995703607727960',
             IMG_URL_BASE + 'gradients_degenerate_2pt/10552995703607727960.png',
-            '11198253335583713230',
+            'gradients_degenerate_2pt/11198253335583713230',
             IMG_URL_BASE + 'gradients_degenerate_2pt/11198253335583713230.png',
             '100.0000', '66.6667', [255, 0, 255],
         ],

gm/rebaseline_server/imagepair.py

@@ -0,0 +1,83 @@
+#!/usr/bin/python
+
+"""
+Copyright 2014 Google Inc.
+
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+ImagePair class (see class docstring for details)
+"""
+
+import posixpath
+
+# Keys used within ImagePair dictionary representations.
+KEY_DIFFERENCE_DATA = 'differenceData'
+KEY_EXPECTATIONS_DATA = 'expectationsData'
+KEY_EXTRA_COLUMN_VALUES = 'extraColumnValues'
+KEY_IMAGE_A_URL = 'imageAUrl'
+KEY_IMAGE_B_URL = 'imageBUrl'
+KEY_IS_DIFFERENT = 'isDifferent'
+
+
+class ImagePair(object):
+  """
+  Describes a pair of images, along with optional metadata (pixel difference
+  metrics, whether to ignore mismatches, etc.)
+  """
+
+  def __init__(self, image_diff_db,
+               base_url, imageA_relative_url, imageB_relative_url,
+               expectations=None, extra_columns=None):
+    """
+    Args:
+      image_diff_db: ImageDiffDB instance we use to generate/store image diffs
+      base_url: base of all image URLs
+      imageA_relative_url: URL pointing at an image, relative to base_url
+      imageB_relative_url: URL pointing at an image, relative to base_url
+      expectations: optional dictionary containing expectations-specific
+          metadata (ignore-failure, bug numbers, etc.)
+      extra_columns: optional dictionary containing more metadata (test name,
+          builder name, etc.)
+    """
+    self.base_url = base_url
+    self.imageA_relative_url = imageA_relative_url
+    self.imageB_relative_url = imageB_relative_url
+    self.expectations_dict = expectations
+    self.extra_columns_dict = extra_columns
+    if imageA_relative_url == imageB_relative_url:
+      self.diff_record = None
+    else:
+      # TODO(epoger): Rather than blocking until image_diff_db can read in
+      # the image pair and generate diffs, it would be better to do it
+      # asynchronously: tell image_diff_db to download a bunch of file pairs,
+      # and only block later if we're still waiting for diff_records to come
+      # back.
+      image_diff_db.add_image_pair(
+          expected_image_locator=imageA_relative_url,
+          expected_image_url=posixpath.join(base_url, imageA_relative_url),
+          actual_image_locator=imageB_relative_url,
+          actual_image_url=posixpath.join(base_url, imageB_relative_url))
+      self.diff_record = image_diff_db.get_diff_record(
+          expected_image_locator=imageA_relative_url,
+          actual_image_locator=imageB_relative_url)
+
+  def as_dict(self):
+    """
+    Return a dictionary describing this ImagePair, as needed when constructing
+    the JSON representation.  Uses the KEY_* constants as keys.
+    """
+    asdict = {
+        KEY_IMAGE_A_URL: self.imageA_relative_url,
+        KEY_IMAGE_B_URL: self.imageB_relative_url,
+    }
+    if self.expectations_dict:
+      asdict[KEY_EXPECTATIONS_DATA] = self.expectations_dict
+    if self.extra_columns_dict:
+      asdict[KEY_EXTRA_COLUMN_VALUES] = self.extra_columns_dict
+    if self.diff_record and (self.diff_record.get_num_pixels_differing() > 0):
+      asdict[KEY_IS_DIFFERENT] = True
+      asdict[KEY_DIFFERENCE_DATA] = self.diff_record.as_dict()
+    else:
+      asdict[KEY_IS_DIFFERENT] = False
+    return asdict

gm/rebaseline_server/imagepair_test.py

@@ -0,0 +1,153 @@
+#!/usr/bin/python
+
+"""
+Copyright 2014 Google Inc.
+
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+
+Test imagepair.py
+"""
+
+# System-level imports
+import shutil
+import tempfile
+import unittest
+
+# Local imports
+import imagediffdb
+import imagepair
+
+
+IMG_URL_BASE = 'http://chromium-skia-gm.commondatastorage.googleapis.com/gm/bitmap-64bitMD5/'
+
+
+class ImagePairTest(unittest.TestCase):
+
+  def setUp(self):
+    self._temp_dir = tempfile.mkdtemp()
+    self.maxDiff = None
+
+  def tearDown(self):
+    shutil.rmtree(self._temp_dir)
+
+  def shortDescription(self):
+    """Tell unittest framework to not print docstrings for test cases."""
+    return None
+
+  def test_endToEnd(self):
+    """Test ImagePair, using a real ImageDiffDB to download real images.
+
+    TODO(epoger): Either in addition to or instead of this end-to-end test,
+    we should perform some tests using either:
+    1. a mock ImageDiffDB, or
+    2. a real ImageDiffDB that doesn't hit Google Storage looking for input
+       image files (maybe a file:// IMG_URL_BASE)
+    """
+    # params for each self-test:
+    #
+    # inputs:
+    #  0. imageA_relative_URL
+    #  1. imageB_relative_URL
+    #  2. expectations dict
+    #  3. extra_columns dict
+    # expected output:
+    #  4. expected result of ImagePair.as_dict()
+    selftests = [
+        [
+            # inputs:
+            'arcofzorro/16206093933823793653.png',
+            'arcofzorro/16206093933823793653.png',
+            None,
+            {
+                'builder': 'MyBuilder',
+                'test': 'MyTest',
+            },
+            # expected output:
+            {
+                'extraColumnValues': {
+                    'builder': 'MyBuilder',
+                    'test': 'MyTest',
+                },
+                'imageAUrl': 'arcofzorro/16206093933823793653.png',
+                'imageBUrl': 'arcofzorro/16206093933823793653.png',
+                'isDifferent': False,
+            },
+        ],
+
+        [
+            # inputs:
+            'arcofzorro/16206093933823793653.png',
+            'arcofzorro/13786535001616823825.png',
+            None,
+            None,
+            # expected output:
+            {
+                'differenceData': {
+                    'maxDiffPerChannel': [255, 255, 247],
+                    'numDifferingPixels': 662,
+                    'percentDifferingPixels': 0.0662,
+                    'weightedDiffMeasure': 0.01127756555171088,
+                },
+                'imageAUrl': 'arcofzorro/16206093933823793653.png',
+                'imageBUrl': 'arcofzorro/13786535001616823825.png',
+                'isDifferent': True,
+            },
+        ],
+
+        [
+            # inputs:
+            'gradients_degenerate_2pt/10552995703607727960.png',
+            'gradients_degenerate_2pt/11198253335583713230.png',
+            {
+                'ignoreFailure': True,
+                'bugs': [1001, 1002],
+            },
+            {
+                'builder': 'MyBuilder',
+                'test': 'MyTest',
+            },
+            # expected output:
+            {
+                'differenceData': {
+                    'maxDiffPerChannel': [255, 0, 255],
+                    'numDifferingPixels': 102400,
+                    'percentDifferingPixels': 100.00,
+                    'weightedDiffMeasure': 66.66666666666667,
+                },
+                'expectationsData': {
+                    'bugs': [1001, 1002],
+                    'ignoreFailure': True,
+                },
+                'extraColumnValues': {
+                    'builder': 'MyBuilder',
+                    'test': 'MyTest',
+                },
+                'imageAUrl':
+                    'gradients_degenerate_2pt/10552995703607727960.png',
+                'imageBUrl':
+                    'gradients_degenerate_2pt/11198253335583713230.png',
+                'isDifferent': True,
+            },
+        ],
+    ]
+
+    db = imagediffdb.ImageDiffDB(self._temp_dir)
+    for selftest in selftests:
+      image_pair = imagepair.ImagePair(
+          image_diff_db=db,
+          base_url=IMG_URL_BASE,
+          imageA_relative_url=selftest[0],
+          imageB_relative_url=selftest[1],
+          expectations=selftest[2],
+          extra_columns=selftest[3])
+      self.assertEqual(image_pair.as_dict(), selftest[4])
+
+
+def main():
+  suite = unittest.TestLoader().loadTestsFromTestCase(ImagePairTest)
+  unittest.TextTestRunner(verbosity=2).run(suite)
+
+
+if __name__ == '__main__':
+  main()