diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 91912226..6b2d16f5 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -7,6 +7,7 @@ import os.path import sys from pre_commit import output +from pre_commit.repository import repositories from pre_commit.util import cmd_output from pre_commit.util import make_executable from pre_commit.util import mkdirp @@ -36,7 +37,7 @@ def is_our_script(filename): def install( - runner, overwrite=False, hooks=False, hook_type='pre-commit', + runner, store, overwrite=False, hooks=False, hook_type='pre-commit', skip_on_missing_conf=False, ): """Install the pre-commit hooks.""" @@ -89,13 +90,13 @@ def install( # If they requested we install all of the hooks, do so. if hooks: - install_hooks(runner) + install_hooks(runner, store) return 0 -def install_hooks(runner): - for repository in runner.repositories: +def install_hooks(runner, store): + for repository in repositories(runner.config, store): repository.require_installed() diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index a0725660..b5dcc1e2 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -13,6 +13,7 @@ from pre_commit import color from pre_commit import git from pre_commit import output from pre_commit.output import get_hook_message +from pre_commit.repository import repositories from pre_commit.staged_files_only import staged_files_only from pre_commit.util import cmd_output from pre_commit.util import memoize_by_cwd @@ -223,7 +224,7 @@ def _has_unstaged_config(runner): return retcode == 1 -def run(runner, args, environ=os.environ): +def run(runner, store, args, environ=os.environ): no_stash = args.all_files or bool(args.files) # Check if we have unresolved merge conflict files and fail fast. @@ -248,11 +249,11 @@ def run(runner, args, environ=os.environ): if no_stash: ctx = noop_context() else: - ctx = staged_files_only(runner.store.directory) + ctx = staged_files_only(store.directory) with ctx: repo_hooks = [] - for repo in runner.repositories: + for repo in repositories(runner.config, store): for _, hook in repo.hooks: if ( (not args.hook or hook['id'] == args.hook) and diff --git a/pre_commit/commands/try_repo.py b/pre_commit/commands/try_repo.py index 68154316..431db141 100644 --- a/pre_commit/commands/try_repo.py +++ b/pre_commit/commands/try_repo.py @@ -20,10 +20,11 @@ def try_repo(args): ref = args.ref or git.head_rev(args.repo) with tmpdir() as tempdir: + store = Store(tempdir) if args.hook: hooks = [{'id': args.hook}] else: - repo_path = Store(tempdir).clone(args.repo, ref) + repo_path = store.clone(args.repo, ref) manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE)) manifest = sorted(manifest, key=lambda hook: hook['id']) hooks = [{'id': hook['id']} for hook in manifest] @@ -42,5 +43,4 @@ def try_repo(args): output.write(config_s) output.write_line('=' * 79) - runner = Runner('.', config_filename, store_dir=tempdir) - return run(runner, args) + return run(Runner('.', config_filename), store, args) diff --git a/pre_commit/main.py b/pre_commit/main.py index 4ff8073d..fafe36b1 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -21,6 +21,7 @@ from pre_commit.commands.try_repo import try_repo from pre_commit.error_handler import error_handler from pre_commit.logging_handler import add_logging_handler from pre_commit.runner import Runner +from pre_commit.store import Store logger = logging.getLogger('pre_commit') @@ -230,32 +231,34 @@ def main(argv=None): with error_handler(): add_logging_handler(args.color) runner = Runner.create(args.config) + store = Store() git.check_for_cygwin_mismatch() if args.command == 'install': return install( - runner, overwrite=args.overwrite, hooks=args.install_hooks, + runner, store, + overwrite=args.overwrite, hooks=args.install_hooks, hook_type=args.hook_type, skip_on_missing_conf=args.allow_missing_config, ) elif args.command == 'install-hooks': - return install_hooks(runner) + return install_hooks(runner, store) elif args.command == 'uninstall': return uninstall(runner, hook_type=args.hook_type) elif args.command == 'clean': - return clean(runner.store) + return clean(store) elif args.command == 'autoupdate': if args.tags_only: logger.warning('--tags-only is the default') return autoupdate( - runner, runner.store, + runner, store, tags_only=not args.bleeding_edge, repos=args.repos, ) elif args.command == 'migrate-config': return migrate_config(runner) elif args.command == 'run': - return run(runner, args) + return run(runner, store, args) elif args.command == 'sample-config': return sample_config() elif args.command == 'try-repo': diff --git a/pre_commit/meta_hooks/check_hooks_apply.py b/pre_commit/meta_hooks/check_hooks_apply.py index 20d7f069..23420f46 100644 --- a/pre_commit/meta_hooks/check_hooks_apply.py +++ b/pre_commit/meta_hooks/check_hooks_apply.py @@ -2,17 +2,18 @@ import argparse import pre_commit.constants as C from pre_commit import git +from pre_commit.clientlib import load_config from pre_commit.commands.run import _filter_by_include_exclude from pre_commit.commands.run import _filter_by_types -from pre_commit.runner import Runner +from pre_commit.repository import repositories +from pre_commit.store import Store def check_all_hooks_match_files(config_file): - runner = Runner.create(config_file) files = git.get_all_files() retv = 0 - for repo in runner.repositories: + for repo in repositories(load_config(config_file), Store()): for hook_id, hook in repo.hooks: if hook['always_run']: continue diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 0647d9df..0f12bd9e 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -282,3 +282,7 @@ class MetaRepository(LocalRepository): (hook['id'], _hook(self.manifest_hooks[hook['id']], hook)) for hook in self.repo_config['hooks'] ) + + +def repositories(config, store): + return tuple(Repository.create(x, store) for x in config['repos']) diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 420c62df..a6d0f576 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -6,8 +6,6 @@ from cached_property import cached_property from pre_commit import git from pre_commit.clientlib import load_config -from pre_commit.repository import Repository -from pre_commit.store import Store class Runner(object): @@ -15,10 +13,9 @@ class Runner(object): repository under test. """ - def __init__(self, git_root, config_file, store_dir=None): + def __init__(self, git_root, config_file): self.git_root = git_root self.config_file = config_file - self._store_dir = store_dir @classmethod def create(cls, config_file): @@ -42,12 +39,6 @@ class Runner(object): def config(self): return load_config(self.config_file_path) - @cached_property - def repositories(self): - """Returns a tuple of the configured repositories.""" - repos = self.config['repos'] - return tuple(Repository.create(x, self.store) for x in repos) - def get_hook_path(self, hook_type): return os.path.join(self.git_dir, 'hooks', hook_type) @@ -58,7 +49,3 @@ class Runner(object): @cached_property def pre_push_path(self): return self.get_hook_path('pre-push') - - @cached_property - def store(self): - return Store(self._store_dir) diff --git a/pre_commit/store.py b/pre_commit/store.py index 8251e21b..0ca6b706 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -39,10 +39,7 @@ class Store(object): __created = False def __init__(self, directory=None): - if directory is None: - directory = self.get_default_directory() - - self.directory = directory + self.directory = directory or Store.get_default_directory() @contextlib.contextmanager def exclusive_lock(self): diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 491495f3..83ea38d3 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -45,44 +45,44 @@ def test_is_previous_pre_commit(tmpdir): assert is_our_script(f.strpath) -def test_install_pre_commit(tempdir_factory): +def test_install_pre_commit(tempdir_factory, store): path = git_dir(tempdir_factory) runner = Runner(path, C.CONFIG_FILE) - assert not install(runner) + assert not install(runner, store) assert os.access(runner.pre_commit_path, os.X_OK) - assert not install(runner, hook_type='pre-push') + assert not install(runner, store, hook_type='pre-push') assert os.access(runner.pre_push_path, os.X_OK) -def test_install_hooks_directory_not_present(tempdir_factory): +def test_install_hooks_directory_not_present(tempdir_factory, store): path = git_dir(tempdir_factory) # Simulate some git clients which don't make .git/hooks #234 hooks = os.path.join(path, '.git', 'hooks') if os.path.exists(hooks): # pragma: no cover (latest git) shutil.rmtree(hooks) runner = Runner(path, C.CONFIG_FILE) - install(runner) + install(runner, store) assert os.path.exists(runner.pre_commit_path) -def test_install_refuses_core_hookspath(tempdir_factory): +def test_install_refuses_core_hookspath(tempdir_factory, store): path = git_dir(tempdir_factory) with cwd(path): cmd_output('git', 'config', '--local', 'core.hooksPath', 'hooks') runner = Runner(path, C.CONFIG_FILE) - assert install(runner) + assert install(runner, store) @xfailif_no_symlink def test_install_hooks_dead_symlink( - tempdir_factory, + tempdir_factory, store, ): # pragma: no cover (non-windows) path = git_dir(tempdir_factory) 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) + install(runner, store) assert os.path.exists(runner.pre_commit_path) @@ -93,11 +93,11 @@ def test_uninstall_does_not_blow_up_when_not_there(tempdir_factory): assert ret == 0 -def test_uninstall(tempdir_factory): +def test_uninstall(tempdir_factory, store): path = git_dir(tempdir_factory) runner = Runner(path, C.CONFIG_FILE) assert not os.path.exists(runner.pre_commit_path) - install(runner) + install(runner, store) assert os.path.exists(runner.pre_commit_path) uninstall(runner) assert not os.path.exists(runner.pre_commit_path) @@ -136,29 +136,29 @@ NORMAL_PRE_COMMIT_RUN = re.compile( ) -def test_install_pre_commit_and_run(tempdir_factory): +def test_install_pre_commit_and_run(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) -def test_install_pre_commit_and_run_custom_path(tempdir_factory): +def test_install_pre_commit_and_run_custom_path(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): cmd_output('git', 'mv', C.CONFIG_FILE, 'custom-config.yaml') cmd_output('git', 'commit', '-m', 'move pre-commit config') - assert install(Runner(path, 'custom-config.yaml')) == 0 + assert install(Runner(path, 'custom-config.yaml'), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) -def test_install_in_submodule_and_run(tempdir_factory): +def test_install_in_submodule_and_run(tempdir_factory, store): src_path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') parent_path = git_dir(tempdir_factory) cmd_output('git', 'submodule', 'add', src_path, 'sub', cwd=parent_path) @@ -166,13 +166,13 @@ 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, C.CONFIG_FILE)) == 0 + assert install(Runner(sub_pth, C.CONFIG_FILE), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) -def test_commit_am(tempdir_factory): +def test_commit_am(tempdir_factory, store): """Regression test for #322.""" path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): @@ -183,16 +183,16 @@ def test_commit_am(tempdir_factory): with io.open('unstaged', 'w') as foo_file: foo_file.write('Oh hai') - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 -def test_unicode_merge_commit_message(tempdir_factory): +def test_unicode_merge_commit_message(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 cmd_output('git', 'checkout', 'master', '-b', 'foo') cmd_output('git', 'commit', '--allow-empty', '-n', '-m', 'branch2') cmd_output('git', 'checkout', 'master') @@ -204,11 +204,11 @@ def test_unicode_merge_commit_message(tempdir_factory): ) -def test_install_idempotent(tempdir_factory): +def test_install_idempotent(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path, C.CONFIG_FILE)) == 0 - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -223,12 +223,12 @@ def _path_without_us(): ]) -def test_environment_not_sourced(tempdir_factory): +def test_environment_not_sourced(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): # Patch the executable to simulate rming virtualenv with mock.patch.object(sys, 'executable', '/does-not-exist'): - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 # Use a specific homedir to ignore --user installs homedir = tempdir_factory.get() @@ -264,10 +264,10 @@ FAILING_PRE_COMMIT_RUN = re.compile( ) -def test_failing_hooks_returns_nonzero(tempdir_factory): +def test_failing_hooks_returns_nonzero(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'failing_hook_repo') with cwd(path): - assert install(Runner(path, C.CONFIG_FILE)) == 0 + assert install(Runner(path, C.CONFIG_FILE), store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 1 @@ -282,7 +282,7 @@ EXISTING_COMMIT_RUN = re.compile( ) -def test_install_existing_hooks_no_overwrite(tempdir_factory): +def test_install_existing_hooks_no_overwrite(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -299,7 +299,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory): assert EXISTING_COMMIT_RUN.match(output) # Now install pre-commit (no-overwrite) - assert install(runner) == 0 + assert install(runner, store) == 0 # We should run both the legacy and pre-commit hooks ret, output = _get_commit_output(tempdir_factory) @@ -308,7 +308,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory): assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) -def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory): +def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -320,8 +320,8 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory): make_executable(runner.pre_commit_path) # Install twice - assert install(runner) == 0 - assert install(runner) == 0 + assert install(runner, store) == 0 + assert install(runner, store) == 0 # We should run both the legacy and pre-commit hooks ret, output = _get_commit_output(tempdir_factory) @@ -337,7 +337,7 @@ FAIL_OLD_HOOK = re.compile( ) -def test_failing_existing_hook_returns_1(tempdir_factory): +def test_failing_existing_hook_returns_1(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -348,7 +348,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory): hook_file.write('#!/usr/bin/env bash\necho "fail!"\nexit 1\n') make_executable(runner.pre_commit_path) - assert install(runner) == 0 + assert install(runner, store) == 0 # We should get a failure from the legacy hook ret, output = _get_commit_output(tempdir_factory) @@ -356,17 +356,18 @@ def test_failing_existing_hook_returns_1(tempdir_factory): assert FAIL_OLD_HOOK.match(output) -def test_install_overwrite_no_existing_hooks(tempdir_factory): +def test_install_overwrite_no_existing_hooks(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - assert install(Runner(path, C.CONFIG_FILE), overwrite=True) == 0 + runner = Runner(path, C.CONFIG_FILE) + assert install(runner, store, overwrite=True) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) -def test_install_overwrite(tempdir_factory): +def test_install_overwrite(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -377,14 +378,14 @@ def test_install_overwrite(tempdir_factory): hook_file.write('#!/usr/bin/env bash\necho "legacy hook"\n') make_executable(runner.pre_commit_path) - assert install(runner, overwrite=True) == 0 + assert install(runner, store, overwrite=True) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) -def test_uninstall_restores_legacy_hooks(tempdir_factory): +def test_uninstall_restores_legacy_hooks(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -396,7 +397,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory): make_executable(runner.pre_commit_path) # Now install and uninstall pre-commit - assert install(runner) == 0 + assert install(runner, store) == 0 assert uninstall(runner) == 0 # Make sure we installed the "old" hook correctly @@ -405,7 +406,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory): assert EXISTING_COMMIT_RUN.match(output) -def test_replace_old_commit_script(tempdir_factory): +def test_replace_old_commit_script(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) @@ -424,7 +425,7 @@ def test_replace_old_commit_script(tempdir_factory): make_executable(runner.pre_commit_path) # Install normally - assert install(runner) == 0 + assert install(runner, store) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -453,39 +454,36 @@ PRE_INSTALLED = re.compile( ) -def test_installs_hooks_with_hooks_True( - tempdir_factory, - mock_out_store_directory, -): +def test_installs_hooks_with_hooks_True(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - install(Runner(path, C.CONFIG_FILE), hooks=True) + install(Runner(path, C.CONFIG_FILE), store, hooks=True) ret, output = _get_commit_output( - tempdir_factory, pre_commit_home=mock_out_store_directory, + tempdir_factory, pre_commit_home=store.directory, ) assert ret == 0 assert PRE_INSTALLED.match(output) -def test_install_hooks_command(tempdir_factory, mock_out_store_directory): +def test_install_hooks_command(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) - install(runner) - install_hooks(runner) + install(runner, store) + install_hooks(runner, store) ret, output = _get_commit_output( - tempdir_factory, pre_commit_home=mock_out_store_directory, + tempdir_factory, pre_commit_home=store.directory, ) assert ret == 0 assert PRE_INSTALLED.match(output) -def test_installed_from_venv(tempdir_factory): +def test_installed_from_venv(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): - install(Runner(path, C.CONFIG_FILE)) + install(Runner(path, C.CONFIG_FILE), store) # 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( @@ -519,12 +517,12 @@ def _get_push_output(tempdir_factory): )[:2] -def test_pre_push_integration_failing(tempdir_factory): +def test_pre_push_integration_failing(tempdir_factory, store): upstream = make_consuming_repo(tempdir_factory, 'failing_hook_repo') path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), store, hook_type='pre-push') # commit succeeds because pre-commit is only installed for pre-push assert _get_commit_output(tempdir_factory)[0] == 0 @@ -535,12 +533,12 @@ def test_pre_push_integration_failing(tempdir_factory): assert 'hookid: failing_hook' in output -def test_pre_push_integration_accepted(tempdir_factory): +def test_pre_push_integration_accepted(tempdir_factory, store): upstream = make_consuming_repo(tempdir_factory, 'script_hooks_repo') path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), store, hook_type='pre-push') assert _get_commit_output(tempdir_factory)[0] == 0 retc, output = _get_push_output(tempdir_factory) @@ -549,13 +547,13 @@ def test_pre_push_integration_accepted(tempdir_factory): assert 'Passed' in output -def test_pre_push_new_upstream(tempdir_factory): +def test_pre_push_new_upstream(tempdir_factory, store): upstream = make_consuming_repo(tempdir_factory, 'script_hooks_repo') upstream2 = git_dir(tempdir_factory) path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), store, hook_type='pre-push') assert _get_commit_output(tempdir_factory)[0] == 0 cmd_output('git', 'remote', 'rename', 'origin', 'upstream') @@ -566,19 +564,19 @@ def test_pre_push_new_upstream(tempdir_factory): assert 'Passed' in output -def test_pre_push_integration_empty_push(tempdir_factory): +def test_pre_push_integration_empty_push(tempdir_factory, store): upstream = make_consuming_repo(tempdir_factory, 'script_hooks_repo') path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) with cwd(path): - install(Runner(path, C.CONFIG_FILE), hook_type='pre-push') + install(Runner(path, C.CONFIG_FILE), store, hook_type='pre-push') _get_push_output(tempdir_factory) retc, output = _get_push_output(tempdir_factory) assert output == 'Everything up-to-date\n' assert retc == 0 -def test_pre_push_legacy(tempdir_factory): +def test_pre_push_legacy(tempdir_factory, store): upstream = make_consuming_repo(tempdir_factory, 'script_hooks_repo') path = tempdir_factory.get() cmd_output('git', 'clone', upstream, path) @@ -597,7 +595,7 @@ def test_pre_push_legacy(tempdir_factory): ) make_executable(hook_path) - install(runner, hook_type='pre-push') + install(runner, store, hook_type='pre-push') assert _get_commit_output(tempdir_factory)[0] == 0 retc, output = _get_push_output(tempdir_factory) @@ -608,16 +606,22 @@ def test_pre_push_legacy(tempdir_factory): assert third_line.endswith('Passed') -def test_commit_msg_integration_failing(commit_msg_repo, tempdir_factory): - install(Runner(commit_msg_repo, C.CONFIG_FILE), hook_type='commit-msg') +def test_commit_msg_integration_failing( + commit_msg_repo, tempdir_factory, store, +): + runner = Runner(commit_msg_repo, C.CONFIG_FILE) + install(runner, store, hook_type='commit-msg') retc, out = _get_commit_output(tempdir_factory) assert retc == 1 assert out.startswith('Must have "Signed off by:"...') assert out.strip().endswith('...Failed') -def test_commit_msg_integration_passing(commit_msg_repo, tempdir_factory): - install(Runner(commit_msg_repo, C.CONFIG_FILE), hook_type='commit-msg') +def test_commit_msg_integration_passing( + commit_msg_repo, tempdir_factory, store, +): + runner = Runner(commit_msg_repo, C.CONFIG_FILE) + install(runner, store, hook_type='commit-msg') msg = 'Hi\nSigned off by: me, lol' retc, out = _get_commit_output(tempdir_factory, commit_msg=msg) assert retc == 0 @@ -626,7 +630,7 @@ def test_commit_msg_integration_passing(commit_msg_repo, tempdir_factory): assert first_line.endswith('...Passed') -def test_commit_msg_legacy(commit_msg_repo, tempdir_factory): +def test_commit_msg_legacy(commit_msg_repo, tempdir_factory, store): runner = Runner(commit_msg_repo, C.CONFIG_FILE) hook_path = runner.get_hook_path('commit-msg') @@ -640,7 +644,7 @@ def test_commit_msg_legacy(commit_msg_repo, tempdir_factory): ) make_executable(hook_path) - install(runner, hook_type='commit-msg') + install(runner, store, hook_type='commit-msg') msg = 'Hi\nSigned off by: asottile' retc, out = _get_commit_output(tempdir_factory, commit_msg=msg) @@ -650,25 +654,31 @@ def test_commit_msg_legacy(commit_msg_repo, tempdir_factory): assert second_line.startswith('Must have "Signed off by:"...') -def test_install_disallow_mising_config(tempdir_factory): +def test_install_disallow_mising_config(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) remove_config_from_repo(path) - assert install(runner, overwrite=True, skip_on_missing_conf=False) == 0 + ret = install( + runner, store, overwrite=True, skip_on_missing_conf=False, + ) + assert ret == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 1 -def test_install_allow_mising_config(tempdir_factory): +def test_install_allow_mising_config(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) remove_config_from_repo(path) - assert install(runner, overwrite=True, skip_on_missing_conf=True) == 0 + ret = install( + runner, store, overwrite=True, skip_on_missing_conf=True, + ) + assert ret == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 @@ -679,13 +689,16 @@ def test_install_allow_mising_config(tempdir_factory): assert expected in output -def test_install_temporarily_allow_mising_config(tempdir_factory): +def test_install_temporarily_allow_mising_config(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): runner = Runner(path, C.CONFIG_FILE) remove_config_from_repo(path) - assert install(runner, overwrite=True, skip_on_missing_conf=False) == 0 + ret = install( + runner, store, overwrite=True, skip_on_missing_conf=False, + ) + assert ret == 0 env = dict(os.environ, PRE_COMMIT_ALLOW_NO_CONFIG='1') ret, output = _get_commit_output(tempdir_factory, env=env) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 91e84d98..70a6b6ec 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -48,33 +48,32 @@ def stage_a_file(filename='foo.py'): cmd_output('git', 'add', filename) -def _do_run(cap_out, repo, args, environ={}, config_file=C.CONFIG_FILE): +def _do_run(cap_out, store, 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) + ret = run(runner, store, args, environ=environ) printed = cap_out.get_bytes() return ret, printed def _test_run( - cap_out, repo, opts, expected_outputs, expected_ret, stage, + cap_out, store, repo, opts, expected_outputs, expected_ret, stage, config_file=C.CONFIG_FILE, ): if stage: stage_a_file() args = run_opts(**opts) - ret, printed = _do_run(cap_out, repo, args, config_file=config_file) + ret, printed = _do_run(cap_out, store, repo, args, config_file=config_file) assert ret == expected_ret, (ret, expected_ret, printed) for expected_output_part in expected_outputs: assert expected_output_part in printed -def test_run_all_hooks_failing( - cap_out, repo_with_failing_hook, mock_out_store_directory, -): +def test_run_all_hooks_failing(cap_out, store, repo_with_failing_hook): _test_run( cap_out, + store, repo_with_failing_hook, {}, ( @@ -88,17 +87,15 @@ def test_run_all_hooks_failing( ) -def test_arbitrary_bytes_hook( - cap_out, tempdir_factory, mock_out_store_directory, -): +def test_arbitrary_bytes_hook(cap_out, store, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'arbitrary_bytes_repo') with cwd(git_path): - _test_run(cap_out, git_path, {}, (b'\xe2\x98\x83\xb2\n',), 1, True) + _test_run( + cap_out, store, git_path, {}, (b'\xe2\x98\x83\xb2\n',), 1, True, + ) -def test_hook_that_modifies_but_returns_zero( - cap_out, tempdir_factory, mock_out_store_directory, -): +def test_hook_that_modifies_but_returns_zero(cap_out, store, tempdir_factory): git_path = make_consuming_repo( tempdir_factory, 'modified_file_returns_zero_repo', ) @@ -106,6 +103,7 @@ def test_hook_that_modifies_but_returns_zero( stage_a_file('bar.py') _test_run( cap_out, + store, git_path, {}, ( @@ -126,22 +124,18 @@ def test_hook_that_modifies_but_returns_zero( ) -def test_types_hook_repository( - cap_out, tempdir_factory, mock_out_store_directory, -): +def test_types_hook_repository(cap_out, store, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'types_repo') with cwd(git_path): stage_a_file('bar.py') stage_a_file('bar.notpy') - ret, printed = _do_run(cap_out, git_path, run_opts()) + ret, printed = _do_run(cap_out, store, git_path, run_opts()) assert ret == 1 assert b'bar.py' in printed assert b'bar.notpy' not in printed -def test_exclude_types_hook_repository( - cap_out, tempdir_factory, mock_out_store_directory, -): +def test_exclude_types_hook_repository(cap_out, store, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'exclude_types_repo') with cwd(git_path): with io.open('exe', 'w') as exe: @@ -149,13 +143,13 @@ def test_exclude_types_hook_repository( make_executable('exe') cmd_output('git', 'add', 'exe') stage_a_file('bar.py') - ret, printed = _do_run(cap_out, git_path, run_opts()) + ret, printed = _do_run(cap_out, store, git_path, run_opts()) assert ret == 1 assert b'bar.py' in printed assert b'exe' not in printed -def test_global_exclude(cap_out, tempdir_factory, mock_out_store_directory): +def test_global_exclude(cap_out, store, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(git_path): with modify_config() as config: @@ -163,23 +157,22 @@ def test_global_exclude(cap_out, tempdir_factory, mock_out_store_directory): open('foo.py', 'a').close() open('bar.py', 'a').close() cmd_output('git', 'add', '.') - ret, printed = _do_run(cap_out, git_path, run_opts(verbose=True)) + opts = run_opts(verbose=True) + ret, printed = _do_run(cap_out, store, git_path, opts) assert ret == 0 # Does not contain foo.py since it was excluded expected = b'hookid: bash_hook\n\nbar.py\nHello World\n\n' assert printed.endswith(expected) -def test_show_diff_on_failure( - capfd, cap_out, tempdir_factory, mock_out_store_directory, -): +def test_show_diff_on_failure(capfd, cap_out, store, tempdir_factory): git_path = make_consuming_repo( tempdir_factory, 'modified_file_returns_zero_repo', ) with cwd(git_path): stage_a_file('bar.py') _test_run( - cap_out, git_path, {'show_diff_on_failure': True}, + cap_out, store, git_path, {'show_diff_on_failure': True}, # we're only testing the output after running (), 1, True, ) @@ -211,15 +204,16 @@ def test_show_diff_on_failure( ) def test_run( cap_out, + store, repo_with_passing_hook, options, outputs, expected_ret, stage, - mock_out_store_directory, ): _test_run( cap_out, + store, repo_with_passing_hook, options, outputs, @@ -228,12 +222,7 @@ def test_run( ) -def test_run_output_logfile( - cap_out, - tempdir_factory, - mock_out_store_directory, -): - +def test_run_output_logfile(cap_out, store, tempdir_factory): expected_output = ( b'This is STDOUT output\n', b'This is STDERR output\n', @@ -243,6 +232,7 @@ def test_run_output_logfile( with cwd(git_path): _test_run( cap_out, + store, git_path, {}, expected_output, expected_ret=1, @@ -257,13 +247,12 @@ def test_run_output_logfile( assert expected_output_part in logfile_content -def test_always_run( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_always_run(cap_out, store, repo_with_passing_hook): with modify_config() as config: config['repos'][0]['hooks'][0]['always_run'] = True _test_run( cap_out, + store, repo_with_passing_hook, {}, (b'Bash hook', b'Passed'), @@ -272,9 +261,7 @@ def test_always_run( ) -def test_always_run_alt_config( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_always_run_alt_config(cap_out, store, repo_with_passing_hook): repo_root = '.' config = read_config(repo_root) config['repos'][0]['hooks'][0]['always_run'] = True @@ -283,6 +270,7 @@ def test_always_run_alt_config( _test_run( cap_out, + store, repo_with_passing_hook, {}, (b'Bash hook', b'Passed'), @@ -292,15 +280,14 @@ def test_always_run_alt_config( ) -def test_hook_verbose_enabled( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_hook_verbose_enabled(cap_out, store, repo_with_passing_hook): with modify_config() as config: config['repos'][0]['hooks'][0]['always_run'] = True config['repos'][0]['hooks'][0]['verbose'] = True _test_run( cap_out, + store, repo_with_passing_hook, {}, (b'Hello World',), @@ -310,26 +297,22 @@ def test_hook_verbose_enabled( @pytest.mark.parametrize( - ('origin', 'source', 'expect_failure'), - ( - ('master', 'master', False), - ('master', '', True), - ('', 'master', True), - ), + ('origin', 'source'), (('master', ''), ('', 'master')), ) -def test_origin_source_error_msg( - repo_with_passing_hook, origin, source, expect_failure, - mock_out_store_directory, cap_out, +def test_origin_source_error_msg_error( + cap_out, store, repo_with_passing_hook, origin, source, ): args = run_opts(origin=origin, source=source) - ret, printed = _do_run(cap_out, repo_with_passing_hook, args) - warning_msg = b'Specify both --origin and --source.' - if expect_failure: - assert ret == 1 - assert warning_msg in printed - else: - assert ret == 0 - assert warning_msg not in printed + ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args) + assert ret == 1 + assert b'Specify both --origin and --source.' in printed + + +def test_origin_source_both_ok(cap_out, store, repo_with_passing_hook): + args = run_opts(origin='master', source='master') + ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args) + assert ret == 0 + assert b'Specify both --origin and --source.' not in printed def test_has_unmerged_paths(in_merge_conflict): @@ -338,30 +321,26 @@ def test_has_unmerged_paths(in_merge_conflict): assert _has_unmerged_paths() is False -def test_merge_conflict(cap_out, in_merge_conflict, mock_out_store_directory): - ret, printed = _do_run(cap_out, in_merge_conflict, run_opts()) +def test_merge_conflict(cap_out, store, in_merge_conflict): + ret, printed = _do_run(cap_out, store, in_merge_conflict, run_opts()) assert ret == 1 assert b'Unmerged files. Resolve before committing.' in printed -def test_merge_conflict_modified( - cap_out, in_merge_conflict, mock_out_store_directory, -): +def test_merge_conflict_modified(cap_out, store, in_merge_conflict): # Touch another file so we have unstaged non-conflicting things assert os.path.exists('dummy') with open('dummy', 'w') as dummy_file: dummy_file.write('bar\nbaz\n') - ret, printed = _do_run(cap_out, in_merge_conflict, run_opts()) + ret, printed = _do_run(cap_out, store, in_merge_conflict, run_opts()) assert ret == 1 assert b'Unmerged files. Resolve before committing.' in printed -def test_merge_conflict_resolved( - cap_out, in_merge_conflict, mock_out_store_directory, -): +def test_merge_conflict_resolved(cap_out, store, in_merge_conflict): cmd_output('git', 'add', '.') - ret, printed = _do_run(cap_out, in_merge_conflict, run_opts()) + ret, printed = _do_run(cap_out, store, in_merge_conflict, run_opts()) for msg in ( b'Checking merge-conflict files only.', b'Bash hook', b'Passed', ): @@ -402,51 +381,45 @@ def test_get_skips(environ, expected_output): assert ret == expected_output -def test_skip_hook(cap_out, repo_with_passing_hook, mock_out_store_directory): +def test_skip_hook(cap_out, store, repo_with_passing_hook): ret, printed = _do_run( - cap_out, repo_with_passing_hook, run_opts(), {'SKIP': 'bash_hook'}, + cap_out, store, repo_with_passing_hook, run_opts(), + {'SKIP': 'bash_hook'}, ) for msg in (b'Bash hook', b'Skipped'): assert msg in printed def test_hook_id_not_in_non_verbose_output( - cap_out, repo_with_passing_hook, mock_out_store_directory, + cap_out, store, repo_with_passing_hook, ): ret, printed = _do_run( - cap_out, repo_with_passing_hook, run_opts(verbose=False), + cap_out, store, repo_with_passing_hook, run_opts(verbose=False), ) assert b'[bash_hook]' not in printed -def test_hook_id_in_verbose_output( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_hook_id_in_verbose_output(cap_out, store, repo_with_passing_hook): ret, printed = _do_run( - cap_out, repo_with_passing_hook, run_opts(verbose=True), + cap_out, store, repo_with_passing_hook, run_opts(verbose=True), ) assert b'[bash_hook] Bash hook' in printed -def test_multiple_hooks_same_id( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_multiple_hooks_same_id(cap_out, store, repo_with_passing_hook): with cwd(repo_with_passing_hook): # Add bash hook on there again with modify_config() as config: config['repos'][0]['hooks'].append({'id': 'bash_hook'}) stage_a_file() - ret, output = _do_run(cap_out, repo_with_passing_hook, run_opts()) + ret, output = _do_run(cap_out, store, repo_with_passing_hook, run_opts()) assert ret == 0 assert output.count(b'Bash hook') == 2 -def test_non_ascii_hook_id( - repo_with_passing_hook, mock_out_store_directory, tempdir_factory, -): +def test_non_ascii_hook_id(repo_with_passing_hook, tempdir_factory): with cwd(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, @@ -456,15 +429,13 @@ def test_non_ascii_hook_id( assert 'UnicodeEncodeError' not in stdout -def test_stdout_write_bug_py26( - repo_with_failing_hook, mock_out_store_directory, tempdir_factory, -): +def test_stdout_write_bug_py26(repo_with_failing_hook, store, tempdir_factory): with cwd(repo_with_failing_hook): with modify_config() as config: config['repos'][0]['hooks'][0]['args'] = ['☃'] stage_a_file() - install(Runner(repo_with_failing_hook, C.CONFIG_FILE)) + install(Runner(repo_with_failing_hook, C.CONFIG_FILE), store) # Have to use subprocess because pytest monkeypatches sys.stdout _, stdout, _ = cmd_output_mocked_pre_commit_home( @@ -479,7 +450,7 @@ def test_stdout_write_bug_py26( assert 'UnicodeDecodeError' not in stdout -def test_lots_of_files(mock_out_store_directory, tempdir_factory): +def test_lots_of_files(store, tempdir_factory): # windows xargs seems to have a bug, here's a regression test for # our workaround git_path = make_consuming_repo(tempdir_factory, 'python_hooks_repo') @@ -494,7 +465,7 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): open(filename, 'w').close() cmd_output('git', 'add', '.') - install(Runner(git_path, C.CONFIG_FILE)) + install(Runner(git_path, C.CONFIG_FILE), store) cmd_output_mocked_pre_commit_home( 'git', 'commit', '-m', 'Commit!', @@ -504,7 +475,7 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): ) -def test_stages(cap_out, repo_with_passing_hook, mock_out_store_directory): +def test_stages(cap_out, store, repo_with_passing_hook): config = OrderedDict(( ('repo', 'local'), ( @@ -526,7 +497,7 @@ def test_stages(cap_out, repo_with_passing_hook, mock_out_store_directory): def _run_for_stage(stage): args = run_opts(hook_stage=stage) - ret, printed = _do_run(cap_out, repo_with_passing_hook, args) + ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args) assert not ret, (ret, printed) # this test should only run one hook assert printed.count(b'hook ') == 1 @@ -537,13 +508,14 @@ def test_stages(cap_out, repo_with_passing_hook, mock_out_store_directory): assert _run_for_stage('manual').startswith(b'hook 3...') -def test_commit_msg_hook(cap_out, commit_msg_repo, mock_out_store_directory): +def test_commit_msg_hook(cap_out, store, commit_msg_repo): filename = '.git/COMMIT_EDITMSG' with io.open(filename, 'w') as f: f.write('This is the commit message') _test_run( cap_out, + store, commit_msg_repo, {'hook_stage': 'commit-msg', 'commit_msg_filename': filename}, expected_outputs=[b'Must have "Signed off by:"', b'Failed'], @@ -552,9 +524,7 @@ def test_commit_msg_hook(cap_out, commit_msg_repo, mock_out_store_directory): ) -def test_local_hook_passes( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_local_hook_passes(cap_out, store, repo_with_passing_hook): config = OrderedDict(( ('repo', 'local'), ( @@ -583,6 +553,7 @@ def test_local_hook_passes( _test_run( cap_out, + store, repo_with_passing_hook, opts={}, expected_outputs=[b''], @@ -591,9 +562,7 @@ def test_local_hook_passes( ) -def test_local_hook_fails( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_local_hook_fails(cap_out, store, repo_with_passing_hook): config = OrderedDict(( ('repo', 'local'), ( @@ -614,6 +583,7 @@ def test_local_hook_fails( _test_run( cap_out, + store, repo_with_passing_hook, opts={}, expected_outputs=[b''], @@ -622,9 +592,7 @@ def test_local_hook_fails( ) -def test_pcre_deprecation_warning( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_pcre_deprecation_warning(cap_out, store, repo_with_passing_hook): config = OrderedDict(( ('repo', 'local'), ( @@ -640,6 +608,7 @@ def test_pcre_deprecation_warning( _test_run( cap_out, + store, repo_with_passing_hook, opts={}, expected_outputs=[ @@ -651,9 +620,7 @@ def test_pcre_deprecation_warning( ) -def test_meta_hook_passes( - cap_out, repo_with_passing_hook, mock_out_store_directory, -): +def test_meta_hook_passes(cap_out, store, repo_with_passing_hook): config = OrderedDict(( ('repo', 'meta'), ( @@ -668,6 +635,7 @@ def test_meta_hook_passes( _test_run( cap_out, + store, repo_with_passing_hook, opts={}, expected_outputs=[b'Check for useless excludes'], @@ -684,32 +652,25 @@ def modified_config_repo(repo_with_passing_hook): yield repo_with_passing_hook -def test_error_with_unstaged_config( - cap_out, modified_config_repo, mock_out_store_directory, -): +def test_error_with_unstaged_config(cap_out, store, modified_config_repo): args = run_opts() - ret, printed = _do_run(cap_out, modified_config_repo, args) + ret, printed = _do_run(cap_out, store, modified_config_repo, args) assert b'Your pre-commit configuration is unstaged.' in printed assert ret == 1 @pytest.mark.parametrize( - 'opts', ({'all_files': True}, {'files': [C.CONFIG_FILE]}), + 'opts', (run_opts(all_files=True), run_opts(files=[C.CONFIG_FILE])), ) def test_no_unstaged_error_with_all_files_or_files( - cap_out, modified_config_repo, mock_out_store_directory, opts, + cap_out, store, modified_config_repo, opts, ): - args = run_opts(**opts) - ret, printed = _do_run(cap_out, modified_config_repo, args) + ret, printed = _do_run(cap_out, store, modified_config_repo, opts) assert b'Your pre-commit configuration is unstaged.' not in printed -def test_files_running_subdir( - repo_with_passing_hook, mock_out_store_directory, tempdir_factory, -): +def test_files_running_subdir(repo_with_passing_hook, tempdir_factory): with cwd(repo_with_passing_hook): - install(Runner(repo_with_passing_hook, C.CONFIG_FILE)) - os.mkdir('subdir') open('subdir/foo.py', 'w').close() cmd_output('git', 'add', 'subdir/foo.py') @@ -735,35 +696,30 @@ def test_files_running_subdir( ), ) def test_pass_filenames( - cap_out, repo_with_passing_hook, mock_out_store_directory, - pass_filenames, - hook_args, - expected_out, + cap_out, store, repo_with_passing_hook, + pass_filenames, hook_args, expected_out, ): with modify_config() as config: config['repos'][0]['hooks'][0]['pass_filenames'] = pass_filenames config['repos'][0]['hooks'][0]['args'] = hook_args stage_a_file() ret, printed = _do_run( - cap_out, repo_with_passing_hook, run_opts(verbose=True), + cap_out, store, repo_with_passing_hook, run_opts(verbose=True), ) assert expected_out + b'\nHello World' in printed assert (b'foo.py' in printed) == pass_filenames -def test_fail_fast( - cap_out, repo_with_failing_hook, mock_out_store_directory, -): - with cwd(repo_with_failing_hook): - with modify_config() as config: - # More than one hook - config['fail_fast'] = True - config['repos'][0]['hooks'] *= 2 - stage_a_file() +def test_fail_fast(cap_out, store, repo_with_failing_hook): + with modify_config() as config: + # More than one hook + config['fail_fast'] = True + config['repos'][0]['hooks'] *= 2 + stage_a_file() - ret, printed = _do_run(cap_out, repo_with_failing_hook, run_opts()) - # it should have only run one hook - assert printed.count(b'Failing hook') == 1 + ret, printed = _do_run(cap_out, store, repo_with_failing_hook, run_opts()) + # it should have only run one hook + assert printed.count(b'Failing hook') == 1 @pytest.fixture diff --git a/tests/conftest.py b/tests/conftest.py index eb2ecf6e..f56bb8f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -134,7 +134,7 @@ def configure_logging(): @pytest.fixture -def mock_out_store_directory(tempdir_factory): +def mock_store_dir(tempdir_factory): tmpdir = tempdir_factory.get() with mock.patch.object( Store, diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 36eb1faf..40299b14 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -73,14 +73,14 @@ def test_error_handler_uncaught_error(mocked_log_and_exit): ) -def test_log_and_exit(cap_out, mock_out_store_directory): +def test_log_and_exit(cap_out, mock_store_dir): with pytest.raises(SystemExit): error_handler._log_and_exit( 'msg', error_handler.FatalError('hai'), "I'm a stacktrace", ) printed = cap_out.get() - log_file = os.path.join(mock_out_store_directory, 'pre-commit.log') + log_file = os.path.join(mock_store_dir, 'pre-commit.log') assert printed == ( 'msg: FatalError: hai\n' 'Check the log at {}\n'.format(log_file) @@ -94,7 +94,7 @@ def test_log_and_exit(cap_out, mock_out_store_directory): ) -def test_error_handler_non_ascii_exception(mock_out_store_directory): +def test_error_handler_non_ascii_exception(mock_store_dir): with pytest.raises(SystemExit): with error_handler.error_handler(): raise ValueError('☃') diff --git a/tests/main_test.py b/tests/main_test.py index ae6a73e7..65adc477 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -92,13 +92,13 @@ def test_help_other_command( @pytest.mark.parametrize('command', CMDS) -def test_all_cmds(command, mock_commands): +def test_all_cmds(command, mock_commands, mock_store_dir): main.main((command,)) assert getattr(mock_commands, command.replace('-', '_')).call_count == 1 assert_only_one_mock_called(mock_commands) -def test_try_repo(): +def test_try_repo(mock_store_dir): with mock.patch.object(main, 'try_repo') as patch: main.main(('try-repo', '.')) assert patch.call_count == 1 @@ -123,12 +123,12 @@ def test_help_cmd_in_empty_directory( def test_expected_fatal_error_no_git_repo( - tempdir_factory, cap_out, mock_out_store_directory, + tempdir_factory, cap_out, mock_store_dir, ): with cwd(tempdir_factory.get()): with pytest.raises(SystemExit): main.main([]) - log_file = os.path.join(mock_out_store_directory, 'pre-commit.log') + log_file = os.path.join(mock_store_dir, 'pre-commit.log') assert cap_out.get() == ( 'An error has occurred: FatalError: git failed. ' 'Is it installed, and are you in a Git repository directory?\n' @@ -136,6 +136,6 @@ def test_expected_fatal_error_no_git_repo( ) -def test_warning_on_tags_only(mock_commands, cap_out): +def test_warning_on_tags_only(mock_commands, cap_out, mock_store_dir): main.main(('autoupdate', '--tags-only')) assert '--tags-only is the default' in cap_out.get() diff --git a/tests/meta_hooks/check_hooks_apply_test.py b/tests/meta_hooks/check_hooks_apply_test.py index c777daa8..f0f38d69 100644 --- a/tests/meta_hooks/check_hooks_apply_test.py +++ b/tests/meta_hooks/check_hooks_apply_test.py @@ -6,9 +6,7 @@ from testing.fixtures import git_dir from testing.util import cwd -def test_hook_excludes_everything( - capsys, tempdir_factory, mock_out_store_directory, -): +def test_hook_excludes_everything(capsys, tempdir_factory, mock_store_dir): config = OrderedDict(( ('repo', 'meta'), ( @@ -31,9 +29,7 @@ def test_hook_excludes_everything( assert 'check-useless-excludes does not apply to this repository' in out -def test_hook_includes_nothing( - capsys, tempdir_factory, mock_out_store_directory, -): +def test_hook_includes_nothing(capsys, tempdir_factory, mock_store_dir): config = OrderedDict(( ('repo', 'meta'), ( @@ -56,9 +52,7 @@ def test_hook_includes_nothing( assert 'check-useless-excludes does not apply to this repository' in out -def test_hook_types_not_matched( - capsys, tempdir_factory, mock_out_store_directory, -): +def test_hook_types_not_matched(capsys, tempdir_factory, mock_store_dir): config = OrderedDict(( ('repo', 'meta'), ( @@ -82,7 +76,7 @@ def test_hook_types_not_matched( def test_hook_types_excludes_everything( - capsys, tempdir_factory, mock_out_store_directory, + capsys, tempdir_factory, mock_store_dir, ): config = OrderedDict(( ('repo', 'meta'), @@ -106,9 +100,7 @@ def test_hook_types_excludes_everything( assert 'check-useless-excludes does not apply to this repository' in out -def test_valid_includes( - capsys, tempdir_factory, mock_out_store_directory, -): +def test_valid_includes(capsys, tempdir_factory, mock_store_dir): config = OrderedDict(( ('repo', 'meta'), ( diff --git a/tests/runner_test.py b/tests/runner_test.py index df324712..10b1409f 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -2,14 +2,11 @@ from __future__ import absolute_import from __future__ import unicode_literals import os.path -from collections import OrderedDict import pre_commit.constants as C from pre_commit.runner import Runner from pre_commit.util import cmd_output -from testing.fixtures import add_config_to_repo from testing.fixtures import git_dir -from testing.fixtures import make_consuming_repo from testing.util import cwd @@ -48,77 +45,6 @@ def test_config_file_path(): 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, C.CONFIG_FILE) - assert len(runner.repositories) == 1 - - -def test_local_hooks(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', 'do_not_commit'), - ('name', 'Block if "DO NOT COMMIT" is found'), - ('entry', 'DO NOT COMMIT'), - ('language', 'pygrep'), - ('files', '^(.*)$'), - )), - ), - ), - )) - git_path = git_dir(tempdir_factory) - add_config_to_repo(git_path, config) - runner = Runner(git_path, C.CONFIG_FILE) - assert len(runner.repositories) == 1 - 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', 'pygrep'), - ('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)