Changeset 57066 in webkit


Ignore:
Timestamp:
Apr 5, 2010 12:41:03 AM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-05 Chris Jerdonek <Chris Jerdonek>

Reviewed by Shinichiro Hamaji.

Removed the PatchStyleErrorHandler class and incorporated its
functionality into the DefaultStyleErrorHandler class.

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

  • Scripts/webkitpy/style/checker.py:
    • In the StyleChecker class:
      • Added a line_number parameter to the check_file() method.
      • Renamed the handle_style_error parameter to mock_handle_style_error to be consistent with the other mock_* parameter names.
      • Added a mock_check_file parameter to the check_patch() method to facilitate unit testing the changes in this patch.
      • Rewrote the check_patch() method with the patch-parsing logic taken from the PatchStyleErrorHandler class.
  • Scripts/webkitpy/style/checker_unittest.py:
    • Added a StyleCheckerCheckFileBase class and sub-classed the existing StyleCheckerCheckFileTest class from it.
    • Added a StyleCheckerCheckPatchTest class to unit-test the rewritten check_patch() method.
    • Removed the vestigial main code at the bottom of the file. This is left over from when check-webkit-style was implemented as a module and a wrapper module.
  • Scripts/webkitpy/style/error_handlers.py:
    • Added an optional line_numbers parameter to the DefaultStyleErrorHandler class constructor and adjusted the call() method as necessary.
    • Removed the PatchStyleErrorHandler class.
  • Scripts/webkitpy/style/error_handlers_unittest.py:
    • Removed the PatchStyleErrorHandlerTest class which unit-tested the PatchStyleErrorHandler class which is being removed in this patch.
    • Added a test_line_numbers() test method to the DefaultStyleErrorHandlerTest class to test use of the DefaultStyleErrorHandler's new line_numbers attribute.
Location:
trunk/WebKitTools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r57065 r57066  
     12010-04-05  Chris Jerdonek  <cjerdonek@webkit.org>
     2
     3        Reviewed by Shinichiro Hamaji.
     4
     5        Removed the PatchStyleErrorHandler class and incorporated its
     6        functionality into the DefaultStyleErrorHandler class.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=37067
     9
     10        * Scripts/webkitpy/style/checker.py:
     11          - In the StyleChecker class:
     12            - Added a line_number parameter to the check_file() method.
     13            - Renamed the handle_style_error parameter to
     14              mock_handle_style_error to be consistent with the other mock_*
     15              parameter names.
     16            - Added a mock_check_file parameter to the check_patch() method
     17              to facilitate unit testing the changes in this patch.
     18            - Rewrote the check_patch() method with the patch-parsing logic
     19              taken from the PatchStyleErrorHandler class.
     20
     21        * Scripts/webkitpy/style/checker_unittest.py:
     22          - Added a StyleCheckerCheckFileBase class and sub-classed the
     23            existing StyleCheckerCheckFileTest class from it.
     24          - Added a StyleCheckerCheckPatchTest class to unit-test the
     25            rewritten check_patch() method.
     26          - Removed the vestigial __main__ code at the bottom of the file.
     27            This is left over from when check-webkit-style was implemented
     28            as a module and a wrapper module.
     29
     30        * Scripts/webkitpy/style/error_handlers.py:
     31          - Added an optional line_numbers parameter to the
     32            DefaultStyleErrorHandler class constructor and adjusted the
     33            __call__() method as necessary.
     34          - Removed the PatchStyleErrorHandler class.
     35
     36        * Scripts/webkitpy/style/error_handlers_unittest.py:
     37          - Removed the PatchStyleErrorHandlerTest class which unit-tested
     38            the PatchStyleErrorHandler class which is being removed in this
     39            patch.
     40          - Added a test_line_numbers() test method to the
     41            DefaultStyleErrorHandlerTest class to test use of the
     42            DefaultStyleErrorHandler's new line_numbers attribute.
     43
    1442010-04-05  Adam Barth  <abarth@webkit.org>
    245

  • trunk/WebKitTools/Scripts/webkitpy/style/checker.py

    r57048 r57066  
    3737
    3838from error_handlers import DefaultStyleErrorHandler
    39 from error_handlers import PatchStyleErrorHandler
    4039from filter import FilterConfiguration
    4140from optparser import ArgumentParser
    … …  
    637636                check_file(file_path)
    638637
    639     def check_file(self, file_path, handle_style_error=None,
     638    def check_file(self, file_path, line_numbers=None,
     639                   mock_handle_style_error=None,
    640640                   mock_os_path_exists=None,
    641641                   mock_process_file=None):
    … …  
    646646                     should be relative to the source root.  Otherwise,
    647647                     path-specific logic may not behave as expected.
    648           handle_style_error: The function to call when a style error
    649                               occurs. This parameter is meant for internal
    650                               use within this class. Defaults to a
    651                               DefaultStyleErrorHandler instance.
     648          line_numbers: An array of line numbers of the lines for which
     649                        style errors should be reported, or None if errors
     650                        for all lines should be reported.  Normally, this
     651                        array contains the line numbers corresponding to the
     652                        modified lines of a patch.
     653          mock_handle_style_error: A unit-testing replacement for the function
     654                                   to call when a style error occurs. Defaults
     655                                   to a DefaultStyleErrorHandler instance.
    652656          mock_os_path_exists: A unit-test replacement for os.path.exists.
    653657                               This parameter should only be used for unit
    … …  
    659663
    660664        """
     665        if mock_handle_style_error is None:
     666            increment = self._increment_error_count
     667            handle_style_error = DefaultStyleErrorHandler(
     668                                     configuration=self._configuration,
     669                                     file_path=file_path,
     670                                     increment_error_count=increment,
     671                                     line_numbers=line_numbers)
     672        else:
     673            handle_style_error = mock_handle_style_error
     674
    661675        os_path_exists = (os.path.exists if mock_os_path_exists is None else
    662676                          mock_os_path_exists)
    … …  
    669683
    670684        _log.debug("Checking: " + file_path)
    671         if handle_style_error is None:
    672             handle_style_error = DefaultStyleErrorHandler(
    673                                      configuration=self._configuration,
    674                                      file_path=file_path,
    675                                      increment_error_count=
    676                                          self._increment_error_count)
     685
    677686        self.file_count += 1
    678687
    … …  
    695704            return
    696705
    697 
    698706        _log.debug("Using class: " + processor.__class__.__name__)
    699707
    700708        process_file(processor, file_path, handle_style_error)
    701709
    702     def check_patch(self, patch_string):
     710    # FIXME: Eliminate this method and move its logic to style/main.py.
     711    #        Calls to check_patch() can be replaced by appropriate calls
     712    #        to check_file() using the optional line_numbers parameter.
     713    def check_patch(self, patch_string, mock_check_file=None):
    703714        """Check style in the given patch.
    704715
    … …  
    707718
    708719        """
     720        check_file = (self.check_file if mock_check_file is None else
     721                      mock_check_file)
     722
    709723        patch_files = parse_patch(patch_string)
     724
     725        # The diff variable is a DiffFile instance.
    710726        for file_path, diff in patch_files.iteritems():
    711             style_error_handler = PatchStyleErrorHandler(diff,
    712                                       file_path,
    713                                       self._configuration,
    714                                       self._increment_error_count)
    715 
    716             self.check_file(file_path, style_error_handler)
     727            line_numbers = set()
     728            for line in diff.lines:
     729                # When deleted line is not set, it means that
     730                # the line is newly added (or modified).
     731                if not line[0]:
     732                    line_numbers.add(line[1])
     733
     734            _log.debug('Found %s modified lines in patch for: %s'
     735                       % (len(line_numbers), file_path))
     736
     737            check_file(file_path=file_path, line_numbers=line_numbers)

  • trunk/WebKitTools/Scripts/webkitpy/style/checker_unittest.py

    r57056 r57066  
    4040
    4141import checker as style
     42from webkitpy.style_references import parse_patch
    4243from webkitpy.style_references import LogTesting
    4344from webkitpy.style_references import TestLogStream
    … …  
    537538
    538539
    539 class StyleCheckerCheckFileTest(LoggingTestCase):
     540class StyleCheckerCheckFileBase(LoggingTestCase):
     541
     542    def setUp(self):
     543        LoggingTestCase.setUp(self)
     544        self.warning_messages = ""
     545
     546    def mock_stderr_write(self, warning_message):
     547        self.warning_messages += warning_message
     548
     549    def _style_checker_configuration(self):
     550        return StyleCheckerConfiguration(
     551            filter_configuration=FilterConfiguration(),
     552            max_reports_per_category={"whitespace/newline": 1},
     553            min_confidence=3,
     554            output_format="vs7",
     555            stderr_write=self.mock_stderr_write)
     556
     557
     558class StyleCheckerCheckFileTest(StyleCheckerCheckFileBase):
    540559
    541560    """Test the check_file() method of the StyleChecker class.
    … …  
    563582    """
    564583    def setUp(self):
    565         LoggingTestCase.setUp(self)
     584        StyleCheckerCheckFileBase.setUp(self)
    566585        self.got_file_path = None
    567586        self.got_handle_style_error = None
    568587        self.got_processor = None
    569         self.warning_messages = ""
    570 
    571     def mock_stderr_write(self, warning_message):
    572         self.warning_messages += warning_message
    573588
    574589    def mock_handle_style_error(self):
    … …  
    607622        self.assert_attributes(None, None, None, "")
    608623
    609         configuration = StyleCheckerConfiguration(
    610                             filter_configuration=FilterConfiguration(),
    611                             max_reports_per_category={"whitespace/newline": 1},
    612                             min_confidence=3,
    613                             output_format="vs7",
    614                             stderr_write=self.mock_stderr_write)
     624        configuration = self._style_checker_configuration()
    615625
    616626        style_checker = StyleChecker(configuration)
    617627
    618628        style_checker.check_file(file_path=file_path,
    619             handle_style_error=self.mock_handle_style_error,
     629            mock_handle_style_error=self.mock_handle_style_error,
    620630            mock_os_path_exists=self.mock_os_path_exists,
    621631            mock_process_file=self.mock_process_file)
    … …  
    686696                               expected_processor,
    687697                               "")
     698
     699
     700class StyleCheckerCheckPatchTest(StyleCheckerCheckFileBase):
     701
     702    """Test the check_patch() method of the StyleChecker class.
     703
     704    Internally, the check_patch() method calls StyleChecker.check_file() for
     705    each file that appears in the patch string.  This class passes a mock
     706    check_file() method to check_patch() to facilitate unit-testing.  The
     707    "got_*" attributes of this class are the parameters that check_patch()
     708    passed to check_file().  (We test only a single call.)  These attributes
     709    let us check that check_patch() is calling check_file() correctly.
     710
     711    Attributes:
     712      got_file_path: The value that check_patch() passed as the file_path
     713                     parameter to the mock_check_file() function.
     714      got_line_numbers: The value that check_patch() passed as the line_numbers
     715                        parameter to the mock_check_file() function.
     716
     717    """
     718
     719    _file_path = "__init__.py"
     720
     721    # The modified line_numbers array for this patch is: [2].
     722    _patch_string = """diff --git a/__init__.py b/__init__.py
     723index ef65bee..e3db70e 100644
     724--- a/__init__.py
     725+++ b/__init__.py
     726@@ -1,1 +1,2 @@
     727 # Required for Python to search this directory for module files
     728+# New line
     729"""
     730
     731    def setUp(self):
     732        StyleCheckerCheckFileBase.setUp(self)
     733        self._got_file_path = None
     734        self._got_line_numbers = None
     735
     736    def _mock_check_file(self, file_path, line_numbers):
     737        self._got_file_path = file_path
     738        self._got_line_numbers = line_numbers
     739
     740    def test_check_patch(self):
     741        patch_files = parse_patch(self._patch_string)
     742        diff = patch_files[self._file_path]
     743
     744        configuration = self._style_checker_configuration()
     745
     746        style_checker = StyleChecker(configuration)
     747
     748        style_checker.check_patch(patch_string=self._patch_string,
     749                                  mock_check_file=self._mock_check_file)
     750
     751        self.assertEquals(self._got_file_path, "__init__.py")
     752        self.assertEquals(self._got_line_numbers, set([2]))
    688753
    689754
    … …  
    734799                           os.path.join("dir_path1", "file2"),
    735800                           os.path.join("dir_path2", "file3")])
    736 
    737 
    738 if __name__ == '__main__':
    739     import sys
    740 
    741     unittest.main()

  • trunk/WebKitTools/Scripts/webkitpy/style/error_handlers.py

    r56692 r57066  
    5757    """The default style error handler."""
    5858
    59     def __init__(self, file_path, configuration, increment_error_count):
     59    def __init__(self, file_path, configuration, increment_error_count,
     60                 line_numbers=None):
    6061        """Create a default style error handler.
    6162
    … …  
    6768                                 increments the total count of reportable
    6869                                 errors.
     70          line_numbers: An array of line numbers of the lines for which
     71                        style errors should be reported, or None if errors
     72                        for all lines should be reported.  When it is not
     73                        None, this array normally contains the line numbers
     74                        corresponding to the modified lines of a patch.
    6975
    7076        """
     77        if line_numbers is not None:
     78            line_numbers = set(line_numbers)
     79
    7180        self._file_path = file_path
    7281        self._configuration = configuration
    7382        self._increment_error_count = increment_error_count
     83        self._line_numbers = line_numbers
    7484
    7585        # A string to integer dictionary cache of the number of reportable
    … …  
    101111
    102112        """
     113        if (self._line_numbers is not None and
     114            line_number not in self._line_numbers):
     115            # Then the error occurred in a line that was not modified, so
     116            # the error is not reportable.
     117            return
     118
    103119        if not self._configuration.is_reportable(category=category,
    104120                                                 confidence_in_error=confidence,
    … …  
    123139            self._configuration.stderr_write("Suppressing further [%s] reports "
    124140                                             "for this file.\n" % category)
    125 
    126 
    127 class PatchStyleErrorHandler(object):
    128 
    129     """The style error function for patch files."""
    130 
    131     def __init__(self, diff, file_path, configuration, increment_error_count):
    132         """Create a patch style error handler for the given path.
    133 
    134         Args:
    135           diff: A DiffFile instance.
    136           Other arguments: see the DefaultStyleErrorHandler.__init__()
    137                            documentation for the other arguments.
    138 
    139         """
    140         self._diff = diff
    141 
    142         self._default_error_handler = DefaultStyleErrorHandler(
    143                                           configuration=configuration,
    144                                           file_path=file_path,
    145                                           increment_error_count=
    146                                               increment_error_count)
    147 
    148         # The line numbers of the modified lines. This is set lazily.
    149         self._line_numbers = set()
    150 
    151     def _get_line_numbers(self):
    152         """Return the line numbers of the modified lines."""
    153         if not self._line_numbers:
    154             for line in self._diff.lines:
    155                 # When deleted line is not set, it means that
    156                 # the line is newly added (or modified).
    157                 if not line[0]:
    158                     self._line_numbers.add(line[1])
    159 
    160         return self._line_numbers
    161 
    162     def __call__(self, line_number, category, confidence, message):
    163         """Handle the occurrence of a style error.
    164 
    165         This function does not report errors occurring in lines not
    166         marked as modified or added in the patch.
    167 
    168         See the docstring of this module for more information.
    169 
    170         """
    171         if line_number not in self._get_line_numbers():
    172             # Then the error is not reportable.
    173             return
    174 
    175         self._default_error_handler(line_number, category, confidence,
    176                                     message)
    177 

  • trunk/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py

    r56692 r57066  
    2626import unittest
    2727
    28 from .. style_references import parse_patch
    2928from checker import StyleCheckerConfiguration
    3029from error_handlers import DefaultStyleErrorHandler
    31 from error_handlers import PatchStyleErrorHandler
    3230from filter import FilterConfiguration
    3331
    … …  
    7270        self.assertEquals(0, len(self._error_messages))
    7371
    74     def _error_handler(self, configuration):
     72    def _error_handler(self, configuration, line_numbers=None):
    7573        return DefaultStyleErrorHandler(configuration=configuration,
    7674                   file_path=self._file_path,
    77                    increment_error_count=self._mock_increment_error_count)
     75                   increment_error_count=self._mock_increment_error_count,
     76                   line_numbers=line_numbers)
    7877
    79     def _call_error_handler(self, handle_error, confidence):
     78    def _call_error_handler(self, handle_error, confidence, line_number=100):
    8079        """Call the given error handler with a test error."""
    81         handle_error(line_number=100,
     80        handle_error(line_number=line_number,
    8281                     category=self._category,
    8382                     confidence=confidence,
    … …  
    133132        self.assertEquals(3, len(self._error_messages))
    134133
     134    def test_line_numbers(self):
     135        """Test the line_numbers parameter."""
     136        self._check_initialized()
     137        configuration = self._style_checker_configuration()
     138        error_handler = self._error_handler(configuration,
     139                                            line_numbers=[50])
     140        confidence = 5
    135141
    136 class PatchStyleErrorHandlerTest(StyleErrorHandlerTestBase):
     142        # Error on non-modified line: no error.
     143        self._call_error_handler(error_handler, confidence, line_number=60)
     144        self.assertEquals(0, self._error_count)
     145        self.assertEquals([], self._error_messages)
    137146
    138     """Tests PatchStyleErrorHandler class."""
    139 
    140     _file_path = "__init__.py"
    141 
    142     _patch_string = """diff --git a/__init__.py b/__init__.py
    143 index ef65bee..e3db70e 100644
    144 --- a/__init__.py
    145 +++ b/__init__.py
    146 @@ -1 +1,2 @@
    147  # Required for Python to search this directory for module files
    148 +# New line
    149 
    150 """
    151 
    152     def test_call(self):
    153         patch_files = parse_patch(self._patch_string)
    154         diff = patch_files[self._file_path]
    155 
    156         configuration = self._style_checker_configuration()
    157 
    158         handle_error = PatchStyleErrorHandler(diff=diff,
    159                                               file_path=self._file_path,
    160                                               configuration=configuration,
    161                                               increment_error_count=
    162                                               self._mock_increment_error_count)
    163 
    164         category = "whitespace/tab"
    165         confidence = 5
    166         message = "message"
    167 
    168         # Confirm error is reportable.
    169         self.assertTrue(configuration.is_reportable(category,
    170                                                     confidence,
    171                                                     self._file_path))
    172 
    173         # Confirm error count initialized to zero.
    174         self.assertEquals(0, self._error_count)
    175 
    176         # Test error in unmodified line (error count does not increment).
    177         handle_error(1, category, confidence, message)
    178         self.assertEquals(0, self._error_count)
    179 
    180         # Test error in modified line (error count increments).
    181         handle_error(2, category, confidence, message)
     147        # Error on modified line: error.
     148        self._call_error_handler(error_handler, confidence, line_number=50)
    182149        self.assertEquals(1, self._error_count)
    183 
     150        self.assertEquals(self._error_messages,
     151                          ["foo.h(50):  message  [whitespace/tab] [5]\n"])
Note: See TracChangeset for help on using the changeset viewer.