[webkit-changes] [126188] trunk/Tools

Tue, 21 Aug 2012 14:35:44 -0700

Title: [126188] trunk/Tools
Revision
126188
Author
[email protected]
Date
2012-08-21 14:36:15 -0700 (Tue, 21 Aug 2012)

Log Message

_compare_image() swaps actual and expected images by mistake
https://bugs.webkit.org/show_bug.cgi?id=94567

Reviewed by Ojan Vafai.

Re-work the code so that we consistently pass (expected, actual)
across all of the compare/diff routines.

* Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
(SingleTestRunner._run_compare_test):
(SingleTestRunner._compare_output):
(SingleTestRunner._compare_text):
(SingleTestRunner._compare_audio):
(SingleTestRunner._compare_image):
(SingleTestRunner._run_reftest):
(SingleTestRunner._compare_output_with_reference):
* Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
(write_test_result):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (126187 => 126188)


--- trunk/Tools/ChangeLog	2012-08-21 21:30:42 UTC (rev 126187)
+++ trunk/Tools/ChangeLog	2012-08-21 21:36:15 UTC (rev 126188)
@@ -1,3 +1,24 @@
+2012-08-21  Dirk Pranke  <[email protected]>
+
+        _compare_image() swaps actual and expected images by mistake
+        https://bugs.webkit.org/show_bug.cgi?id=94567
+
+        Reviewed by Ojan Vafai.
+
+        Re-work the code so that we consistently pass (expected, actual)
+        across all of the compare/diff routines.
+
+        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
+        (SingleTestRunner._run_compare_test):
+        (SingleTestRunner._compare_output):
+        (SingleTestRunner._compare_text):
+        (SingleTestRunner._compare_audio):
+        (SingleTestRunner._compare_image):
+        (SingleTestRunner._run_reftest):
+        (SingleTestRunner._compare_output_with_reference):
+        * Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:
+        (write_test_result):
+
 2012-08-21  Adam Barth  <[email protected]>
 
         Unreviewed. Move the commit-queue to building release only. Previously,

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py (126187 => 126188)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py	2012-08-21 21:30:42 UTC (rev 126187)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py	2012-08-21 21:36:15 UTC (rev 126188)
@@ -110,7 +110,7 @@
             expected_driver_output.strip_metrics()
             driver_output.strip_metrics()
 
-        test_result = self._compare_output(driver_output, expected_driver_output)
+        test_result = self._compare_output(expected_driver_output, driver_output)
         if self._options.new_test_results:
             self._add_missing_baselines(test_result, driver_output)
         test_result_writer.write_test_result(self._filesystem, self._port, self._test_name, driver_output, expected_driver_output, test_result.failures)
@@ -209,7 +209,7 @@
             _log.debug("  %s" % line)
         return failures
 
-    def _compare_output(self, driver_output, expected_driver_output):
+    def _compare_output(self, expected_driver_output, driver_output):
         failures = []
         failures.extend(self._handle_error(driver_output))
 
@@ -218,26 +218,26 @@
             # In case of timeouts, we continue since we still want to see the text and image output.
             return TestResult(self._test_name, failures, driver_output.test_time, driver_output.has_stderr())
 
-        failures.extend(self._compare_text(driver_output.text, expected_driver_output.text))
-        failures.extend(self._compare_audio(driver_output.audio, expected_driver_output.audio))
+        failures.extend(self._compare_text(expected_driver_output.text, driver_output.text))
+        failures.extend(self._compare_audio(expected_driver_output.audio, driver_output.audio))
         if self._should_run_pixel_test:
-            failures.extend(self._compare_image(driver_output, expected_driver_output))
+            failures.extend(self._compare_image(expected_driver_output, driver_output))
         return TestResult(self._test_name, failures, driver_output.test_time, driver_output.has_stderr())
 
-    def _compare_text(self, actual_text, expected_text):
+    def _compare_text(self, expected_text, actual_text):
         failures = []
         if (expected_text and actual_text and
             # Assuming expected_text is already normalized.
-            self._port.do_text_results_differ(self._get_normalized_output_text(actual_text), expected_text)):
+            self._port.do_text_results_differ(expected_text, self._get_normalized_output_text(actual_text))):
             failures.append(test_failures.FailureTextMismatch())
         elif actual_text and not expected_text:
             failures.append(test_failures.FailureMissingResult())
         return failures
 
-    def _compare_audio(self, actual_audio, expected_audio):
+    def _compare_audio(self, expected_audio, actual_audio):
         failures = []
         if (expected_audio and actual_audio and
-            self._port.do_audio_results_differ(actual_audio, expected_audio)):
+            self._port.do_audio_results_differ(expected_audio, actual_audio)):
             failures.append(test_failures.FailureAudioMismatch())
         elif actual_audio and not expected_audio:
             failures.append(test_failures.FailureMissingAudio())
@@ -254,7 +254,7 @@
 
     # FIXME: This function also creates the image diff. Maybe that work should
     # be handled elsewhere?
-    def _compare_image(self, driver_output, expected_driver_output):
+    def _compare_image(self, expected_driver_output, driver_output):
         failures = []
         # If we didn't produce a hash file, this test must be text-only.
         if driver_output.image_hash is None:
@@ -264,7 +264,7 @@
         elif not expected_driver_output.image_hash:
             failures.append(test_failures.FailureMissingImageHash())
         elif driver_output.image_hash != expected_driver_output.image_hash:
-            diff_result = self._port.diff_image(driver_output.image, expected_driver_output.image)
+            diff_result = self._port.diff_image(expected_driver_output.image, driver_output.image)
             err_str = diff_result[2]
             # FIXME: see https://bugs.webkit.org/show_bug.cgi?id=94277 and
             # https://bugs.webkit.org/show_bug.cgi?id=81962; ImageDiff doesn't
@@ -298,7 +298,7 @@
         for expectation, reference_filename in putAllMismatchBeforeMatch(self._reference_files):
             reference_test_name = self._port.relative_test_filename(reference_filename)
             reference_output = self._driver.run_test(DriverInput(reference_test_name, self._timeout, test_output.image_hash, should_run_pixel_test=True), self._stop_when_done)
-            test_result = self._compare_output_with_reference(test_output, reference_output, reference_filename, expectation == '!=')
+            test_result = self._compare_output_with_reference(reference_output, test_output, reference_filename, expectation == '!=')
 
             if (expectation == '!=' and test_result.failures) or (expectation == '==' and not test_result.failures):
                 break
@@ -308,15 +308,15 @@
         test_result_writer.write_test_result(self._filesystem, self._port, self._test_name, test_output, reference_output, test_result.failures)
         return TestResult(self._test_name, test_result.failures, total_test_time + test_result.test_run_time, test_result.has_stderr)
 
-    def _compare_output_with_reference(self, driver_output1, driver_output2, reference_filename, mismatch):
-        total_test_time = driver_output1.test_time + driver_output2.test_time
-        has_stderr = driver_output1.has_stderr() or driver_output2.has_stderr()
+    def _compare_output_with_reference(self, reference_driver_output, actual_driver_output, reference_filename, mismatch):
+        total_test_time = reference_driver_output.test_time + actual_driver_output.test_time
+        has_stderr = reference_driver_output.has_stderr() or actual_driver_output.has_stderr()
         failures = []
-        failures.extend(self._handle_error(driver_output1))
+        failures.extend(self._handle_error(actual_driver_output))
         if failures:
             # Don't continue any more if we already have crash or timeout.
             return TestResult(self._test_name, failures, total_test_time, has_stderr)
-        failures.extend(self._handle_error(driver_output2, reference_filename=reference_filename))
+        failures.extend(self._handle_error(reference_driver_output, reference_filename=reference_filename))
         if failures:
             return TestResult(self._test_name, failures, total_test_time, has_stderr)
 
@@ -324,15 +324,15 @@
             # don't check pixel results for WTR/WK2; they're broken.
             return TestResult(self._test_name, failures, total_test_time, has_stderr)
 
-        if not driver_output1.image_hash and not driver_output2.image_hash:
+        if not reference_driver_output.image_hash and not actual_driver_output.image_hash:
             failures.append(test_failures.FailureReftestNoImagesGenerated(reference_filename))
         elif mismatch:
-            if driver_output1.image_hash == driver_output2.image_hash:
+            if reference_driver_output.image_hash == actual_driver_output.image_hash:
                 failures.append(test_failures.FailureReftestMismatchDidNotOccur(reference_filename))
-        elif driver_output1.image_hash != driver_output2.image_hash:
+        elif reference_driver_output.image_hash != actual_driver_output.image_hash:
             failures.append(test_failures.FailureReftestMismatch(reference_filename))
 
         # recompute in case we added to stderr during diff_image
-        has_stderr = driver_output1.has_stderr() or driver_output2.has_stderr()
+        has_stderr = reference_driver_output.has_stderr() or actual_driver_output.has_stderr()
 
         return TestResult(self._test_name, failures, total_test_time, has_stderr)

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py (126187 => 126188)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py	2012-08-21 21:30:42 UTC (rev 126187)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py	2012-08-21 21:36:15 UTC (rev 126188)
@@ -69,7 +69,7 @@
             # 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:
-                diff_image, diff_percent, err_str = port.diff_image(driver_output.image, expected_driver_output.image, tolerance=0)
+                diff_image, diff_percent, err_str = port.diff_image(expected_driver_output.image, driver_output.image, tolerance=0)
                 if diff_image:
                     writer.write_image_diff_files(diff_image)
                     failure.diff_percent = diff_percent

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (126187 => 126188)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py	2012-08-21 21:30:42 UTC (rev 126187)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py	2012-08-21 21:36:15 UTC (rev 126188)
@@ -216,6 +216,7 @@
         native_diff_filename = self._convert_path(diff_filename)
 
         executable = self._path_to_image_diff()
+        # Note that although we are handed 'old', 'new', image_diff wants 'new', 'old'.
         comand = [executable, '--diff', native_actual_filename, native_expected_filename, native_diff_filename]
 
         result = None

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py (126187 => 126188)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py	2012-08-21 21:30:42 UTC (rev 126187)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/image_diff.py	2012-08-21 21:36:15 UTC (rev 126188)
@@ -57,6 +57,7 @@
 
             if not self._process:
                 self._start(tolerance)
+            # Note that although we are handed 'old', 'new', ImageDiff wants 'new', 'old'.
             self._process.write('Content-Length: %d\n%sContent-Length: %d\n%s' % (
                 len(actual_contents), actual_contents,
                 len(expected_contents), expected_contents))

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to