unify helper vars across all sections am: b960818021 am: 90f1f4558c am: 9f6c447004

am: 17fc41882e

Change-Id: I8d3e074e58b14da099ffa527e9d561a838ccd512

6 files changed, 535 insertions, 41 deletions

diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 2565503..e82319c 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -1,6 +1,7 @@
 [Hook Scripts]
 # Only list fast unittests here.
 config_unittest = ./rh/config_unittest.py
+hooks_unittest  = ./rh/hooks_unittest.py
 shell_unittest  = ./rh/shell_unittest.py
 
 [Builtin Hooks]
diff --git a/README.md b/README.md
index d656ef3..c4c5dd9 100644
--- a/README.md
+++ b/README.md
@@ -87,6 +87,24 @@ A few environment variables are set so scripts don't need to discover things.
 * `PREUPLOAD_COMMIT`: The commit that is currently being checked.
    e.g. `1f89dce0468448fa36f632d2fc52175cd6940a91`
 
+## Placeholders
+
+A few keywords are recognized to pass down settings.  These are **not**
+environment variables, but are expanded inline.  Files with whitespace and
+such will be expanded correctly via argument positions, so do not try to
+force your own quote handling.
+
+* `${PREUPLOAD_FILES}`: List of files to operate on.
+* `${PREUPLOAD_COMMIT}`: Commit hash.
+* `${PREUPLOAD_COMMIT_MESSAGE}`: Commit message.
+
+Some variables are available to make it easier to handle OS differences.  These
+are automatically expanded for you:
+
+* `${REPO_ROOT}`: The absolute path of the root of the repo checkout.
+* `${BUILD_OS}`: The string `darwin-x86` for macOS and the string `linux-x86`
+  for Linux/x86.
+
 ## [Options]
 
 This section allows for setting options that affect the overall behavior of the
@@ -150,14 +168,7 @@ override any existing default options, so be sure to include everything you need
 Quoting is handled naturally.  i.e. use `"a b c"` to pass an argument with
 whitespace.
 
-A few keywords are recognized to pass down settings.  These are **not**
-environment variables, but are expanded inline.  Files with whitespace and
-such will be expanded correctly via argument positions, so do not try to
-force your own quote handling.
-
-* `${PREUPLOAD_FILES}`: List of files to operate on.
-* `${PREUPLOAD_COMMIT}`: Commit hash.
-* `${PREUPLOAD_COMMIT_MESSAGE}`: Commit message.
+See [Placeholders](#Placeholders) for variables you can expand automatically.
 
 ```
 [Builtin Hooks Options]
@@ -179,12 +190,7 @@ distros/versions.  The following tools are recognized:
 * `gofmt`: used for the `gofmt` builtin hook.
 * `pylint`: used for the `pylint` builtin hook.
 
-Some variables are available to make it easier to handle OS differences.  These
-are automatically expanded for you:
-
-* `${REPO_ROOT}`: The absolute path of the root of the repo checkout.
-* `${BUILD_OS}`: The string `darwin-x86` for macOS and the string `linux-x86`
-  for Linux/x86.
+See [Placeholders](#Placeholders) for variables you can expand automatically.
 
 ```
 [Tool Paths]
diff --git a/pre-upload.py b/pre-upload.py
index cc8d24a..f15a9b7 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -23,7 +23,6 @@ when developing.
 from __future__ import print_function
 
 import argparse
-import collections
 import os
 import sys
 
@@ -38,6 +37,7 @@ if sys.path[0] != _path:
     sys.path.insert(0, _path)
 del _path
 
+import rh
 import rh.results
 import rh.config
 import rh.git
@@ -50,9 +50,6 @@ import rh.utils
 REPOHOOKS_URL = 'https://android.googlesource.com/platform/tools/repohooks/'
 
 
-Project = collections.namedtuple('Project', ['name', 'dir', 'remote'])
-
-
 class Output(object):
     """Class for reporting hook status."""
 
@@ -219,7 +216,7 @@ def _run_project_hooks(project_name, proj_dir=None,
     })
 
     output = Output(project_name, len(hooks))
-    project = Project(name=project_name, dir=proj_dir, remote=remote)
+    project = rh.Project(name=project_name, dir=proj_dir, remote=remote)
 
     if not commit_list:
         commit_list = rh.git.get_commits(
diff --git a/rh/__init__.py b/rh/__init__.py
index e69de29..c36cb89 100644
--- a/rh/__init__.py
+++ b/rh/__init__.py
@@ -0,0 +1,24 @@
+# -*- coding:utf-8 -*-
+# Copyright 2016 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Common repohook objects/constants."""
+
+from __future__ import print_function
+
+import collections
+
+
+# An object representing the git project that we're testing currently.
+Project = collections.namedtuple('Project', ['name', 'dir', 'remote'])
diff --git a/rh/hooks.py b/rh/hooks.py
index 1f59dd2..9612bf7 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -33,6 +33,100 @@ import rh.git
 import rh.utils
 
 
+class Placeholders(object):
+    """Holder class for replacing ${vars} in arg lists.
+
+    To add a new variable to replace in config files, just add it as a @property
+    to this class using the form.  So to add support for BIRD:
+      @property
+      def var_BIRD(self):
+        return <whatever this is>
+
+    You can return either a string or an iterable (e.g. a list or tuple).
+    """
+
+    def __init__(self, diff=()):
+        """Initialize.
+
+        Args:
+          diff: The list of files that changed.
+        """
+        self.diff = diff
+
+    def expand_vars(self, args):
+        """Perform place holder expansion on all of |args|.
+
+        Args:
+          args: The args to perform expansion on.
+
+        Returns:
+          The updated |args| list.
+        """
+        all_vars = set(self.vars())
+        replacements = dict((var, self.get(var)) for var in all_vars)
+
+        ret = []
+        for arg in args:
+            # First scan for exact matches
+            for key, val in replacements.items():
+                var = '${%s}' % (key,)
+                if arg == var:
+                    if isinstance(val, str):
+                        ret.append(val)
+                    else:
+                        ret.extend(val)
+                    # We break on first hit to avoid double expansion.
+                    break
+            else:
+                # If no exact matches, do an inline replacement.
+                def replace(m):
+                    val = self.get(m.group(1))
+                    if isinstance(val, str):
+                        return val
+                    else:
+                        return ' '.join(val)
+                ret.append(re.sub(r'\$\{(%s)\}' % ('|'.join(all_vars),),
+                                  replace, arg))
+
+        return ret
+
+    @classmethod
+    def vars(cls):
+        """Yield all replacement variable names."""
+        for key in dir(cls):
+            if key.startswith('var_'):
+                yield key[4:]
+
+    def get(self, var):
+        """Helper function to get the replacement |var| value."""
+        return getattr(self, 'var_%s' % (var,))
+
+    @property
+    def var_PREUPLOAD_COMMIT_MESSAGE(self):
+        """The git commit message."""
+        return os.environ.get('PREUPLOAD_COMMIT_MESSAGE', '')
+
+    @property
+    def var_PREUPLOAD_COMMIT(self):
+        """The git commit sha1."""
+        return os.environ.get('PREUPLOAD_COMMIT', '')
+
+    @property
+    def var_PREUPLOAD_FILES(self):
+        """List of files modified in this git commit."""
+        return [x.file for x in self.diff if x.status != 'D']
+
+    @property
+    def var_REPO_ROOT(self):
+        """The root of the repo checkout."""
+        return rh.git.find_repo_root()
+
+    @property
+    def var_BUILD_OS(self):
+        """The build OS (see _get_build_os_name for details)."""
+        return _get_build_os_name()
+
+
 class HookOptions(object):
     """Holder class for hook options."""
 
@@ -48,6 +142,12 @@ class HookOptions(object):
         self._args = args
         self._tool_paths = tool_paths
 
+    @staticmethod
+    def expand_vars(args, diff=()):
+        """Perform place holder expansion on all of |args|."""
+        replacer = Placeholders(diff=diff)
+        return replacer.expand_vars(args)
+
     def args(self, default_args=(), diff=()):
         """Gets the hook arguments, after performing place holder expansion.
 
@@ -62,18 +162,7 @@ class HookOptions(object):
         if not args:
             args = default_args
 
-        ret = []
-        for arg in args:
-            if arg == '${PREUPLOAD_FILES}':
-                ret.extend(x.file for x in diff if x.status != 'D')
-            elif arg == '${PREUPLOAD_COMMIT_MESSAGE}':
-                ret.append(os.environ['PREUPLOAD_COMMIT_MESSAGE'])
-            elif arg == '${PREUPLOAD_COMMIT}':
-                ret.append(os.environ['PREUPLOAD_COMMIT'])
-            else:
-                ret.append(arg)
-
-        return ret
+        return self.expand_vars(args, diff=diff)
 
     def tool_path(self, tool_name):
         """Gets the path in which the |tool_name| executable can be found.
@@ -92,17 +181,8 @@ class HookOptions(object):
         if tool_name not in self._tool_paths:
             return TOOL_PATHS[tool_name]
 
-        components = []
         tool_path = os.path.normpath(self._tool_paths[tool_name])
-        for component in tool_path.split(os.sep):
-            if component == '${REPO_ROOT}':
-                components.append(rh.git.find_repo_root())
-            elif component == '${BUILD_OS}':
-                components.append(_get_build_os_name())
-            else:
-                components.append(component)
-
-        return os.sep.join(components)
+        return self.expand_vars([tool_path])[0]
 
 
 def _run_command(cmd, **kwargs):
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
new file mode 100755
index 0000000..0b4ef40
--- /dev/null
+++ b/rh/hooks_unittest.py
@@ -0,0 +1,386 @@
+#!/usr/bin/python
+# -*- coding:utf-8 -*-
+# Copyright 2016 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the hooks module."""
+
+from __future__ import print_function
+
+import mock
+import os
+import sys
+import unittest
+
+_path = os.path.realpath(__file__ + '/../..')
+if sys.path[0] != _path:
+    sys.path.insert(0, _path)
+del _path
+
+import rh
+import rh.hooks
+import rh.config
+
+
+class HooksDocsTests(unittest.TestCase):
+    """Make sure all hook features are documented.
+
+    Note: These tests are a bit hokey in that they parse README.md.  But they
+    get the job done, so that's all that matters right?
+    """
+
+    def setUp(self):
+        self.readme = os.path.join(os.path.dirname(os.path.dirname(
+            os.path.realpath(__file__))), 'README.md')
+
+    def _grab_section(self, section):
+        """Extract the |section| text out of the readme."""
+        ret = []
+        in_section = False
+        for line in open(self.readme):
+            if not in_section:
+                # Look for the section like "## [Tool Paths]".
+                if line.startswith('#') and line.lstrip('#').strip() == section:
+                    in_section = True
+            else:
+                # Once we hit the next section (higher or lower), break.
+                if line[0] == '#':
+                    break
+                ret.append(line)
+        return ''.join(ret)
+
+    def testBuiltinHooks(self):
+        """Verify builtin hooks are documented."""
+        data = self._grab_section('[Builtin Hooks]')
+        for hook in rh.hooks.BUILTIN_HOOKS:
+            self.assertIn('* `%s`:' % (hook,), data,
+                          msg='README.md missing docs for hook "%s"' % (hook,))
+
+    def testToolPaths(self):
+        """Verify tools are documented."""
+        data = self._grab_section('[Tool Paths]')
+        for tool in rh.hooks.TOOL_PATHS:
+            self.assertIn('* `%s`:' % (tool,), data,
+                          msg='README.md missing docs for tool "%s"' % (tool,))
+
+    def testPlaceholders(self):
+        """Verify placeholder replacement vars are documented."""
+        data = self._grab_section('Placeholders')
+        for var in rh.hooks.Placeholders.vars():
+            self.assertIn('* `${%s}`:' % (var,), data,
+                          msg='README.md missing docs for var "%s"' % (var,))
+
+
+class PlaceholderTests(unittest.TestCase):
+    """Verify behavior of replacement variables."""
+
+    def setUp(self):
+        self._saved_environ = os.environ.copy()
+        os.environ.update({
+            'PREUPLOAD_COMMIT_MESSAGE': 'commit message',
+            'PREUPLOAD_COMMIT': '5c4c293174bb61f0f39035a71acd9084abfa743d',
+        })
+        self.replacer = rh.hooks.Placeholders()
+
+    def tearDown(self):
+        os.environ.clear()
+        os.environ.update(self._saved_environ)
+
+    def testVars(self):
+        """Light test for the vars inspection generator."""
+        ret = list(self.replacer.vars())
+        self.assertGreater(len(ret), 4)
+        self.assertIn('PREUPLOAD_COMMIT', ret)
+
+    @mock.patch.object(rh.git, 'find_repo_root', return_value='/ ${BUILD_OS}')
+    def testExpandVars(self, _m):
+        """Verify the replacement actually works."""
+        input_args = [
+            # Verify ${REPO_ROOT} is updated, but not REPO_ROOT.
+            # We also make sure that things in ${REPO_ROOT} are not double
+            # expanded (which is why the return includes ${BUILD_OS}).
+            '${REPO_ROOT}/some/prog/REPO_ROOT/ok',
+            # Verify lists are merged rather than inserted.  In this case, the
+            # list is empty, but we'd hit an error still if we saw [] in args.
+            '${PREUPLOAD_FILES}',
+            # Verify values with whitespace don't expand into multiple args.
+            '${PREUPLOAD_COMMIT_MESSAGE}',
+            # Verify multiple values get replaced.
+            '${PREUPLOAD_COMMIT}^${PREUPLOAD_COMMIT_MESSAGE}',
+            # Unknown vars should be left alone.
+            '${THIS_VAR_IS_GOOD}',
+        ]
+        output_args = self.replacer.expand_vars(input_args)
+        exp_args = [
+            '/ ${BUILD_OS}/some/prog/REPO_ROOT/ok',
+            'commit message',
+            '5c4c293174bb61f0f39035a71acd9084abfa743d^commit message',
+            '${THIS_VAR_IS_GOOD}',
+        ]
+        self.assertEqual(output_args, exp_args)
+
+    def testTheTester(self):
+        """Make sure we have a test for every variable."""
+        for var in self.replacer.vars():
+            self.assertIn('test%s' % (var,), dir(self),
+                          msg='Missing unittest for variable %s' % (var,))
+
+    def testPREUPLOAD_COMMIT_MESSAGE(self):
+        """Verify handling of PREUPLOAD_COMMIT_MESSAGE."""
+        self.assertEqual(self.replacer.get('PREUPLOAD_COMMIT_MESSAGE'),
+                         'commit message')
+
+    def testPREUPLOAD_COMMIT(self):
+        """Verify handling of PREUPLOAD_COMMIT."""
+        self.assertEqual(self.replacer.get('PREUPLOAD_COMMIT'),
+                         '5c4c293174bb61f0f39035a71acd9084abfa743d')
+
+    def testPREUPLOAD_FILES(self):
+        """Verify handling of PREUPLOAD_FILES."""
+        self.assertEqual(self.replacer.get('PREUPLOAD_FILES'), [])
+
+    @mock.patch.object(rh.git, 'find_repo_root', return_value='/repo!')
+    def testREPO_ROOT(self, m):
+        """Verify handling of REPO_ROOT."""
+        self.assertEqual(self.replacer.get('REPO_ROOT'), m.return_value)
+
+    @mock.patch.object(rh.hooks, '_get_build_os_name', return_value='vapier os')
+    def testBUILD_OS(self, m):
+        """Verify handling of BUILD_OS."""
+        self.assertEqual(self.replacer.get('BUILD_OS'), m.return_value)
+
+
+class HookOptionsTests(unittest.TestCase):
+    """Verify behavior of HookOptions object."""
+
+    @mock.patch.object(rh.hooks, '_get_build_os_name', return_value='vapier os')
+    def testExpandVars(self, m):
+        """Verify expand_vars behavior."""
+        # Simple pass through.
+        args = ['who', 'goes', 'there ?']
+        self.assertEqual(args, rh.hooks.HookOptions.expand_vars(args))
+
+        # At least one replacement.  Most real testing is in PlaceholderTests.
+        args = ['who', 'goes', 'there ?', '${BUILD_OS} is great']
+        exp_args = ['who', 'goes', 'there ?', '%s is great' % (m.return_value,)]
+        self.assertEqual(exp_args, rh.hooks.HookOptions.expand_vars(args))
+
+    def testArgs(self):
+        """Verify args behavior."""
+        # Verify initial args to __init__ has higher precedent.
+        args = ['start', 'args']
+        options = rh.hooks.HookOptions('hook name', args, {})
+        self.assertEqual(options.args(), args)
+        self.assertEqual(options.args(default_args=['moo']), args)
+
+        # Verify we fall back to default_args.
+        args = ['default', 'args']
+        options = rh.hooks.HookOptions('hook name', [], {})
+        self.assertEqual(options.args(), [])
+        self.assertEqual(options.args(default_args=args), args)
+
+    def testToolPath(self):
+        """Verify tool_path behavior."""
+        options = rh.hooks.HookOptions('hook name', [], {
+            'cpplint': 'my cpplint',
+        })
+        # Check a builtin (and not overridden) tool.
+        self.assertEqual(options.tool_path('pylint'), 'pylint')
+        # Check an overridden tool.
+        self.assertEqual(options.tool_path('cpplint'), 'my cpplint')
+        # Check an unknown tool fails.
+        self.assertRaises(AssertionError, options.tool_path, 'extra_tool')
+
+
+class UtilsTests(unittest.TestCase):
+    """Verify misc utility functions."""
+
+    def testRunCommand(self):
+        """Check _run_command behavior."""
+        # Most testing is done against the utils.RunCommand already.
+        # pylint: disable=protected-access
+        ret = rh.hooks._run_command(['true'])
+        self.assertEqual(ret.returncode, 0)
+
+    def testBuildOs(self):
+        """Check _get_build_os_name behavior."""
+        # Just verify it returns something and doesn't crash.
+        # pylint: disable=protected-access
+        ret = rh.hooks._get_build_os_name()
+        self.assertTrue(isinstance(ret, str))
+        self.assertNotEqual(ret, '')
+
+    def testGetHelperPath(self):
+        """Check get_helper_path behavior."""
+        # Just verify it doesn't crash.  It's a dirt simple func.
+        ret = rh.hooks.get_helper_path('booga')
+        self.assertTrue(isinstance(ret, str))
+        self.assertNotEqual(ret, '')
+
+
+
+@mock.patch.object(rh.utils, 'run_command')
+@mock.patch.object(rh.hooks, '_check_cmd', return_value=['check_cmd'])
+class BuiltinHooksTests(unittest.TestCase):
+    """Verify the builtin hooks."""
+
+    def setUp(self):
+        self.project = rh.Project(name='project-name', dir='/.../repo/dir',
+                                  remote='remote')
+        self.options = rh.hooks.HookOptions('hook name', [], {})
+
+    def _test_commit_messages(self, func, accept, msgs):
+        """Helper for testing commit message hooks.
+
+        Args:
+          func: The hook function to test.
+          accept: Whether all the |msgs| should be accepted.
+          msgs: List of messages to test.
+        """
+        for desc in msgs:
+            ret = func(self.project, 'commit', desc, (), options=self.options)
+            if accept:
+                self.assertEqual(
+                    ret, None, msg='Should have accepted: {{{%s}}}' % (desc,))
+            else:
+                self.assertNotEqual(
+                    ret, None, msg='Should have rejected: {{{%s}}}' % (desc,))
+
+    def _test_file_filter(self, mock_check, func, files):
+        """Helper for testing hooks that filter by files and run external tools.
+
+        Args:
+          mock_check: The mock of _check_cmd.
+          func: The hook function to test.
+          files: A list of files that we'd check.
+        """
+        # First call should do nothing as there are no files to check.
+        ret = func(self.project, 'commit', 'desc', (), options=self.options)
+        self.assertEqual(ret, None)
+        self.assertFalse(mock_check.called)
+
+        # Second call should include some checks.
+        diff = [rh.git.RawDiffEntry(file=x) for x in files]
+        ret = func(self.project, 'commit', 'desc', diff, options=self.options)
+        self.assertEqual(ret, mock_check.return_value)
+
+    def testTheTester(self, _mock_check, _mock_run):
+        """Make sure we have a test for every hook."""
+        for hook in rh.hooks.BUILTIN_HOOKS:
+            self.assertIn('test_%s' % (hook,), dir(self),
+                          msg='Missing unittest for builtin hook %s' % (hook,))
+
+    def test_checkpatch(self, mock_check, _mock_run):
+        """Verify the checkpatch builtin hook."""
+        ret = rh.hooks.check_checkpatch(
+            self.project, 'commit', 'desc', (), options=self.options)
+        self.assertEqual(ret, mock_check.return_value)
+
+    def test_clang_format(self, mock_check, _mock_run):
+        """Verify the clang_format builtin hook."""
+        ret = rh.hooks.check_clang_format(
+            self.project, 'commit', 'desc', (), options=self.options)
+        self.assertEqual(ret, mock_check.return_value)
+
+    def test_commit_msg_bug_field(self, _mock_check, _mock_run):
+        """Verify the commit_msg_bug_field builtin hook."""
+        # Check some good messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_bug_field, True, (
+                'subj\n\nBug: 1234\n',
+                'subj\n\nBug: 1234\nChange-Id: blah\n',
+            ))
+
+        # Check some bad messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_bug_field, False, (
+                'subj',
+                'subj\n\nBUG=1234\n',
+                'subj\n\nBUG: 1234\n',
+            ))
+
+    def test_commit_msg_changeid_field(self, _mock_check, _mock_run):
+        """Verify the commit_msg_changeid_field builtin hook."""
+        # Check some good messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_changeid_field, True, (
+                'subj\n\nChange-Id: I1234\n',
+            ))
+
+        # Check some bad messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_changeid_field, False, (
+                'subj',
+                'subj\n\nChange-Id: 1234\n',
+                'subj\n\nChange-ID: I1234\n',
+            ))
+
+    def test_commit_msg_test_field(self, _mock_check, _mock_run):
+        """Verify the commit_msg_test_field builtin hook."""
+        # Check some good messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_test_field, True, (
+                'subj\n\nTest: i did done dood it\n',
+            ))
+
+        # Check some bad messages.
+        self._test_commit_messages(
+            rh.hooks.check_commit_msg_test_field, False, (
+                'subj',
+                'subj\n\nTEST=1234\n',
+                'subj\n\nTEST: I1234\n',
+            ))
+
+    def test_cpplint(self, mock_check, _mock_run):
+        """Verify the cpplint builtin hook."""
+        self._test_file_filter(mock_check, rh.hooks.check_cpplint,
+                               ('foo.cpp', 'foo.cxx'))
+
+    def test_gofmt(self, mock_check, _mock_run):
+        """Verify the gofmt builtin hook."""
+        # First call should do nothing as there are no files to check.
+        ret = rh.hooks.check_gofmt(
+            self.project, 'commit', 'desc', (), options=self.options)
+        self.assertEqual(ret, None)
+        self.assertFalse(mock_check.called)
+
+        # Second call will have some results.
+        diff = [rh.git.RawDiffEntry(file='foo.go')]
+        ret = rh.hooks.check_gofmt(
+            self.project, 'commit', 'desc', diff, options=self.options)
+        self.assertNotEqual(ret, None)
+
+    def test_jsonlint(self, mock_check, _mock_run):
+        """Verify the jsonlint builtin hook."""
+        # First call should do nothing as there are no files to check.
+        ret = rh.hooks.check_json(
+            self.project, 'commit', 'desc', (), options=self.options)
+        self.assertEqual(ret, None)
+        self.assertFalse(mock_check.called)
+
+        # TODO: Actually pass some valid/invalid json data down.
+
+    def test_pylint(self, mock_check, _mock_run):
+        """Verify the pylint builtin hook."""
+        self._test_file_filter(mock_check, rh.hooks.check_pylint,
+                               ('foo.py',))
+
+    def test_xmllint(self, mock_check, _mock_run):
+        """Verify the xmllint builtin hook."""
+        self._test_file_filter(mock_check, rh.hooks.check_xmllint,
+                               ('foo.xml',))
+
+
+if __name__ == '__main__':
+    unittest.main()