2010-11-17 Hayato Ito <hayato@chromium.org>
authorhayato@chromium.org <hayato@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Nov 2010 23:02:39 +0000 (23:02 +0000)
committerhayato@chromium.org <hayato@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Nov 2010 23:02:39 +0000 (23:02 +0000)
        Refactor TestTypeBase.compare_output().

        Introduce a TestOutput class and update compare_output() of each test
        types so that they can take both actual and expected TestOutput objects.

        https://bugs.webkit.org/show_bug.cgi?id=49431

        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_output.py: Added.
        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        * Scripts/webkitpy/layout_tests/port/dryrun.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/port/webkit.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/test_types/image_diff.py:
        * Scripts/webkitpy/layout_tests/test_types/test_type_base.py:
        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:

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

12 files changed:
WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/test_types/image_diff.py
WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base.py
WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py

index e244f79..cdac1da 100644 (file)
@@ -1,3 +1,24 @@
+2010-11-17  Hayato Ito  <hayato@chromium.org>
+
+        Refactor TestTypeBase.compare_output().
+
+        Introduce a TestOutput class and update compare_output() of each test
+        types so that they can take both actual and expected TestOutput objects.
+
+        https://bugs.webkit.org/show_bug.cgi?id=49431
+
+        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_output.py: Added.
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/dryrun.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/test_types/image_diff.py:
+        * Scripts/webkitpy/layout_tests/test_types/test_type_base.py:
+        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:
+
 2010-11-17  Adam Roben  <aroben@apple.com>
 
         Make each Windows Test builder use the same OS for all its slaves
index 703f9de..8142867 100644 (file)
@@ -51,6 +51,7 @@ import time
 import traceback
 
 import test_failures
+import test_output
 import test_results
 
 _log = logging.getLogger("webkitpy.layout_tests.layout_package."
@@ -74,9 +75,14 @@ def log_stack(stack):
             _log.error('  %s' % line.strip())
 
 
+def _expected_test_output(port, filename):
+    """Returns an expected TestOutput object."""
+    return test_output.TestOutput(port.expected_text(filename),
+                                  port.expected_image(filename),
+                                  port.expected_checksum(filename))
+
 def _process_output(port, options, test_input, test_types, test_args,
-                    crash, timeout, test_run_time, actual_checksum,
-                    output, error):
+                    test_output):
     """Receives the output from a DumpRenderTree process, subjects it to a
     number of tests, and returns a list of failure types the test produced.
 
@@ -87,24 +93,20 @@ def _process_output(port, options, test_input, test_types, test_args,
       test_input: Object containing the test filename, uri and timeout
       test_types: list of test types to subject the output to
       test_args: arguments to be passed to each test
+      test_output: a TestOutput object containing the output of the test
 
     Returns: a TestResult object
     """
     failures = []
 
-    # Some test args, such as the image hash, may be added or changed on a
-    # test-by-test basis.
-    local_test_args = copy.copy(test_args)
-
-    local_test_args.hash = actual_checksum
-
-    if crash:
+    if test_output.crash:
         failures.append(test_failures.FailureCrash())
-    if timeout:
+    if test_output.timeout:
         failures.append(test_failures.FailureTimeout())
 
-    if crash:
-        _log.debug("Stacktrace for %s:\n%s" % (test_input.filename, error))
+    if test_output.crash:
+        _log.debug("Stacktrace for %s:\n%s" % (test_input.filename,
+                                               test_output.error))
         # Strip off "file://" since RelativeTestFilename expects
         # filesystem paths.
         filename = os.path.join(options.results_directory,
@@ -113,9 +115,11 @@ def _process_output(port, options, test_input, test_types, test_args,
         filename = os.path.splitext(filename)[0] + "-stack.txt"
         port.maybe_make_directory(os.path.split(filename)[0])
         with codecs.open(filename, "wb", "utf-8") as file:
-            file.write(error)
-    elif error:
-        _log.debug("Previous test output stderr lines:\n%s" % error)
+            file.write(test_output.error)
+    elif test_output.error:
+        _log.debug("Previous test output stderr lines:\n%s" % test_output.error)
+
+    expected_test_output = _expected_test_output(port, test_input.filename)
 
     # Check the output and save the results.
     start_time = time.time()
@@ -123,18 +127,18 @@ def _process_output(port, options, test_input, test_types, test_args,
     for test_type in test_types:
         start_diff_time = time.time()
         new_failures = test_type.compare_output(port, test_input.filename,
-                                                output, local_test_args,
-                                                options.configuration)
+                                                test_args, test_output,
+                                                expected_test_output)
         # Don't add any more failures if we already have a crash, so we don't
         # double-report those tests. We do double-report for timeouts since
         # we still want to see the text and image output.
-        if not crash:
+        if not test_output.crash:
             failures.extend(new_failures)
         time_for_diffs[test_type.__class__.__name__] = (
             time.time() - start_diff_time)
 
     total_time_for_all_diffs = time.time() - start_diff_time
-    return test_results.TestResult(test_input.filename, failures, test_run_time,
+    return test_results.TestResult(test_input.filename, failures, test_output.test_time,
                                    total_time_for_all_diffs, time_for_diffs)
 
 
@@ -157,6 +161,23 @@ def _should_fetch_expected_checksum(options):
     return options.pixel_tests and not (options.new_baseline or options.reset_results)
 
 
+def _run_single_test(port, options, test_input, test_types, test_args, driver):
+    # FIXME: Pull this into TestShellThread._run().
+
+    # The image hash is used to avoid doing an image dump if the
+    # checksums match, so it should be set to a blank value if we
+    # are generating a new baseline.  (Otherwise, an image from a
+    # previous run will be copied into the baseline."""
+    if _should_fetch_expected_checksum(options):
+        image_hash_to_driver = port.expected_checksum(test_input.filename)
+    else:
+        image_hash_to_driver = None
+    test_input.uri = port.filename_to_uri(test_input.filename).strip()
+    test_output = driver.run_test(test_input.uri, test_input.timeout, image_hash_to_driver)
+    return _process_output(port, options, test_input, test_types, test_args,
+                           test_output)
+
+
 class SingleTestThread(threading.Thread):
     """Thread wrapper for running a single test file."""
 
@@ -185,25 +206,12 @@ class SingleTestThread(threading.Thread):
     def _covered_run(self):
         # FIXME: this is a separate routine to work around a bug
         # in coverage: see http://bitbucket.org/ned/coveragepy/issue/85.
-
-        # FIXME: Pull this into TestShellThread._run().
-        test_input = self._test_input
-        test_input.uri = self._port.filename_to_uri(test_input.filename)
-        if _should_fetch_expected_checksum(self._options):
-            test_input.image_checksum = self._port.expected_checksum(test_input.filename)
-
-        start = time.time()
         self._driver = self._port.create_driver(self._test_args.png_path,
                                                 self._options)
         self._driver.start()
-        crash, timeout, actual_checksum, output, error = \
-            self._driver.run_test(test_input.uri, test_input.timeout,
-                                  test_input.image_checksum)
-        end = time.time()
-        self._test_result = _process_output(self._port, self._options,
-            test_input, self._test_types, self._test_args,
-            crash, timeout, end - start,
-            actual_checksum, output, error)
+        self._test_result = _run_single_test(self._port, self._options,
+                                             self._test_input, self._test_types,
+                                             self._test_args, self._driver)
         self._driver.stop()
 
     def get_test_result(self):
@@ -501,32 +509,14 @@ class TestShellThread(WatchableThread):
         Returns: a TestResult object.
         """
         self._ensure_dump_render_tree_is_running()
-        # The pixel_hash is used to avoid doing an image dump if the
-        # checksums match, so it should be set to a blank value if we
-        # are generating a new baseline.  (Otherwise, an image from a
-        # previous run will be copied into the baseline.)
-
-        # FIXME: Pull this into TestShellThread._run().
-        test_input.uri = self._port.filename_to_uri(test_input.filename)
-        if _should_fetch_expected_checksum(self._options):
-            test_input.image_checksum = self._port.expected_checksum(test_input.filename)
-        start = time.time()
-
         thread_timeout = _milliseconds_to_seconds(
              _pad_timeout(int(test_input.timeout)))
-        self._next_timeout = start + thread_timeout
-
-        crash, timeout, actual_checksum, output, error = \
-           self._driver.run_test(test_input.uri, test_input.timeout, test_input.image_checksum)
-        end = time.time()
-
-        result = _process_output(self._port, self._options,
-                                 test_input, self._test_types,
-                                 self._test_args, crash,
-                                 timeout, end - start, actual_checksum,
-                                 output, error)
-        self._test_results.append(result)
-        return result
+        self._next_timeout = time.time() + thread_timeout
+        test_result = _run_single_test(self._port, self._options, test_input,
+                                       self._test_types, self._test_args,
+                                       self._driver)
+        self._test_results.append(test_result)
+        return test_result
 
     def _ensure_dump_render_tree_is_running(self):
         """Start the shared DumpRenderTree, if it's not running.
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_output.py
new file mode 100644 (file)
index 0000000..e809be6
--- /dev/null
@@ -0,0 +1,56 @@
+# Copyright (C) 2010 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.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# 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.
+
+
+class TestOutput(object):
+    """Groups information about a test output for easy passing of data.
+
+    This is used not only for a actual test output, but also for grouping
+    expected test output.
+    """
+
+    def __init__(self, text, image, image_hash,
+                 crash=None, test_time=None, timeout=None, error=None):
+        """Initializes a TestOutput object.
+
+        Args:
+          text: a text output
+          image: an image output
+          image_hash: a string containing the checksum of the image
+          crash: a boolean indicating whether the driver crashed on the test
+          test_time: a time which the test has taken
+          timeout: a boolean indicating whehter the test timed out
+          error: any unexpected or additional (or error) text output
+        """
+        self.text = text
+        self.image = image
+        self.image_hash = image_hash
+        self.crash = crash
+        self.test_time = test_time
+        self.timeout = timeout
+        self.error = error
index 4a2970e..632806f 100644 (file)
@@ -832,16 +832,8 @@ class Driver:
         checksum - if present, the expected checksum for the image for this
             test
 
-        Returns a tuple of the following:
-            crash - a boolean indicating whether the driver crashed on the test
-            timeout - a boolean indicating whehter the test timed out
-            checksum - a string containing the checksum of the image, if
-                present
-            output - any text output
-            error - any unexpected or additional (or error) text output
-
-        Note that the image itself should be written to the path that was
-        specified in the __init__() call."""
+        Returns a TestOutput object.
+        """
         raise NotImplementedError('Driver.run_test')
 
     # FIXME: This is static so we can test it w/o creating a Base instance.
index f93f9a8..3149290 100644 (file)
@@ -46,13 +46,11 @@ import webbrowser
 from webkitpy.common.system.executive import Executive
 from webkitpy.common.system.path import cygpath
 from webkitpy.layout_tests.layout_package import test_expectations
+from webkitpy.layout_tests.layout_package import test_output
 
 import base
 import http_server
 
-from webkitpy.common.system.executive import Executive
-from webkitpy.layout_tests.layout_package import test_expectations
-
 # Chromium DRT on OSX uses WebKitDriver.
 if sys.platform == 'darwin':
     import webkit
@@ -447,6 +445,15 @@ class ChromiumDriver(base.Driver):
         cmd += "\n"
         return cmd
 
+    def _output_image(self):
+        """Returns the image output which driver generated."""
+        png_path = self._image_path
+        if png_path and os.path.isfile(png_path):
+            with open(png_path, 'rb') as image_file:
+                return image_file.read()
+        else:
+            return None
+
     def run_test(self, uri, timeoutms, checksum):
         output = []
         error = []
@@ -498,8 +505,9 @@ class ChromiumDriver(base.Driver):
 
             (line, crash) = self._write_command_and_read_line(input=None)
 
-        return (crash, timeout, actual_checksum, ''.join(output),
-                ''.join(error))
+        return test_output.TestOutput(
+            ''.join(output), self._output_image(), actual_checksum,
+            crash, time.time() - start_time, timeout, ''.join(error))
 
     def stop(self):
         if self._proc:
index 8a6af56..96d0d55 100644 (file)
@@ -48,6 +48,9 @@ from __future__ import with_statement
 
 import os
 import sys
+import time
+
+from webkitpy.layout_tests.layout_package import test_output
 
 import base
 import factory
@@ -109,19 +112,19 @@ class DryrunDriver(base.Driver):
         return None
 
     def run_test(self, uri, timeoutms, image_hash):
+        start_time = time.time()
         test_name = self._port.uri_to_test_name(uri)
         path = os.path.join(self._port.layout_tests_dir(), test_name)
         text_output = self._port.expected_text(path)
 
         if image_hash is not None:
             image = self._port.expected_image(path)
-            if image and self._image_path:
-                with open(self._image_path, 'w') as f:
-                    f.write(image)
             hash = self._port.expected_checksum(path)
         else:
+            image = None
             hash = None
-        return (False, False, hash, text_output, None)
+        return test_output.TestOutput(text_output, image, hash, False,
+                                      time.time() - start_time, False, None)
 
     def start(self):
         pass
index ff4086c..0a27821 100644 (file)
@@ -36,6 +36,8 @@ import os
 import sys
 import time
 
+from webkitpy.layout_tests.layout_package import test_output
+
 import base
 
 
@@ -289,6 +291,7 @@ class TestDriver(base.Driver):
         return True
 
     def run_test(self, uri, timeoutms, image_hash):
+        start_time = time.time()
         test_name = self._port.uri_to_test_name(uri)
         test = self._port._tests[test_name]
         if test.keyboard:
@@ -297,13 +300,10 @@ class TestDriver(base.Driver):
             raise ValueError('exception from ' + test_name)
         if test.hang:
             time.sleep((float(timeoutms) * 4) / 1000.0)
-
-        if self._port.get_option('pixel_tests') and test.actual_image:
-            with open(self._image_path, 'w') as file:
-                file.write(test.actual_image)
-
-        return (test.crash, test.timeout, test.actual_checksum,
-                test.actual_text, test.error)
+        return test_output.TestOutput(test.actual_text, test.actual_image,
+                                      test.actual_checksum, test.crash,
+                                      time.time() - start_time, test.timeout,
+                                      test.error)
 
     def start(self):
         pass
index 51f57c4..06797c6 100644 (file)
@@ -49,6 +49,7 @@ import shutil
 from webkitpy.common.system.executive import Executive
 
 import webkitpy.common.system.ospath as ospath
+import webkitpy.layout_tests.layout_package.test_output as test_output
 import webkitpy.layout_tests.port.base as base
 import webkitpy.layout_tests.port.server_process as server_process
 
@@ -442,6 +443,7 @@ class WebKitDriver(base.Driver):
             command += "'" + image_hash
         command += "\n"
 
+        start_time = time.time()
         self._server_process.write(command)
 
         have_seen_content_type = False
@@ -486,10 +488,6 @@ class WebKitDriver(base.Driver):
             timeout = deadline - time.time()
             line = self._server_process.read_line(timeout)
 
-        if self._image_path and len(self._image_path):
-            with open(self._image_path, "wb") as image_file:
-                image_file.write(image)
-
         error_lines = self._server_process.error.splitlines()
         # FIXME: This is a hack.  It is unclear why sometimes
         # we do not get any error lines from the server_process
@@ -500,11 +498,11 @@ class WebKitDriver(base.Driver):
         # FIXME: This seems like the wrong section of code to be doing
         # this reset in.
         self._server_process.error = ""
-        return (self._server_process.crashed,
-                self._server_process.timed_out,
-                actual_image_hash,
-                output,
-                error)
+        return test_output.TestOutput(output, image, actual_image_hash,
+                                      self._server_process.crashed,
+                                      time.time() - start_time,
+                                      self._server_process.timed_out,
+                                      error)
 
     def stop(self):
         if self._server_process:
index 680ba5e..b37fa1e 100755 (executable)
@@ -101,9 +101,6 @@ class TestInput:
           """
         # FIXME: filename should really be test_name as a relative path.
         self.filename = filename
-        # The image checksum is passed to the driver so that the driver can optionally not have
-        # to output the image if the checksums match.
-        self.image_checksum = None
         self.timeout = timeout
 
         # FIXME: Maybe the URI shouldn't be part of the TestInput at all?
index 0b05802..41fe9bd 100644 (file)
@@ -54,116 +54,93 @@ _log = logging.getLogger("webkitpy.layout_tests.test_types.image_diff")
 
 class ImageDiff(test_type_base.TestTypeBase):
 
-    def _copy_output_png(self, test_filename, source_image, extension):
-        """Copies result files into the output directory with appropriate
-        names.
-
-        Args:
-          test_filename: the test filename
-          source_file: path to the image file (either actual or expected)
-          extension: extension to indicate -actual.png or -expected.png
-        """
-        self._make_output_directory(test_filename)
-        dest_image = self.output_filename(test_filename, extension)
-
-        if os.path.exists(source_image):
-            shutil.copyfile(source_image, dest_image)
-
-    def _save_baseline_files(self, filename, png_path, checksum,
+    def _save_baseline_files(self, filename, image, image_hash,
                              generate_new_baseline):
         """Saves new baselines for the PNG and checksum.
 
         Args:
           filename: test filename
-          png_path: path to the actual PNG result file
-          checksum: value of the actual checksum result
+          image: a image output
+          image_hash: a checksum of the image
           generate_new_baseline: whether to generate a new, platform-specific
             baseline, or update the existing one
         """
-        with open(png_path, "rb") as png_file:
-            png_data = png_file.read()
-        self._save_baseline_data(filename, png_data, ".png", encoding=None,
+        self._save_baseline_data(filename, image, ".png", encoding=None,
                                  generate_new_baseline=generate_new_baseline)
-        self._save_baseline_data(filename, checksum, ".checksum",
+        self._save_baseline_data(filename, image_hash, ".checksum",
                                  encoding="ascii",
                                  generate_new_baseline=generate_new_baseline)
 
-    def _create_image_diff(self, port, filename, configuration):
+    def _copy_image(self, filename, actual_image, expected_image):
+        self.write_output_files(filename, '.png',
+                                output=actual_image, expected=expected_image,
+                                encoding=None, print_text_diffs=False)
+
+    def _copy_image_hash(self, filename, actual_image_hash, expected_image_hash):
+        self.write_output_files(filename, '.checksum',
+                                actual_image_hash, expected_image_hash,
+                                encoding="ascii", print_text_diffs=False)
+
+    def _create_diff_image(self, port, filename, actual_image, expected_image):
         """Creates the visual diff of the expected/actual PNGs.
 
-        Args:
-          filename: the name of the test
-          configuration: Debug or Release
-        Returns True if the files are different, False if they match
+        Returns True if the images are different.
         """
         diff_filename = self.output_filename(filename,
-          self.FILENAME_SUFFIX_COMPARE)
-        actual_filename = self.output_filename(filename,
-          self.FILENAME_SUFFIX_ACTUAL + '.png')
-        expected_filename = self.output_filename(filename,
-          self.FILENAME_SUFFIX_EXPECTED + '.png')
+                                             self.FILENAME_SUFFIX_COMPARE)
+        return port.diff_image(actual_image, expected_image, diff_filename)
 
-        expected_image = port.expected_image(filename)
-        with codecs.open(actual_filename, 'r+b', None) as file:
-            actual_image = file.read()
-
-        result = port.diff_image(expected_image, actual_image,
-                                 diff_filename)
-        return result
-
-    def compare_output(self, port, filename, output, test_args, configuration):
+    def compare_output(self, port, filename, test_args, actual_test_output,
+                       expected_test_output):
         """Implementation of CompareOutput that checks the output image and
         checksum against the expected files from the LayoutTest directory.
         """
         failures = []
 
         # If we didn't produce a hash file, this test must be text-only.
-        if test_args.hash is None:
+        if actual_test_output.image_hash is None:
             return failures
 
         # If we're generating a new baseline, we pass.
         if test_args.new_baseline or test_args.reset_results:
-            self._save_baseline_files(filename, test_args.png_path,
-                                      test_args.hash, test_args.new_baseline)
+            self._save_baseline_files(filename, actual_test_output.image_hash,
+                                      actual_test_output.image,
+                                      test_args.new_baseline)
             return failures
 
-        # Compare hashes.
-        expected_hash = self._port.expected_checksum(filename)
-        expected_png = self._port.expected_image(filename)
-
-        if not expected_png:
+        if not expected_test_output.image:
             # Report a missing expected PNG file.
-            self.write_output_files(filename, '.checksum',
-                                    test_args.hash, expected_hash,
-                                    encoding="ascii",
-                                    print_text_diffs=False)
-            self._copy_output_png(filename, test_args.png_path, '-actual.png')
+            self._copy_image(filename, actual_test_output.image, expected_image=None)
+            self._copy_image_hash(filename, actual_test_output.image_hash,
+                                  expected_test_output.image_hash)
             failures.append(test_failures.FailureMissingImage())
             return failures
-        elif test_args.hash == expected_hash:
+        if not expected_test_output.image_hash:
+            # Report a missing expected checksum file.
+            self._copy_image(filename, actual_test_output.image,
+                             expected_test_output.image)
+            self._copy_image_hash(filename, actual_test_output.image_hash,
+                                  expected_image_hash=None)
+            failures.append(test_failures.FailureMissingImageHash())
+            return failures
+
+        if actual_test_output.image_hash == expected_test_output.image_hash:
             # Hash matched (no diff needed, okay to return).
             return failures
 
-        self.write_output_files(filename, '.checksum',
-                                test_args.hash, expected_hash,
-                                encoding="ascii",
-                                print_text_diffs=False)
-
-        # FIXME: combine next two lines
-        self._copy_output_png(filename, test_args.png_path, '-actual.png')
-        self.write_output_files(filename, '.png', output=None,
-                                expected=expected_png,
-                                encoding=None, print_text_diffs=False)
+        self._copy_image(filename, actual_test_output.image,
+                         expected_test_output.image)
+        self._copy_image_hash(filename, actual_test_output.image_hash,
+                              expected_test_output.image_hash)
 
         # Even though we only use the result in one codepath below but we
         # still need to call CreateImageDiff for other codepaths.
-        images_are_different = self._create_image_diff(port, filename, configuration)
-        if not expected_hash:
-            failures.append(test_failures.FailureMissingImageHash())
-        elif test_args.hash != expected_hash:
-            if images_are_different:
-                failures.append(test_failures.FailureImageHashMismatch())
-            else:
-                failures.append(test_failures.FailureImageHashIncorrect())
+        images_are_different = self._create_diff_image(port, filename,
+                                                       actual_test_output.image,
+                                                       expected_test_output.image)
+        if not images_are_different:
+            failures.append(test_failures.FailureImageHashIncorrect())
+        else:
+            failures.append(test_failures.FailureImageHashMismatch())
 
         return failures
index dcc64a3..4b96b3a 100644 (file)
@@ -140,18 +140,22 @@ class TestTypeBase(object):
             self._port.relative_test_filename(filename))
         return os.path.splitext(output_filename)[0] + modifier
 
-    def compare_output(self, port, filename, output, test_args, configuration):
+    def compare_output(self, port, filename, test_args, actual_test_output,
+                        expected_test_output):
         """Method that compares the output from the test with the
         expected value.
 
         This is an abstract method to be implemented by all sub classes.
 
         Args:
+          port: object implementing port-specific information and methods
           filename: absolute filename to test file
-          output: a string containing the output of the test
           test_args: a TestArguments object holding optional additional
               arguments
-          configuration: Debug or Release
+          actual_test_output: a TestOutput object which represents actual test
+              output
+          expected_test_output: a TestOutput object which represents a expected
+              test output
 
         Return:
           a list of TestFailure objects, empty if the test passes
index 4c32f0d..66e42ba 100644 (file)
@@ -55,13 +55,8 @@ class TestTextDiff(test_type_base.TestTypeBase):
              "\r\n", "\n")
         return norm + "\n"
 
-    def _get_normalized_expected_text(self, filename):
-        """Given the filename of the test, read the expected output from a file
-        and normalize the text.  Returns a string with the expected text, or ''
-        if the expected output file was not found."""
-        return self._port.expected_text(filename)
-
-    def compare_output(self, port, filename, output, test_args, configuration):
+    def compare_output(self, port, filename, test_args, actual_test_output,
+                        expected_test_output):
         """Implementation of CompareOutput that checks the output text against
         the expected text from the LayoutTest directory."""
         failures = []
@@ -76,17 +71,18 @@ class TestTextDiff(test_type_base.TestTypeBase):
             return failures
 
         # Normalize text to diff
-        output = self._get_normalized_output_text(output)
-        expected = self._get_normalized_expected_text(filename)
+        actual_text = self._get_normalized_output_text(actual_test_output.text)
+        # Assuming expected_text is already normalized.
+        expected_text = expected_test_output.text
 
         # Write output files for new tests, too.
-        if port.compare_text(output, expected):
+        if port.compare_text(actual_text, expected_text):
             # Text doesn't match, write output files.
-            self.write_output_files(filename, ".txt", output,
-                                    expected, encoding=None,
+            self.write_output_files(filename, ".txt", actual_text,
+                                    expected_text, encoding=None,
                                     print_text_diffs=True)
 
-            if expected == '':
+            if expected_text == '':
                 failures.append(test_failures.FailureMissingResult())
             else:
                 failures.append(test_failures.FailureTextMismatch())