Repository: audreyr/cookiecutter (python)

Issue:
Simplify cookiecutter.hooks.find_hooks
We should rename `cookiecutter.hooks.find_hooks` to `find_hook(hook_name)` and explicitly look for the requested hook, instead of processing all the files in the hooks directory.

See https://github.com/audreyr/cookiecutter/pull/768/files/9a94484093ca23e9d55d42a53f096f67535b0b63#r68646614


Pull request description:
Find the requested hook only
Fixes #769.


Patch:
diff --git a/cookiecutter/hooks.py b/cookiecutter/hooks.py
index bd39be8..bffd5d7 100644
--- a/cookiecutter/hooks.py
+++ b/cookiecutter/hooks.py
@@ -16,38 +16,53 @@ from .exceptions import FailedHookException
 
 logger = logging.getLogger(__name__)
 
-
 _HOOKS = [
     'pre_gen_project',
     'post_gen_project',
-    # TODO: other hooks should be listed here
 ]
 EXIT_SUCCESS = 0
 
 
-def find_hooks():
+def valid_hook(hook_file, hook_name):
+    """Determine if a hook file is valid.
+
+    :param hook_file: The hook file to consider for validity
+    :param hook_name: The hook to find
+    :return: The hook file validity
+    """
+    filename = os.path.basename(hook_file)
+    basename = os.path.splitext(filename)[0]
+
+    matching_hook = basename == hook_name
+    supported_hook = basename in _HOOKS
+    backup_file = filename.endswith('~')
+
+    return matching_hook and supported_hook and not backup_file
+
+
+def find_hook(hook_name, hooks_dir='hooks'):
     """Return a dict of all hook scripts provided.
 
     Must be called with the project template as the current working directory.
     Dict's key will be the hook/script's name, without extension, while values
     will be the absolute path to the script. Missing scripts will not be
     included in the returned dict.
+
+    :param hook_name: The hook to find
+    :param hooks_dir: The hook directory in the template
+    :return: The absolute path to the hook script or None
     """
-    hooks_dir = 'hooks'
-    hooks = {}
-    logger.debug('hooks_dir is {}'.format(hooks_dir))
+    logger.debug('hooks_dir is {}'.format(os.path.abspath(hooks_dir)))
 
     if not os.path.isdir(hooks_dir):
         logger.debug('No hooks/ dir in template_dir')
-        return hooks
+        return None
 
-    for f in os.listdir(hooks_dir):
-        filename = os.path.basename(f)
-        basename = os.path.splitext(filename)[0]
+    for hook_file in os.listdir(hooks_dir):
+        if valid_hook(hook_file, hook_name):
+            return os.path.abspath(os.path.join(hooks_dir, hook_file))
 
-        if basename in _HOOKS and not filename.endswith('~'):
-            hooks[basename] = os.path.abspath(os.path.join(hooks_dir, f))
-    return hooks
+    return None
 
 
 def run_script(script_path, cwd='.'):
@@ -105,7 +120,7 @@ def run_hook(hook_name, project_dir, context):
     :param project_dir: The directory to execute the script from.
     :param context: Cookiecutter project context.
     """
-    script = find_hooks().get(hook_name)
+    script = find_hook(hook_name)
     if script is None:
         logger.debug('No hooks found')
         return


Test patch:
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index 56e1feb..4402599 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -71,19 +71,30 @@ class TestFindHooks(object):
         """Finds the specified hook."""
 
         with utils.work_in(self.repo_path):
-            expected = {
-                'pre_gen_project': os.path.abspath('hooks/pre_gen_project.py'),
-                'post_gen_project': os.path.abspath(
-                    os.path.join('hooks', self.post_hook)
-                ),
-            }
-            assert expected == hooks.find_hooks()
+            expected_pre = os.path.abspath('hooks/pre_gen_project.py')
+            actual_hook_path = hooks.find_hook('pre_gen_project')
+            assert expected_pre == actual_hook_path
+
+            expected_post = os.path.abspath('hooks/{}'.format(self.post_hook))
+            actual_hook_path = hooks.find_hook('post_gen_project')
+            assert expected_post == actual_hook_path
 
     def test_no_hooks(self):
         """find_hooks should return None if the hook could not be found."""
 
         with utils.work_in('tests/fake-repo'):
-            assert {} == hooks.find_hooks()
+            assert None is hooks.find_hook('pre_gen_project')
+
+    def test_unknown_hooks_dir(self):
+        with utils.work_in(self.repo_path):
+            assert hooks.find_hook(
+                'pre_gen_project',
+                hooks_dir='hooks_dir'
+            ) is None
+
+    def test_hook_not_found(self):
+        with utils.work_in(self.repo_path):
+            assert hooks.find_hook('unknown_hook') is None
 
 
 class TestExternalHooks(object):
@@ -220,4 +231,5 @@ def dir_with_hooks(tmpdir):
 def test_ignore_hook_backup_files(monkeypatch, dir_with_hooks):
     # Change the current working directory that contains `hooks/`
     monkeypatch.chdir(dir_with_hooks)
-    assert hooks.find_hooks() == {}
+    assert hooks.find_hook('pre_gen_project') is None
+    assert hooks.find_hook('post_gen_project') is None