2010-08-24 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Aug 2010 00:10:30 +0000 (00:10 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Aug 2010 00:10:30 +0000 (00:10 +0000)
        Reviewed by Ojan Vafai.

        new-run-webkit-tests: clean up code for test_types, test_failures

        Add a bunch of unit tests for webkitpy.layout_tests.test_types and
        webkitpy.layout_tests.layout_package.test_failures, and remove
        some dead code and otherwise clean up things.

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

        * Scripts/webkitpy/layout_tests/layout_package/test_failures.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py: Added.
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_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/test_type_base_unittest.py: Added.
        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.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/test_type_base_unittest.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py

index ccdff3ef3de8d6dec2833d8ba538c28e68abc2b7..2f07142dd795b5eadd7580e9d121f1a2e1f9376c 100644 (file)
@@ -1,3 +1,23 @@
+2010-08-24  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Ojan Vafai.
+
+        new-run-webkit-tests: clean up code for test_types, test_failures
+
+        Add a bunch of unit tests for webkitpy.layout_tests.test_types and
+        webkitpy.layout_tests.layout_package.test_failures, and remove
+        some dead code and otherwise clean up things.
+
+        https://bugs.webkit.org/show_bug.cgi?id=44559
+
+        * Scripts/webkitpy/layout_tests/layout_package/test_failures.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py: Added.
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_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/test_type_base_unittest.py: Added.
+        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:
+
 2010-08-24  Dirk Pranke  <dpranke@chromium.org>
 
         Reviewed by Eric Seidel.
index 3be924083f04309947e5129129fd2b96b2482b02..340d0753bb10f3a795ec8b8873abf2b81b2f3f35 100644 (file)
@@ -73,11 +73,11 @@ class TestFailure(object):
     @staticmethod
     def message():
         """Returns a string describing the failure in more detail."""
-        raise NotImplemented
+        raise NotImplementedError
 
     def result_html_output(self, filename):
         """Returns an HTML string to be included on the results.html page."""
-        raise NotImplemented
+        raise NotImplementedError
 
     def should_kill_dump_render_tree(self):
         """Returns True if we should kill DumpRenderTree before the next
@@ -108,10 +108,8 @@ class FailureWithType(TestFailure):
     use the standard OutputLinks.
     """
 
-    def __init__(self, test_type):
+    def __init__(self):
         TestFailure.__init__(self)
-        # FIXME: This class no longer needs to know the test_type.
-        self._test_type = test_type
 
     # Filename suffixes used by ResultHtmlOutput.
     OUT_FILENAMES = []
@@ -202,8 +200,8 @@ class FailureTextMismatch(FailureWithType):
     OUT_FILENAMES_WDIFF = ["-actual.txt", "-expected.txt", "-diff.txt",
                            "-wdiff.html", "-pretty-diff.html"]
 
-    def __init__(self, test_type, has_wdiff):
-        FailureWithType.__init__(self, test_type)
+    def __init__(self, has_wdiff):
+        FailureWithType.__init__(self)
         if has_wdiff:
             self.OUT_FILENAMES = self.OUT_FILENAMES_WDIFF
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py
new file mode 100644 (file)
index 0000000..65c4a0b
--- /dev/null
@@ -0,0 +1,69 @@
+# 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.
+
+""""Tests code paths not covered by the regular unit tests."""
+
+from webkitpy.layout_tests.layout_package.test_failures import *
+import unittest
+
+
+class Test(unittest.TestCase):
+    def assertResultHtml(self, failure_obj):
+        self.assertNotEqual(failure_obj.result_html_output('foo'), None)
+
+    def test_crash(self):
+        self.assertResultHtml(FailureCrash())
+
+    def test_hash_incorrect(self):
+        self.assertResultHtml(FailureImageHashIncorrect())
+
+    def test_missing(self):
+        self.assertResultHtml(FailureMissingResult())
+
+    def test_missing_image(self):
+        self.assertResultHtml(FailureMissingImage())
+
+    def test_missing_image_hash(self):
+        self.assertResultHtml(FailureMissingImageHash())
+
+    def test_timeout(self):
+        self.assertResultHtml(FailureTimeout())
+
+    def test_unknown_failure_type(self):
+        class UnknownFailure(TestFailure):
+            pass
+
+        failure_obj = UnknownFailure()
+        self.assertRaises(ValueError, determine_result_type, [failure_obj])
+        self.assertRaises(NotImplementedError, failure_obj.message)
+        self.assertRaises(NotImplementedError, failure_obj.result_html_output,
+                          "foo.txt")
+
+
+if __name__ == '__main__':
+    unittest.main()
index 92f103258246268e54cfa0251777e6db4f04039e..3a9f923a05629752a49b83a25b98113165057b87 100644 (file)
@@ -63,8 +63,6 @@ import webkitpy.common.checkout.scm as scm
 
 import port
 from layout_package import test_expectations
-from test_types import image_diff
-from test_types import text_diff
 
 _log = logging.getLogger("webkitpy.layout_tests."
                          "rebaseline_chromium_webkit_tests")
@@ -549,11 +547,13 @@ class Rebaseliner(object):
             return True
 
         if ext1 == '.PNG':
-            return image_diff.ImageDiff(self._port,
-               '').diff_files(self._port, file1, file2)
+            return self._port.diff_image(file1, file2)
         else:
-            return text_diff.TestTextDiff(self._port,
-                '').diff_files(self._port, file1, file2)
+            with codecs.open(file1, "r", "utf8") as file_handle1:
+                output1 = file_handle1.read()
+            with codecs.open(file2, "r", "utf8") as file_handle2:
+                output2 = file_handle2.read()
+            return self._port.compare_text(output1, output2)
 
     def _delete_baseline(self, filename):
         """Remove the file from repository and delete it from disk.
index 121b64eade35c7b7f6843f2c224982d1cec72314..dbb2b91caf10ea8ca6e51a0c6b303e7dc7505472 100644 (file)
@@ -29,6 +29,7 @@
 
 """Unit tests for rebaseline_chromium_webkit_tests.py."""
 
+import os
 import unittest
 
 from webkitpy.layout_tests import port
@@ -85,18 +86,32 @@ class TestGetHostPortObject(unittest.TestCase):
 
 
 class TestRebaseliner(unittest.TestCase):
-
-    def test_noop(self):
-        # this method tests that was can at least instantiate an object, even
-        # if there is nothing to do.
+    def make_rebaseliner(self):
         options = MockOptions()
         host_port_obj = port.get('test', options)
         target_options = options
         target_port_obj = port.get('test', target_options)
         platform = 'test'
-        rebaseliner = rebaseline_chromium_webkit_tests.Rebaseliner(
+        return rebaseline_chromium_webkit_tests.Rebaseliner(
             host_port_obj, target_port_obj, platform, options)
+
+    def test_noop(self):
+        # this method tests that was can at least instantiate an object, even
+        # if there is nothing to do.
+        rebaseliner = self.make_rebaseliner()
         self.assertNotEqual(rebaseliner, None)
 
+    def test_diff_baselines_txt(self):
+        rebaseliner = self.make_rebaseliner()
+        path = os.path.join(rebaseliner._port.layout_tests_dir(),
+                             "passes", "text-expected.txt")
+        self.assertFalse(rebaseliner._diff_baselines(path, path))
+
+    def test_diff_baselines_png(self):
+        rebaseliner = self.make_rebaseliner()
+        path = os.path.join(rebaseliner._port.layout_tests_dir(),
+                            "passes", "image-expected.png")
+        self.assertFalse(rebaseliner._diff_baselines(path, path))
+
 if __name__ == '__main__':
     unittest.main()
index c9f4107998b1dd03f865036a6f00b77a00342c65..879646cfffa919a9035ad85f603f59e063a3fb58 100644 (file)
@@ -66,12 +66,8 @@ class ImageDiff(test_type_base.TestTypeBase):
         self._make_output_directory(test_filename)
         dest_image = self.output_filename(test_filename, extension)
 
-        try:
+        if os.path.exists(source_image):
             shutil.copyfile(source_image, dest_image)
-        except IOError, e:
-            # A missing expected PNG has already been recorded as an error.
-            if errno.ENOENT != e.errno:
-                raise
 
     def _save_baseline_files(self, filename, png_path, checksum,
                              generate_new_baseline):
@@ -107,20 +103,8 @@ class ImageDiff(test_type_base.TestTypeBase):
         expected_filename = self.output_filename(filename,
           self.FILENAME_SUFFIX_EXPECTED + '.png')
 
-        result = True
-        try:
-            _compare_available = True
-            result = port.diff_image(expected_filename, actual_filename,
-                                     diff_filename)
-        except ValueError:
-            _compare_available = False
-
-        global _compare_msg_printed
-        if not _compare_available and not _compare_msg_printed:
-            _compare_msg_printed = True
-            print('image_diff not found. Make sure you have a ' +
-                  configuration + ' build of the image_diff executable.')
-
+        result = port.diff_image(expected_filename, actual_filename,
+                                 diff_filename)
         return result
 
     def compare_output(self, port, filename, output, test_args, configuration):
@@ -145,14 +129,10 @@ class ImageDiff(test_type_base.TestTypeBase):
         expected_png_file = self._port.expected_filename(filename, '.png')
 
         # FIXME: We repeat this pattern often, we should share code.
-        try:
+        expected_hash = ''
+        if os.path.exists(expected_hash_file):
             with codecs.open(expected_hash_file, "r", "ascii") as file:
                 expected_hash = file.read()
-        except IOError, e:
-            if errno.ENOENT != e.errno:
-                raise
-            expected_hash = ''
-
 
         if not os.path.isfile(expected_png_file):
             # Report a missing expected PNG file.
@@ -161,7 +141,7 @@ class ImageDiff(test_type_base.TestTypeBase):
                                     encoding="ascii",
                                     print_text_diffs=False)
             self._copy_output_png(filename, test_args.png_path, '-actual.png')
-            failures.append(test_failures.FailureMissingImage(self))
+            failures.append(test_failures.FailureMissingImage())
             return failures
         elif test_args.hash == expected_hash:
             # Hash matched (no diff needed, okay to return).
@@ -178,26 +158,11 @@ class ImageDiff(test_type_base.TestTypeBase):
         # still need to call CreateImageDiff for other codepaths.
         images_are_different = self._create_image_diff(port, filename, configuration)
         if expected_hash == '':
-            failures.append(test_failures.FailureMissingImageHash(self))
+            failures.append(test_failures.FailureMissingImageHash())
         elif test_args.hash != expected_hash:
             if images_are_different:
-                failures.append(test_failures.FailureImageHashMismatch(self))
+                failures.append(test_failures.FailureImageHashMismatch())
             else:
-                failures.append(test_failures.FailureImageHashIncorrect(self))
+                failures.append(test_failures.FailureImageHashIncorrect())
 
         return failures
-
-    def diff_files(self, port, file1, file2):
-        """Diff two image files exactly.
-
-        Args:
-          file1, file2: full paths of the files to compare.
-
-        Returns:
-          True if two files are different.
-          False otherwise.
-        """
-        try:
-            return port.diff_image(file1, file2, None, 0)
-        except ValueError, e:
-            return True
index dd44642b2512246aec3853e601fc011182b2f8b8..753dbeec46d21a849e5356f4dba9c980961a8e75 100644 (file)
@@ -156,7 +156,7 @@ class TestTypeBase(object):
         Return:
           a list of TestFailure objects, empty if the test passes
         """
-        raise NotImplemented
+        raise NotImplementedError
 
     def _write_into_file_at_path(self, file_path, contents, encoding):
         """This method assumes that byte_array is already encoded
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/test_types/test_type_base_unittest.py
new file mode 100644 (file)
index 0000000..5dbfcb6
--- /dev/null
@@ -0,0 +1,47 @@
+# 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.
+
+""""Tests stray tests not covered by regular code paths."""
+
+import test_type_base
+import unittest
+
+from webkitpy.thirdparty.mock import Mock
+
+
+class Test(unittest.TestCase):
+
+    def test_compare_output_notimplemented(self):
+        test_type = test_type_base.TestTypeBase(None, None)
+        self.assertRaises(NotImplementedError, test_type.compare_output,
+                          None, "foo.txt", '',
+                          test_type_base.TestArguments(), 'Debug')
+
+
+if __name__ == '__main__':
+    unittest.main()
index d06ec8d34659f122ed5e8a91e0bc6e26d75d44fa..50a99954522e0ec125364e6b08bcf2331d6dcc18 100644 (file)
@@ -65,18 +65,16 @@ class TestTextDiff(test_type_base.TestTypeBase):
 
     def _get_normalized_text(self, filename):
         # FIXME: We repeat this pattern often, we should share code.
-        try:
-            # NOTE: -expected.txt files are ALWAYS utf-8.  However,
-            # we do not decode the output from DRT, so we should not
-            # decode the -expected.txt values either to allow comparisons.
-            with codecs.open(filename, "r", encoding=None) as file:
-                text = file.read()
-                # We could assert that the text is valid utf-8.
-        except IOError, e:
-            if errno.ENOENT != e.errno:
-                raise
+        if not os.path.exists(filename):
             return ''
 
+        # NOTE: -expected.txt files are ALWAYS utf-8.  However,
+        # we do not decode the output from DRT, so we should not
+        # decode the -expected.txt values either to allow comparisons.
+        with codecs.open(filename, "r", encoding=None) as file:
+            text = file.read()
+            # We could assert that the text is valid utf-8.
+
         # Normalize line endings
         return text.strip("\r\n").replace("\r\n", "\n") + "\n"
 
@@ -106,22 +104,8 @@ class TestTextDiff(test_type_base.TestTypeBase):
                                     print_text_diffs=True)
 
             if expected == '':
-                failures.append(test_failures.FailureMissingResult(self))
+                failures.append(test_failures.FailureMissingResult())
             else:
-                failures.append(test_failures.FailureTextMismatch(self, True))
+                failures.append(test_failures.FailureTextMismatch(True))
 
         return failures
-
-    def diff_files(self, port, file1, file2):
-        """Diff two text files.
-
-        Args:
-          file1, file2: full paths of the files to compare.
-
-        Returns:
-          True if two files are different.
-          False otherwise.
-        """
-
-        return port.compare_text(self._get_normalized_text(file1),
-                                 self._get_normalized_text(file2))