[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

cjerdonek at webkit.org cjerdonek at webkit.org
Thu Apr 8 02:06:31 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 444ae13755ceb5c68dd73d8d52f37ed22fe1c2a4
Author: cjerdonek at webkit.org <cjerdonek at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Mar 2 16:48:05 2010 +0000

    2010-03-02  Chris Jerdonek  <cjerdonek at webkit.org>
    
            Reviewed by Shinichiro Hamaji.
    
            Started using the logging module in check-webkit-style.
            This provides more options for debugging and a more flexible,
            uniform way to report messages to the end-user.
    
            https://bugs.webkit.org/show_bug.cgi?id=35484
    
            Also included classes in a central location to facilitate
            the unit testing of logging code (setUp and tearDown of unit
            test logging configurations, etc).
    
            * Scripts/check-webkit-style:
              - Added a call to configure_logging() in the beginning of main().
              - Replaced two calls to sys.stderr.write() with appropriate
                logging calls.
    
            * Scripts/webkitpy/init/__init__.py: Copied from WebKitTools/QueueStatusServer/filters/__init__.py.
    
            * Scripts/webkitpy/init/logtesting.py: Added.
              - Added a UnitTestLogStream class to capture log output
                during unit tests.
              - Added a UnitTestLog class that provides convenience methods
                for unit-testing logging code.
    
            * Scripts/webkitpy/style/checker.py:
              - Added a configure_logging() method.
              - Added a _LevelLoggingFilter class to filter out log messages
                above a certain logging level.
              - Removed the _stderr_write() method from the StyleChecker class
                and replaced its use with appropriate logging calls.
    
            * Scripts/webkitpy/style/checker_unittest.py:
              - Added a ConfigureLoggingTest class to unit test the
                configure_logging() method.
              - Updated the StyleCheckerCheckFileTest class as necessary.
    
            * Scripts/webkitpy/style_references.py:
              - Added references to logtesting.UnitTestLog and
                logtesting.UnitTestLogStream.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55409 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index e20cb40..537f2ee 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,46 @@
+2010-03-02  Chris Jerdonek  <cjerdonek at webkit.org>
+
+        Reviewed by Shinichiro Hamaji.
+
+        Started using the logging module in check-webkit-style.
+        This provides more options for debugging and a more flexible,
+        uniform way to report messages to the end-user.
+
+        https://bugs.webkit.org/show_bug.cgi?id=35484
+
+        Also included classes in a central location to facilitate
+        the unit testing of logging code (setUp and tearDown of unit
+        test logging configurations, etc).
+
+        * Scripts/check-webkit-style:
+          - Added a call to configure_logging() in the beginning of main().
+          - Replaced two calls to sys.stderr.write() with appropriate
+            logging calls.
+
+        * Scripts/webkitpy/init/__init__.py: Copied from WebKitTools/QueueStatusServer/filters/__init__.py.
+
+        * Scripts/webkitpy/init/logtesting.py: Added.
+          - Added a UnitTestLogStream class to capture log output
+            during unit tests.
+          - Added a UnitTestLog class that provides convenience methods
+            for unit-testing logging code.
+
+        * Scripts/webkitpy/style/checker.py:
+          - Added a configure_logging() method.
+          - Added a _LevelLoggingFilter class to filter out log messages
+            above a certain logging level.
+          - Removed the _stderr_write() method from the StyleChecker class
+            and replaced its use with appropriate logging calls.
+
+        * Scripts/webkitpy/style/checker_unittest.py:
+          - Added a ConfigureLoggingTest class to unit test the
+            configure_logging() method.
+          - Updated the StyleCheckerCheckFileTest class as necessary.
+
+        * Scripts/webkitpy/style_references.py:
+          - Added references to logtesting.UnitTestLog and
+            logtesting.UnitTestLogStream.
+
 2010-03-01  Chris Fleizach  <cfleizach at apple.com>
 
         Fixing broken DRT on Leopard/Tiger. Second try.
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style
index ea2e943..f1885b2 100755
--- a/WebKitTools/Scripts/check-webkit-style
+++ b/WebKitTools/Scripts/check-webkit-style
@@ -43,6 +43,7 @@ same line, but it is far from perfect (in either direction).
 """
 
 import codecs
+import logging
 import os
 import os.path
 import sys
@@ -50,13 +51,28 @@ import sys
 import webkitpy.style.checker as checker
 from webkitpy.style_references import SimpleScm
 
+_log = logging.getLogger("check-webkit-style")
+
 def main():
     # Change stderr to write with replacement characters so we don't die
     # if we try to print something containing non-ASCII characters.
-    sys.stderr = codecs.StreamReaderWriter(sys.stderr,
-                                           codecs.getreader('utf8'),
-                                           codecs.getwriter('utf8'),
-                                           'replace')
+    stderr = codecs.StreamReaderWriter(sys.stderr,
+                                       codecs.getreader('utf8'),
+                                       codecs.getwriter('utf8'),
+                                       'replace')
+    # Setting an "encoding" attribute on the stream is necessary to
+    # prevent the logging module from raising an error.  See
+    # the checker.configure_logging() function for more information.
+    stderr.encoding = "UTF-8"
+
+    # FIXME: Change webkitpy.style so that we do not need to overwrite
+    #        the global sys.stderr.  This involves updating the code to
+    #        accept a stream parameter where necessary, and not calling
+    #        sys.stderr explicitly anywhere.
+    sys.stderr = stderr
+
+    checker.configure_logging(stderr)
+
     parser = checker.check_webkit_style_parser()
     (files, options) = parser.parse(sys.argv[1:])
 
@@ -78,7 +94,8 @@ def main():
                 # FIXME: If the range is a "...", the code should find the common ancestor and
                 # start there (see git diff --help for information about how ... usually works).
                 commit = commit[:commit.find('..')]
-                print >> sys.stderr, "Warning: Ranges are not supported for --git-commit. Checking all changes since %s.\n" % commit
+                _log.warn("Ranges are not supported for --git-commit. "
+                          "Checking all changes since %s." % commit)
             patch = scm.create_patch_since_local_commit(commit)
         else:
             patch = scm.create_patch()
@@ -86,8 +103,8 @@ def main():
 
     error_count = style_checker.error_count
     file_count = style_checker.file_count
-    sys.stderr.write('Total errors found: %d in %d files\n'
-                     % (error_count, file_count))
+    _log.info("Total errors found: %d in %d files"
+              % (error_count, file_count))
     # We fail when style errors are found or there are no checked files.
     sys.exit(error_count > 0 or file_count == 0)
 
diff --git a/WebKitTools/QueueStatusServer/filters/__init__.py b/WebKitTools/Scripts/webkitpy/init/__init__.py
similarity index 100%
copy from WebKitTools/QueueStatusServer/filters/__init__.py
copy to WebKitTools/Scripts/webkitpy/init/__init__.py
diff --git a/WebKitTools/Scripts/webkitpy/init/logtesting.py b/WebKitTools/Scripts/webkitpy/init/logtesting.py
new file mode 100644
index 0000000..6363115
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/init/logtesting.py
@@ -0,0 +1,122 @@
+# Copyright (C) 2010 Chris Jerdonek (cjerdonek at webkit.org)
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1.  Redistributions of source code must retain the above copyright
+#     notice, this list of conditions and the following disclaimer.
+# 2.  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.
+#
+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS 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 APPLE INC. OR ITS 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.
+
+"""Supports the unit-testing of logging."""
+
+import logging
+
+
+class UnitTestLogStream(object):
+
+    """Represents a file-like object for unit-testing logging.
+
+    This is meant for passing to the logging.StreamHandler constructor.
+
+    """
+
+    def __init__(self):
+        self.messages = []
+        """A list of log messages written to the stream."""
+
+    def write(self, message):
+        self.messages.append(message)
+
+    def flush(self):
+        pass
+
+
+class UnitTestLog(object):
+
+    """Supports unit-testing logging.
+
+    Sample usage:
+
+    # The following line should go in the setUp() method of a
+    # unittest.TestCase.  In particular, self is a TestCase instance.
+    self._log = UnitTestLog(self)
+
+    # Call the following in a test method of a unittest.TestCase.
+    log = logging.getLogger("webkitpy")
+    log.info("message1")
+    log.warn("message2")
+    self._log.assertMessages(["INFO: message1\n",
+                              "WARN: message2\n"])  # Should succeed.
+
+    # The following line should go in the tearDown() method of
+    # unittest.TestCase.
+    self._log.tearDown()
+
+    """
+
+    def __init__(self, test_case, logging_level=None):
+        """Configure unit test logging, and return an instance.
+
+        This method configures the root logger to log to a testing
+        log stream.  Only messages logged at or above the given level
+        are logged to the stream.  Messages are formatted in the
+        following way, for example--
+
+        "INFO: This is a test log message."
+
+        This method should normally be called in the setUp() method
+        of a unittest.TestCase.
+
+        Args:
+          test_case: A unittest.TestCase instance.
+          logging_level: A logging level.  Only messages logged at or
+                         above this level are written to the testing
+                         log stream.  Defaults to logging.INFO.
+
+        """
+        if logging_level is None:
+            logging_level = logging.INFO
+
+        stream = UnitTestLogStream()
+        handler = logging.StreamHandler(stream)
+        formatter = logging.Formatter("%(levelname)s: %(message)s")
+        handler.setFormatter(formatter)
+
+        logger = logging.getLogger()
+        logger.setLevel(logging_level)
+        logger.addHandler(handler)
+
+        self._handler = handler
+        self._messages = stream.messages
+        self._test_case = test_case
+
+    def _logger(self):
+        """Return the root logger used for logging."""
+        return logging.getLogger()
+
+    def assertMessages(self, messages):
+        """Assert that the given messages match the logged messages."""
+        self._test_case.assertEquals(messages, self._messages)
+
+    def tearDown(self):
+        """Unconfigure unit test logging.
+
+        This should normally be called in the tearDown method of a
+        unittest.TestCase
+
+        """
+        logger = self._logger()
+        logger.removeHandler(self._handler)
diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py
index 8494e1b..c74aece 100644
--- a/WebKitTools/Scripts/webkitpy/style/checker.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker.py
@@ -30,6 +30,7 @@
 """Front end of some style-checker modules."""
 
 import codecs
+import logging
 import os.path
 import sys
 
@@ -44,6 +45,7 @@ from processors.common import categories as CommonCategories
 from processors.cpp import CppProcessor
 from processors.text import TextProcessor
 
+_log = logging.getLogger("webkitpy.style.checker")
 
 # These are default option values for the command-line option parser.
 _DEFAULT_VERBOSITY = 1
@@ -224,6 +226,88 @@ def check_webkit_style_configuration(options):
                verbosity=options.verbosity)
 
 
+# FIXME: Add support for more verbose logging for debug purposes.
+#        This can use a formatter like the following, for example--
+#
+#        formatter = logging.Formatter("%(name)s: [%(levelname)s] %(message)s")
+def configure_logging(stream):
+    """Configure logging, and return the list of handlers added.
+
+    Configures the root logger to log INFO messages and higher.
+    Formats WARNING messages and above to display the logging level
+    and messages strictly below WARNING not to display it.
+
+    Returns:
+      A list of references to the logging handlers added to the root
+      logger.  This allows the caller to later remove the handlers
+      using logger.removeHandler.  This is useful primarily during unit
+      testing where the caller may want to configure logging temporarily
+      and then undo the configuring.
+
+    Args:
+      stream: A file-like object to which to log.  The stream must
+              define an "encoding" data attribute, or else logging
+              raises an error.
+
+    """
+    # If the stream does not define an "encoding" data attribute, the
+    # logging module can throw an error like the following:
+    #
+    # Traceback (most recent call last):
+    #   File "/System/Library/Frameworks/Python.framework/Versions/2.6/...
+    #         lib/python2.6/logging/__init__.py", line 761, in emit
+    #     self.stream.write(fs % msg.encode(self.stream.encoding))
+    # LookupError: unknown encoding: unknown
+
+    # Handles logging.WARNING and above.
+    error_handler = logging.StreamHandler(stream)
+    error_handler.setLevel(logging.WARNING)
+    formatter = logging.Formatter("%(levelname)s: %(message)s")
+    error_handler.setFormatter(formatter)
+
+    # Handles records strictly below logging.WARNING.
+    non_error_handler = logging.StreamHandler(stream)
+    non_error_filter = _LevelLoggingFilter(logging.WARNING)
+    non_error_handler.addFilter(non_error_filter)
+    formatter = logging.Formatter("%(message)s")
+    non_error_handler.setFormatter(formatter)
+
+    logger = logging.getLogger()
+    logger.setLevel(logging.INFO)
+
+    handlers = [error_handler, non_error_handler]
+
+    for handler in handlers:
+        logger.addHandler(handler)
+
+    return handlers
+
+
+# FIXME: Consider moving this class into a module in webkitpy.init after
+#        getting more experience with its use.  We want to make sure
+#        we have the right API before doing so.  For example, we may
+#        want to provide a constructor that has both upper and lower
+#        bounds, and not just an upper bound.
+class _LevelLoggingFilter(object):
+
+    """A logging filter for blocking records at or above a certain level."""
+
+    def __init__(self, logging_level):
+        """Create a _LevelLoggingFilter.
+
+        Args:
+          logging_level: The logging level cut-off.  Logging levels at
+                         or above this level will not be logged.
+
+        """
+        self._logging_level = logging_level
+
+    # The logging module requires that this method be defined.
+    def filter(self, log_record):
+        """Return whether given the LogRecord should be logged."""
+        return log_record.levelno < self._logging_level
+
+
 # Enum-like idiom
 class FileType:
 
@@ -322,6 +406,8 @@ class ProcessorDispatcher(object):
         return processor
 
 
+# FIXME: Remove the stderr_write attribute from this class and replace
+#        its use with calls to a logging module logger.
 class StyleCheckerConfiguration(object):
 
     """Stores configuration values for the StyleChecker class.
@@ -439,9 +525,6 @@ class StyleChecker(object):
         self.error_count = 0
         self.file_count = 0
 
-    def _stderr_write(self, message):
-        self._configuration.stderr_write(message)
-
     def _increment_error_count(self):
         """Increment the total count of reported errors."""
         self.error_count += 1
@@ -469,11 +552,15 @@ class StyleChecker(object):
             contents = file.read()
 
         except IOError:
-            self._stderr_write("Skipping input '%s': Can't open for reading\n" % file_path)
+            message = 'Could not read file. Skipping: "%s"' % file_path
+            _log.warn(message)
             return
 
         lines = contents.split("\n")
 
+        # FIXME: Make a CarriageReturnProcessor for this logic, and put
+        #        it in processors.common.  The process() method should
+        #        return the lines with the carriage returns stripped.
         for line_number in range(len(lines)):
             # FIXME: We should probably use the SVN "eol-style" property
             #        or a white list to decide whether or not to do
@@ -520,8 +607,8 @@ class StyleChecker(object):
         if dispatcher.should_skip_without_warning(file_path):
             return
         if dispatcher.should_skip_with_warning(file_path):
-            self._stderr_write('Ignoring "%s": this file is exempt from the '
-                               "style guide.\n" % file_path)
+            _log.warn('File exempt from style guide. Skipping: "%s"'
+                      % file_path)
             return
 
         verbosity = self._configuration.verbosity
diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
index ae8cccc..d866663 100755
--- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py
@@ -34,15 +34,19 @@
 
 """Unit tests for style.py."""
 
+import logging
 import unittest
 
 import checker as style
+from ..style_references import UnitTestLog
+from ..style_references import UnitTestLogStream
 from checker import _BASE_FILTER_RULES
 from checker import _MAX_REPORTS_PER_CATEGORY
 from checker import _PATH_RULES_SPECIFIER as PATH_RULES_SPECIFIER
 from checker import _all_categories
 from checker import check_webkit_style_configuration
 from checker import check_webkit_style_parser
+from checker import configure_logging
 from checker import ProcessorDispatcher
 from checker import StyleChecker
 from checker import StyleCheckerConfiguration
@@ -54,6 +58,58 @@ from processors.cpp import CppProcessor
 from processors.text import TextProcessor
 
 
+class ConfigureLoggingTest(unittest.TestCase):
+
+    """Tests the configure_logging() function."""
+
+    def setUp(self):
+        log_stream = UnitTestLogStream()
+
+        self._handlers = configure_logging(log_stream)
+        self._log_stream = log_stream
+
+    def tearDown(self):
+        """Reset logging to its original state.
+
+        This method ensures that the logging configuration set up
+        for a unit test does not affect logging in other unit tests.
+
+        """
+        # This should be the same as the logger configured in the
+        # configure_logging() method.
+        logger = logging.getLogger()
+        for handler in self._handlers:
+            logger.removeHandler(handler)
+
+    def _log(self):
+        return logging.getLogger("webkitpy")
+
+    def assert_log_messages(self, messages):
+        """Assert that the logged messages equal the given messages."""
+        self.assertEquals(messages, self._log_stream.messages)
+
+    def test_warning_message(self):
+        self._log().warn("test message")
+        self.assert_log_messages(["WARNING: test message\n"])
+
+    def test_below_warning_message(self):
+        # We test the boundary case of a logging level equal to 29.
+        # In practice, we will probably only be calling log.info(),
+        # which corresponds to a logging level of 20.
+        level = logging.WARNING - 1  # Equals 29.
+        self._log().log(level, "test message")
+        self.assert_log_messages(["test message\n"])
+
+    def test_debug_message(self):
+        self._log().debug("test message")
+        self.assert_log_messages([])
+
+    def test_two_messages(self):
+        self._log().info("message1")
+        self._log().info("message2")
+        self.assert_log_messages(["message1\n", "message2\n"])
+
+
 class GlobalVariablesTest(unittest.TestCase):
 
     """Tests validity of the global variables."""
@@ -446,11 +502,15 @@ class StyleCheckerCheckFileTest(unittest.TestCase):
 
     """
     def setUp(self):
+        self._log = UnitTestLog(self)
         self.got_file_path = None
         self.got_handle_style_error = None
         self.got_processor = None
         self.warning_messages = ""
 
+    def tearDown(self):
+        self._log.tearDown()
+
     def mock_stderr_write(self, warning_message):
         self.warning_messages += warning_message
 
@@ -523,8 +583,9 @@ class StyleCheckerCheckFileTest(unittest.TestCase):
 
         # Check the outcome.
         self.call_check_file(file_path)
-        self.assert_attributes(None, None, None,
-                               'Ignoring "gtk2drawing.c": this file is exempt from the style guide.\n')
+        self.assert_attributes(None, None, None, "")
+        self._log.assertMessages(["WARNING: File exempt from style guide. "
+                                  'Skipping: "gtk2drawing.c"\n'])
 
     def test_check_file_on_non_skipped(self):
 
diff --git a/WebKitTools/Scripts/webkitpy/style_references.py b/WebKitTools/Scripts/webkitpy/style_references.py
index 2528c4d..cb8a985 100644
--- a/WebKitTools/Scripts/webkitpy/style_references.py
+++ b/WebKitTools/Scripts/webkitpy/style_references.py
@@ -41,6 +41,8 @@
 import os
 
 from diff_parser import DiffParser
+from init.logtesting import UnitTestLog
+from init.logtesting import UnitTestLogStream
 from scm import detect_scm_system
 
 

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list