From 931c69b3faccffda8c18233c743cda6493737dcd Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 14 Jan 2015 19:21:32 -0800 Subject: [PATCH] Simplify a few things --- pre_commit/commands/install_uninstall.py | 21 +++++++-------------- pre_commit/commands/run.py | 13 +++++-------- pre_commit/main.py | 4 ++-- pre_commit/runner.py | 23 ----------------------- tests/commands/install_uninstall_test.py | 22 ++++------------------ tests/commands/run_test.py | 20 +++++++++++--------- tests/runner_test.py | 23 ----------------------- 7 files changed, 29 insertions(+), 97 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 61c8e062..b2af14d4 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -9,6 +9,7 @@ import stat import sys from pre_commit.logging_handler import LoggingHandler +from pre_commit.util import resource_filename logger = logging.getLogger('pre_commit') @@ -41,19 +42,10 @@ def make_executable(filename): ) -def get_hook_path(runner, hook_type): - if hook_type == 'pre-commit': - hook_path = runner.pre_commit_path - legacy_path = runner.pre_commit_legacy_path - else: - hook_path = runner.pre_push_path - legacy_path = runner.pre_push_legacy_path - return hook_path, legacy_path - - def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): """Install the pre-commit hooks.""" - hook_path, legacy_path = get_hook_path(runner, hook_type) + hook_path = runner.get_hook_path(hook_type) + legacy_path = hook_path + '.legacy' # If we have an existing hook, move it to pre-commit.legacy if ( @@ -76,12 +68,12 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): with io.open(hook_path, 'w') as pre_commit_file_obj: if hook_type == 'pre-push': - with io.open(runner.pre_push_template) as fp: + with io.open(resource_filename('pre-push-tmpl')) as fp: pre_push_contents = fp.read() else: pre_push_contents = '' - contents = io.open(runner.pre_template).read().format( + contents = io.open(resource_filename('hook-tmpl')).read().format( sys_executable=sys.executable, hook_type=hook_type, pre_push=pre_push_contents, @@ -104,7 +96,8 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): def uninstall(runner, hook_type='pre-commit'): """Uninstall the pre-commit hooks.""" - hook_path, legacy_path = get_hook_path(runner, hook_type) + hook_path = runner.get_hook_path(hook_type) + legacy_path = hook_path + '.legacy' # If our file doesn't exist or it isn't ours, gtfo. if ( not os.path.exists(hook_path) or ( diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 2817a533..a9e59d55 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -50,18 +50,16 @@ def _print_user_skipped(hook, write, args): def get_changed_files(new, old): - changed_files = cmd_output( + return cmd_output( 'git', 'diff', '--name-only', '{0}..{1}'.format(old, new), )[1].splitlines() - for f in changed_files: - if f: - yield f def _run_single_hook(runner, repository, hook, args, write, skips=set()): if args.origin and args.source: get_filenames = git.get_files_matching( - lambda: get_changed_files(args.origin, args.source)) + lambda: get_changed_files(args.origin, args.source), + ) elif args.files: get_filenames = git.get_files_matching(lambda: args.files) elif args.all_files: @@ -150,9 +148,8 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ): if _has_unmerged_paths(runner): logger.error('Unmerged files. Resolve before committing.') return 1 - if (args.source and not args.origin) or \ - (args.origin and not args.source): - logger.error('--origin and --source depend on each other.') + if bool(args.source) != bool(args.origin): + logger.error('Specify both --origin and --source.') return 1 # Don't stash if specified or files are specified diff --git a/pre_commit/main.py b/pre_commit/main.py index 6cf6bd33..64c04047 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -79,11 +79,11 @@ def main(argv=None): ) run_parser.add_argument( - '--origin', '-o', default='', + '--origin', '-o', help='The origin branch"s commit_id when using `git push`', ) run_parser.add_argument( - '--source', '-s', default='', + '--source', '-s', help='The remote branch"s commit_id when using `git push`', ) run_mutex_group = run_parser.add_mutually_exclusive_group(required=False) diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 302bacb0..ae720d05 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -10,7 +10,6 @@ from pre_commit import git from pre_commit.clientlib.validate_config import load_config from pre_commit.repository import Repository from pre_commit.store import Store -from pre_commit.util import resource_filename class Runner(object): @@ -55,28 +54,6 @@ class Runner(object): def pre_push_path(self): return self.get_hook_path('pre-push') - @cached_property - def pre_template(self): - return resource_filename('hook-tmpl') - - @cached_property - def pre_push_template(self): - return resource_filename('pre-push-tmpl') - - @property - def pre_commit_legacy_path(self): - """The path in the 'hooks' directory representing the temporary - storage for existing pre-commit hooks. - """ - return self.pre_commit_path + '.legacy' - - @property - def pre_push_legacy_path(self): - """The path in the 'hooks' directory representing the temporary - storage for existing pre-push hooks. - """ - return self.pre_push_path + '.legacy' - @cached_property def cmd_runner(self): # TODO: remove this and inline runner.store.cmd_runner diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 9733b175..658e8966 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -10,7 +10,7 @@ import subprocess import sys import mock -from pre_commit.commands.install_uninstall import get_hook_path + from pre_commit.commands.install_uninstall import IDENTIFYING_HASH from pre_commit.commands.install_uninstall import install from pre_commit.commands.install_uninstall import is_our_pre_commit @@ -22,7 +22,6 @@ from pre_commit.runner import Runner from pre_commit.util import cmd_output from pre_commit.util import cwd from pre_commit.util import resource_filename - from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo @@ -71,11 +70,12 @@ def test_install_pre_commit(tmpdir_factory): assert ret == 0 assert os.path.exists(runner.pre_push_path) pre_push_contents = io.open(runner.pre_push_path).read() - pre_push_template_contents = io.open(runner.pre_push_template).read() + pre_push_tmpl = resource_filename('pre-push-tmpl') + pre_push_template_contents = io.open(pre_push_tmpl).read() expected_contents = io.open(pre_commit_script).read().format( sys_executable=sys.executable, hook_type='pre-push', - pre_push=pre_push_template_contents + pre_push=pre_push_template_contents, ) assert pre_push_contents == expected_contents @@ -406,17 +406,3 @@ def test_installed_from_venv(tmpdir_factory): ) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) - - -def test_get_hook_path(tmpdir_factory): - path = git_dir(tmpdir_factory) - with cwd(path): - runner = Runner(path) - expected_paths = (os.path.join(path, '.git/hooks/pre-commit'), - os.path.join(path, '.git/hooks/pre-commit.legacy') - ) - assert expected_paths == get_hook_path(runner, 'pre-commit') - expected_paths = (os.path.join(path, '.git/hooks/pre-push'), - os.path.join(path, '.git/hooks/pre-push.legacy') - ) - assert expected_paths == get_hook_path(runner, 'pre-push') diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 1a1a2fb9..e67c02bd 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -132,20 +132,21 @@ def test_run( @pytest.mark.parametrize( - ('origin', 'source', 'expect_stash'), + ('origin', 'source', 'expect_failure'), ( ('master', 'master', False), ('master', '', True), ('', 'master', True), ) ) -def test_origin_source_define( - repo_with_passing_hook, origin, source, expect_stash, - mock_out_store_directory): +def test_origin_source_error_msg( + repo_with_passing_hook, origin, source, expect_failure, + mock_out_store_directory, +): args = _get_opts(origin=origin, source=source) ret, printed = _do_run(repo_with_passing_hook, args) - warning_msg = '--origin and --source depend on each other.' - if expect_stash: + warning_msg = 'Specify both --origin and --source.' + if expect_failure: assert ret == 1 assert warning_msg in printed else: @@ -297,7 +298,8 @@ def test_stdout_write_bug_py26( def test_get_changed_files(): - files = list(get_changed_files('78c682a1d13ba20e7cb735313b9314a74365cd3a', - '3387edbb1288a580b37fe25225aa0b856b18ad1a' - )) + files = get_changed_files( + '78c682a1d13ba20e7cb735313b9314a74365cd3a', + '3387edbb1288a580b37fe25225aa0b856b18ad1a', + ) assert files == ['CHANGELOG.md', 'setup.py'] diff --git a/tests/runner_test.py b/tests/runner_test.py index 41bffa1c..249cc2c4 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -7,7 +7,6 @@ import os.path import pre_commit.constants as C from pre_commit.runner import Runner from pre_commit.util import cwd -from pre_commit.util import resource_filename from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo @@ -65,28 +64,6 @@ def test_pre_push_path(): assert runner.pre_push_path == expected_path -def test_pre_commit_legacy_path(): - runner = Runner('foo/bar') - expected_path = os.path.join('foo/bar', '.git/hooks/pre-commit.legacy') - assert runner.pre_commit_legacy_path == expected_path - - -def test_pre_push_legacy_path(): - runner = Runner('foo/bar') - expected_path = os.path.join('foo/bar', '.git/hooks/pre-push.legacy') - assert runner.pre_push_legacy_path == expected_path - - -def test_pre_template(): - runner = Runner('foo/bar') - assert runner.pre_template == resource_filename('hook-tmpl') - - -def test_pre_push_template(): - runner = Runner('foo/bar') - assert runner.pre_push_template == resource_filename('pre-push-tmpl') - - def test_cmd_runner(mock_out_store_directory): runner = Runner('foo/bar') ret = runner.cmd_runner