fix contents of render_pictures JSON summary

tools/tests/render_pictures_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 the render_pictures binary.
 """
 
 # System-level imports
 import json
 import os
 import shutil
 import tempfile
 
 # Imports from within Skia
 import base_unittest
 
 # Maximum length of text diffs to show when tests fail
 MAX_DIFF_LENGTH = 30000
 
 EXPECTED_HEADER_CONTENTS = {
     "type" : "ChecksummedImages",
     "revision" : 1,
 }
 
+# Manually verified: 640x400 red rectangle with black border
+RED_WHOLEIMAGE = {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 11092453015575919668,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp.png",
+}
+
+# Manually verified: 640x400 green rectangle with black border
+GREEN_WHOLEIMAGE = {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 8891695120562235492,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp.png",
+}
+
+# Manually verified these 6 images, all 256x256 tiles,
+# consistent with a tiled version of the 640x400 red rect
+# with black borders.
+RED_TILES = [{
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 5815827069051002745,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile0.png",
+},{
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 9323613075234140270,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile1.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 16670399404877552232,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile2.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 2507897274083364964,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile3.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 7325267995523877959,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile4.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 2181381724594493116,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "red_skp-tile5.png",
+}]
+
+# Manually verified these 6 images, all 256x256 tiles,
+# consistent with a tiled version of the 640x400 green rect
+# with black borders.
+GREEN_TILES = [{
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 12587324416545178013,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile0.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 7624374914829746293,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile1.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 5686489729535631913,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile2.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 7980646035555096146,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile3.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 17817086664365875131,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile4.png",
+}, {
+    "checksumAlgorithm" : "bitmap-64bitMD5",
+    "checksumValue" : 10673669813016809363,
+    "comparisonResult" : "no-comparison",
+    "filepath" : "green_skp-tile5.png",
+}]
+
 
 class RenderPicturesTest(base_unittest.TestCase):
 
   def setUp(self):
     self._input_skp_dir = tempfile.mkdtemp()
     self._temp_dir = tempfile.mkdtemp()
     self.maxDiff = MAX_DIFF_LENGTH
 
   def tearDown(self):
     shutil.rmtree(self._input_skp_dir)
     shutil.rmtree(self._temp_dir)
 
   def test_tiled_whole_image(self):
-    """Run render_pictures with tiles and --writeWholeImage flag."""
+    """Run render_pictures with tiles and --writeWholeImage flag.
+
+    TODO(epoger): This test generates undesired results!  The JSON summary

[epoger, 2014/04/24 20:51:20]

This behavior CHANGED with this CL, but it was already broken.  Now it's just
broken in a different way.

+    includes both whole-image and tiled-images (as it should), but only
+    whole-images are written out to disk.
+
+    TODO(epoger): I noticed that when this is run without --writePath being

[scroggo, 2014/04/24 21:15:08]

Are there bugs for these TODOs? Should they be referenced in the file?

[epoger, 2014/04/25 15:26:43]

Now they do, and they are.  Good idea, thanks.

+    specified, this test writes red_skp.png and green_skp.png to the current
+    directory.  We should fix that... if --writePath is not specified, this
+    probably shouldn't write out red_skp.png and green_skp.png at all!
+    """
     output_json_path = os.path.join(self._temp_dir, 'output.json')
     self._generate_skps()
-    # TODO(epoger): I noticed that when this is run without --writePath being
-    # specified, this test writes red_skp.png and green_skp.png to the current
-    # directory.  We should fix that... if --writePath is not specified, this
-    # probably shouldn't write out red_skp.png and green_skp.png at all!
     self._run_render_pictures(['-r', self._input_skp_dir,
                                '--bbh', 'grid', '256', '256',
                                '--mode', 'tile', '256', '256',
                                '--writeJsonSummaryPath', output_json_path,
                                '--writePath', self._temp_dir,
                                '--writeWholeImage'])
     expected_summary_dict = {
         "header" : EXPECTED_HEADER_CONTENTS,
         "actual-results" : {
             "red.skp": {
-                # Manually verified: 640x400 red rectangle with black border
-                "whole-image": {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 11092453015575919668,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp.png",
-                },
+                "tiled-images": RED_TILES,
+                "whole-image": RED_WHOLEIMAGE,
             },
             "green.skp": {
-                # Manually verified: 640x400 green rectangle with black border
-                "whole-image": {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 8891695120562235492,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp.png",
-                },
+                "tiled-images": GREEN_TILES,
+                "whole-image": GREEN_WHOLEIMAGE,
             }
         }
     }
     self._assert_json_contents(output_json_path, expected_summary_dict)
+    # The disk contents are not what they should be--we should see all the

[scroggo, 2014/04/24 21:15:08]

I suppose this goes with the TODO above?

[epoger, 2014/04/25 15:26:43]

Yes.  Removed this redundant comment.

+    # tile images written out, too.
     self._assert_directory_contents(
         self._temp_dir, ['red_skp.png', 'green_skp.png', 'output.json'])
 
   def test_untiled(self):
     """Run without tiles."""
     output_json_path = os.path.join(self._temp_dir, 'output.json')
     self._generate_skps()
     self._run_render_pictures(['-r', self._input_skp_dir,
                                '--writePath', self._temp_dir,
                                '--writeJsonSummaryPath', output_json_path])
-    # TODO(epoger): These expectations are the same as for above unittest.
-    # Define the expectations once, and share them.
     expected_summary_dict = {
         "header" : EXPECTED_HEADER_CONTENTS,
         "actual-results" : {
             "red.skp": {
-                # Manually verified: 640x400 red rectangle with black border
-                "whole-image": {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 11092453015575919668,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp.png",
-                },
+                "whole-image": RED_WHOLEIMAGE,
             },
             "green.skp": {
-                # Manually verified: 640x400 green rectangle with black border
-                "whole-image": {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 8891695120562235492,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp.png",
-                },
+                "whole-image": GREEN_WHOLEIMAGE,
             }
         }
     }
     self._assert_json_contents(output_json_path, expected_summary_dict)
     self._assert_directory_contents(
         self._temp_dir, ['red_skp.png', 'green_skp.png', 'output.json'])
 
   def test_untiled_writeChecksumBasedFilenames(self):
     """Same as test_untiled, but with --writeChecksumBasedFilenames."""
     output_json_path = os.path.join(self._temp_dir, 'output.json')
@@ skipping 29 matching lines @@
     self._assert_directory_contents(self._temp_dir, [
         'red_skp', 'green_skp', 'output.json'])
     self._assert_directory_contents(
         os.path.join(self._temp_dir, 'red_skp'),
         ['bitmap-64bitMD5_11092453015575919668.png'])
     self._assert_directory_contents(
         os.path.join(self._temp_dir, 'green_skp'),
         ['bitmap-64bitMD5_8891695120562235492.png'])
 
   def test_untiled_validate(self):
-    """Same as test_untiled, but with --validate.
-
-    TODO(epoger): This test generates undesired results!  The call
-    to render_pictures should succeed, and generate the same output as
-    test_untiled.
-    See https://code.google.com/p/skia/issues/detail?id=2044 ('render_pictures:

[scroggo, 2014/04/24 21:15:08]

I'm glad to know that using the flag "validate" no longer fails! Did that get
fixed in this CL by returning true when there's no writePath?

[epoger, 2014/04/25 15:26:43]

Probably so.

This code really should have had unittests from the beginning.  :-(

-    --validate fails')
-    """
+    """Same as test_untiled, but with --validate."""
     output_json_path = os.path.join(self._temp_dir, 'output.json')
     self._generate_skps()
-    with self.assertRaises(Exception):
-      self._run_render_pictures(['-r', self._input_skp_dir,
-                                 '--validate',
-                                 '--writePath', self._temp_dir,
-                                 '--writeJsonSummaryPath', output_json_path])
+    self._run_render_pictures(['-r', self._input_skp_dir,
+                               '--validate',
+                               '--writePath', self._temp_dir,
+                               '--writeJsonSummaryPath', output_json_path])
+    expected_summary_dict = {
+        "header" : EXPECTED_HEADER_CONTENTS,
+        "actual-results" : {
+            "red.skp": {
+                "whole-image": RED_WHOLEIMAGE,
+            },
+            "green.skp": {
+                "whole-image": GREEN_WHOLEIMAGE,
+            }
+        }
+    }
+    self._assert_json_contents(output_json_path, expected_summary_dict)
+    self._assert_directory_contents(
+        self._temp_dir, ['red_skp.png', 'green_skp.png', 'output.json'])
 
   def test_untiled_without_writePath(self):
-    """Same as test_untiled, but without --writePath.
-
-    TODO(epoger): This test generates undesired results!
-    See https://code.google.com/p/skia/issues/detail?id=2043 ('render_pictures:
-    --writeJsonSummaryPath fails unless --writePath is specified')
-    """
+    """Same as test_untiled, but without --writePath."""
     output_json_path = os.path.join(self._temp_dir, 'output.json')
     self._generate_skps()
     self._run_render_pictures(['-r', self._input_skp_dir,
                                '--writeJsonSummaryPath', output_json_path])
     expected_summary_dict = {
         "header" : EXPECTED_HEADER_CONTENTS,
-        "actual-results" : None,
+        "actual-results" : {
+            "red.skp": {
+                "whole-image": RED_WHOLEIMAGE,
+            },
+            "green.skp": {
+                "whole-image": GREEN_WHOLEIMAGE,
+            }
+        }
     }
     self._assert_json_contents(output_json_path, expected_summary_dict)
 
   def test_tiled(self):
     """Generate individual tiles."""
     output_json_path = os.path.join(self._temp_dir, 'output.json')
     self._generate_skps()
     self._run_render_pictures(['-r', self._input_skp_dir,
                                '--bbh', 'grid', '256', '256',
                                '--mode', 'tile', '256', '256',
                                '--writePath', self._temp_dir,
                                '--writeJsonSummaryPath', output_json_path])
     expected_summary_dict = {
         "header" : EXPECTED_HEADER_CONTENTS,
         "actual-results" : {
             "red.skp": {
-                # Manually verified these 6 images, all 256x256 tiles,
-                # consistent with a tiled version of the 640x400 red rect
-                # with black borders.
-                "tiled-images": [{
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 5815827069051002745,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile0.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 9323613075234140270,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile1.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 16670399404877552232,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile2.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 2507897274083364964,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile3.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 7325267995523877959,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile4.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 2181381724594493116,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "red_skp-tile5.png",
-                }],
+                "tiled-images": RED_TILES,
             },
             "green.skp": {
-                # Manually verified these 6 images, all 256x256 tiles,
-                # consistent with a tiled version of the 640x400 green rect
-                # with black borders.
-                "tiled-images": [{
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 12587324416545178013,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile0.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 7624374914829746293,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile1.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 5686489729535631913,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile2.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 7980646035555096146,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile3.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 17817086664365875131,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile4.png",
-                }, {
-                    "checksumAlgorithm" : "bitmap-64bitMD5",
-                    "checksumValue" : 10673669813016809363,
-                    "comparisonResult" : "no-comparison",
-                    "filepath" : "green_skp-tile5.png",
-                }],
+                "tiled-images": GREEN_TILES,
             }
         }
     }
     self._assert_json_contents(output_json_path, expected_summary_dict)
     self._assert_directory_contents(
         self._temp_dir,
         ['red_skp-tile0.png', 'red_skp-tile1.png', 'red_skp-tile2.png',
          'red_skp-tile3.png', 'red_skp-tile4.png', 'red_skp-tile5.png',
          'green_skp-tile0.png', 'green_skp-tile1.png', 'green_skp-tile2.png',
          'green_skp-tile3.png', 'green_skp-tile4.png', 'green_skp-tile5.png',
@@ skipping 172 matching lines @@
     actual_dict = json.loads(file_contents)
     self.assertEqual(actual_dict, expected_dict)
 
 
 def main():
   base_unittest.main(RenderPicturesTest)
 
 
 if __name__ == '__main__':
   main()