Diff
Modified: trunk/Tools/ChangeLog (101738 => 101739)
--- trunk/Tools/ChangeLog 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/ChangeLog 2011-12-02 04:32:26 UTC (rev 101739)
@@ -1,3 +1,30 @@
+2011-12-01 Hayato Ito <[email protected]>
+
+ Explicitly pass tolerance=0 to port.diff_image in case of RefTestMismatch failure.
+ https://bugs.webkit.org/show_bug.cgi?id=73406
+
+ Reviewed by Ryosuke Niwa.
+
+ WebKitPort's image_diff uses tolerance='0.1' in default.
+ When reftests fail, we should use tolerace=0 when diff-ing images.
+
+ Since ImageDiff on chromium port doesn't use tolerance value as of now,
+ this change doesn't affect chromium port's behavior.
+
+ * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+ (write_test_result):
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ (Port.diff_image):
+ * Scripts/webkitpy/layout_tests/port/chromium.py:
+ (ChromiumPort.diff_image):
+ * Scripts/webkitpy/layout_tests/port/test.py:
+ (TestPort.diff_image):
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ (WebKitPort.diff_image):
+ (WebKitPort._start_image_diff_process):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (MainTest.test_tolerance.ImageDiffTestPort.diff_image):
+
2011-12-01 Adam Klein <[email protected]>
Add Chromium ToT GTest build bots (and group selection support) to flakiness dashboard
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -70,10 +70,11 @@
# FIXME: This work should be done earlier in the pipeline (e.g., when we compare images for non-ref tests).
# FIXME: We should always have 2 images here.
if driver_output.image and expected_driver_output.image:
- image_diff = port.diff_image(driver_output.image, expected_driver_output.image)[0]
- # Non-chromium ports return 'None' since they don't implement diff_image.
- if image_diff is not None:
+ image_diff = port.diff_image(driver_output.image, expected_driver_output.image, tolerance=0)[0]
+ if image_diff:
writer.write_image_diff_files(image_diff)
+ else:
+ _log.warn('Can not get image diff. ImageDiff program might not work correctly.')
writer.copy_file(failure.reference_filename)
elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur):
writer.write_image_files(driver_output.image, expected_image=None)
Added: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py (0 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py (rev 0)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -0,0 +1,59 @@
+#!/usr/bin/env python
+# Copyright (C) 2011 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+from webkitpy.layout_tests.controllers import test_result_writer
+from webkitpy.layout_tests.models import test_failures
+from webkitpy.layout_tests.port.driver import DriverOutput
+from webkitpy.layout_tests.port.test import TestPort
+
+
+class TestResultWriterTest(unittest.TestCase):
+
+ def test_reftest_diff_imaget(self):
+ """A write_test_result should call port.diff_image with tolerance=0 in case of FailureReftestMismatch."""
+ used_tolerance_values = []
+
+ class ImageDiffTestPort(TestPort):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
+ used_tolerance_values.append(tolerance)
+ return (True, 1)
+
+ port = ImageDiffTestPort()
+ fs = port._filesystem
+ test_name = 'failures/unexpected/reftest.html'
+ test_reference_file = fs.join(port.layout_tests_dir(), port.reftest_expected_filename(test_name))
+ driver_output1 = DriverOutput('text1', 'image1', 'imagehash1', 'audio1')
+ driver_output2 = DriverOutput('text2', 'image2', 'imagehash2', 'audio2')
+ failures = [test_failures.FailureReftestMismatch(test_reference_file)]
+ test_result_writer.write_test_result(ImageDiffTestPort(), test_name,
+ driver_output1, driver_output2, failures)
+ self.assertEqual([0], used_tolerance_values)
+
+
+if __name__ == '__main__':
+ unittest.main()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -260,7 +260,7 @@
"""Return whether the two audio files are *not* equal."""
return expected_audio != actual_audio
- def diff_image(self, expected_contents, actual_contents, tolerance=0):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
"""Compare two images and return a tuple of an image diff, and a percentage difference (0-100).
|tolerance| should be a percentage value (0.0 - 100.0).
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -165,9 +165,12 @@
return self._check_file_exists(image_diff_path, 'image diff exe',
override_step, logging)
- def diff_image(self, expected_contents, actual_contents):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
# FIXME: need unit tests for this.
+ # tolerance is not used in chromium. Make sure caller doesn't pass tolerance other than zero or None.
+ assert (tolerance is None) or tolerance == 0
+
# If only one of them exists, return that one.
if not actual_contents and not expected_contents:
return (None, 0)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -354,7 +354,7 @@
def default_configuration(self):
return 'Release'
- def diff_image(self, expected_contents, actual_contents):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
diffed = actual_contents != expected_contents
if diffed:
return ["< %s\n---\n> %s\n" % (expected_contents, actual_contents), 1]
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -157,7 +157,7 @@
return False
return True
- def diff_image(self, expected_contents, actual_contents):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
# Handle the case where the test didn't actually generate an image.
# FIXME: need unit tests for this.
if not actual_contents and not expected_contents:
@@ -167,17 +167,18 @@
# Maybe we should throw an exception?
return (True, 0)
- process = self._start_image_diff_process(expected_contents, actual_contents)
+ process = self._start_image_diff_process(expected_contents, actual_contents, tolerance=tolerance)
return self._read_image_diff(process)
- def _start_image_diff_process(self, expected_contents, actual_contents):
+ def _start_image_diff_process(self, expected_contents, actual_contents, tolerance=None):
# FIXME: There needs to be a more sane way of handling default
# values for options so that you can distinguish between a default
# value of None and a default value that wasn't set.
- if self.get_option('tolerance') is not None:
- tolerance = self.get_option('tolerance')
- else:
- tolerance = 0.1
+ if tolerance is None:
+ if self.get_option('tolerance') is not None:
+ tolerance = self.get_option('tolerance')
+ else:
+ tolerance = 0.1
command = [self._path_to_image_diff(), '--tolerance', str(tolerance)]
process = server_process.ServerProcess(self, 'ImageDiff', command)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (101738 => 101739)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-12-02 04:12:05 UTC (rev 101738)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-12-02 04:32:26 UTC (rev 101739)
@@ -665,7 +665,7 @@
def test_tolerance(self):
class ImageDiffTestPort(TestPort):
- def diff_image(self, expected_contents, actual_contents):
+ def diff_image(self, expected_contents, actual_contents, tolerance=None):
self.tolerance_used_for_diff_image = self._options.tolerance
return (True, 1)
