Show an error in the pretty diff when an image lacks a checksum
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 May 2012 02:18:08 +0000 (02:18 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 May 2012 02:18:08 +0000 (02:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87791

Reviewed by Dirk Pranke.

* PrettyPatch/PrettyPatch.rb:
* PrettyPatch/PrettyPatch_test.rb:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@118882 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/bugs.webkit.org/ChangeLog
Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
Websites/bugs.webkit.org/PrettyPatch/PrettyPatch_test.rb

index e995b83..979dfc5 100644 (file)
@@ -1,3 +1,13 @@
+2012-05-29  Ojan Vafai  <ojan@chromium.org>
+
+        Show an error in the pretty diff when an image lacks a checksum
+        https://bugs.webkit.org/show_bug.cgi?id=87791
+
+        Reviewed by Dirk Pranke.
+
+        * PrettyPatch/PrettyPatch.rb:
+        * PrettyPatch/PrettyPatch_test.rb:
+
 2012-04-01  Adam Barth  <abarth@webkit.org>
 
         Code review tool no longer needs to work around position:fixed handling on iPad
index f672163..4d38bd2 100644 (file)
@@ -98,6 +98,8 @@ private
         Websites
     ]
 
+    IMAGE_CHECKSUM_ERROR = "<p>INVALID: Image lacks a checksum. This will fail with a MISSING error in run-webkit-tests. Always generate new png files using run-webkit-tests.</p>"
+
     def self.normalize_line_ending(s)
         s.gsub /\r\n?/, "\n"
     end
@@ -578,7 +580,7 @@ EOF
             end
             image_snippet = "<img class='image' src='" + @image_url + "' />"
             if not @image_checksum then
-                return image_snippet
+                return IMAGE_CHECKSUM_ERROR + image_snippet
             end
             return "<p>" + @image_checksum + "</p>" + image_snippet
         end
@@ -601,6 +603,8 @@ EOF
 
                         if image_checksum
                             str += image_checksum + "<br>"
+                        else
+                            str += IMAGE_CHECKSUM_ERROR + "<br>"
                         end
                         if image_url
                             str += "<img class='image' src='" + image_url + "' />"
index ed41693..fef8ca0 100755 (executable)
@@ -60,9 +60,21 @@ class PrettyPatch_test < Test::Unit::TestCase
         assert_equal(info[Info::SHARED], $last_prettify_part_count["shared"], "Wrong number of 'shared' parts in " + description)
         assert_equal(0, $last_prettify_part_count["binary"], "Wrong number of 'binary' parts in " + description)
         assert_equal(0, $last_prettify_part_count["extract-error"], "Wrong number of 'extract-error' parts in " + description)
+        return pretty
     end
 
     def test_patches
         PATCHES.each { |id, info| check_one_patch(id, info) }
     end
+
+    def test_images_without_checksum
+        pretty = check_one_patch(144064, ["Images without checksums", 10, 5, 4, 8])
+        puts pretty
+        matches = pretty.match("INVALID: Image lacks a checksum.")
+        assert(matches, "Should have invalid checksums")
+        # FIXME: This should only have 4 invalid images, but git apply needs an actual copy of the before binary
+        # in order to apply diffs correctly. The end result is that all images in the patch are empty and thus
+        # thought to have no checksum, instead of the 4 images that actually don't have a checksum.
+        assert_equal(10, pretty.scan(/INVALID\: Image lacks a checksum\./).size)
+    end
 end