Separate store from runner

This commit is contained in:
Anthony Sottile 2018-06-29 22:35:53 -07:00
parent 6d683a5fac
commit c01ffc83f8
15 changed files with 228 additions and 347 deletions

View file

@ -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