diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b3907cc3..5d709431 100644 --- a/.pre-commit-config.yaml +++ b/.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$' diff --git a/manifest.yaml b/manifest.yaml index e3569e75..062e09c5 100644 --- a/manifest.yaml +++ b/manifest.yaml @@ -1,4 +1,3 @@ - - id: validate_manifest name: Validate Pre-Commit Manifest description: This validator validates a pre-commit hooks manifest file diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py index 5496cb21..ed0b1581 100644 --- a/pre_commit/clientlib/validate_config.py +++ b/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', - nargs='?', default=None, - help='Config filename. Defaults to {0} at root of git repo'.format( + 'filenames', + nargs='*', default=None, + help='Config filenames. Defaults to {0} at root of git repo'.format( C.CONFIG_FILE, ) ) args = parser.parse_args(argv) - try: - validate_config(args.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])) - return 1 + filenames = args.filenames or [C.CONFIG_FILE] + retval = 0 - 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__': diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py index 168c0ddc..ed6818af 100644 --- a/pre_commit/clientlib/validate_manifest.py +++ b/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', - nargs='?', default=None, - help='Manifest filename. Defaults to {0} at root of git repo'.format( + 'filenames', + nargs='*', default=None, + help='Manifest filenames. Defaults to {0} at root of git repo'.format( C.MANIFEST_FILE, ) ) args = parser.parse_args(argv) - try: - validate_manifest(args.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])) - return 1 + filenames = args.filenames or [C.MANIFEST_FILE] + retval = 0 - 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__': diff --git a/pre_commit/repository.py b/pre_commit/repository.py index f9a4b7cb..abb9044b 100644 --- a/pre_commit/repository.py +++ b/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 diff --git a/pre_commit/run.py b/pre_commit/run.py index cd1afc0c..e4e220f3 100644 --- a/pre_commit/run.py +++ b/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.repository import Repository +from pre_commit.runner import Runner 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( - hook['name'], - '.' * (COLS - len(hook['name']) - len(pass_fail) - 6), - color, - pass_fail, - NORMAL, - ) + print '{0}{1}{2}'.format(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([]) - for config in configs: - repo = Repository(config) + runner = Runner.create() + for repo in runner.repositories: 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): - configs = configs or validate_config([]) - for config in configs: - repo = Repository(config) +def run_single_hook(hook_id, run_all_the_things=False): + runner = Runner.create() + for repo in runner.repositories: if hook_id in repo.hooks: return _run_single_hook( repo, diff --git a/pre_commit/runner.py b/pre_commit/runner.py new file mode 100644 index 00000000..7cb7dee2 --- /dev/null +++ b/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)) diff --git a/testing/__init__.py b/testing/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/data/array_yaml_file.yaml b/testing/resources/array_yaml_file.yaml similarity index 100% rename from tests/data/array_yaml_file.yaml rename to testing/resources/array_yaml_file.yaml diff --git a/testing/resources/consumer_repo/.pre-commit-config.yaml b/testing/resources/consumer_repo/.pre-commit-config.yaml new file mode 100644 index 00000000..da23b5c3 --- /dev/null +++ b/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 diff --git a/testing/resources/node_hooks_repo/bin/main.js b/testing/resources/node_hooks_repo/bin/main.js new file mode 100644 index 00000000..8e0f025a --- /dev/null +++ b/testing/resources/node_hooks_repo/bin/main.js @@ -0,0 +1,3 @@ +#!/usr/bin/env node + +console.log('Hello World'); diff --git a/testing/resources/node_hooks_repo/manifest.yaml b/testing/resources/node_hooks_repo/manifest.yaml new file mode 100644 index 00000000..2088caa1 --- /dev/null +++ b/testing/resources/node_hooks_repo/manifest.yaml @@ -0,0 +1,4 @@ +- id: foo + name: Foo + entry: foo + language: node diff --git a/testing/resources/node_hooks_repo/package.json b/testing/resources/node_hooks_repo/package.json new file mode 100644 index 00000000..050b6300 --- /dev/null +++ b/testing/resources/node_hooks_repo/package.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "version": "0.0.1", + "bin": {"foo": "./bin/main.js"} +} diff --git a/tests/data/non_parseable_yaml_file.yaml b/testing/resources/non_parseable_yaml_file.yaml similarity index 100% rename from tests/data/non_parseable_yaml_file.yaml rename to testing/resources/non_parseable_yaml_file.yaml diff --git a/testing/resources/python_hooks_repo/foo/__init__.py b/testing/resources/python_hooks_repo/foo/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testing/resources/python_hooks_repo/foo/main.py b/testing/resources/python_hooks_repo/foo/main.py new file mode 100644 index 00000000..5be7cd29 --- /dev/null +++ b/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 diff --git a/testing/resources/python_hooks_repo/manifest.yaml b/testing/resources/python_hooks_repo/manifest.yaml new file mode 100644 index 00000000..661835e7 --- /dev/null +++ b/testing/resources/python_hooks_repo/manifest.yaml @@ -0,0 +1,4 @@ +- id: foo + name: Foo + entry: foo + language: python diff --git a/testing/resources/python_hooks_repo/setup.py b/testing/resources/python_hooks_repo/setup.py new file mode 100644 index 00000000..556dd8f5 --- /dev/null +++ b/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'], + }, +) diff --git a/tests/data/valid_yaml_but_invalid_config.yaml b/testing/resources/valid_yaml_but_invalid_config.yaml similarity index 100% rename from tests/data/valid_yaml_but_invalid_config.yaml rename to testing/resources/valid_yaml_but_invalid_config.yaml diff --git a/tests/data/valid_yaml_but_invalid_manifest.yaml b/testing/resources/valid_yaml_but_invalid_manifest.yaml similarity index 100% rename from tests/data/valid_yaml_but_invalid_manifest.yaml rename to testing/resources/valid_yaml_but_invalid_manifest.yaml diff --git a/testing/util.py b/testing/util.py new file mode 100644 index 00000000..4e5acf77 --- /dev/null +++ b/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) diff --git a/tests/clientlib/validate_base_test.py b/tests/clientlib/validate_base_test.py index f900d4ae..b74fba31 100644 --- a/tests/clientlib/validate_base_test.py +++ b/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'] diff --git a/tests/conftest.py b/tests/conftest.py index 11063ec1..f2ac3d6c 100644 --- a/tests/conftest.py +++ b/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): - local.path(C.MANIFEST_FILE).write(""" -- - id: foo - name: Foo - entry: foo - language: python - """) - +def python_hooks_repo(dummy_git_repo): + copy_tree_to_path( + get_resource_path('python_hooks_repo'), + dummy_git_repo, + ) 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): - local.path(C.MANIFEST_FILE).write(""" -- - id: foo - name: Foo - entry: foo - language: node - """) - +def node_hooks_repo(dummy_git_repo): + copy_tree_to_path( + get_resource_path('node_hooks_repo'), + dummy_git_repo, + ) 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, - 'sha': git.get_head_sha(node_pre_commit_git_repo), + 'repo': node_hooks_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, - 'sha': git.get_head_sha(python_pre_commit_git_repo), + 'repo': python_hooks_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 diff --git a/tests/repository_test.py b/tests/repository_test.py index d6125274..0f3ccf35 100644 --- a/tests/repository_test.py +++ b/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): - repo = Repository(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_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): - repo = Repository(config_for_python_pre_commit_git_repo) +def test_run_a_python_hook(config_for_python_hooks_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): - repo = Repository(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_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): - repo = Repository(config_for_node_pre_commit_git_repo) +def test_run_a_node_hook(config_for_node_hooks_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): - repo = Repository(config_for_python_pre_commit_git_repo) +def test_languages(config_for_python_hooks_repo): + repo = Repository(config_for_python_hooks_repo) assert repo.languages == set(['python']) diff --git a/tests/runner_test.py b/tests/runner_test.py new file mode 100644 index 00000000..0ec6bbe5 --- /dev/null +++ b/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', + ]