2010-04-11 Chris Jerdonek <cjerdonek@webkit.org>

        Reviewed by Shinichiro Hamaji.

        Refactored check-webkit-style so that the StyleChecker class
        has no dependencies on patch-related concepts.

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

        This patch is an intermediate step towards making the StyleChecker
        class a generalized file processor that can do arbitary operations
        on the files corresponding to a list of paths.  This patch
        also simplifies the unit-testing of patch-checking code.

        * Scripts/check-webkit-style:
          - Updated to use the new PatchChecker class.

        * Scripts/webkitpy/style/checker.py:
          - Refactored the StyleChecker.check_patch() method into the
            check() method of a new PatchChecker class.

        * Scripts/webkitpy/style/checker_unittest.py:
          - Refactored the unit tests as necessary, changing the
            StyleCheckerCheckPatchTest class to a PatchCheckerTest class.

Canonical link: https://commits.webkit.org/48755@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@57467 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog

@@ -1,3 +1,28 @@
+2010-04-11  Chris Jerdonek  <cjerdonek@webkit.org>
+
+        Reviewed by Shinichiro Hamaji.
+
+        Refactored check-webkit-style so that the StyleChecker class
+        has no dependencies on patch-related concepts.
+
+        https://bugs.webkit.org/show_bug.cgi?id=37065
+
+        This patch is an intermediate step towards making the StyleChecker
+        class a generalized file processor that can do arbitary operations
+        on the files corresponding to a list of paths.  This patch
+        also simplifies the unit-testing of patch-checking code.
+
+        * Scripts/check-webkit-style:
+          - Updated to use the new PatchChecker class.
+
+        * Scripts/webkitpy/style/checker.py:
+          - Refactored the StyleChecker.check_patch() method into the
+            check() method of a new PatchChecker class.
+
+        * Scripts/webkitpy/style/checker_unittest.py:
+          - Refactored the unit tests as necessary, changing the
+            StyleCheckerCheckPatchTest class to a PatchCheckerTest class.
+
 2010-04-11  Adam Barth  <abarth@webkit.org>
 
         Unreviewed.

WebKitTools/Scripts/check-webkit-style

@@ -49,8 +49,9 @@ import os.path
 import sys
 
 from webkitpy.style_references import detect_checkout
-from webkitpy.style.main import change_directory
 import webkitpy.style.checker as checker
+from webkitpy.style.checker import PatchChecker
+from webkitpy.style.main import change_directory
 
 _log = logging.getLogger("check-webkit-style")
 
@@ -113,7 +114,8 @@ def main():
             patch = checkout.create_patch_since_local_commit(options.git_commit)
         else:
             patch = checkout.create_patch()
-        style_checker.check_patch(patch)
+        patch_checker = PatchChecker(style_checker)
+        patch_checker.check(patch)
 
     error_count = style_checker.error_count
     file_count = style_checker.file_count

WebKitTools/Scripts/webkitpy/style/checker.py

@@ -707,31 +707,35 @@ def check_file(self, file_path, line_numbers=None,
 
         process_file(processor, file_path, handle_style_error)
 
-    # FIXME: Eliminate this method and move its logic to style/main.py.
-    #        Calls to check_patch() can be replaced by appropriate calls
-    #        to check_file() using the optional line_numbers parameter.
-    def check_patch(self, patch_string, mock_check_file=None):
-        """Check style in the given patch.
+
+class PatchChecker(object):
+
+    """Supports checking style in patches."""
+
+    def __init__(self, style_checker):
+        """Create a PatchChecker instance.
 
         Args:
-          patch_string: A string that is a patch string.
+          style_checker: A StyleChecker instance.
 
         """
-        check_file = (self.check_file if mock_check_file is None else
-                      mock_check_file)
+        self._file_checker = style_checker
 
+    def check(self, patch_string):
+        """Check style in the given patch."""
         patch_files = parse_patch(patch_string)
 
         # The diff variable is a DiffFile instance.
-        for file_path, diff in patch_files.iteritems():
+        for path, diff in patch_files.iteritems():
             line_numbers = set()
             for line in diff.lines:
                 # When deleted line is not set, it means that
                 # the line is newly added (or modified).
                 if not line[0]:
                     line_numbers.add(line[1])
 
-            _log.debug('Found %s modified lines in patch for: %s'
-                       % (len(line_numbers), file_path))
+            _log.debug('Found %s new or modified lines in: %s'
+                       % (len(line_numbers), path))
 
-            check_file(file_path=file_path, line_numbers=line_numbers)
+            self._file_checker.check_file(file_path=path,
+                                          line_numbers=line_numbers)

WebKitTools/Scripts/webkitpy/style/checker_unittest.py

@@ -50,6 +50,7 @@
 from checker import check_webkit_style_parser
 from checker import configure_logging
 from checker import ProcessorDispatcher
+from checker import PatchChecker
 from checker import StyleChecker
 from checker import StyleCheckerConfiguration
 from filter import validate_filter_rules
@@ -698,61 +699,6 @@ def test_check_file_on_non_skipped(self):
                                "")
 
 
-class StyleCheckerCheckPatchTest(StyleCheckerCheckFileBase):
-
-    """Test the check_patch() method of the StyleChecker class.
-
-    Internally, the check_patch() method calls StyleChecker.check_file() for
-    each file that appears in the patch string.  This class passes a mock
-    check_file() method to check_patch() to facilitate unit-testing.  The
-    "got_*" attributes of this class are the parameters that check_patch()
-    passed to check_file().  (We test only a single call.)  These attributes
-    let us check that check_patch() is calling check_file() correctly.
-
-    Attributes:
-      got_file_path: The value that check_patch() passed as the file_path
-                     parameter to the mock_check_file() function.
-      got_line_numbers: The value that check_patch() passed as the line_numbers
-                        parameter to the mock_check_file() function.
-
-    """
-
-    _file_path = "__init__.py"
-
-    # The modified line_numbers array for this patch is: [2].
-    _patch_string = """diff --git a/__init__.py b/__init__.py
-index ef65bee..e3db70e 100644
---- a/__init__.py
-+++ b/__init__.py
-@@ -1,1 +1,2 @@
- # Required for Python to search this directory for module files
-+# New line
-"""
-
-    def setUp(self):
-        StyleCheckerCheckFileBase.setUp(self)
-        self._got_file_path = None
-        self._got_line_numbers = None
-
-    def _mock_check_file(self, file_path, line_numbers):
-        self._got_file_path = file_path
-        self._got_line_numbers = line_numbers
-
-    def test_check_patch(self):
-        patch_files = parse_patch(self._patch_string)
-        diff = patch_files[self._file_path]
-
-        configuration = self._style_checker_configuration()
-
-        style_checker = StyleChecker(configuration)
-
-        style_checker.check_patch(patch_string=self._patch_string,
-                                  mock_check_file=self._mock_check_file)
-
-        self.assertEquals(self._got_file_path, "__init__.py")
-        self.assertEquals(self._got_line_numbers, set([2]))
-
-
 class StyleCheckerCheckPathsTest(unittest.TestCase):
 
     """Test the check_paths() method of the StyleChecker class."""
@@ -799,3 +745,40 @@ def test_check_paths(self):
                            os.path.join("dir_path1", "file1"),
                            os.path.join("dir_path1", "file2"),
                            os.path.join("dir_path2", "file3")])
+
+
+class PatchCheckerTest(unittest.TestCase):
+
+    """Test the PatchChecker class."""
+
+    class MockStyleChecker(object):
+
+        def __init__(self):
+            self.checked_files = []
+            """A list of (file_path, line_numbers) pairs."""
+
+        def check_file(self, file_path, line_numbers):
+            self.checked_files.append((file_path, line_numbers))
+
+    def setUp(self):
+        style_checker = self.MockStyleChecker()
+        self._style_checker = style_checker
+        self._patch_checker = PatchChecker(style_checker)
+
+    def _call_check_patch(self, patch_string):
+        self._patch_checker.check(patch_string)
+
+    def _assert_checked(self, checked_files):
+        self.assertEquals(self._style_checker.checked_files, checked_files)
+
+    def test_check_patch(self):
+        # The modified line_numbers array for this patch is: [2].
+        self._call_check_patch("""diff --git a/__init__.py b/__init__.py
+index ef65bee..e3db70e 100644
+--- a/__init__.py
++++ b/__init__.py
+@@ -1,1 +1,2 @@
+ # Required for Python to search this directory for module files
++# New line
+""")
+        self._assert_checked([("__init__.py", set([2]))])