From c5cbd473c782c32cf1e964b24eb51d9ab76fbe91 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 2 May 2014 22:47:23 -0700 Subject: [PATCH] Simplify Repository.cmd_runner --- pre_commit/commands.py | 6 +---- pre_commit/prefixed_command_runner.py | 2 +- pre_commit/repository.py | 33 ++++++++++----------------- tests/repository_test.py | 29 ++++++++--------------- 4 files changed, 24 insertions(+), 46 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index e027946c..4ff8d01e 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -195,11 +195,7 @@ def _run_single_hook(runner, repository, hook_id, args, write, skips=set()): write(get_hook_message(_hook_msg_start(hook, args.verbose), end_len=6)) sys.stdout.flush() - retcode, stdout, stderr = repository.run_hook( - runner.cmd_runner, - hook_id, - filenames, - ) + retcode, stdout, stderr = repository.run_hook(hook_id, filenames) if retcode != repository.hooks[hook_id]['expected_return_value']: retcode = 1 diff --git a/pre_commit/prefixed_command_runner.py b/pre_commit/prefixed_command_runner.py index d8a562a2..9c3c6b39 100644 --- a/pre_commit/prefixed_command_runner.py +++ b/pre_commit/prefixed_command_runner.py @@ -40,7 +40,7 @@ class PrefixedCommandRunner(object): For instance: PrefixedCommandRunner('/tmp/foo').run(['{prefix}foo.sh', 'bar', 'baz']) - will run ['/tmpl/foo/foo.sh', 'bar', 'baz'] + will run ['/tmp/foo/foo.sh', 'bar', 'baz'] """ def __init__(self, prefix_dir, popen=subprocess.Popen, makedirs=os.makedirs): self.prefix_dir = prefix_dir.rstrip(os.sep) + os.sep diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 977afdb6..7b29dea2 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -43,47 +43,38 @@ class Repository(object): def manifest(self): return Manifest(self.repo_path_getter) - def get_cmd_runner(self, hooks_cmd_runner): - # TODO: this effectively throws away the original cmd runner - return PrefixedCommandRunner.from_command_runner( - hooks_cmd_runner, self.repo_path_getter.repo_path, - ) + @cached_property + def cmd_runner(self): + return PrefixedCommandRunner(self.repo_path_getter.repo_path) - def require_installed(self, cmd_runner): + def require_installed(self): if self.__installed: return - self.install(cmd_runner) + self.install() self.__installed = True - def install(self, cmd_runner): - """Install the hook repository. - - Args: - cmd_runner - A `PrefixedCommandRunner` bound to the hooks workspace - """ - repo_cmd_runner = self.get_cmd_runner(cmd_runner) + def install(self): + """Install the hook repository.""" for language_name in self.languages: language = languages[language_name] if ( language.ENVIRONMENT_DIR is None or - repo_cmd_runner.exists(language.ENVIRONMENT_DIR) + self.cmd_runner.exists(language.ENVIRONMENT_DIR) ): # The language is already installed continue - language.install_environment(repo_cmd_runner) + language.install_environment(self.cmd_runner) - def run_hook(self, cmd_runner, hook_id, file_args): + def run_hook(self, hook_id, file_args): """Run a hook. Args: - cmd_runner - A `PrefixedCommandRunner` bound to the hooks workspace hook_id - Id of the hook file_args - List of files to run """ - self.require_installed(cmd_runner) - repo_cmd_runner = self.get_cmd_runner(cmd_runner) + self.require_installed() hook = self.hooks[hook_id] return languages[hook['language']].run_hook( - repo_cmd_runner, hook, file_args, + self.cmd_runner, hook, file_args, ) diff --git a/tests/repository_test.py b/tests/repository_test.py index 58088aac..1a8fa6e4 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -5,23 +5,20 @@ from plumbum import local from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.jsonschema_extensions import apply_defaults -from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.repository import Repository @pytest.mark.integration def test_install_python_repo_in_env(config_for_python_hooks_repo, store): repo = Repository.create(config_for_python_hooks_repo, store) - repo.install(PrefixedCommandRunner(store.directory)) + repo.install() assert os.path.exists(os.path.join(store.directory, repo.sha, 'py_env')) @pytest.mark.integration def test_run_a_python_hook(config_for_python_hooks_repo, store): repo = Repository.create(config_for_python_hooks_repo, store) - ret = repo.run_hook( - PrefixedCommandRunner(store.directory), 'foo', ['/dev/null'], - ) + ret = repo.run_hook('foo', ['/dev/null']) assert ret[0] == 0 assert ret[1] == "['/dev/null']\nHello World\n" @@ -30,9 +27,7 @@ def test_run_a_python_hook(config_for_python_hooks_repo, store): @pytest.mark.integration def test_lots_of_files(config_for_python_hooks_repo, store): repo = Repository.create(config_for_python_hooks_repo, store) - ret = repo.run_hook( - PrefixedCommandRunner(store.directory), 'foo', ['/dev/null'] * 15000, - ) + ret = repo.run_hook('foo', ['/dev/null'] * 15000) assert ret[0] == 0 @@ -41,9 +36,7 @@ def test_lots_of_files(config_for_python_hooks_repo, store): def test_cwd_of_hook(config_for_prints_cwd_repo, store): # Note: this doubles as a test for `system` hooks repo = Repository.create(config_for_prints_cwd_repo, store) - ret = repo.run_hook( - PrefixedCommandRunner(store.directory), 'prints_cwd', [], - ) + ret = repo.run_hook('prints_cwd', []) assert ret[0] == 0 assert ret[1] == repo.repo_url + '\n' @@ -56,7 +49,7 @@ def test_cwd_of_hook(config_for_prints_cwd_repo, store): @pytest.mark.integration def test_run_a_node_hook(config_for_node_hooks_repo, store): repo = Repository.create(config_for_node_hooks_repo, store) - ret = repo.run_hook(PrefixedCommandRunner(store.directory), 'foo', []) + ret = repo.run_hook('foo', []) assert ret[0] == 0 assert ret[1] == 'Hello World\n' @@ -65,9 +58,7 @@ def test_run_a_node_hook(config_for_node_hooks_repo, store): @pytest.mark.integration def test_run_a_script_hook(config_for_script_hooks_repo, store): repo = Repository.create(config_for_script_hooks_repo, store) - ret = repo.run_hook( - PrefixedCommandRunner(store.directory), 'bash_hook', ['bar'], - ) + ret = repo.run_hook('bash_hook', ['bar']) assert ret[0] == 0 assert ret[1] == 'bar\nHello World\n' @@ -106,14 +97,14 @@ def test_languages(config_for_python_hooks_repo, store): def test_reinstall(config_for_python_hooks_repo, store): repo = Repository.create(config_for_python_hooks_repo, store) - repo.require_installed(PrefixedCommandRunner(store.directory)) + repo.require_installed() # Reinstall with same repo should not trigger another install # TODO: how to assert this? - repo.require_installed(PrefixedCommandRunner(store.directory)) + repo.require_installed() # Reinstall on another run should not trigger another install # TODO: how to assert this? repo = Repository.create(config_for_python_hooks_repo, store) - repo.require_installed(PrefixedCommandRunner(store.directory)) + repo.require_installed() @pytest.mark.integration @@ -122,4 +113,4 @@ def test_really_long_file_paths(config_for_python_hooks_repo, store): local['git']['init', path]() with local.cwd(path): repo = Repository.create(config_for_python_hooks_repo, store) - repo.require_installed(PrefixedCommandRunner(store.directory)) + repo.require_installed()