From 5eda1212607c9c8c37cc55e43341b1406e207297 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 14:07:52 -0700 Subject: [PATCH] Move stuff into commands and write tests. --- pre_commit/commands.py | 91 ++++++++++++ pre_commit/run.py | 95 +------------ .../resources/failing_hook_repo/bin/hook.sh | 6 + .../resources/failing_hook_repo/hooks.yaml | 4 + .../prints_cwd_repo/prints_cwd/__init__.py | 0 .../prints_cwd_repo/prints_cwd/main.py | 5 - tests/commands_test.py | 130 +++++++++++++++--- tests/conftest.py | 31 ++++- 8 files changed, 243 insertions(+), 119 deletions(-) create mode 100755 testing/resources/failing_hook_repo/bin/hook.sh create mode 100644 testing/resources/failing_hook_repo/hooks.yaml delete mode 100644 testing/resources/prints_cwd_repo/prints_cwd/__init__.py delete mode 100644 testing/resources/prints_cwd_repo/prints_cwd/main.py diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 15813fd1..1db112ae 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -1,19 +1,33 @@ from __future__ import print_function +import logging import os import pkg_resources import shutil import stat +import subprocess +import sys from asottile.ordereddict import OrderedDict from asottile.yaml import ordered_dump from asottile.yaml import ordered_load from plumbum import local import pre_commit.constants as C +from pre_commit import git +from pre_commit import color from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import load_config from pre_commit.jsonschema_extensions import remove_defaults +from pre_commit.logging_handler import LoggingHandler from pre_commit.repository import Repository +from pre_commit.staged_files_only import staged_files_only + + +logger = logging.getLogger('pre_commit') + +COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0]) + +PASS_FAIL_LENGTH = 6 def install(runner): @@ -127,3 +141,80 @@ def clean(runner): shutil.rmtree(runner.hooks_workspace_path) print('Cleaned {0}.'.format(runner.hooks_workspace_path)) return 0 + + +def _run_single_hook(runner, repository, hook_id, args, write): + if args.all_files: + get_filenames = git.get_all_files_matching + else: + get_filenames = git.get_staged_files_matching + + hook = repository.hooks[hook_id] + + # Print the hook and the dots first in case the hook takes hella long to + # run. + write( + '{0}{1}'.format( + hook['name'], + '.' * (COLS - len(hook['name']) - PASS_FAIL_LENGTH - 6), + ), + ) + sys.stdout.flush() + + retcode, stdout, stderr = repository.run_hook( + runner.cmd_runner, + hook_id, + get_filenames(hook['files'], hook['exclude']), + ) + + if retcode != repository.hooks[hook_id]['expected_return_value']: + retcode = 1 + print_color = color.RED + pass_fail = 'Failed' + else: + retcode = 0 + print_color = color.GREEN + pass_fail = 'Passed' + + write(color.format_color(pass_fail, print_color, args.color) + '\n') + + if (stdout or stderr) and (retcode or args.verbose): + write('\n') + for output in (stdout, stderr): + if output.strip(): + write(output.strip() + '\n') + write('\n') + + return retcode + + +def _run_hooks(runner, args, write): + """Actually run the hooks.""" + retval = 0 + + for repo in runner.repositories: + for hook_id in repo.hooks: + retval |= _run_single_hook(runner, repo, hook_id, args, write=write) + + return retval + + +def _run_hook(runner, hook_id, args, write): + for repo in runner.repositories: + if hook_id in repo.hooks: + return _run_single_hook(runner, repo, hook_id, args, write=write) + else: + write('No hook with id `{0}`\n'.format(hook_id)) + return 1 + + +def run(runner, args, write=sys.stdout.write): + # Set up our logging handler + logger.addHandler(LoggingHandler(args.color)) + logger.setLevel(logging.INFO) + + with staged_files_only(runner.cmd_runner): + if args.hook: + return _run_hook(runner, args.hook, args, write=write) + else: + return _run_hooks(runner, args, write=write) diff --git a/pre_commit/run.py b/pre_commit/run.py index fe6c94ee..2bcb5d86 100644 --- a/pre_commit/run.py +++ b/pre_commit/run.py @@ -1,103 +1,12 @@ -from __future__ import print_function - import argparse -import logging -import subprocess import sys from pre_commit import color from pre_commit import commands -from pre_commit import git -from pre_commit.logging_handler import LoggingHandler from pre_commit.runner import Runner -from pre_commit.staged_files_only import staged_files_only from pre_commit.util import entry -logger = logging.getLogger('pre_commit') - -COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0]) - -PASS_FAIL_LENGTH = 6 - - -def _run_single_hook(runner, repository, hook_id, args): - if args.all_files: - get_filenames = git.get_all_files_matching - else: - get_filenames = git.get_staged_files_matching - - 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), - ), - end='', - ) - - retcode, stdout, stderr = repository.run_hook( - runner.cmd_runner, - hook_id, - get_filenames(hook['files'], hook['exclude']), - ) - - if retcode != repository.hooks[hook_id]['expected_return_value']: - retcode = 1 - print_color = color.RED - pass_fail = 'Failed' - else: - retcode = 0 - print_color = color.GREEN - pass_fail = 'Passed' - - print(color.format_color(pass_fail, print_color, args.color)) - - if (stdout or stderr) and (retcode or args.verbose): - print() - for output in (stdout, stderr): - if output.strip(): - print(output.strip()) - print() - - return retcode - - -def run_hooks(runner, args): - """Actually run the hooks.""" - retval = 0 - - for repo in runner.repositories: - for hook_id in repo.hooks: - retval |= _run_single_hook(runner, repo, hook_id, args) - - return retval - - -def run_single_hook(runner, hook_id, args): - for repo in runner.repositories: - if hook_id in repo.hooks: - return _run_single_hook(runner, repo, hook_id, args) - else: - print('No hook with id `{0}`'.format(hook_id)) - return 1 - - -def _run(runner, args): - # Set up our logging handler - logger.addHandler(LoggingHandler(args.color)) - logger.setLevel(logging.INFO) - - with staged_files_only(runner.cmd_runner): - if args.hook: - return run_single_hook(runner, args.hook, args) - else: - return run_hooks(runner, args) - - @entry def run(argv): parser = argparse.ArgumentParser() @@ -118,11 +27,11 @@ def run(argv): '--all-files', '-a', action='store_true', default=False, help='Run on all the files in the repo.', ) - run_parser.add_argument('--verbose', '-v', action='store_true', default=False) run_parser.add_argument( '--color', default='auto', type=color.use_color, help='Whether to use color in output. Defaults to `auto`', ) + run_parser.add_argument('--verbose', '-v', action='store_true', default=False) help = subparsers.add_parser('help', help='Show help for a specific command.') help.add_argument('help_cmd', nargs='?', help='Command to show help for.') @@ -143,7 +52,7 @@ def run(argv): elif args.command == 'autoupdate': return commands.autoupdate(runner) elif args.command == 'run': - return _run(runner, args) + return commands.run(runner, args) elif args.command == 'help': if args.help_cmd: parser.parse_args([args.help_cmd, '--help']) diff --git a/testing/resources/failing_hook_repo/bin/hook.sh b/testing/resources/failing_hook_repo/bin/hook.sh new file mode 100755 index 00000000..832b6cd1 --- /dev/null +++ b/testing/resources/failing_hook_repo/bin/hook.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + + +echo 'Fail' +echo $@ +exit 1 diff --git a/testing/resources/failing_hook_repo/hooks.yaml b/testing/resources/failing_hook_repo/hooks.yaml new file mode 100644 index 00000000..ecdf9a00 --- /dev/null +++ b/testing/resources/failing_hook_repo/hooks.yaml @@ -0,0 +1,4 @@ +- id: failing_hook + name: Failing hook + entry: bin/hook.sh + language: script diff --git a/testing/resources/prints_cwd_repo/prints_cwd/__init__.py b/testing/resources/prints_cwd_repo/prints_cwd/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/testing/resources/prints_cwd_repo/prints_cwd/main.py b/testing/resources/prints_cwd_repo/prints_cwd/main.py deleted file mode 100644 index 5e854ab1..00000000 --- a/testing/resources/prints_cwd_repo/prints_cwd/main.py +++ /dev/null @@ -1,5 +0,0 @@ -import os - -def func(): - print os.getcwd() - return 0 diff --git a/tests/commands_test.py b/tests/commands_test.py index 381762ae..cf60cb52 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -1,3 +1,4 @@ +import mock import os import os.path import pkg_resources @@ -10,15 +11,10 @@ from plumbum import local import pre_commit.constants as C +from pre_commit import commands 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 pre_commit.commands import autoupdate -from pre_commit.commands import clean -from pre_commit.commands import install -from pre_commit.commands import RepositoryCannotBeUpdatedError -from pre_commit.commands import uninstall -from pre_commit.commands import _update_repository from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.runner import Runner from testing.auto_namedtuple import auto_namedtuple @@ -27,7 +23,7 @@ from testing.util import get_resource_path def test_install_pre_commit(empty_git_dir): runner = Runner(empty_git_dir) - ret = install(runner) + ret = commands.install(runner) assert ret == 0 assert os.path.exists(runner.pre_commit_path) pre_commit_contents = open(runner.pre_commit_path).read() @@ -40,16 +36,16 @@ def test_install_pre_commit(empty_git_dir): def test_uninstall_pre_commit_does_not_blow_up_when_not_there(empty_git_dir): runner = Runner(empty_git_dir) - ret = uninstall(runner) + ret = commands.uninstall(runner) assert ret == 0 def test_uninstall(empty_git_dir): runner = Runner(empty_git_dir) assert not os.path.exists(runner.pre_commit_path) - install(runner) + commands.install(runner) assert os.path.exists(runner.pre_commit_path) - uninstall(runner) + commands.uninstall(runner) assert not os.path.exists(runner.pre_commit_path) @@ -77,14 +73,14 @@ def up_to_date_repo(python_hooks_repo): def test_up_to_date_repo(up_to_date_repo): input_sha = up_to_date_repo.repo_config['sha'] - ret = _update_repository(up_to_date_repo.repo_config) + ret = commands._update_repository(up_to_date_repo.repo_config) assert ret['sha'] == input_sha def test_autoupdate_up_to_date_repo(up_to_date_repo): before = open(C.CONFIG_FILE).read() runner = Runner(up_to_date_repo.python_hooks_repo) - ret = autoupdate(runner) + ret = commands.autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 0 assert before == after @@ -116,12 +112,12 @@ def out_of_date_repo(python_hooks_repo): def test_out_of_date_repo(out_of_date_repo): - ret = _update_repository(out_of_date_repo.repo_config) + ret = commands._update_repository(out_of_date_repo.repo_config) assert ret['sha'] == out_of_date_repo.head_sha def test_removes_defaults(out_of_date_repo): - ret = _update_repository(out_of_date_repo.repo_config) + ret = commands._update_repository(out_of_date_repo.repo_config) assert 'args' not in ret['hooks'][0] assert 'expected_return_value' not in ret['hooks'][0] @@ -129,7 +125,7 @@ def test_removes_defaults(out_of_date_repo): def test_autoupdate_out_of_date_repo(out_of_date_repo): before = open(C.CONFIG_FILE).read() runner = Runner(out_of_date_repo.python_hooks_repo) - ret = autoupdate(runner) + ret = commands.autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 0 assert before != after @@ -162,14 +158,14 @@ def hook_disappearing_repo(python_hooks_repo): def test_hook_disppearing_repo_raises(hook_disappearing_repo): - with pytest.raises(RepositoryCannotBeUpdatedError): - _update_repository(hook_disappearing_repo.repo_config) + with pytest.raises(commands.RepositoryCannotBeUpdatedError): + commands._update_repository(hook_disappearing_repo.repo_config) def test_autoupdate_hook_disappearing_repo(hook_disappearing_repo): before = open(C.CONFIG_FILE).read() runner = Runner(hook_disappearing_repo.python_hooks_repo) - ret = autoupdate(runner) + ret = commands.autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 1 assert before == after @@ -177,11 +173,105 @@ def test_autoupdate_hook_disappearing_repo(hook_disappearing_repo): def test_clean(empty_git_dir): os.mkdir(C.HOOKS_WORKSPACE) - clean(Runner(empty_git_dir)) + commands.clean(Runner(empty_git_dir)) assert not os.path.exists(C.HOOKS_WORKSPACE) def test_clean_empty(empty_git_dir): assert not os.path.exists(C.HOOKS_WORKSPACE) - clean(Runner(empty_git_dir)) + commands.clean(Runner(empty_git_dir)) assert not os.path.exists(C.HOOKS_WORKSPACE) + + +def stage_a_file(): + local['touch']['foo.py']() + local['git']['add', 'foo.py']() + + +def get_write_mock_output(write_mock): + return ''.join(call[0][0] for call in write_mock.call_args_list) + + +def test_run_all_hooks_passing(repo_with_passing_hook): + stage_a_file() + runner = Runner(repo_with_passing_hook) + args = auto_namedtuple( + all_files=False, color=False, verbose=False, hook=None, + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 0 + printed = get_write_mock_output(write_mock) + assert 'Bash hook' in printed + assert 'Passed' in printed + + +def test_verbose_prints_output(repo_with_passing_hook): + stage_a_file() + runner = Runner(repo_with_passing_hook) + args = auto_namedtuple( + all_files=False, color=False, verbose=True, hook=None, + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 0 + printed = get_write_mock_output(write_mock) + assert 'foo.py\nHello World\n' in printed + + +def test_run_all_hooks_failing(repo_with_failing_hook): + stage_a_file() + runner = Runner(repo_with_failing_hook) + args = auto_namedtuple( + all_files=False, color=False, verbose=False, hook=None, + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 1 + printed = get_write_mock_output(write_mock) + assert 'Failing hook' in printed + assert 'Failed' in printed + assert 'Fail\nfoo.py\n' in printed + + +def test_run_a_specific_hook(repo_with_passing_hook): + stage_a_file() + runner = Runner(repo_with_passing_hook) + args = auto_namedtuple( + all_files=False, color=False, verbose=False, hook='bash_hook', + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 0 + printed = get_write_mock_output(write_mock) + assert 'Bash hook' in printed + assert 'Passed' in printed + + +def test_run_a_non_existing_hook(repo_with_passing_hook): + stage_a_file() + runner = Runner(repo_with_passing_hook) + args = auto_namedtuple( + all_files=False, color=False, verbose=False, hook='nope', + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 1 + printed = get_write_mock_output(write_mock) + assert 'No hook with id `nope`' in printed + + +def test_run_all_files(repo_with_passing_hook): + stage_a_file() + runner = Runner(repo_with_passing_hook) + args = auto_namedtuple( + all_files=True, color=False, verbose=True, hook=None, + ) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + assert ret == 0 + printed = get_write_mock_output(write_mock) + # These are all the files checked into the repo. + # This seems kind of weird but it is because py.test reuses fixtures + for filename in 'hooks.yaml bin/hook.sh foo.py dummy'.split(): + assert filename in printed diff --git a/tests/conftest.py b/tests/conftest.py index 6c0ee1e0..e7be74a0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,8 +2,10 @@ from __future__ import absolute_import import pytest import time +import yaml 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 @@ -68,6 +70,11 @@ def script_hooks_repo(dummy_git_repo): yield _make_repo(dummy_git_repo, 'script_hooks_repo') +@pytest.yield_fixture +def failing_hook_repo(dummy_git_repo): + yield _make_repo(dummy_git_repo, 'failing_hook_repo') + + def _make_config(path, hook_id, file_regex): config = { 'repo': path, @@ -96,4 +103,26 @@ def config_for_prints_cwd_repo(prints_cwd_repo): @pytest.yield_fixture def config_for_script_hooks_repo(script_hooks_repo): - yield _make_config(script_hooks_repo, 'bash_hook', '^$') + yield _make_config(script_hooks_repo, 'bash_hook', '') + + +def _make_repo_from_configs(*configs): + with open(C.CONFIG_FILE, 'w') as config_file: + yaml.dump( + configs, + stream=config_file, + Dumper=yaml.SafeDumper, + **C.YAML_DUMP_KWARGS + ) + + +@pytest.yield_fixture +def repo_with_passing_hook(config_for_script_hooks_repo, empty_git_dir): + _make_repo_from_configs(config_for_script_hooks_repo) + yield empty_git_dir + + +@pytest.yield_fixture +def repo_with_failing_hook(failing_hook_repo, empty_git_dir): + _make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook', '')) + yield empty_git_dir