[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

dpranke at chromium.org dpranke at chromium.org
Wed Dec 22 12:36:03 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit a1951a06b6b958aa1013c9717470f3c566409868
Author: dpranke at chromium.org <dpranke at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Aug 25 21:41:12 2010 +0000

    2010-08-25  Dirk Pranke  <dpranke at chromium.org>
    
            Reviewed by Ojan Vafai.
    
            new-run-webkit-tests: add more unit tests for layout_package/printing.py
    
            This change adds more unit tests to get the test coverage to 100%
            for the printing module. This code is actually pretty crufty and
            has some layering violations that need to be cleaned up but I'll
            save that for another CL after we get all the unit tests written and
            we fix the multithreading issues. At least now we'll be able to tell
            if we break things.
    
            https://bugs.webkit.org/show_bug.cgi?id=44576
    
            * Scripts/webkitpy/layout_tests/layout_package/printing.py:
            * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66043 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 034789f..c27ea11 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,21 @@
+2010-08-25  Dirk Pranke  <dpranke at chromium.org>
+
+        Reviewed by Ojan Vafai.
+
+        new-run-webkit-tests: add more unit tests for layout_package/printing.py
+
+        This change adds more unit tests to get the test coverage to 100%
+        for the printing module. This code is actually pretty crufty and
+        has some layering violations that need to be cleaned up but I'll
+        save that for another CL after we get all the unit tests written and
+        we fix the multithreading issues. At least now we'll be able to tell
+        if we break things.
+
+        https://bugs.webkit.org/show_bug.cgi?id=44576
+
+        * Scripts/webkitpy/layout_tests/layout_package/printing.py:
+        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
+
 2010-08-25  Ojan Vafai  <ojan at chromium.org>
 
         Reviewed by Adam Barth.
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py
index 81cdc9b..a9e015f 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py
@@ -298,7 +298,20 @@ class Printer(object):
         self._write("")
 
     def print_test_result(self, result, expected, exp_str, got_str):
-        """Print the result of the test as determined by --print."""
+        """Print the result of the test as determined by --print.
+
+        This routine is used to print the details of each test as it completes.
+
+        Args:
+            result   - The actual TestResult object
+            expected - Whether the result we got was an expected result
+            exp_str  - What we expected to get (used for tracing)
+            got_str  - What we actually got (used for tracing)
+
+        Note that we need all of these arguments even though they seem
+        somewhat redundant, in order to keep this routine from having to
+        known anything about the set of expectations.
+        """
         if (self.enabled('trace-everything') or
             self.enabled('trace-unexpected') and not expected):
             self._print_test_trace(result, exp_str, got_str)
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
index c8648bc..40c691f 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
@@ -51,27 +51,6 @@ def get_options(args):
     return option_parser.parse_args(args)
 
 
-def get_result(filename, result_type=test_expectations.PASS, run_time=0):
-    failures = []
-    if result_type == test_expectations.TIMEOUT:
-        failures = [test_failures.FailureTimeout()]
-    elif result_type == test_expectations.CRASH:
-        failures = [test_failures.FailureCrash()]
-    return dump_render_tree_thread.TestResult(filename, failures, run_time,
-                                              total_time_for_all_diffs=0,
-                                              time_for_diffs=0)
-
-
-def get_result_summary(port_obj, test_files, expectations_str):
-    expectations = test_expectations.TestExpectations(
-        port_obj, test_files, expectations_str,
-        port_obj.test_platform_name(), is_debug_mode=False,
-        is_lint_mode=False, tests_are_present=False)
-
-    rs = run_webkit_tests.ResultSummary(expectations, test_files)
-    return rs, expectations
-
-
 class TestUtilityFunctions(unittest.TestCase):
     def test_configure_logging(self):
         # FIXME: We need to figure out how to reset the basic logger.
@@ -155,6 +134,28 @@ class  Testprinter(unittest.TestCase):
                                    is_fully_parallel)
         return printer, regular_output, buildbot_output
 
+    def get_result(self, test, result_type=test_expectations.PASS, run_time=0):
+        failures = []
+        if result_type == test_expectations.TIMEOUT:
+            failures = [test_failures.FailureTimeout()]
+        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)
+
+    def get_result_summary(self, tests, expectations_str):
+        test_paths = [os.path.join(self._port.layout_tests_dir(), test) for
+                      test in tests]
+        expectations = test_expectations.TestExpectations(
+            self._port, test_paths, expectations_str,
+            self._port.test_platform_name(), is_debug_mode=False,
+            is_lint_mode=False, tests_are_present=False)
+
+        rs = run_webkit_tests.ResultSummary(expectations, test_paths)
+        return test_paths, rs, expectations
+
     def test_help_printer(self):
         # Here and below we'll call the "regular" printer err and the
         # buildbot printer out; this corresponds to how things run on the
@@ -244,10 +245,18 @@ class  Testprinter(unittest.TestCase):
 
 
     def test_print_test_result(self):
-        result = get_result('foo.html')
+        # Note here that we don't use meaningful exp_str and got_str values;
+        # the actual contents of the string are treated opaquely by
+        # print_test_result() when tracing, and usually we don't want
+        # to test what exactly is printed, just that something
+        # was printed (or that nothing was printed).
+        #
+        # FIXME: this is actually some goofy layering; it would be nice
+        # we could refactor it so that the args weren't redundant. Maybe
+        # the TestResult should contain what was expected, and the
+        # strings could be derived from the TestResult?
         printer, err, out = self.get_printer(['--print', 'nothing'])
-        result = get_result(os.path.join(self._port.layout_tests_dir(),
-                                         'foo.html'))
+        result = self.get_result('passes/image.html')
         printer.print_test_result(result, expected=False, exp_str='',
                                   got_str='')
         self.assertTrue(err.empty())
@@ -259,7 +268,7 @@ class  Testprinter(unittest.TestCase):
         printer.print_test_result(result, expected=False, exp_str='',
                                   got_str='')
         self.assertEquals(err.get(),
-                          ['  foo.html -> unexpected pass\n'])
+                          ['  passes/image.html -> unexpected pass\n'])
 
         printer, err, out = self.get_printer(['--print', 'everything'])
         printer.print_test_result(result, expected=True, exp_str='',
@@ -269,7 +278,7 @@ class  Testprinter(unittest.TestCase):
         printer.print_test_result(result, expected=False, exp_str='',
                                   got_str='')
         self.assertEquals(err.get(),
-                          ['  foo.html -> unexpected pass\n'])
+                          ['  passes/image.html -> unexpected pass\n'])
 
         printer, err, out = self.get_printer(['--print', 'nothing'])
         printer.print_test_result(result, expected=False, exp_str='',
@@ -277,11 +286,24 @@ class  Testprinter(unittest.TestCase):
         self.assertTrue(err.empty())
 
         printer, err, out = self.get_printer(['--print',
-                                                 'trace-unexpected'])
+                                              'trace-unexpected'])
         printer.print_test_result(result, expected=True, exp_str='',
                                   got_str='')
         self.assertTrue(err.empty())
 
+        printer, err, out = self.get_printer(['--print',
+                                              'trace-unexpected'])
+        printer.print_test_result(result, expected=False, exp_str='',
+                                  got_str='')
+        self.assertFalse(err.empty())
+
+        printer, err, out = self.get_printer(['--print',
+                                              'trace-unexpected'])
+        result = self.get_result("passes/text.html")
+        printer.print_test_result(result, expected=False, exp_str='',
+                                  got_str='')
+        self.assertFalse(err.empty())
+
         err.reset()
         printer.print_test_result(result, expected=False, exp_str='',
                                   got_str='')
@@ -297,104 +319,106 @@ class  Testprinter(unittest.TestCase):
                                   got_str='')
 
     def test_print_progress(self):
-        test_files = ['foo.html', 'bar.html']
         expectations = ''
 
         # test that we print nothing
         printer, err, out = self.get_printer(['--print', 'nothing'])
-        rs, exp = get_result_summary(self._port, test_files, expectations)
+        tests = ['passes/text.html', 'failures/expected/timeout.html',
+                 'failures/expected/crash.html']
+        paths, rs, exp = self.get_result_summary(tests, expectations)
 
-        printer.print_progress(rs, False, test_files)
+        printer.print_progress(rs, False, paths)
         self.assertTrue(out.empty())
         self.assertTrue(err.empty())
 
-        printer.print_progress(rs, True, test_files)
+        printer.print_progress(rs, True, paths)
         self.assertTrue(out.empty())
         self.assertTrue(err.empty())
 
         # test regular functionality
         printer, err, out = self.get_printer(['--print',
                                               'one-line-progress'])
-        printer.print_progress(rs, False, test_files)
+        printer.print_progress(rs, False, paths)
         self.assertTrue(out.empty())
         self.assertFalse(err.empty())
 
         err.reset()
         out.reset()
-        printer.print_progress(rs, True, test_files)
+        printer.print_progress(rs, True, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
     def test_print_progress__detailed(self):
-        test_files = ['pass/pass.html', 'pass/timeout.html', 'fail/crash.html']
-        expectations = 'pass/timeout.html = TIMEOUT'
+        tests = ['passes/text.html', 'failures/expected/timeout.html',
+                 'failures/expected/crash.html']
+        expectations = 'failures/expected/timeout.html = TIMEOUT'
 
         # first, test that it is disabled properly
         # should still print one-line-progress
         printer, err, out = self.get_printer(
             ['--print', 'detailed-progress'], single_threaded=False)
-        rs, exp = get_result_summary(self._port, test_files, expectations)
-        printer.print_progress(rs, False, test_files)
+        paths, rs, exp = self.get_result_summary(tests, expectations)
+        printer.print_progress(rs, False, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         # now test the enabled paths
         printer, err, out = self.get_printer(
             ['--print', 'detailed-progress'], single_threaded=True)
-        rs, exp = get_result_summary(self._port, test_files, expectations)
-        printer.print_progress(rs, False, test_files)
+        paths, rs, exp = self.get_result_summary(tests, expectations)
+        printer.print_progress(rs, False, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         err.reset()
         out.reset()
-        printer.print_progress(rs, True, test_files)
+        printer.print_progress(rs, True, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
-        rs.add(get_result('pass/pass.html', test_expectations.TIMEOUT), False)
-        rs.add(get_result('pass/timeout.html'), True)
-        rs.add(get_result('fail/crash.html', test_expectations.CRASH), True)
+        rs.add(self.get_result('passes/text.html', test_expectations.TIMEOUT), False)
+        rs.add(self.get_result('failures/expected/timeout.html'), True)
+        rs.add(self.get_result('failures/expected/crash.html', test_expectations.CRASH), True)
         err.reset()
         out.reset()
-        printer.print_progress(rs, False, test_files)
+        printer.print_progress(rs, False, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         # We only clear the meter when retrying w/ detailed-progress.
         err.reset()
         out.reset()
-        printer.print_progress(rs, True, test_files)
-        self.assertEqual(err.get(), [''])
+        printer.print_progress(rs, True, paths)
+        self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         printer, err, out = self.get_printer(
             ['--print', 'detailed-progress,unexpected'], single_threaded=True)
-        rs, exp = get_result_summary(self._port, test_files, expectations)
-        printer.print_progress(rs, False, test_files)
+        paths, rs, exp = self.get_result_summary(tests, expectations)
+        printer.print_progress(rs, False, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         err.reset()
         out.reset()
-        printer.print_progress(rs, True, test_files)
+        printer.print_progress(rs, True, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
-        rs.add(get_result('pass/pass.html', test_expectations.TIMEOUT), False)
-        rs.add(get_result('pass/timeout.html'), True)
-        rs.add(get_result('fail/crash.html', test_expectations.CRASH), True)
+        rs.add(self.get_result('passes/text.html', test_expectations.TIMEOUT), False)
+        rs.add(self.get_result('failures/expected/timeout.html'), True)
+        rs.add(self.get_result('failures/expected/crash.html', test_expectations.CRASH), True)
         err.reset()
         out.reset()
-        printer.print_progress(rs, False, test_files)
+        printer.print_progress(rs, False, paths)
         self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
         # We only clear the meter when retrying w/ detailed-progress.
         err.reset()
         out.reset()
-        printer.print_progress(rs, True, test_files)
-        self.assertEqual(err.get(), [''])
+        printer.print_progress(rs, True, paths)
+        self.assertFalse(err.empty())
         self.assertTrue(out.empty())
 
     def test_write(self):
@@ -416,42 +440,72 @@ class  Testprinter(unittest.TestCase):
         printer.write("foo", "config")
         self.assertFalse(err.empty())
 
+        # FIXME: this should be logged somewhere, but it actually
+        # disappears into the ether in the logging subsystem.
+        printer, err, out = self.get_printer(['--verbose'])
+        printer.write("foo")
+        self.assertTrue(err.empty())
+        self.assertTrue(out.empty())
+
     def test_print_unexpected_results(self):
         # This routine is the only one that prints stuff that the bots
         # care about.
+        #
+        # FIXME: there's some weird layering going on here. It seems
+        # like we shouldn't be both using an expectations string and
+        # having to specify whether or not the result was expected.
+        # This whole set of tests should probably be rewritten.
+        #
+        # FIXME: Plus, the fact that we're having to call into
+        # run_webkit_tests is clearly a layering inversion.
         def get_unexpected_results(expected, passing, flaky):
-            rs, exp = get_result_summary(self._port, test_files, expectations)
+            """Return an unexpected results summary matching the input description.
+
+            There are a lot of different combinations of test results that
+            can be tested; this routine produces various combinations based
+            on the values of the input flags.
+
+            Args
+                expected: whether the tests ran as expected
+                passing: whether the tests should all pass
+                flaky: whether the tests should be flaky (if False, they
+                    produce the same results on both runs; if True, they
+                    all pass on the second run).
+
+            """
+            paths, rs, exp = self.get_result_summary(tests, expectations)
             if expected:
-                rs.add(get_result('pass/pass.html', test_expectations.PASS),
+                rs.add(self.get_result('passes/text.html', test_expectations.PASS),
                        expected)
-                rs.add(get_result('pass/timeout.html',
+                rs.add(self.get_result('failures/expected/timeout.html',
                        test_expectations.TIMEOUT), expected)
-                rs.add(get_result('fail/crash.html', test_expectations.CRASH),
+                rs.add(self.get_result('failures/expected/crash.html', test_expectations.CRASH),
                    expected)
             elif passing:
-                rs.add(get_result('pass/pass.html'), expected)
-                rs.add(get_result('pass/timeout.html'), expected)
-                rs.add(get_result('fail/crash.html'), expected)
+                rs.add(self.get_result('passes/text.html'), expected)
+                rs.add(self.get_result('failures/expected/timeout.html'), expected)
+                rs.add(self.get_result('failures/expected/crash.html'), expected)
             else:
-                rs.add(get_result('pass/pass.html', test_expectations.TIMEOUT),
+                rs.add(self.get_result('passes/text.html', test_expectations.TIMEOUT),
                        expected)
-                rs.add(get_result('pass/timeout.html',
+                rs.add(self.get_result('failures/expected/timeout.html',
                        test_expectations.CRASH), expected)
-                rs.add(get_result('fail/crash.html',
+                rs.add(self.get_result('failures/expected/crash.html',
                                   test_expectations.TIMEOUT),
                    expected)
             retry = rs
             if flaky:
-                retry, exp = get_result_summary(self._port, test_files,
+                paths, retry, exp = self.get_result_summary(tests,
                                                 expectations)
-                retry.add(get_result('pass/pass.html'), True)
-                retry.add(get_result('pass/timeout.html'), True)
-                retry.add(get_result('fail/crash.html'), True)
+                retry.add(self.get_result('passes/text.html'), True)
+                retry.add(self.get_result('failures/expected/timeout.html'), True)
+                retry.add(self.get_result('failures/expected/crash.html'), True)
             unexpected_results = run_webkit_tests.summarize_unexpected_results(
                 self._port, exp, rs, retry)
             return unexpected_results
 
-        test_files = ['pass/pass.html', 'pass/timeout.html', 'fail/crash.html']
+        tests = ['passes/text.html', 'failures/expected/timeout.html',
+                 'failures/expected/crash.html']
         expectations = ''
 
         printer, err, out = self.get_printer(['--print', 'nothing'])
@@ -501,6 +555,33 @@ class  Testprinter(unittest.TestCase):
         self.assertTrue(err.empty())
         self.assertFalse(out.empty())
 
+        expectations = """
+failures/expected/crash.html = CRASH
+failures/expected/timeout.html = TIMEOUT
+"""
+        err.reset()
+        out.reset()
+        ur = get_unexpected_results(expected=False, passing=False, flaky=False)
+        printer.print_unexpected_results(ur)
+        self.assertTrue(err.empty())
+        self.assertFalse(out.empty())
+
+        err.reset()
+        out.reset()
+        ur = get_unexpected_results(expected=False, passing=True, flaky=False)
+        printer.print_unexpected_results(ur)
+        self.assertTrue(err.empty())
+        self.assertFalse(out.empty())
+
+        # Test handling of --verbose as well.
+        err.reset()
+        out.reset()
+        printer, err, out = self.get_printer(['--verbose'])
+        ur = get_unexpected_results(expected=False, passing=False, flaky=False)
+        printer.print_unexpected_results(ur)
+        self.assertTrue(err.empty())
+        self.assertFalse(out.empty())
+
     def test_print_unexpected_results_buildbot(self):
         # FIXME: Test that print_unexpected_results() produces the printer the
         # buildbot is expecting.
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
index 4c0472d..af1af93 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
@@ -30,6 +30,8 @@
 """Abstract base class of Port-specific entrypoints for the layout tests
 test infrastructure (the Port and Driver classes)."""
 
+from __future__ import with_statement
+
 import cgi
 import codecs
 import difflib

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list