diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 36b0d7d7..6e09dabd 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -56,16 +56,21 @@ def install( with io.open(hook_path, 'w') as pre_commit_file_obj: if hook_type == 'pre-push': - with io.open(resource_filename('pre-push-tmpl')) as fp: - pre_push_contents = fp.read() + with io.open(resource_filename('pre-push-tmpl')) as f: + hook_specific_contents = f.read() + elif hook_type == 'commit-msg': + with io.open(resource_filename('commit-msg-tmpl')) as f: + hook_specific_contents = f.read() + elif hook_type == 'pre-commit': + hook_specific_contents = '' else: - pre_push_contents = '' + raise AssertionError('Unknown hook type: {}'.format(hook_type)) skip_on_missing_conf = 'true' if skip_on_missing_conf else 'false' contents = io.open(resource_filename('hook-tmpl')).read().format( sys_executable=sys.executable, hook_type=hook_type, - pre_push=pre_push_contents, + hook_specific=hook_specific_contents, skip_on_missing_conf=skip_on_missing_conf, ) pre_commit_file_obj.write(contents) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 676c5044..c18f2aac 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -58,6 +58,9 @@ def get_filenames(args, include_expr, exclude_expr): getter = git.get_files_matching( lambda: get_changed_files(args.origin, args.source), ) + elif args.hook_stage == 'commit-msg': + def getter(*_): + return (args.commit_msg_filename,) elif args.files: getter = git.get_files_matching(lambda: args.files) elif args.all_files: diff --git a/pre_commit/main.py b/pre_commit/main.py index 02eed40e..37fb264d 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -76,7 +76,7 @@ def main(argv=None): ), ) install_parser.add_argument( - '-t', '--hook-type', choices=('pre-commit', 'pre-push'), + '-t', '--hook-type', choices=('pre-commit', 'pre-push', 'commit-msg'), default='pre-commit', ) install_parser.add_argument( @@ -149,6 +149,10 @@ def main(argv=None): '--source', '-s', help="The remote branch's commit_id when using `git push`.", ) + run_parser.add_argument( + '--commit-msg-filename', + help='Filename to check when running during `commit-msg`', + ) run_parser.add_argument( '--allow-unstaged-config', default=False, action='store_true', help=( @@ -157,7 +161,8 @@ def main(argv=None): ), ) run_parser.add_argument( - '--hook-stage', choices=('commit', 'push'), default='commit', + '--hook-stage', choices=('commit', 'push', 'commit-msg'), + default='commit', help='The stage during which the hook is fired e.g. commit or push.', ) run_parser.add_argument( diff --git a/pre_commit/resources/commit-msg-tmpl b/pre_commit/resources/commit-msg-tmpl new file mode 100644 index 00000000..b11521b0 --- /dev/null +++ b/pre_commit/resources/commit-msg-tmpl @@ -0,0 +1 @@ +args="run --hook-stage=commit-msg --commit-msg-filename=$1" diff --git a/pre_commit/resources/hook-tmpl b/pre_commit/resources/hook-tmpl index da939ff1..3bfce5c7 100644 --- a/pre_commit/resources/hook-tmpl +++ b/pre_commit/resources/hook-tmpl @@ -52,7 +52,7 @@ if [ ! -f $CONF_FILE ]; then fi fi -{pre_push} +{hook_specific} # Run pre-commit if ((WHICH_RETV == 0)); then diff --git a/testing/fixtures.py b/testing/fixtures.py index be434216..eda2e09a 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -23,8 +23,7 @@ from testing.util import get_resource_path def git_dir(tempdir_factory): path = tempdir_factory.get() - with cwd(path): - cmd_output('git', 'init') + cmd_output('git', 'init', path) return path diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 1fb0f8f1..94d396a9 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -56,7 +56,7 @@ def test_install_pre_commit(tempdir_factory): expected_contents = io.open(pre_commit_script).read().format( sys_executable=sys.executable, hook_type='pre-commit', - pre_push='', + hook_specific='', skip_on_missing_conf='false', ) assert pre_commit_contents == expected_contents @@ -71,7 +71,7 @@ def test_install_pre_commit(tempdir_factory): expected_contents = io.open(pre_commit_script).read().format( sys_executable=sys.executable, hook_type='pre-push', - pre_push=pre_push_template_contents, + hook_specific=pre_push_template_contents, skip_on_missing_conf='false', ) assert pre_push_contents == expected_contents @@ -118,10 +118,11 @@ def test_uninstall(tempdir_factory): def _get_commit_output(tempdir_factory, touch_file='foo', **kwargs): + commit_msg = kwargs.pop('commit_msg', 'Commit!') open(touch_file, 'a').close() cmd_output('git', 'add', touch_file) return cmd_output_mocked_pre_commit_home( - 'git', 'commit', '-am', 'Commit!', '--allow-empty', + 'git', 'commit', '-am', commit_msg, '--allow-empty', # git commit puts pre-commit to stderr stderr=subprocess.STDOUT, retcode=None, @@ -560,6 +561,24 @@ def test_pre_push_integration_empty_push(tempdir_factory): assert retc == 0 +def test_commit_msg_integration_failing(commit_msg_repo, tempdir_factory): + install(Runner(commit_msg_repo, C.CONFIG_FILE), 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') + msg = 'Hi\nSigned off by: me, lol' + retc, out = _get_commit_output(tempdir_factory, commit_msg=msg) + assert retc == 0 + first_line = out.splitlines()[0] + assert first_line.startswith('Must have "Signed off by:"...') + assert first_line.endswith('...Passed') + + def test_install_disallow_mising_config(tempdir_factory): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 59f2ec9a..c360fde9 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -60,6 +60,7 @@ def _get_opts( allow_unstaged_config=False, hook_stage='commit', show_diff_on_failure=False, + commit_msg_filename='', ): # These are mutually exclusive assert not (all_files and files) @@ -75,6 +76,7 @@ def _get_opts( allow_unstaged_config=allow_unstaged_config, hook_stage=hook_stage, show_diff_on_failure=show_diff_on_failure, + commit_msg_filename=commit_msg_filename, ) @@ -572,40 +574,7 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): ) -@pytest.mark.parametrize( - ( - 'hook_stage', 'stage_for_first_hook', 'stage_for_second_hook', - 'expected_output', - ), - ( - ('push', ['commit'], ['commit'], [b'', b'']), - ( - 'push', ['commit', 'push'], ['commit', 'push'], - [b'hook 1', b'hook 2'], - ), - ('push', [], [], [b'hook 1', b'hook 2']), - ('push', [], ['commit'], [b'hook 1', b'']), - ('push', ['push'], ['commit'], [b'hook 1', b'']), - ('push', ['commit'], ['push'], [b'', b'hook 2']), - ( - 'commit', ['commit', 'push'], ['commit', 'push'], - [b'hook 1', b'hook 2'], - ), - ('commit', ['commit'], ['commit'], [b'hook 1', b'hook 2']), - ('commit', [], [], [b'hook 1', b'hook 2']), - ('commit', [], ['commit'], [b'hook 1', b'hook 2']), - ('commit', ['push'], ['commit'], [b'', b'hook 2']), - ('commit', ['commit'], ['push'], [b'hook 1', b'']), - ), -) -def test_local_hook_for_stages( - cap_out, - repo_with_passing_hook, mock_out_store_directory, - stage_for_first_hook, - stage_for_second_hook, - hook_stage, - expected_output, -): +def test_push_hook(cap_out, repo_with_passing_hook, mock_out_store_directory): config = OrderedDict(( ('repo', 'local'), ( @@ -613,36 +582,60 @@ def test_local_hook_for_stages( OrderedDict(( ('id', 'flake8'), ('name', 'hook 1'), - ('entry', 'python -m flake8.__main__'), + ('entry', "'{}' -m flake8".format(sys.executable)), ('language', 'system'), - ('files', r'\.py$'), - ('stages', stage_for_first_hook), - )), OrderedDict(( + ('types', ['python']), + ('stages', ['commit']), + )), + OrderedDict(( ('id', 'do_not_commit'), ('name', 'hook 2'), ('entry', 'DO NOT COMMIT'), ('language', 'pcre'), - ('files', '^(.*)$'), - ('stages', stage_for_second_hook), + ('types', ['text']), + ('stages', ['push']), )), ), ), )) add_config_to_repo(repo_with_passing_hook, config) - with io.open('dummy.py', 'w') as staged_file: - staged_file.write('"""TODO: something"""\n') + open('dummy.py', 'a').close() cmd_output('git', 'add', 'dummy.py') _test_run( cap_out, repo_with_passing_hook, - {'hook_stage': hook_stage}, - expected_outputs=expected_output, + {'hook_stage': 'commit'}, + expected_outputs=[b'hook 1'], expected_ret=0, stage=False, ) + _test_run( + cap_out, + repo_with_passing_hook, + {'hook_stage': 'push'}, + expected_outputs=[b'hook 2'], + expected_ret=0, + stage=False, + ) + + +def test_commit_msg_hook(cap_out, commit_msg_repo, mock_out_store_directory): + filename = '.git/COMMIT_EDITMSG' + with io.open(filename, 'w') as f: + f.write('This is the commit message') + + _test_run( + cap_out, + commit_msg_repo, + {'hook_stage': 'commit-msg', 'commit_msg_filename': filename}, + expected_outputs=[b'Must have "Signed off by:"', b'Failed'], + expected_ret=1, + stage=False, + ) + def test_local_hook_passes( cap_out, repo_with_passing_hook, mock_out_store_directory, @@ -654,7 +647,7 @@ def test_local_hook_passes( OrderedDict(( ('id', 'flake8'), ('name', 'flake8'), - ('entry', 'python -m flake8.__main__'), + ('entry', "'{}' -m flake8".format(sys.executable)), ('language', 'system'), ('files', r'\.py$'), )), OrderedDict(( diff --git a/tests/conftest.py b/tests/conftest.py index 3d97695a..36743d88 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ from __future__ import absolute_import from __future__ import unicode_literals +import collections import functools import io import logging @@ -20,6 +21,7 @@ from pre_commit.util import cmd_output from pre_commit.util import cwd from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo +from testing.fixtures import write_config @pytest.yield_fixture @@ -92,6 +94,29 @@ def in_conflicting_submodule(tempdir_factory): yield +@pytest.fixture +def commit_msg_repo(tempdir_factory): + path = git_dir(tempdir_factory) + config = collections.OrderedDict(( + ('repo', 'local'), + ( + 'hooks', + [collections.OrderedDict(( + ('id', 'must-have-signoff'), + ('name', 'Must have "Signed off by:"'), + ('entry', 'grep -q "Signed off by:"'), + ('language', 'system'), + ('stages', ['commit-msg']), + ))], + ), + )) + write_config(path, config) + with cwd(path): + cmd_output('git', 'add', '.') + cmd_output('git', 'commit', '-m', 'add hooks') + yield path + + @pytest.yield_fixture(autouse=True, scope='session') def dont_write_to_home_directory(): """pre_commit.store.Store will by default write to the home directory