From f1c00eefe4d8874baef2b4ab6f2a50fb474abe89 Mon Sep 17 00:00:00 2001 From: Jacob Scott Date: Fri, 2 Dec 2016 11:06:15 -0800 Subject: [PATCH 1/4] Add option to run from alternate config file --- pre_commit/main.py | 11 ++++++++++- pre_commit/runner.py | 9 +++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pre_commit/main.py b/pre_commit/main.py index 9d9329a2..ab3bbea7 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -34,6 +34,10 @@ def _add_color_option(parser): ) +def _add_config_option(parser): + parser.add_argument('-c', '--config', help='Path to alternate config file') + + def main(argv=None): argv = argv if argv is not None else sys.argv[1:] argv = [five.to_text(arg) for arg in argv] @@ -89,6 +93,7 @@ def main(argv=None): help="Auto-update pre-commit config to the latest repos' versions.", ) _add_color_option(autoupdate_parser) + _add_config_option(autoupdate_parser) run_parser = subparsers.add_parser('run', help='Run hooks.') _add_color_option(run_parser) @@ -119,6 +124,7 @@ def main(argv=None): '--hook-stage', choices=('commit', 'push'), default='commit', help='The stage during which the hook is fired e.g. commit or push.', ) + _add_config_option(run_parser) run_mutex_group = run_parser.add_mutually_exclusive_group(required=False) run_mutex_group.add_argument( '--all-files', '-a', action='store_true', default=False, @@ -152,7 +158,10 @@ def main(argv=None): with error_handler(): add_logging_handler(args.color) - runner = Runner.create() + runner_kwargs = {} + if hasattr(args, 'config_file'): + runner_kwargs['config_file'] = args.config_file + runner = Runner.create(**runner_kwargs) git.check_for_cygwin_mismatch() if args.command == 'install': diff --git a/pre_commit/runner.py b/pre_commit/runner.py index c055b126..c44f00ae 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -16,18 +16,19 @@ class Runner(object): repository under test. """ - def __init__(self, git_root): + def __init__(self, git_root, config_file=None): self.git_root = git_root + self.config_file = config_file or C.CONFIG_FILE @classmethod - def create(cls): + def create(cls, config_file=None): """Creates a PreCommitRunner by doing the following: - Finds the root of the current git repository - chdir to that directory """ root = git.get_root() os.chdir(root) - return cls(root) + return cls(root, config_file=config_file) @cached_property def git_dir(self): @@ -35,7 +36,7 @@ class Runner(object): @cached_property def config_file_path(self): - return os.path.join(self.git_root, C.CONFIG_FILE) + return os.path.join(self.git_root, self.config_file) @cached_property def repositories(self): From f205e6d1702114a4d0a3b9fe069405259dfe3994 Mon Sep 17 00:00:00 2001 From: Jacob Scott Date: Fri, 2 Dec 2016 13:53:59 -0800 Subject: [PATCH 2/4] Incoroporate PR feedback * Make config_file a required argument to Runner * Update main.py * Update tests to make them all green New test to test alternate config functionality coming in next commit --- pre_commit/main.py | 17 ++++---- pre_commit/runner.py | 9 ++--- tests/commands/autoupdate_test.py | 12 +++--- tests/commands/install_uninstall_test.py | 51 ++++++++++++------------ tests/commands/run_test.py | 12 +++--- tests/conftest.py | 3 +- tests/runner_test.py | 18 ++++----- 7 files changed, 63 insertions(+), 59 deletions(-) diff --git a/pre_commit/main.py b/pre_commit/main.py index ab3bbea7..280f5a5b 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -35,7 +35,10 @@ def _add_color_option(parser): def _add_config_option(parser): - parser.add_argument('-c', '--config', help='Path to alternate config file') + parser.add_argument( + '-c', '--config', default='.pre-commit-config.yaml', + help='Path to alternate config file' + ) def main(argv=None): @@ -43,6 +46,7 @@ def main(argv=None): argv = [five.to_text(arg) for arg in argv] parser = argparse.ArgumentParser() + parser.set_defaults(config='.pre-commit-config.yaml') # http://stackoverflow.com/a/8521644/812183 parser.add_argument( '-V', '--version', @@ -58,6 +62,7 @@ def main(argv=None): 'install', help='Install the pre-commit script.', ) _add_color_option(install_parser) + _add_config_option(install_parser) install_parser.add_argument( '-f', '--overwrite', action='store_true', help='Overwrite existing hooks / remove migration mode.', @@ -78,6 +83,7 @@ def main(argv=None): 'uninstall', help='Uninstall the pre-commit script.', ) _add_color_option(uninstall_parser) + _add_config_option(uninstall_parser) uninstall_parser.add_argument( '-t', '--hook-type', choices=('pre-commit', 'pre-push'), default='pre-commit', @@ -87,7 +93,7 @@ def main(argv=None): 'clean', help='Clean out pre-commit files.', ) _add_color_option(clean_parser) - + _add_config_option(clean_parser) autoupdate_parser = subparsers.add_parser( 'autoupdate', help="Auto-update pre-commit config to the latest repos' versions.", @@ -97,6 +103,7 @@ def main(argv=None): run_parser = subparsers.add_parser('run', help='Run hooks.') _add_color_option(run_parser) + _add_config_option(run_parser) run_parser.add_argument('hook', nargs='?', help='A single hook-id to run') run_parser.add_argument( '--no-stash', default=False, action='store_true', @@ -124,7 +131,6 @@ def main(argv=None): '--hook-stage', choices=('commit', 'push'), default='commit', help='The stage during which the hook is fired e.g. commit or push.', ) - _add_config_option(run_parser) run_mutex_group = run_parser.add_mutually_exclusive_group(required=False) run_mutex_group.add_argument( '--all-files', '-a', action='store_true', default=False, @@ -158,10 +164,7 @@ def main(argv=None): with error_handler(): add_logging_handler(args.color) - runner_kwargs = {} - if hasattr(args, 'config_file'): - runner_kwargs['config_file'] = args.config_file - runner = Runner.create(**runner_kwargs) + runner = Runner.create(args.config) git.check_for_cygwin_mismatch() if args.command == 'install': diff --git a/pre_commit/runner.py b/pre_commit/runner.py index c44f00ae..985e6456 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -4,7 +4,6 @@ import os.path from cached_property import cached_property -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 @@ -16,19 +15,19 @@ class Runner(object): repository under test. """ - def __init__(self, git_root, config_file=None): + def __init__(self, git_root, config_file): self.git_root = git_root - self.config_file = config_file or C.CONFIG_FILE + self.config_file = config_file @classmethod - def create(cls, config_file=None): + def create(cls, config_file): """Creates a PreCommitRunner by doing the following: - Finds the root of the current git repository - chdir to that directory """ root = git.get_root() os.chdir(root) - return cls(root, config_file=config_file) + return cls(root, config_file) @cached_property def git_dir(self): diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index 62a0269f..8924fb84 100644 --- a/tests/commands/autoupdate_test.py +++ b/tests/commands/autoupdate_test.py @@ -45,7 +45,7 @@ def test_autoupdate_up_to_date_repo( before = open(C.CONFIG_FILE).read() assert '^$' not in before - runner = Runner('.') + runner = Runner('.', C.CONFIG_FILE) ret = autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 0 @@ -86,7 +86,7 @@ def test_autoupdate_out_of_date_repo( write_config('.', config) before = open(C.CONFIG_FILE).read() - runner = Runner('.') + runner = Runner('.', C.CONFIG_FILE) ret = autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 0 @@ -111,7 +111,7 @@ def test_autoupdate_tagged_repo( ) write_config('.', config) - ret = autoupdate(Runner('.')) + ret = autoupdate(Runner('.', C.CONFIG_FILE)) assert ret == 0 assert 'v1.2.3' in open(C.CONFIG_FILE).read() @@ -156,7 +156,7 @@ def test_autoupdate_hook_disappearing_repo( write_config('.', config) before = open(C.CONFIG_FILE).read() - runner = Runner('.') + runner = Runner('.', C.CONFIG_FILE) ret = autoupdate(runner) after = open(C.CONFIG_FILE).read() assert ret == 1 @@ -167,7 +167,7 @@ def test_autoupdate_local_hooks(tempdir_factory): git_path = git_dir(tempdir_factory) config = config_with_local_hooks() path = add_config_to_repo(git_path, config) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) assert autoupdate(runner) == 0 new_config_writen = load_config(runner.config_file_path) assert len(new_config_writen) == 1 @@ -183,7 +183,7 @@ def test_autoupdate_local_hooks_with_out_of_date_repo( local_config = config_with_local_hooks() config = [local_config, stale_config] write_config('.', config) - runner = Runner('.') + runner = Runner('.', C.CONFIG_FILE) assert autoupdate(runner) == 0 new_config_writen = load_config(runner.config_file_path) assert len(new_config_writen) == 2 diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 2d80d49f..4b285186 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -10,6 +10,7 @@ import sys import mock +import pre_commit.constants as C 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 @@ -53,7 +54,7 @@ def test_is_previous_pre_commit(in_tmpdir): def test_install_pre_commit(tempdir_factory): path = git_dir(tempdir_factory) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) ret = install(runner) assert ret == 0 assert os.path.exists(runner.pre_commit_path) @@ -87,7 +88,7 @@ def test_install_hooks_directory_not_present(tempdir_factory): hooks = os.path.join(path, '.git', 'hooks') if os.path.exists(hooks): # pragma: no cover (latest git) shutil.rmtree(hooks) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) install(runner) assert os.path.exists(runner.pre_commit_path) @@ -97,7 +98,7 @@ def test_install_hooks_dead_symlink( tempdir_factory, ): # pragma: no cover (non-windows) path = git_dir(tempdir_factory) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) mkdirp(os.path.dirname(runner.pre_commit_path)) os.symlink('/fake/baz', os.path.join(path, '.git', 'hooks', 'pre-commit')) install(runner) @@ -106,14 +107,14 @@ def test_install_hooks_dead_symlink( def test_uninstall_does_not_blow_up_when_not_there(tempdir_factory): path = git_dir(tempdir_factory) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) ret = uninstall(runner) assert ret == 0 def test_uninstall(tempdir_factory): path = git_dir(tempdir_factory) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) assert not os.path.exists(runner.pre_commit_path) install(runner) assert os.path.exists(runner.pre_commit_path) @@ -156,7 +157,7 @@ NORMAL_PRE_COMMIT_RUN = re.compile( def test_install_pre_commit_and_run(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -172,7 +173,7 @@ def test_install_in_submodule_and_run(tempdir_factory): sub_pth = os.path.join(parent_path, 'sub') with cwd(sub_pth): - assert install(Runner(sub_pth)) == 0 + assert install(Runner(sub_pth, C.CONFIG_FILE)) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) @@ -189,7 +190,7 @@ def test_commit_am(tempdir_factory): with io.open('unstaged', 'w') as foo_file: foo_file.write('Oh hai') - assert install(Runner(path)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -198,8 +199,8 @@ def test_commit_am(tempdir_factory): def test_install_idempotent(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path)) == 0 - assert install(Runner(path)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -219,7 +220,7 @@ def test_environment_not_sourced(tempdir_factory): with cwd(path): # Patch the executable to simulate rming virtualenv with mock.patch.object(sys, 'executable', '/bin/false'): - assert install(Runner(path)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 # Use a specific homedir to ignore --user installs homedir = tempdir_factory.get() @@ -257,7 +258,7 @@ FAILING_PRE_COMMIT_RUN = re.compile( def test_failing_hooks_returns_nonzero(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'failing_hook_repo') with cwd(path): - assert install(Runner(path)) == 0 + assert install(Runner(path, C.CONFIG_FILE)) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 1 @@ -275,7 +276,7 @@ EXISTING_COMMIT_RUN = re.compile( def test_install_existing_hooks_no_overwrite(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Write out an "old" hook mkdirp(os.path.dirname(runner.pre_commit_path)) @@ -301,7 +302,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory): def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Write out an "old" hook mkdirp(os.path.dirname(runner.pre_commit_path)) @@ -330,7 +331,7 @@ FAIL_OLD_HOOK = re.compile( def test_failing_existing_hook_returns_1(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Write out a failing "old" hook mkdirp(os.path.dirname(runner.pre_commit_path)) @@ -349,7 +350,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory): def test_install_overwrite_no_existing_hooks(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path), overwrite=True) == 0 + assert install(Runner(path, C.CONFIG_FILE), overwrite=True) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -359,7 +360,7 @@ def test_install_overwrite_no_existing_hooks(tempdir_factory): def test_install_overwrite(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Write out the "old" hook mkdirp(os.path.dirname(runner.pre_commit_path)) @@ -377,7 +378,7 @@ def test_install_overwrite(tempdir_factory): def test_uninstall_restores_legacy_hooks(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Write out an "old" hook mkdirp(os.path.dirname(runner.pre_commit_path)) @@ -398,7 +399,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory): def test_replace_old_commit_script(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) # Install a script that looks like our old script pre_commit_contents = io.open( @@ -424,7 +425,7 @@ def test_replace_old_commit_script(tempdir_factory): def test_uninstall_doesnt_remove_not_our_hooks(tempdir_factory): path = git_dir(tempdir_factory) with cwd(path): - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) mkdirp(os.path.dirname(runner.pre_commit_path)) with io.open(runner.pre_commit_path, 'w') as pre_commit_file: pre_commit_file.write('#!/usr/bin/env bash\necho 1\n') @@ -449,7 +450,7 @@ def test_installs_hooks_with_hooks_True( ): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - install(Runner(path), hooks=True) + install(Runner(path, C.CONFIG_FILE), hooks=True) ret, output = _get_commit_output( tempdir_factory, pre_commit_home=mock_out_store_directory, ) @@ -461,7 +462,7 @@ def test_installs_hooks_with_hooks_True( def test_installed_from_venv(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - install(Runner(path)) + install(Runner(path, C.CONFIG_FILE)) # No environment so pre-commit is not on the path when running! # Should still pick up the python from when we installed ret, output = _get_commit_output( @@ -495,7 +496,7 @@ def test_pre_push_integration_failing(tempdir_factory): path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') # commit succeeds because pre-commit is only installed for pre-push assert _get_commit_output(tempdir_factory)[0] == 0 @@ -511,7 +512,7 @@ def test_pre_push_integration_accepted(tempdir_factory): path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') assert _get_commit_output(tempdir_factory)[0] == 0 retc, output = _get_push_output(tempdir_factory) @@ -525,7 +526,7 @@ def test_pre_push_integration_empty_push(tempdir_factory): path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') _get_push_output(tempdir_factory) retc, output = _get_push_output(tempdir_factory) assert output == 'Everything up-to-date\n' diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index d0948378..a7a651ed 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -75,7 +75,7 @@ def _get_opts( def _do_run(cap_out, repo, args, environ={}): - runner = Runner(repo) + runner = Runner(repo, C.CONFIG_FILE) with cwd(runner.git_root): # replicates Runner.create behaviour ret = run(runner, args, environ=environ) printed = cap_out.get_bytes() @@ -375,7 +375,7 @@ def test_non_ascii_hook_id( repo_with_passing_hook, mock_out_store_directory, tempdir_factory, ): with cwd(repo_with_passing_hook): - install(Runner(repo_with_passing_hook)) + install(Runner(repo_with_passing_hook, C.CONFIG_FILE)) _, stdout, _ = cmd_output_mocked_pre_commit_home( sys.executable, '-m', 'pre_commit.main', 'run', '☃', retcode=None, tempdir_factory=tempdir_factory, @@ -393,7 +393,7 @@ def test_stdout_write_bug_py26( config[0]['hooks'][0]['args'] = ['☃'] stage_a_file() - install(Runner(repo_with_failing_hook)) + install(Runner(repo_with_failing_hook, C.CONFIG_FILE)) # Have to use subprocess because pytest monkeypatches sys.stdout _, stdout, _ = cmd_output_mocked_pre_commit_home( @@ -411,7 +411,7 @@ def test_stdout_write_bug_py26( def test_hook_install_failure(mock_out_store_directory, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'not_installable_repo') with cwd(git_path): - install(Runner(git_path)) + install(Runner(git_path, C.CONFIG_FILE)) _, stdout, _ = cmd_output_mocked_pre_commit_home( 'git', 'commit', '-m', 'Commit!', @@ -460,7 +460,7 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): open(filename, 'w').close() cmd_output('bash', '-c', 'git add .') - install(Runner(git_path)) + install(Runner(git_path, C.CONFIG_FILE)) cmd_output_mocked_pre_commit_home( 'git', 'commit', '-m', 'Commit!', @@ -646,7 +646,7 @@ def test_files_running_subdir( repo_with_passing_hook, mock_out_store_directory, tempdir_factory, ): with cwd(repo_with_passing_hook): - install(Runner(repo_with_passing_hook)) + install(Runner(repo_with_passing_hook, C.CONFIG_FILE)) os.mkdir('subdir') open('subdir/foo.py', 'w').close() diff --git a/tests/conftest.py b/tests/conftest.py index f32cc6c7..058780bb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,7 @@ import os.path import mock import pytest +import pre_commit.constants as C from pre_commit import five from pre_commit import output from pre_commit.logging_handler import add_logging_handler @@ -136,7 +137,7 @@ def cmd_runner(tempdir_factory): @pytest.yield_fixture def runner_with_mocked_store(mock_out_store_directory): - yield Runner('/') + yield Runner('/', C.CONFIG_FILE) @pytest.yield_fixture diff --git a/tests/runner_test.py b/tests/runner_test.py index 29bbee3e..f93cb360 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -15,7 +15,7 @@ from testing.fixtures import make_consuming_repo def test_init_has_no_side_effects(tmpdir): current_wd = os.getcwd() - runner = Runner(tmpdir.strpath) + runner = Runner(tmpdir.strpath, C.CONFIG_FILE) assert runner.git_root == tmpdir.strpath assert os.getcwd() == current_wd @@ -23,7 +23,7 @@ def test_init_has_no_side_effects(tmpdir): def test_create_sets_correct_directory(tempdir_factory): path = git_dir(tempdir_factory) with cwd(path): - runner = Runner.create() + runner = Runner.create(C.CONFIG_FILE) assert os.path.normcase(runner.git_root) == os.path.normcase(path) assert os.path.normcase(os.getcwd()) == os.path.normcase(path) @@ -37,20 +37,20 @@ def test_create_changes_to_git_root(tempdir_factory): os.chdir(foo_path) assert os.getcwd() != path - runner = Runner.create() + runner = Runner.create(C.CONFIG_FILE) assert os.path.normcase(runner.git_root) == os.path.normcase(path) assert os.path.normcase(os.getcwd()) == os.path.normcase(path) def test_config_file_path(): - runner = Runner(os.path.join('foo', 'bar')) + runner = Runner(os.path.join('foo', 'bar'), C.CONFIG_FILE) expected_path = os.path.join('foo', 'bar', C.CONFIG_FILE) assert runner.config_file_path == expected_path def test_repositories(tempdir_factory, mock_out_store_directory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) assert len(runner.repositories) == 1 @@ -74,7 +74,7 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory): )) git_path = git_dir(tempdir_factory) add_config_to_repo(git_path, config) - runner = Runner(git_path) + runner = Runner(git_path, C.CONFIG_FILE) assert len(runner.repositories) == 1 assert len(runner.repositories[0].hooks) == 2 @@ -82,7 +82,7 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory): def test_pre_commit_path(in_tmpdir): path = os.path.join('foo', 'bar') cmd_output('git', 'init', path) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) expected_path = os.path.join(path, '.git', 'hooks', 'pre-commit') assert runner.pre_commit_path == expected_path @@ -90,12 +90,12 @@ def test_pre_commit_path(in_tmpdir): def test_pre_push_path(in_tmpdir): path = os.path.join('foo', 'bar') cmd_output('git', 'init', path) - runner = Runner(path) + runner = Runner(path, C.CONFIG_FILE) expected_path = os.path.join(path, '.git', 'hooks', 'pre-push') assert runner.pre_push_path == expected_path def test_cmd_runner(mock_out_store_directory): - runner = Runner(os.path.join('foo', 'bar')) + runner = Runner(os.path.join('foo', 'bar'), C.CONFIG_FILE) ret = runner.cmd_runner assert ret.prefix_dir == os.path.join(mock_out_store_directory) + os.sep From 727247e6edfa2fb48ff050958eeac65564693394 Mon Sep 17 00:00:00 2001 From: Jacob Scott Date: Fri, 2 Dec 2016 16:19:34 -0800 Subject: [PATCH 3/4] Add tests for alternate config --- testing/fixtures.py | 18 ++++++++++++------ tests/commands/run_test.py | 30 ++++++++++++++++++++++++++---- tests/runner_test.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/testing/fixtures.py b/testing/fixtures.py index 36c7400c..fdf651e8 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -94,18 +94,24 @@ def make_config_from_repo(repo_path, sha=None, hooks=None, check=True): return config -def write_config(directory, config): +def read_config(directory, config_file=C.CONFIG_FILE): + config_path = os.path.join(directory, config_file) + config = ordered_load(io.open(config_path).read()) + return config + + +def write_config(directory, config, config_file=C.CONFIG_FILE): if type(config) is not list: assert type(config) is OrderedDict config = [config] - with io.open(os.path.join(directory, C.CONFIG_FILE), 'w') as config_file: - config_file.write(ordered_dump(config, **C.YAML_DUMP_KWARGS)) + with io.open(os.path.join(directory, config_file), 'w') as outfile: + outfile.write(ordered_dump(config, **C.YAML_DUMP_KWARGS)) -def add_config_to_repo(git_path, config): - write_config(git_path, config) +def add_config_to_repo(git_path, config, config_file=C.CONFIG_FILE): + write_config(git_path, config, config_file=config_file) with cwd(git_path): - cmd_output('git', 'add', C.CONFIG_FILE) + cmd_output('git', 'add', config_file) cmd_output('git', 'commit', '-m', 'Add hooks config') return git_path diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index a7a651ed..86d0ecd4 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -24,6 +24,7 @@ from testing.auto_namedtuple import auto_namedtuple from testing.fixtures import add_config_to_repo from testing.fixtures import make_consuming_repo from testing.fixtures import modify_config +from testing.fixtures import read_config from testing.util import cmd_output_mocked_pre_commit_home @@ -74,19 +75,20 @@ def _get_opts( ) -def _do_run(cap_out, repo, args, environ={}): - runner = Runner(repo, C.CONFIG_FILE) +def _do_run(cap_out, repo, args, environ={}, config_file=C.CONFIG_FILE): + runner = Runner(repo, config_file) with cwd(runner.git_root): # replicates Runner.create behaviour ret = run(runner, args, environ=environ) printed = cap_out.get_bytes() return ret, printed -def _test_run(cap_out, repo, opts, expected_outputs, expected_ret, stage): +def _test_run(cap_out, repo, opts, expected_outputs, expected_ret, stage, + config_file=C.CONFIG_FILE): if stage: stage_a_file() args = _get_opts(**opts) - ret, printed = _do_run(cap_out, repo, args) + ret, printed = _do_run(cap_out, repo, args, config_file=config_file) assert ret == expected_ret, (ret, expected_ret, printed) for expected_output_part in expected_outputs: @@ -205,6 +207,26 @@ def test_always_run( ) +def test_always_run_alt_config( + cap_out, repo_with_passing_hook, mock_out_store_directory, +): + repo_root = '.' + config = read_config(repo_root) + config[0]['hooks'][0]['always_run'] = True + alt_config_file = 'alternate_config.yaml' + add_config_to_repo(repo_root, config, config_file=alt_config_file) + + _test_run( + cap_out, + repo_with_passing_hook, + {}, + (b'Bash hook', b'Passed'), + 0, + stage=False, + config_file=alt_config_file + ) + + @pytest.mark.parametrize( ('origin', 'source', 'expect_failure'), ( diff --git a/tests/runner_test.py b/tests/runner_test.py index f93cb360..9039e573 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -79,6 +79,38 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory): assert len(runner.repositories[0].hooks) == 2 +def test_local_hooks_alt_config(tempdir_factory, mock_out_store_directory): + config = OrderedDict(( + ('repo', 'local'), + ('hooks', (OrderedDict(( + ('id', 'arg-per-line'), + ('name', 'Args per line hook'), + ('entry', 'bin/hook.sh'), + ('language', 'script'), + ('files', ''), + ('args', ['hello', 'world']), + )), OrderedDict(( + ('id', 'ugly-format-json'), + ('name', 'Ugly format json'), + ('entry', 'ugly-format-json'), + ('language', 'python'), + ('files', ''), + )), OrderedDict(( + ('id', 'do_not_commit'), + ('name', 'Block if "DO NOT COMMIT" is found'), + ('entry', 'DO NOT COMMIT'), + ('language', 'pcre'), + ('files', '^(.*)$'), + )))) + )) + git_path = git_dir(tempdir_factory) + alt_config_file = 'alternate_config.yaml' + add_config_to_repo(git_path, config, config_file=alt_config_file) + runner = Runner(git_path, alt_config_file) + assert len(runner.repositories) == 1 + assert len(runner.repositories[0].hooks) == 3 + + def test_pre_commit_path(in_tmpdir): path = os.path.join('foo', 'bar') cmd_output('git', 'init', path) From 372069f3e5dd7ffa9bb17baf4c046faf72579f85 Mon Sep 17 00:00:00 2001 From: Jacob Scott Date: Sat, 3 Dec 2016 10:47:38 -0800 Subject: [PATCH 4/4] minor cleanup --- pre_commit/main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pre_commit/main.py b/pre_commit/main.py index 280f5a5b..e1101209 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -46,7 +46,6 @@ def main(argv=None): argv = [five.to_text(arg) for arg in argv] parser = argparse.ArgumentParser() - parser.set_defaults(config='.pre-commit-config.yaml') # http://stackoverflow.com/a/8521644/812183 parser.add_argument( '-V', '--version',