Add ability to output ImageBaseGSUrl to render_picture and use in rebaseline server

gm/rebaseline_server/imagepair_test.py

 #!/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/')
+ALT_IMG_URL_BASE = ('http://chromium-skia-gm.commondatastorage.googleapis.com/'
+                    'test/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)
@@ skipping 33 matching lines @@
             # expected output:
             {
                 'extraColumns': {
                     'builder': 'MyBuilder',
                     'test': 'MyTest',
                 },
                 'imageAUrl': 'arcofzorro/16206093933823793653.png',
                 'imageBUrl': 'arcofzorro/16206093933823793653.png',
                 'isDifferent': False,
             },
+            # use alternate imageB base url:

[epoger, 2014/08/21 16:15:22]

AFAICT, the different settings for this parameter do not affect any of the
unittest results.  So are they being exercised?

[rmistry, 2014/08/21 18:15:06]

This parameter was read and exercised at line 209. Is that what you meant?

[epoger, 2014/08/21 18:25:06]

I see that the parameter was read, but the parameter's value doesn't modify the
unittest results (the "expected output" above).  So we know it didn't cause an
assertion to fail, but we don't see that it is functioning correctly...

It may be that it just doesn't make sense to test this parameter within this
file, because it only shows up when you dump out an ImagePairSet instead of just
one ImagePair???

[rmistry, 2014/08/21 19:07:01]

The parameter's value did modify the results of this test. Setting it to True in
any of the selftests here (except the first one) fails the test because the
image does not exist in the new location. It only exists for selftest 1.

The failure if you change any of the other values is:

FAIL: test_endToEnd (__main__.ImagePairTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "imagepair_test.py", line 217, in test_endToEnd
    self.assertEqual(image_pair.as_dict(), selftest[4])
AssertionError: {'isDifferent': True, 'imageBUrl':
'arcofzorro/13786535001616823825.png', 'image [truncated]... != {'isDifferent':
True, 'imageBUrl': 'arcofzorro/13786535001616823825.png', 'image [truncated]...
+ {'differenceData': {'diffUrl':
'arcofzorro_16206093933823793653_png_png-vs-arcofzorro_13786535001616823825_png_png.png',
+                     'maxDiffPerChannel': [255, 255, 247],
+                     'numDifferingPixels': 662,
+                     'percentDifferingPixels': 0.0662,
+                     'perceptualDifference': 0.06620300000000157,
+                     'whiteDiffUrl':
'arcofzorro_16206093933823793653_png_png-vs-arcofzorro_13786535001616823825_png_png.png'},
- {'imageAUrl': 'arcofzorro/16206093933823793653.png',
? ^

+  'imageAUrl': 'arcofzorro/16206093933823793653.png',
? ^

   'imageBUrl': 'arcofzorro/13786535001616823825.png',
   'isDifferent': True}

----------------------------------------------------------------------
Ran 1 test in 1.006s

FAILED (failures=1)



The expected output above was not modified because it never contained the base
url, I'm not sure if it needs to, please let me know if I missed something here.

[epoger, 2014/08/21 19:20:30]

Ah, so you added a new file to GS (
gs://chromium-skia/gm/test/gm/bitmap-64bitMD5/arcofzorro/16206093933823793653.png
) ...

If I'm not mistaken, the first selftest will succeed with either
(IMG_URL_BASE, ALT_IMG_URL_BASE)
or
(IMG_URL_BASE, IMG_URL_BASE)

(the latter is the same functionality that was being tested before your CL)

I think it would be a good idea to note the assumptions being made here about
which files are present in Google Storage, and which are not.

At any rate, I think it's a good idea to focus on the other test case, where we
make the code spit out a whole JSON file and we make sure it looks OK to us.

+            True,

[epoger, 2014/08/21 16:15:22]

Assuming that this parameter is really being exercised meaningfully...

I think it would be more straightforward just to specify the base URLs within
each test case setup.  For each one, either set it to

(IMG_URL_BASE, IMG_URL_BASE)
or
(IMG_URL_BASE, ALT_IMG_URL_BASE)


If you really prefer the boolean, you could do it this way to make it more
self-documenting:

DO_USE_ALTERNATE_IMAGEB_BASE_URL = True
DONT_USE_ALTERNATE_IMAGEB_BASE_URL = False

and then just use those constants.

[rmistry, 2014/08/21 18:15:06]

Used the tuple suggestion. Done.

         ],
 
         [
             # inputs:
             'arcofzorro/16206093933823793653.png',
             'arcofzorro/13786535001616823825.png',
             None,
             None,
             # expected output:
             {
                 'differenceData': {
                     'maxDiffPerChannel': [255, 255, 247],
                     'numDifferingPixels': 662,
                     'percentDifferingPixels': 0.0662,
                     'perceptualDifference': 0.06620300000000157,
                     'diffUrl': 'arcofzorro_16206093933823793653_png_png-vs-' +
                         'arcofzorro_13786535001616823825_png_png.png',
-                    'whiteDiffUrl': 'arcofzorro_16206093933823793653_png_png' + 
+                    'whiteDiffUrl': 'arcofzorro_16206093933823793653_png_png' +
                         '-vs-arcofzorro_13786535001616823825_png_png.png',
                 },
                 'imageAUrl': 'arcofzorro/16206093933823793653.png',
                 'imageBUrl': 'arcofzorro/13786535001616823825.png',
                 'isDifferent': True,
             },
+            # use alternate imageB base url:
+            False,
         ],
 
         [
             # inputs:
             'gradients_degenerate_2pt/10552995703607727960.png',
             'gradients_degenerate_2pt/11198253335583713230.png',
             {
                 'ignoreFailure': True,
                 'bugs': [1001, 1002],
             },
@@ skipping 23 matching lines @@
                 'extraColumns': {
                     'builder': 'MyBuilder',
                     'test': 'MyTest',
                 },
                 'imageAUrl':
                     'gradients_degenerate_2pt/10552995703607727960.png',
                 'imageBUrl':
                     'gradients_degenerate_2pt/11198253335583713230.png',
                 'isDifferent': True,
             },
+            # use alternate imageB base url:
+            False,
         ],
 
         # Test fix for http://skbug.com/2368 -- how do we handle an ImagePair
         # missing one of its images?
         [
             # inputs:
             'arcofzorro/16206093933823793653.png',
             'nonexistentDir/111111.png',
             {
                 'ignoreFailure': True,
@@ skipping 10 matching lines @@
                     'ignoreFailure': True,
                 },
                 'extraColumns': {
                     'builder': 'MyBuilder',
                     'test': 'MyTest',
                 },
                 'imageAUrl': 'arcofzorro/16206093933823793653.png',
                 'imageBUrl': 'nonexistentDir/111111.png',
                 'isDifferent': True,
             },
+            # use alternate imageB base url:
+            False,
         ],
 
         # One of the two images is missing, but download_all_images=True so we
         # should download it anyway.
         [
             # inputs:
             None,
             'arcofzorro/13786535001616823825.png',
             None,
             None,
             # expected output:
             {
                 'imageAUrl': None,
                 'imageBUrl': 'arcofzorro/13786535001616823825.png',
                 'isDifferent': True,
             },
+            # use alternate imageB base url:
+            False,
         ],
 
     ]
 
     db = imagediffdb.ImageDiffDB(self.temp_dir)
     for selftest in selftests:
+      imageA_base_url = IMG_URL_BASE
+      if selftest[5]:
+        imageB_base_url = ALT_IMG_URL_BASE
+      else:
+        imageB_base_url = IMG_URL_BASE
       image_pair = imagepair.ImagePair(
           image_diff_db=db,
-          base_url=IMG_URL_BASE,
+          imageA_base_url=imageA_base_url,
+          imageB_base_url=imageB_base_url,
           imageA_relative_url=selftest[0],
           imageB_relative_url=selftest[1],
           expectations=selftest[2],
           extra_columns=selftest[3],
           download_all_images=True)
       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()