From c418f2b94e7352fca2112c23327b17c5c7cb8adf Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 30 Mar 2014 15:15:13 -0700 Subject: [PATCH] Implement no-dependency system and script hook types. Closes #39. --- example_hooks.yaml | 1 + pre_commit/clientlib/validate_manifest.py | 7 +-- pre_commit/constants.py | 7 --- pre_commit/languages/all.py | 7 +++ pre_commit/languages/script.py | 13 +++++ pre_commit/languages/system.py | 13 +++++ pre_commit/repository.py | 5 +- testing/resources/prints_cwd_repo/hooks.yaml | 4 +- .../resources/script_hooks_repo/bin/hook.sh | 4 ++ .../resources/script_hooks_repo/hooks.yaml | 4 ++ tests/clientlib/validate_manifest_test.py | 2 +- tests/conftest.py | 51 ++++++++----------- tests/languages/all_test.py | 8 +-- tests/repository_test.py | 12 +++++ 14 files changed, 86 insertions(+), 52 deletions(-) create mode 100644 pre_commit/languages/script.py create mode 100644 pre_commit/languages/system.py create mode 100755 testing/resources/script_hooks_repo/bin/hook.sh create mode 100644 testing/resources/script_hooks_repo/hooks.yaml diff --git a/example_hooks.yaml b/example_hooks.yaml index 564f2ccf..05be1940 100644 --- a/example_hooks.yaml +++ b/example_hooks.yaml @@ -23,4 +23,5 @@ name: My Bash Based Hook description: This is a hook that uses grep to validate some stuff entry: ./my_grep_based_hook.sh + language: script expected_return_value: 1 diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py index ed6818af..28f544cf 100644 --- a/pre_commit/clientlib/validate_manifest.py +++ b/pre_commit/clientlib/validate_manifest.py @@ -6,6 +6,7 @@ import sys import pre_commit.constants as C from pre_commit.clientlib.validate_base import get_validator +from pre_commit.languages.all import all_languages from pre_commit.util import entry @@ -25,7 +26,7 @@ MANIFEST_JSON_SCHEMA = { 'language': {'type': 'string'}, 'expected_return_value': {'type': 'number'}, }, - 'required': ['id', 'name', 'entry'], + 'required': ['id', 'name', 'entry', 'language'], }, } @@ -35,13 +36,13 @@ def additional_manifest_check(obj): language = hook_config.get('language') if language is not None and not any( - language.startswith(lang) for lang in C.SUPPORTED_LANGUAGES + language.startswith(lang) for lang in all_languages ): raise InvalidManifestError( 'Expected language {0} for {1} to start with one of {2!r}'.format( hook_config['id'], hook_config['language'], - C.SUPPORTED_LANGUAGES, + all_languages, ) ) diff --git a/pre_commit/constants.py b/pre_commit/constants.py index 666664f0..1e9137ba 100644 --- a/pre_commit/constants.py +++ b/pre_commit/constants.py @@ -5,13 +5,6 @@ HOOKS_WORKSPACE = '.pre-commit-files' MANIFEST_FILE = 'hooks.yaml' -SUPPORTED_LANGUAGES = set([ - 'python', - 'ruby', - 'node', -]) - - YAML_DUMP_KWARGS = { 'default_flow_style': False, 'indent': 4, diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py index 858af7e5..541b4dca 100644 --- a/pre_commit/languages/all.py +++ b/pre_commit/languages/all.py @@ -2,6 +2,8 @@ from pre_commit.languages import node from pre_commit.languages import python from pre_commit.languages import ruby +from pre_commit.languages import script +from pre_commit.languages import system # A language implements the following two functions in its module: # @@ -29,4 +31,9 @@ languages = { 'node': node, 'python': python, 'ruby': ruby, + 'script': script, + 'system': system, } + + +all_languages = languages.keys() diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py new file mode 100644 index 00000000..783743ae --- /dev/null +++ b/pre_commit/languages/script.py @@ -0,0 +1,13 @@ + +def install_environment(repo_cmd_runner): + """Installation for script type is a noop.""" + pass + + +def run_hook(repo_cmd_runner, hook, file_args): + return repo_cmd_runner.run( + ['xargs', '{{prefix}}{0}'.format(hook['entry'])] + hook.get('args', []), + # TODO: this is duplicated in pre_commit/languages/helpers.py + stdin='\n'.join(list(file_args) + ['']), + retcode=None, + ) diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py new file mode 100644 index 00000000..8439c091 --- /dev/null +++ b/pre_commit/languages/system.py @@ -0,0 +1,13 @@ + +def install_environment(repo_cmd_runner): + """Installation for system type is a noop.""" + pass + + +def run_hook(repo_cmd_runner, hook, file_args): + return repo_cmd_runner.run( + ['xargs', hook['entry']] + hook.get('args', []), + # TODO: this is duplicated in pre_commit/languages/helpers.py + stdin='\n'.join(list(file_args) + ['']), + retcode=None, + ) diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 7e710aaa..f8f4d1c2 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -82,9 +82,8 @@ class Repository(object): """ self.require_created() repo_cmd_runner = self.get_cmd_runner(cmd_runner) - for language in C.SUPPORTED_LANGUAGES: - if language in self.languages: - languages[language].install_environment(repo_cmd_runner) + for language in self.languages: + languages[language].install_environment(repo_cmd_runner) @contextlib.contextmanager def in_checkout(self): diff --git a/testing/resources/prints_cwd_repo/hooks.yaml b/testing/resources/prints_cwd_repo/hooks.yaml index 4a716c21..e743dda0 100644 --- a/testing/resources/prints_cwd_repo/hooks.yaml +++ b/testing/resources/prints_cwd_repo/hooks.yaml @@ -1,4 +1,4 @@ - id: prints_cwd name: Prints Cwd - entry: prints_cwd - language: python + entry: pwd + language: system diff --git a/testing/resources/script_hooks_repo/bin/hook.sh b/testing/resources/script_hooks_repo/bin/hook.sh new file mode 100755 index 00000000..6565ee40 --- /dev/null +++ b/testing/resources/script_hooks_repo/bin/hook.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +echo $@ +echo 'Hello World' diff --git a/testing/resources/script_hooks_repo/hooks.yaml b/testing/resources/script_hooks_repo/hooks.yaml new file mode 100644 index 00000000..6f9c0faa --- /dev/null +++ b/testing/resources/script_hooks_repo/hooks.yaml @@ -0,0 +1,4 @@ +- id: bash_hook + name: Bash hook + entry: bin/hook.sh + language: script diff --git a/tests/clientlib/validate_manifest_test.py b/tests/clientlib/validate_manifest_test.py index f6aed26a..7d64db28 100644 --- a/tests/clientlib/validate_manifest_test.py +++ b/tests/clientlib/validate_manifest_test.py @@ -45,7 +45,7 @@ def is_valid_according_to_schema(obj, schema): @pytest.mark.parametrize(('manifest_obj', 'expected'), ( ([], False), - ([{'id': 'a', 'name': 'b', 'entry': 'c'}], True), + ([{'id': 'a', 'name': 'b', 'entry': 'c', 'language': 'python'}], True), ( [{ 'id': 'a', diff --git a/tests/conftest.py b/tests/conftest.py index 3b989779..70e3a9a1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -59,45 +59,36 @@ def prints_cwd_repo(dummy_git_repo): @pytest.yield_fixture -def config_for_node_hooks_repo(node_hooks_repo): +def script_hooks_repo(dummy_git_repo): + yield _make_repo(dummy_git_repo, 'script_hooks_repo') + + +def _make_config(path, hook_id, file_regex): config = { - 'repo': node_hooks_repo, - 'sha': git.get_head_sha(node_hooks_repo), - 'hooks': [{ - 'id': 'foo', - 'files': '\.js$', - }], + 'repo': path, + 'sha': git.get_head_sha(path), + 'hooks': [{'id': hook_id, 'files': file_regex}], } jsonschema.validate([config], CONFIG_JSON_SCHEMA) validate_config_extra([config]) - yield config + return config + + +@pytest.yield_fixture +def config_for_node_hooks_repo(node_hooks_repo): + yield _make_config(node_hooks_repo, 'foo', '\.js$') @pytest.yield_fixture def config_for_python_hooks_repo(python_hooks_repo): - config = { - 'repo': python_hooks_repo, - 'sha': git.get_head_sha(python_hooks_repo), - 'hooks': [{ - 'id': 'foo', - 'files': '\.py$', - }], - } - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) - yield config + yield _make_config(python_hooks_repo, 'foo', '\.py$') @pytest.yield_fixture def config_for_prints_cwd_repo(prints_cwd_repo): - config = { - 'repo': prints_cwd_repo, - 'sha': git.get_head_sha(prints_cwd_repo), - 'hooks': [{ - 'id': 'prints_cwd', - 'files': '\.py$', - }], - } - jsonschema.validate([config], CONFIG_JSON_SCHEMA) - validate_config_extra([config]) - yield config + yield _make_config(prints_cwd_repo, 'prints_cwd', '^$') + + +@pytest.yield_fixture +def config_for_script_hooks_repo(script_hooks_repo): + yield _make_config(script_hooks_repo, 'bash_hook', '^$') diff --git a/tests/languages/all_test.py b/tests/languages/all_test.py index 3766a888..b4bdaf25 100644 --- a/tests/languages/all_test.py +++ b/tests/languages/all_test.py @@ -1,15 +1,11 @@ import pytest -import pre_commit.constants as C +from pre_commit.languages.all import all_languages from pre_commit.languages.all import languages -def test_all_languages_have_repo_setups(): - assert set(languages.keys()) == C.SUPPORTED_LANGUAGES - - -@pytest.mark.parametrize('language', C.SUPPORTED_LANGUAGES) +@pytest.mark.parametrize('language', all_languages) def test_all_languages_support_interface(language): assert hasattr(languages[language], 'install_environment') assert hasattr(languages[language], 'run_hook') diff --git a/tests/repository_test.py b/tests/repository_test.py index d7002355..0b4011e7 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -67,6 +67,7 @@ def test_run_a_hook_lots_of_files(config_for_python_hooks_repo): @pytest.mark.integration def test_cwd_of_hook(config_for_prints_cwd_repo): + # Note: this doubles as a test for `system` hooks repo = Repository(config_for_prints_cwd_repo) ret = repo.run_hook( PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'prints_cwd', [], @@ -89,6 +90,17 @@ def test_run_a_node_hook(config_for_node_hooks_repo): assert ret[1] == 'Hello World\n' +@pytest.mark.integration +def test_run_a_script_hook(config_for_script_hooks_repo): + repo = Repository(config_for_script_hooks_repo) + ret = repo.run_hook( + PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'bash_hook', ['bar'], + ) + + assert ret[0] == 0 + assert ret[1] == 'bar\nHello World\n' + + @pytest.fixture def mock_repo_config(): config = {