2010-08-25 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Aug 2010 21:41:12 +0000 (21:41 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Aug 2010 21:41:12 +0000 (21:41 +0000)
        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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py

index 034789f7ad3df3e4a7d994f3009224960af43bf6..c27ea1191ef40563e117a9675f9e59d605121a8a 100644 (file)
@@ -1,3 +1,21 @@
+2010-08-25  Dirk Pranke  <dpranke@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@chromium.org>
 
         Reviewed by Adam Barth.
index 81cdc9b67ba555302023965389f4f0bab48a0aad..a9e015fceecb3c9ccb6f8e8c7f9e58b2d6d0988c 100644 (file)
@@ -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)
index c8648bcfc86f008e3494e9048411c8f1ba2090c6..40c691f9a10e84c9cd947c2ec93a8629159fde1a 100644 (file)
@@ -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.
index 4c0472d5d035f2ef69d562733be98b42e8ba1ec9..af1af93d3d0246a5b7999208618a0caf02e8da0d 100644 (file)
@@ -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