From 6c940c5145a0cfcdc95a90ee24c76c73143e3571 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 17:12:34 -0700 Subject: [PATCH 1/2] Simplify commands test. --- tests/commands_test.py | 106 ++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 71 deletions(-) diff --git a/tests/commands_test.py b/tests/commands_test.py index cf60cb52..11f228b4 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -192,86 +192,50 @@ def get_write_mock_output(write_mock): return ''.join(call[0][0] for call in write_mock.call_args_list) -def test_run_all_hooks_passing(repo_with_passing_hook): - stage_a_file() - runner = Runner(repo_with_passing_hook) +def _test_run(repo, options, expected_outputs, expected_ret, stage): + if stage: + stage_a_file() + runner = Runner(repo) args = auto_namedtuple( - all_files=False, color=False, verbose=False, hook=None, + **dict( + dict(all_files=False, color=False, verbose=False, hook=None), + **options + ) ) write_mock = mock.Mock() ret = commands.run(runner, args, write=write_mock) - assert ret == 0 + assert ret == expected_ret printed = get_write_mock_output(write_mock) - assert 'Bash hook' in printed - assert 'Passed' in printed - - -def test_verbose_prints_output(repo_with_passing_hook): - stage_a_file() - runner = Runner(repo_with_passing_hook) - args = auto_namedtuple( - all_files=False, color=False, verbose=True, hook=None, - ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) - assert ret == 0 - printed = get_write_mock_output(write_mock) - assert 'foo.py\nHello World\n' in printed + for expected_output_part in expected_outputs: + assert expected_output_part in printed def test_run_all_hooks_failing(repo_with_failing_hook): - stage_a_file() - runner = Runner(repo_with_failing_hook) - args = auto_namedtuple( - all_files=False, color=False, verbose=False, hook=None, + _test_run( + repo_with_failing_hook, + {}, + ('Failing hook', 'Failed', 'Fail\nfoo.py\n'), + 1, + True, ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) - assert ret == 1 - printed = get_write_mock_output(write_mock) - assert 'Failing hook' in printed - assert 'Failed' in printed - assert 'Fail\nfoo.py\n' in printed -def test_run_a_specific_hook(repo_with_passing_hook): - stage_a_file() - runner = Runner(repo_with_passing_hook) - args = auto_namedtuple( - all_files=False, color=False, verbose=False, hook='bash_hook', +@pytest.mark.parametrize( + ('options', 'outputs', 'expected_ret', 'stage'), + ( + ({}, ('Bash hook', 'Passed'), 0, True), + ({'verbose': True}, ('foo.py\nHello World',), 0, True), + ({'hook': 'bash_hook'}, ('Bash hook', 'Passed'), 0, True), + ({'hook': 'nope'}, ('No hook with id `nope`',), 1, True), + # All the files in the repo. + # This seems kind of weird but it is beacuse py.test reuses fixtures + ( + {'all_files': True, 'verbose': True}, + ('hooks.yaml', 'bin/hook.sh', 'foo.py', 'dummy'), + 0, + True, + ), ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) - assert ret == 0 - printed = get_write_mock_output(write_mock) - assert 'Bash hook' in printed - assert 'Passed' in printed - - -def test_run_a_non_existing_hook(repo_with_passing_hook): - stage_a_file() - runner = Runner(repo_with_passing_hook) - args = auto_namedtuple( - all_files=False, color=False, verbose=False, hook='nope', - ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) - assert ret == 1 - printed = get_write_mock_output(write_mock) - assert 'No hook with id `nope`' in printed - - -def test_run_all_files(repo_with_passing_hook): - stage_a_file() - runner = Runner(repo_with_passing_hook) - args = auto_namedtuple( - all_files=True, color=False, verbose=True, hook=None, - ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) - assert ret == 0 - printed = get_write_mock_output(write_mock) - # These are all the files checked into the repo. - # This seems kind of weird but it is because py.test reuses fixtures - for filename in 'hooks.yaml bin/hook.sh foo.py dummy'.split(): - assert filename in printed +) +def test_run(repo_with_passing_hook, options, outputs, expected_ret, stage): + _test_run(repo_with_passing_hook, options, outputs, expected_ret, stage) From de24712a6fab092c04cd342317ec2d171842b72b Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 17:25:05 -0700 Subject: [PATCH 2/2] Skip hooks if no files to check. Closes #69. --- pre_commit/commands.py | 22 +++++++++++++++++++++- tests/commands_test.py | 1 + 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 1db112ae..909f8de4 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -151,6 +151,26 @@ def _run_single_hook(runner, repository, hook_id, args, write): hook = repository.hooks[hook_id] + filenames = get_filenames(hook['files'], hook['exclude']) + if not filenames: + no_files_msg = '(no files to check) ' + skipped_msg = 'Skipped' + write( + '{0}{1}{2}{3}\n'.format( + hook['name'], + '.' * ( + COLS - + len(hook['name']) - + len(no_files_msg) - + len(skipped_msg) - + 6 + ), + no_files_msg, + color.format_color(skipped_msg, color.TURQUOISE, args.color), + ) + ) + return 0 + # Print the hook and the dots first in case the hook takes hella long to # run. write( @@ -164,7 +184,7 @@ def _run_single_hook(runner, repository, hook_id, args, write): retcode, stdout, stderr = repository.run_hook( runner.cmd_runner, hook_id, - get_filenames(hook['files'], hook['exclude']), + filenames, ) if retcode != repository.hooks[hook_id]['expected_return_value']: diff --git a/tests/commands_test.py b/tests/commands_test.py index 11f228b4..1d1cb1ab 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -235,6 +235,7 @@ def test_run_all_hooks_failing(repo_with_failing_hook): 0, True, ), + ({}, ('Bash hook', '(no files to check)', 'Skipped'), 0, False), ) ) def test_run(repo_with_passing_hook, options, outputs, expected_ret, stage):