Merge pull request #41 from pre-commit/pre_commit_runner

Add Runner interface.

.pre-commit-config.yaml

@@ -17,6 +17,6 @@
     sha: 47b7ca44ed1fcaa83464ed00cef72049ae22c33d
     hooks:
         -   id: validate_manifest
-            files: /manifest.yaml
+            files: '^manifest.yaml$'
         -   id: validate_config
-            files: /\.pre-commit-config.yaml
+            files: '^\.pre-commit-config.yaml$'

manifest.yaml

@@ -1,4 +1,3 @@
-
 -   id: validate_manifest
     name: Validate Pre-Commit Manifest
     description: This validator validates a pre-commit hooks manifest file

pre_commit/clientlib/validate_config.py

@@ -57,7 +57,7 @@ def validate_config_extra(config):
                 )
 
 
-validate_config = get_validator(
+load_config = get_validator(
     C.CONFIG_FILE,
     CONFIG_JSON_SCHEMA,
     InvalidConfigError,
@@ -69,25 +69,28 @@ validate_config = get_validator(
 def run(argv):
     parser = argparse.ArgumentParser()
     parser.add_argument(
-        'filename',
+        'filenames',
-        nargs='?', default=None,
+        nargs='*', default=None,
-        help='Config filename.  Defaults to {0} at root of git repo'.format(
+        help='Config filenames.  Defaults to {0} at root of git repo'.format(
             C.CONFIG_FILE,
         )
     )
     args = parser.parse_args(argv)
 
-    try:
+    filenames = args.filenames or [C.CONFIG_FILE]
-        validate_config(args.filename)
+    retval = 0
-    except InvalidConfigError as e:
-        print(e.args[0])
-        # If we have more than one exception argument print the stringified
-        # version
-        if len(e.args) > 1:
-            print(str(e.args[1]))
-        return 1
 
-    return 0
+    for filename in filenames:
+        try:
+            load_config(filename)
+        except InvalidConfigError as e:
+            print(e.args[0])
+            # If we have more than one exception argument print the stringified
+            # version
+            if len(e.args) > 1:
+                print(str(e.args[1]))
+            retval = 1
+    return retval
 
 
 if __name__ == '__main__':

pre_commit/clientlib/validate_manifest.py

@@ -46,7 +46,7 @@ def additional_manifest_check(obj):
             )
 
 
-validate_manifest = get_validator(
+load_manifest = get_validator(
     C.MANIFEST_FILE,
     MANIFEST_JSON_SCHEMA,
     InvalidManifestError,
@@ -58,25 +58,28 @@ validate_manifest = get_validator(
 def run(argv):
     parser = argparse.ArgumentParser()
     parser.add_argument(
-        'filename',
+        'filenames',
-        nargs='?', default=None,
+        nargs='*', default=None,
-        help='Manifest filename.  Defaults to {0} at root of git repo'.format(
+        help='Manifest filenames.  Defaults to {0} at root of git repo'.format(
             C.MANIFEST_FILE,
         )
     )
     args = parser.parse_args(argv)
 
-    try:
+    filenames = args.filenames or [C.MANIFEST_FILE]
-        validate_manifest(args.filename)
+    retval = 0
-    except InvalidManifestError as e:
-        print(e.args[0])
-        # If we have more than one exception argument print the stringified
-        # version
-        if len(e.args) > 1:
-            print(str(e.args[1]))
-        return 1
 
-    return 0
+    for filename in filenames:
+        try:
+            load_manifest(filename)
+        except InvalidManifestError as e:
+            print(e.args[0])
+            # If we have more than one exception argument print the stringified
+            # version
+            if len(e.args) > 1:
+                print(str(e.args[1]))
+            retval = 1
+    return retval
 
 
 if __name__ == '__main__':

pre_commit/repository.py

@@ -3,7 +3,7 @@ import contextlib
 from plumbum import local
 
 import pre_commit.constants as C
-from pre_commit.clientlib.validate_manifest import validate_manifest
+from pre_commit.clientlib.validate_manifest import load_manifest
 from pre_commit.hooks_workspace import in_hooks_workspace
 from pre_commit.languages.all import languages
 from pre_commit.ordereddict import OrderedDict
@@ -40,7 +40,7 @@ class Repository(object):
         with self.in_checkout():
             return dict(
                 (hook['id'], hook)
-                for hook in validate_manifest(C.MANIFEST_FILE)
+                for hook in load_manifest(C.MANIFEST_FILE)
             )
 
     @contextlib.contextmanager

pre_commit/run.py

@@ -5,8 +5,7 @@ import subprocess
 import sys
 
 from pre_commit import git
-from pre_commit.clientlib.validate_config import validate_config
+from pre_commit.runner import Runner
-from pre_commit.repository import Repository
 from pre_commit.util import entry
 
 
@@ -15,6 +14,8 @@ GREEN = '\033[42m'
 NORMAL = '\033[0m'
 COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0])
 
+PASS_FAIL_LENGTH = 6
+
 
 def _run_single_hook(repository, hook_id, run_all_the_things=False):
     repository.install()
@@ -26,6 +27,13 @@ def _run_single_hook(repository, hook_id, run_all_the_things=False):
 
     hook = repository.hooks[hook_id]
 
+    # Print the hook and the dots first in case the hook takes hella long to
+    # run.
+    print '{0}{1}'.format(
+        hook['name'],
+        '.' * (COLS - len(hook['name']) - PASS_FAIL_LENGTH - 6),
+    ),
+
     retcode, stdout, stderr = repository.run_hook(
         hook_id,
         map(os.path.abspath, get_filenames(hook['files'])),
@@ -43,13 +51,7 @@ def _run_single_hook(repository, hook_id, run_all_the_things=False):
         pass_fail = 'Passed'
 
 
-    print '{0}{1}{2}{3}{4}'.format(
+    print '{0}{1}{2}'.format(color, pass_fail, NORMAL)
-        hook['name'],
-        '.' * (COLS - len(hook['name']) - len(pass_fail) - 6),
-        color,
-        pass_fail,
-        NORMAL,
-    )
 
     if output:
         print
@@ -63,9 +65,8 @@ def run_hooks(run_all_the_things=False):
     """Actually run the hooks."""
     retval = 0
 
-    configs = validate_config([])
+    runner = Runner.create()
-    for config in configs:
+    for repo in runner.repositories:
-        repo = Repository(config)
         for hook_id in repo.hooks:
             retval |= _run_single_hook(
                 repo,
@@ -76,10 +77,9 @@ def run_hooks(run_all_the_things=False):
     return retval
 
 
-def run_single_hook(hook_id, configs=None, run_all_the_things=False):
+def run_single_hook(hook_id, run_all_the_things=False):
-    configs = configs or validate_config([])
+    runner = Runner.create()
-    for config in configs:
+    for repo in runner.repositories:
-        repo = Repository(config)
         if hook_id in repo.hooks:
             return _run_single_hook(
                 repo,

pre_commit/runner.py

@@ -0,0 +1,42 @@
+
+import os
+import os.path
+
+import pre_commit.constants as C
+from pre_commit import git
+from pre_commit.clientlib.validate_config import load_config
+from pre_commit.repository import Repository
+from pre_commit.util import cached_property
+
+
+class Runner(object):
+    """A `Runner` represents the execution context of the hooks.  Notably the
+    repository under test.
+    """
+
+    def __init__(self, git_root):
+        self.git_root = git_root
+
+    @classmethod
+    def create(cls):
+        """Creates a PreCommitRunner by doing the following:
+            - Finds the root of the current git repository
+            - chdirs to that directory
+        """
+        root = git.get_root()
+        os.chdir(root)
+        return cls(root)
+
+    @cached_property
+    def hooks_workspace_path(self):
+        return os.path.join(self.git_root, C.HOOKS_WORKSPACE)
+
+    @cached_property
+    def config_file_path(self):
+        return os.path.join(self.git_root, C.CONFIG_FILE)
+
+    @cached_property
+    def repositories(self):
+        """Returns a tuple of the configured repositories."""
+        config = load_config(self.config_file_path)
+        return tuple(map(Repository, config))

testing/resources/consumer_repo/.pre-commit-config.yaml

@@ -0,0 +1,22 @@
+
+-   repo: git@github.com:pre-commit/pre-commit-hooks
+    sha: 76739902911688e8d7b13241409f9facc0e534e4
+    hooks:
+        -   id: pyflakes
+            files: '\.py$'
+        -   id: debug-statements
+            files: '\.py$'
+        -   id: trailing-whitespace
+            files: '\.(py|sh|yaml)$'
+        -   id: name-tests-test
+            files: 'tests/.+\.py$'
+        -   id: end-of-file-fixer
+            files: '\.(py|sh|yaml)$'
+
+-   repo: git@github.com:pre-commit/pre-commit
+    sha: 47b7ca44ed1fcaa83464ed00cef72049ae22c33d
+    hooks:
+        -   id: validate_manifest
+            files: '^manifest.yaml$'
+        -   id: validate_config
+            files: \.pre-commit-config.yaml

testing/resources/node_hooks_repo/bin/main.js

@@ -0,0 +1,3 @@
+#!/usr/bin/env node
+
+console.log('Hello World');

testing/resources/node_hooks_repo/manifest.yaml

@@ -0,0 +1,4 @@
+-   id: foo
+    name: Foo
+    entry: foo
+    language: node

testing/resources/node_hooks_repo/package.json

@@ -0,0 +1,5 @@
+{
+    "name": "foo",
+    "version": "0.0.1",
+    "bin": {"foo": "./bin/main.js"}
+}

testing/resources/python_hooks_repo/foo/main.py

@@ -0,0 +1,6 @@
+import sys
+
+def func():
+    print repr(sys.argv[1:])
+    print 'Hello World'
+    return 0

testing/resources/python_hooks_repo/manifest.yaml

@@ -0,0 +1,4 @@
+-   id: foo
+    name: Foo
+    entry: foo
+    language: python

testing/resources/python_hooks_repo/setup.py

@@ -0,0 +1,11 @@
+from setuptools import find_packages
+from setuptools import setup
+
+setup(
+    name='Foo',
+    version='0.0.0',
+    packages=find_packages('.'),
+    entry_points={
+        'console_scripts': ['foo = foo.main:func'],
+    },
+)

testing/util.py

@@ -0,0 +1,29 @@
+
+import os
+import os.path
+import shutil
+
+
+TESTING_DIR = os.path.abspath(os.path.dirname(__file__))
+
+
+def get_resource_path(path):
+    return os.path.join(TESTING_DIR, 'resources', path)
+
+
+def copy_tree_to_path(src_dir, dest_dir):
+    """Copies all of the things inside src_dir to an already existing dest_dir.
+
+    This looks eerily similar to shutil.copytree, but copytree has no option
+    for not creating dest_dir.
+    """
+    names = os.listdir(src_dir)
+
+    for name in names:
+        srcname = os.path.join(src_dir, name)
+        destname = os.path.join(dest_dir, name)
+
+        if os.path.isdir(srcname):
+            shutil.copytree(srcname, destname)
+        else:
+            shutil.copy(srcname, destname)

tests/clientlib/validate_base_test.py

@@ -7,6 +7,7 @@ import pytest
 
 from pre_commit import git
 from pre_commit.clientlib.validate_base import get_validator
+from testing.util import get_resource_path
 
 
 class AdditionalValidatorError(ValueError): pass
@@ -42,7 +43,7 @@ def test_raises_for_non_existent_file(noop_validator):
 
 def test_raises_for_invalid_yaml_file(noop_validator):
     with pytest.raises(ValueError):
-        noop_validator('tests/data/non_parseable_yaml_file.yaml')
+        noop_validator(get_resource_path('non_parseable_yaml_file.yaml'))
 
 
 def test_defaults_to_backup_filename(noop_validator):
@@ -55,11 +56,11 @@ def test_defaults_to_backup_filename(noop_validator):
 
 def test_raises_for_failing_schema(array_validator):
     with pytest.raises(ValueError):
-        array_validator('tests/data/valid_yaml_but_invalid_manifest.yaml')
+        array_validator(get_resource_path('valid_yaml_but_invalid_manifest.yaml'))
 
 
 def test_passes_array_schema(array_validator):
-    array_validator('tests/data/array_yaml_file.yaml')
+    array_validator(get_resource_path('array_yaml_file.yaml'))
 
 
 def test_raises_when_additional_validation_fails(additional_validator):
@@ -68,5 +69,5 @@ def test_raises_when_additional_validation_fails(additional_validator):
 
 
 def test_returns_object_after_validating(noop_validator):
-    ret = noop_validator('tests/data/array_yaml_file.yaml')
+    ret = noop_validator(get_resource_path('array_yaml_file.yaml'))
     assert ret == ['foo', 'bar']

tests/conftest.py

@@ -1,14 +1,15 @@
 from __future__ import absolute_import
 
 import jsonschema
-import simplejson
 import pytest
 import time
 from plumbum import local
 
-import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
+from pre_commit.clientlib.validate_config import validate_config_extra
+from testing.util import copy_tree_to_path
+from testing.util import get_resource_path
 
 
 @pytest.yield_fixture
@@ -25,123 +26,69 @@ def add_and_commit():
 
 @pytest.yield_fixture
 def dummy_git_repo(empty_git_dir):
+    # This is needed otherwise there is no `HEAD`
     local['touch']['dummy']()
     add_and_commit()
-
     yield empty_git_dir
 
 
 @pytest.yield_fixture
-def python_pre_commit_git_repo(dummy_git_repo):
+def python_hooks_repo(dummy_git_repo):
-    local.path(C.MANIFEST_FILE).write("""
+    copy_tree_to_path(
--
+        get_resource_path('python_hooks_repo'),
-    id: foo
+        dummy_git_repo,
-    name: Foo
+    )
-    entry: foo
-    language: python
-    """)
-
     add_and_commit()
-
-    local.path('setup.py').write("""
-from setuptools import find_packages
-from setuptools import setup
-
-setup(
-    name='Foo',
-    version='0.0.0',
-    packages=find_packages('.'),
-    entry_points={
-        'console_scripts': [
-            'foo = foo.main:func'
-        ],
-    }
-)
-""")
-
-    foo_module = local.path('foo')
-
-    foo_module.mkdir()
-
-    with local.cwd(foo_module):
-        local.path('__init__.py').write('')
-        local.path('main.py').write("""
-def func():
-    import sys
-    print repr(sys.argv[1:])
-    print 'Hello World'
-    return 0
-""")
-
-    add_and_commit()
-
     yield dummy_git_repo
 
 
 @pytest.yield_fixture
-def node_pre_commit_git_repo(dummy_git_repo):
+def node_hooks_repo(dummy_git_repo):
-    local.path(C.MANIFEST_FILE).write("""
+    copy_tree_to_path(
--
+        get_resource_path('node_hooks_repo'),
-    id: foo
+        dummy_git_repo,
-    name: Foo
+    )
-    entry: foo
-    language: node
-    """)
-
     add_and_commit()
+    yield dummy_git_repo
 
-    local.path('package.json').write(simplejson.dumps({
-        'name': 'foo',
-        'version': '0.0.1',
-        'bin': {
-            'foo': './bin/main.js'
-        },
-    }))
-
-    bin_dir = local.path('bin')
-
-    bin_dir.mkdir()
-
-    with local.cwd(bin_dir):
-        local.path('main.js').write(
-"""#!/usr/bin/env node
-
-console.log('Hello World');
-""")
 
+@pytest.yield_fixture
+def consumer_repo(dummy_git_repo):
+    copy_tree_to_path(
+        get_resource_path('consumer_repo'),
+        dummy_git_repo,
+    )
     add_and_commit()
-
     yield dummy_git_repo
 
 
 @pytest.fixture
-def config_for_node_pre_commit_git_repo(node_pre_commit_git_repo):
+def config_for_node_hooks_repo(node_hooks_repo):
     config = {
-        'repo': node_pre_commit_git_repo,
+        'repo': node_hooks_repo,
-        'sha': git.get_head_sha(node_pre_commit_git_repo),
+        'sha': git.get_head_sha(node_hooks_repo),
         'hooks': [{
             'id': 'foo',
-            'files': '*.js',
+            'files': '\.js$',
         }],
     }
-
     jsonschema.validate([config], CONFIG_JSON_SCHEMA)
+    validate_config_extra([config])
 
     return config
 
 
-
 @pytest.fixture
-def config_for_python_pre_commit_git_repo(python_pre_commit_git_repo):
+def config_for_python_hooks_repo(python_hooks_repo):
     config = {
-        'repo': python_pre_commit_git_repo,
+        'repo': python_hooks_repo,
-        'sha': git.get_head_sha(python_pre_commit_git_repo),
+        'sha': git.get_head_sha(python_hooks_repo),
         'hooks': [{
             'id': 'foo',
-            'files': '*.py',
+            'files': '\.py$',
         }],
     }
-
     jsonschema.validate([config], CONFIG_JSON_SCHEMA)
+    validate_config_extra([config])
 
     return config

tests/repository_test.py

@@ -28,13 +28,13 @@ def test_create_repo_in_env(dummy_repo_config, dummy_git_repo):
     )
 
 @pytest.mark.integration
-def test_install_python_repo_in_env(python_pre_commit_git_repo, config_for_python_pre_commit_git_repo):
+def test_install_python_repo_in_env(python_hooks_repo, config_for_python_hooks_repo):
-    repo = Repository(config_for_python_pre_commit_git_repo)
+    repo = Repository(config_for_python_hooks_repo)
     repo.install()
 
     assert os.path.exists(
         os.path.join(
-            python_pre_commit_git_repo,
+            python_hooks_repo,
             C.HOOKS_WORKSPACE,
             repo.sha,
             'py_env',
@@ -43,8 +43,8 @@ def test_install_python_repo_in_env(python_pre_commit_git_repo, config_for_pytho
 
 
 @pytest.mark.integration
-def test_run_a_python_hook(config_for_python_pre_commit_git_repo):
+def test_run_a_python_hook(config_for_python_hooks_repo):
-    repo = Repository(config_for_python_pre_commit_git_repo)
+    repo = Repository(config_for_python_hooks_repo)
     repo.install()
     ret = repo.run_hook('foo', ['/dev/null'])
 
@@ -53,8 +53,8 @@ def test_run_a_python_hook(config_for_python_pre_commit_git_repo):
 
 
 @pytest.mark.integration
-def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo):
+def test_run_a_hook_lots_of_files(config_for_python_hooks_repo):
-    repo = Repository(config_for_python_pre_commit_git_repo)
+    repo = Repository(config_for_python_hooks_repo)
     repo.install()
     ret = repo.run_hook('foo', ['/dev/null'] * 15000)
 
@@ -66,8 +66,8 @@ def test_run_a_hook_lots_of_files(config_for_python_pre_commit_git_repo):
     reason="TODO: make this test not super slow",
 )
 @pytest.mark.integration
-def test_run_a_node_hook(config_for_node_pre_commit_git_repo):
+def test_run_a_node_hook(config_for_node_hooks_repo):
-    repo = Repository(config_for_node_pre_commit_git_repo)
+    repo = Repository(config_for_node_hooks_repo)
     repo.install()
     ret = repo.run_hook('foo', [])
 
@@ -101,6 +101,6 @@ def test_sha(mock_repo_config):
 
 
 @pytest.mark.integration
-def test_languages(config_for_python_pre_commit_git_repo):
+def test_languages(config_for_python_hooks_repo):
-    repo = Repository(config_for_python_pre_commit_git_repo)
+    repo = Repository(config_for_python_hooks_repo)
     assert repo.languages == set(['python'])

tests/runner_test.py

@@ -0,0 +1,55 @@
+
+import os
+import os.path
+import pytest
+
+import pre_commit.constants as C
+from pre_commit.runner import Runner
+
+
+def test_init_has_no_side_effects(tmpdir):
+    current_wd = os.getcwd()
+    runner = Runner(tmpdir.strpath)
+    assert runner.git_root == tmpdir.strpath
+    assert os.getcwd() == current_wd
+
+
+def test_create_sets_correct_directory(empty_git_dir):
+    runner = Runner.create()
+    assert runner.git_root == empty_git_dir
+    assert os.getcwd() == empty_git_dir
+
+
+@pytest.yield_fixture
+def git_dir_with_directory(empty_git_dir):
+    os.mkdir('foo')
+    yield empty_git_dir
+
+
+def test_changes_to_root_of_git_dir(git_dir_with_directory):
+    os.chdir('foo')
+    assert os.getcwd() != git_dir_with_directory
+    runner = Runner.create()
+    assert runner.git_root == git_dir_with_directory
+    assert os.getcwd() == git_dir_with_directory
+
+
+def test_hooks_workspace_path():
+    runner = Runner('foo/bar')
+    expected_path = os.path.join('foo/bar', C.HOOKS_WORKSPACE)
+    assert runner.hooks_workspace_path == expected_path
+
+
+def test_config_file_path():
+    runner = Runner('foo/bar')
+    expected_path = os.path.join('foo/bar', C.CONFIG_FILE)
+    assert runner.config_file_path == expected_path
+
+
+def test_repositories(consumer_repo):
+    runner = Runner(consumer_repo)
+    assert len(runner.repositories) == 2
+    assert [repo.repo_url for repo in runner.repositories] == [
+        'git@github.com:pre-commit/pre-commit-hooks',
+        'git@github.com:pre-commit/pre-commit',
+    ]