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

gm/rebaseline_server/imagepair_test.py

Index: gm/rebaseline_server/imagepair_test.py
diff --git a/gm/rebaseline_server/imagepair_test.py b/gm/rebaseline_server/imagepair_test.py
index c82e4b59cd62e0a43de4083c194b7b43514e63a4..400c1d6d076fc3f62ae73a5279ebf30e24655900 100755
--- a/gm/rebaseline_server/imagepair_test.py
+++ b/gm/rebaseline_server/imagepair_test.py
@@ -21,6 +21,8 @@ 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):
@@ -74,6 +76,8 @@ class ImagePairTest(unittest.TestCase):
                 '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.

         ],
 
         [
@@ -91,13 +95,15 @@ class ImagePairTest(unittest.TestCase):
                     '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,
         ],
 
         [
@@ -141,6 +147,8 @@ class ImagePairTest(unittest.TestCase):
                     '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
@@ -171,6 +179,8 @@ class ImagePairTest(unittest.TestCase):
                 '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
@@ -187,15 +197,23 @@ class ImagePairTest(unittest.TestCase):
                 '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],