2010-10-29 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Oct 2010 22:46:57 +0000 (22:46 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Oct 2010 22:46:57 +0000 (22:46 +0000)
        Reviewed by Tony Chang.

        new-run-webkit-tests: change TestResults to be serializable

        In preparation for changing new-run-webkit-tests from
        multithreaded to multiprocess, we need to make sure the data
        going between the threads is easily serialized over a socket.

        This change adds serialization/pickling for the TestResults and
        TestFailure objects (using cPickle).

        The TestFailure objects included a "has_wdiff" flag for Text
        results, but the flag wasn't being used, so I've removed it,
        simplifying the state to basically a set of enum objects with
        associated methods.

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

        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_failures.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_results.py: Added.
        * Scripts/webkitpy/layout_tests/layout_package/test_results_unittest.py:Added.
        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results_unittest.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/test_types/text_diff.py

index 08fb07a409cc8175530e37b1d304f0e01f30abe1..f32dcdfff742efb3ab0eb7ccf3df4ad1e038db12 100644 (file)
@@ -1,3 +1,32 @@
+2010-10-29  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Tony Chang.
+
+        new-run-webkit-tests: change TestResults to be serializable
+
+        In preparation for changing new-run-webkit-tests from
+        multithreaded to multiprocess, we need to make sure the data
+        going between the threads is easily serialized over a socket.
+
+        This change adds serialization/pickling for the TestResults and
+        TestFailure objects (using cPickle).
+
+        The TestFailure objects included a "has_wdiff" flag for Text
+        results, but the flag wasn't being used, so I've removed it,
+        simplifying the state to basically a set of enum objects with
+        associated methods.
+
+        https://bugs.webkit.org/show_bug.cgi?id=48616
+
+        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
+        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_failures.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_failures_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_results.py: Added.
+        * Scripts/webkitpy/layout_tests/layout_package/test_results_unittest.py:Added. 
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/test_types/text_diff.py:
+
 2010-10-29  Dirk Pranke  <dpranke@chromium.org>
 
         Reviewed by Ojan Vafai.
index ab10e0a952fb83253437f9d638c173caeb369c8c..3e3ba0b16770450036f7315ac161ef1fb9b7d735 100644 (file)
@@ -51,6 +51,7 @@ import time
 import traceback
 
 import test_failures
+import test_results
 
 _log = logging.getLogger("webkitpy.layout_tests.layout_package."
                          "dump_render_tree_thread")
@@ -133,8 +134,8 @@ def _process_output(port, options, test_info, test_types, test_args,
             time.time() - start_diff_time)
 
     total_time_for_all_diffs = time.time() - start_diff_time
-    return TestResult(test_info.filename, failures, test_run_time,
-                      total_time_for_all_diffs, time_for_diffs)
+    return test_results.TestResult(test_info.filename, failures, test_run_time,
+                                   total_time_for_all_diffs, time_for_diffs)
 
 
 def _pad_timeout(timeout):
@@ -159,18 +160,6 @@ def _image_hash(test_info, test_args, options):
     return test_info.image_hash()
 
 
-class TestResult(object):
-
-    def __init__(self, filename, failures, test_run_time,
-                 total_time_for_all_diffs, time_for_diffs):
-        self.failures = failures
-        self.filename = filename
-        self.test_run_time = test_run_time
-        self.time_for_diffs = time_for_diffs
-        self.total_time_for_all_diffs = total_time_for_all_diffs
-        self.type = test_failures.determine_result_type(failures)
-
-
 class SingleTestThread(threading.Thread):
     """Thread wrapper for running a single test file."""
 
@@ -264,8 +253,8 @@ class TestShellThread(WatchableThread):
           options: command line options argument from optparse
           filename_list_queue: A thread safe Queue class that contains lists
               of tuples of (filename, uri) pairs.
-          result_queue: A thread safe Queue class that will contain tuples of
-              (test, failure lists) for the test results.
+          result_queue: A thread safe Queue class that will contain
+              serialized TestResult objects.
           test_types: A list of TestType objects to run the test output
               against.
           test_args: A TestArguments object to pass to each TestType.
@@ -449,7 +438,7 @@ class TestShellThread(WatchableThread):
             else:
                 _log.debug("%s %s passed" % (self.getName(),
                            self._port.relative_test_filename(filename)))
-            self._result_queue.put(result)
+            self._result_queue.put(result.dumps())
 
             if batch_size > 0 and batch_count > batch_size:
                 # Bounce the shell and reset count.
@@ -505,9 +494,8 @@ class TestShellThread(WatchableThread):
             failures = []
             _log.error('Cannot get results of test: %s' %
                        test_info.filename)
-            result = TestResult(test_info.filename, failures=[],
-                                test_run_time=0, total_time_for_all_diffs=0,
-                                time_for_diffs=0)
+            result = test_results.TestResult(test_info.filename, failures=[],
+                test_run_time=0, total_time_for_all_diffs=0, time_for_diffs=0)
 
         return result
 
@@ -517,9 +505,7 @@ class TestShellThread(WatchableThread):
         Args:
           test_info: Object containing the test filename, uri and timeout
 
-        Returns:
-          A list of TestFailure objects describing the error.
-
+        Returns: a TestResult object.
         """
         self._ensure_dump_render_tree_is_running()
         # The pixel_hash is used to avoid doing an image dump if the
index 0344aa7ac408aeb08241aa3b0d9f7120dc41ba70..9a0f4eea208d21cf88ef8d5eeaf1483a511618e3 100644 (file)
@@ -40,7 +40,7 @@ from webkitpy.common import array_stream
 from webkitpy.common.system import logtesting
 from webkitpy.layout_tests import port
 from webkitpy.layout_tests.layout_package import printing
-from webkitpy.layout_tests.layout_package import dump_render_tree_thread
+from webkitpy.layout_tests.layout_package import test_results
 from webkitpy.layout_tests.layout_package import test_expectations
 from webkitpy.layout_tests.layout_package import test_failures
 from webkitpy.layout_tests import run_webkit_tests
@@ -141,9 +141,9 @@ class  Testprinter(unittest.TestCase):
         elif result_type == test_expectations.CRASH:
             failures = [test_failures.FailureCrash()]
         path = os.path.join(self._port.layout_tests_dir(), test)
-        return dump_render_tree_thread.TestResult(path, failures, run_time,
-                                                  total_time_for_all_diffs=0,
-                                                  time_for_diffs=0)
+        return test_results.TestResult(path, failures, run_time,
+                                       total_time_for_all_diffs=0,
+                                       time_for_diffs=0)
 
     def get_result_summary(self, tests, expectations_str):
         test_paths = [os.path.join(self._port.layout_tests_dir(), test) for
index 340d0753bb10f3a795ec8b8873abf2b81b2f3f35..6d557610fbf1b6364e48e824d021b52ff79c71f8 100644 (file)
@@ -32,6 +32,8 @@
 import os
 import test_expectations
 
+import cPickle
+
 
 def determine_result_type(failure_list):
     """Takes a set of test_failures and returns which result type best fits
@@ -70,11 +72,26 @@ def determine_result_type(failure_list):
 class TestFailure(object):
     """Abstract base class that defines the failure interface."""
 
+    @staticmethod
+    def loads(s):
+        """Creates a TestFailure object from the specified string."""
+        return cPickle.loads(s)
+
     @staticmethod
     def message():
         """Returns a string describing the failure in more detail."""
         raise NotImplementedError
 
+    def __eq__(self, other):
+        return self.__class__.__name__ == other.__class__.__name__
+
+    def __ne__(self, other):
+        return self.__class__.__name__ != other.__class__.__name__
+
+    def dumps(self):
+        """Returns the string/JSON representation of a TestFailure."""
+        return cPickle.dumps(self)
+
     def result_html_output(self, filename):
         """Returns an HTML string to be included on the results.html page."""
         raise NotImplementedError
@@ -112,7 +129,7 @@ class FailureWithType(TestFailure):
         TestFailure.__init__(self)
 
     # Filename suffixes used by ResultHtmlOutput.
-    OUT_FILENAMES = []
+    OUT_FILENAMES = ()
 
     def output_links(self, filename, out_names):
         """Returns a string holding all applicable output file links.
@@ -128,6 +145,10 @@ class FailureWithType(TestFailure):
         # FIXME: Seems like a bad idea to separate the display name data
         # from the path data by hard-coding the display name here
         # and passing in the path information via out_names.
+        #
+        # FIXME: Also, we don't know for sure that these files exist,
+        # and we shouldn't be creating links to files that don't exist
+        # (for example, if we don't actually have wdiff output).
         links = ['']
         uris = [self.relative_output_filename(filename, fn) for
                 fn in out_names]
@@ -170,7 +191,7 @@ class FailureCrash(TestFailure):
         return "Test shell crashed"
 
     def result_html_output(self, filename):
-        # TODO(tc): create a link to the minidump file
+        # FIXME: create a link to the minidump file
         stack = self.relative_output_filename(filename, "-stack.txt")
         return "<strong>%s</strong> <a href=%s>stack</a>" % (self.message(),
                                                              stack)
@@ -181,7 +202,7 @@ class FailureCrash(TestFailure):
 
 class FailureMissingResult(FailureWithType):
     """Expected result was missing."""
-    OUT_FILENAMES = ["-actual.txt"]
+    OUT_FILENAMES = ("-actual.txt",)
 
     @staticmethod
     def message():
@@ -196,14 +217,8 @@ class FailureTextMismatch(FailureWithType):
     """Text diff output failed."""
     # Filename suffixes used by ResultHtmlOutput.
     # FIXME: Why don't we use the constants from TestTypeBase here?
-    OUT_FILENAMES = ["-actual.txt", "-expected.txt", "-diff.txt"]
-    OUT_FILENAMES_WDIFF = ["-actual.txt", "-expected.txt", "-diff.txt",
-                           "-wdiff.html", "-pretty-diff.html"]
-
-    def __init__(self, has_wdiff):
-        FailureWithType.__init__(self)
-        if has_wdiff:
-            self.OUT_FILENAMES = self.OUT_FILENAMES_WDIFF
+    OUT_FILENAMES = ("-actual.txt", "-expected.txt", "-diff.txt",
+                     "-wdiff.html", "-pretty-diff.html")
 
     @staticmethod
     def message():
@@ -214,7 +229,6 @@ class FailureMissingImageHash(FailureWithType):
     """Actual result hash was missing."""
     # Chrome doesn't know to display a .checksum file as text, so don't bother
     # putting in a link to the actual result.
-    OUT_FILENAMES = []
 
     @staticmethod
     def message():
@@ -226,7 +240,7 @@ class FailureMissingImageHash(FailureWithType):
 
 class FailureMissingImage(FailureWithType):
     """Actual result image was missing."""
-    OUT_FILENAMES = ["-actual.png"]
+    OUT_FILENAMES = ("-actual.png",)
 
     @staticmethod
     def message():
@@ -239,7 +253,7 @@ class FailureMissingImage(FailureWithType):
 
 class FailureImageHashMismatch(FailureWithType):
     """Image hashes didn't match."""
-    OUT_FILENAMES = ["-actual.png", "-expected.png", "-diff.png"]
+    OUT_FILENAMES = ("-actual.png", "-expected.png", "-diff.png")
 
     @staticmethod
     def message():
@@ -252,7 +266,6 @@ class FailureImageHashIncorrect(FailureWithType):
     """Actual result hash is incorrect."""
     # Chrome doesn't know to display a .checksum file as text, so don't bother
     # putting in a link to the actual result.
-    OUT_FILENAMES = []
 
     @staticmethod
     def message():
@@ -260,3 +273,10 @@ class FailureImageHashIncorrect(FailureWithType):
 
     def result_html_output(self, filename):
         return "<strong>%s</strong>" % self.message()
+
+# Convenient collection of all failure classes for anything that might
+# need to enumerate over them all.
+ALL_FAILURE_CLASSES = (FailureTimeout, FailureCrash, FailureMissingResult,
+                       FailureTextMismatch, FailureMissingImageHash,
+                       FailureMissingImage, FailureImageHashMismatch,
+                       FailureImageHashIncorrect)
index 92fe2760c06a4569417ba83d10853cdfcc2a9ec4..3e3528dc7807494a0a7eabe413234a6e673ce330 100644 (file)
 
 """"Tests code paths not covered by the regular unit tests."""
 
-from webkitpy.layout_tests.layout_package.test_failures import *
 import unittest
 
+from webkitpy.layout_tests.layout_package.test_failures import *
+
+
 class Test(unittest.TestCase):
     def assertResultHtml(self, failure_obj):
         self.assertNotEqual(failure_obj.result_html_output('foo'), None)
 
+    def assert_loads(self, cls):
+        failure_obj = cls()
+        s = failure_obj.dumps()
+        new_failure_obj = TestFailure.loads(s)
+        self.assertTrue(isinstance(new_failure_obj, cls))
+
+        self.assertEqual(failure_obj, new_failure_obj)
+
+        # Also test that != is implemented.
+        self.assertFalse(failure_obj != new_failure_obj)
+
     def test_crash(self):
         self.assertResultHtml(FailureCrash())
 
@@ -63,6 +76,9 @@ class Test(unittest.TestCase):
         self.assertRaises(NotImplementedError, failure_obj.result_html_output,
                           "foo.txt")
 
+    def test_loads(self):
+        for c in ALL_FAILURE_CLASSES:
+            self.assert_loads(c)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results.py
new file mode 100644 (file)
index 0000000..2417fb7
--- /dev/null
@@ -0,0 +1,61 @@
+# 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.
+
+import cPickle
+
+import test_failures
+
+
+class TestResult(object):
+    """Data object containing the results of a single test."""
+
+    @staticmethod
+    def loads(str):
+        return cPickle.loads(str)
+
+    def __init__(self, filename, failures, test_run_time,
+                 total_time_for_all_diffs, time_for_diffs):
+        self.failures = failures
+        self.filename = filename
+        self.test_run_time = test_run_time
+        self.time_for_diffs = time_for_diffs
+        self.total_time_for_all_diffs = total_time_for_all_diffs
+        self.type = test_failures.determine_result_type(failures)
+
+    def __eq__(self, other):
+        return (self.filename == other.filename and
+                self.failures == other.failures and
+                self.test_run_time == other.test_run_time and
+                self.time_for_diffs == other.time_for_diffs and
+                self.total_time_for_all_diffs == other.total_time_for_all_diffs)
+
+    def __ne__(self, other):
+        return not (self == other)
+
+    def dumps(self):
+        return cPickle.dumps(self)
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_results_unittest.py
new file mode 100644 (file)
index 0000000..5921666
--- /dev/null
@@ -0,0 +1,52 @@
+# 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.
+
+import unittest
+
+from test_results import TestResult
+
+
+class Test(unittest.TestCase):
+    def test_loads(self):
+        result = TestResult(filename='foo',
+                            failures=[],
+                            test_run_time=1.1,
+                            total_time_for_all_diffs=0.5,
+                            time_for_diffs=0.5)
+        s = result.dumps()
+        new_result = TestResult.loads(s)
+        self.assertTrue(isinstance(new_result, TestResult))
+
+        self.assertEqual(new_result, result)
+
+        # Also check that != is implemented.
+        self.assertFalse(new_result != result)
+
+
+if __name__ == '__main__':
+    unittest.main()
index 1350ced1e6501a9ac3d6d7d683b29cb16825fa5d..704180c60ba1cc790a62ee15a574f8d6a1a07fa2 100755 (executable)
@@ -69,6 +69,7 @@ from layout_package import json_layout_results_generator
 from layout_package import printing
 from layout_package import test_expectations
 from layout_package import test_failures
+from layout_package import test_results
 from layout_package import test_results_uploader
 from test_types import image_diff
 from test_types import text_diff
@@ -457,7 +458,7 @@ class TestRunner:
             # subtracted out of self._test_files, above), but we stub out the
             # results here so the statistics can remain accurate.
             for test in skip_chunk:
-                result = dump_render_tree_thread.TestResult(test,
+                result = test_results.TestResult(test,
                     failures=[], test_run_time=0, total_time_for_all_diffs=0,
                     time_for_diffs=0)
                 result.type = test_expectations.SKIP
@@ -852,7 +853,7 @@ class TestRunner:
         """Update the summary and print results with any completed tests."""
         while True:
             try:
-                result = self._result_queue.get_nowait()
+                result = test_results.TestResult.loads(self._result_queue.get_nowait())
             except Queue.Empty:
                 return
 
@@ -1305,7 +1306,8 @@ class TestRunner:
             page += u"<p><a href='%s'>%s</a><br />\n" % (test_url, test_name)
             test_failures = failures.get(test_file, [])
             for failure in test_failures:
-                page += u"&nbsp;&nbsp;%s<br/>" % failure.result_html_output(test_name)
+                page += (u"&nbsp;&nbsp;%s<br/>" %
+                         failure.result_html_output(test_name))
             page += "</p>\n"
         page += "</body></html>\n"
         return page
index b1f621e5b45e5824662b03139fc37b4a237d01a9..4c32f0d023473c63b8e474f77093a0fdfa0aa337 100644 (file)
@@ -89,6 +89,6 @@ class TestTextDiff(test_type_base.TestTypeBase):
             if expected == '':
                 failures.append(test_failures.FailureMissingResult())
             else:
-                failures.append(test_failures.FailureTextMismatch(True))
+                failures.append(test_failures.FailureTextMismatch())
 
         return failures