Merge pull request #78 from pre-commit/improve_test_coverage

Improve test coverage
This commit is contained in:
Anthony Sottile 2014-04-13 15:55:12 -07:00
commit e297b210a5
16 changed files with 294 additions and 155 deletions

View file

@ -1,19 +1,33 @@
from __future__ import print_function
import logging
import os
import pkg_resources
import shutil
import stat
import subprocess
import sys
from asottile.ordereddict import OrderedDict
from asottile.yaml import ordered_dump
from asottile.yaml import ordered_load
from plumbum import local
import pre_commit.constants as C
from pre_commit import git
from pre_commit import color
from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
from pre_commit.clientlib.validate_config import load_config
from pre_commit.jsonschema_extensions import remove_defaults
from pre_commit.logging_handler import LoggingHandler
from pre_commit.repository import Repository
from pre_commit.staged_files_only import staged_files_only
logger = logging.getLogger('pre_commit')
COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0])
PASS_FAIL_LENGTH = 6
def install(runner):
@ -127,3 +141,80 @@ def clean(runner):
shutil.rmtree(runner.hooks_workspace_path)
print('Cleaned {0}.'.format(runner.hooks_workspace_path))
return 0
def _run_single_hook(runner, repository, hook_id, args, write):
if args.all_files:
get_filenames = git.get_all_files_matching
else:
get_filenames = git.get_staged_files_matching
hook = repository.hooks[hook_id]
# Print the hook and the dots first in case the hook takes hella long to
# run.
write(
'{0}{1}'.format(
hook['name'],
'.' * (COLS - len(hook['name']) - PASS_FAIL_LENGTH - 6),
),
)
sys.stdout.flush()
retcode, stdout, stderr = repository.run_hook(
runner.cmd_runner,
hook_id,
get_filenames(hook['files'], hook['exclude']),
)
if retcode != repository.hooks[hook_id]['expected_return_value']:
retcode = 1
print_color = color.RED
pass_fail = 'Failed'
else:
retcode = 0
print_color = color.GREEN
pass_fail = 'Passed'
write(color.format_color(pass_fail, print_color, args.color) + '\n')
if (stdout or stderr) and (retcode or args.verbose):
write('\n')
for output in (stdout, stderr):
if output.strip():
write(output.strip() + '\n')
write('\n')
return retcode
def _run_hooks(runner, args, write):
"""Actually run the hooks."""
retval = 0
for repo in runner.repositories:
for hook_id in repo.hooks:
retval |= _run_single_hook(runner, repo, hook_id, args, write=write)
return retval
def _run_hook(runner, hook_id, args, write):
for repo in runner.repositories:
if hook_id in repo.hooks:
return _run_single_hook(runner, repo, hook_id, args, write=write)
else:
write('No hook with id `{0}`\n'.format(hook_id))
return 1
def run(runner, args, write=sys.stdout.write):
# Set up our logging handler
logger.addHandler(LoggingHandler(args.color))
logger.setLevel(logging.INFO)
with staged_files_only(runner.cmd_runner):
if args.hook:
return _run_hook(runner, args.hook, args, write=write)
else:
return _run_hooks(runner, args, write=write)

View file

@ -8,11 +8,8 @@ def extend_validator_cls(validator_cls, modify):
validate_properties = validator_cls.VALIDATORS['properties']
def new_properties(validator, properties, instance, schema):
for error in validate_properties(
validator, properties, instance, schema,
):
yield error
# Exhaust the validator
list(validate_properties(validator, properties, instance, schema))
modify(properties, instance)
return jsonschema.validators.extend(

View file

@ -2,6 +2,8 @@ import os
import os.path
import subprocess
from pre_commit import five
class CalledProcessError(RuntimeError):
def __init__(self, returncode, cmd, expected_returncode, output=None):
@ -63,6 +65,11 @@ class PrefixedCommandRunner(object):
replaced_cmd = _replace_cmd(cmd, prefix=self.prefix_dir)
proc = self.__popen(replaced_cmd, **popen_kwargs)
stdout, stderr = proc.communicate(stdin)
# TODO: stdout, stderr = from_bytes(stdout), from_bytes(stderr)
if isinstance(stdout, bytes):
stdout = five.text(stdout, 'utf-8')
if isinstance(stderr, bytes):
stderr = five.text(stderr, 'utf-8')
returncode = proc.returncode
if retcode is not None and retcode != returncode:

View file

@ -1,103 +1,12 @@
from __future__ import print_function
import argparse
import logging
import subprocess
import sys
from pre_commit import color
from pre_commit import commands
from pre_commit import git
from pre_commit.logging_handler import LoggingHandler
from pre_commit.runner import Runner
from pre_commit.staged_files_only import staged_files_only
from pre_commit.util import entry
logger = logging.getLogger('pre_commit')
COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0])
PASS_FAIL_LENGTH = 6
def _run_single_hook(runner, repository, hook_id, args):
if args.all_files:
get_filenames = git.get_all_files_matching
else:
get_filenames = git.get_staged_files_matching
hook = repository.hooks[hook_id]
# Print the hook and the dots first in case the hook takes hella long to
# run.
print(
'{0}{1}'.format(
hook['name'],
'.' * (COLS - len(hook['name']) - PASS_FAIL_LENGTH - 6),
),
end='',
)
retcode, stdout, stderr = repository.run_hook(
runner.cmd_runner,
hook_id,
get_filenames(hook['files'], hook['exclude']),
)
if retcode != repository.hooks[hook_id]['expected_return_value']:
retcode = 1
print_color = color.RED
pass_fail = 'Failed'
else:
retcode = 0
print_color = color.GREEN
pass_fail = 'Passed'
print(color.format_color(pass_fail, print_color, args.color))
if (stdout or stderr) and (retcode or args.verbose):
print()
for output in (stdout, stderr):
if output.strip():
print(output.strip())
print()
return retcode
def run_hooks(runner, args):
"""Actually run the hooks."""
retval = 0
for repo in runner.repositories:
for hook_id in repo.hooks:
retval |= _run_single_hook(runner, repo, hook_id, args)
return retval
def run_single_hook(runner, hook_id, args):
for repo in runner.repositories:
if hook_id in repo.hooks:
return _run_single_hook(runner, repo, hook_id, args)
else:
print('No hook with id `{0}`'.format(hook_id))
return 1
def _run(runner, args):
# Set up our logging handler
logger.addHandler(LoggingHandler(args.color))
logger.setLevel(logging.INFO)
with staged_files_only(runner.cmd_runner):
if args.hook:
return run_single_hook(runner, args.hook, args)
else:
return run_hooks(runner, args)
@entry
def run(argv):
parser = argparse.ArgumentParser()
@ -118,11 +27,11 @@ def run(argv):
'--all-files', '-a', action='store_true', default=False,
help='Run on all the files in the repo.',
)
run_parser.add_argument('--verbose', '-v', action='store_true', default=False)
run_parser.add_argument(
'--color', default='auto', type=color.use_color,
help='Whether to use color in output. Defaults to `auto`',
)
run_parser.add_argument('--verbose', '-v', action='store_true', default=False)
help = subparsers.add_parser('help', help='Show help for a specific command.')
help.add_argument('help_cmd', nargs='?', help='Command to show help for.')
@ -143,7 +52,7 @@ def run(argv):
elif args.command == 'autoupdate':
return commands.autoupdate(runner)
elif args.command == 'run':
return _run(runner, args)
return commands.run(runner, args)
elif args.command == 'help':
if args.help_cmd:
parser.parse_args([args.help_cmd, '--help'])

View file

@ -0,0 +1,6 @@
#!/usr/bin/env bash
echo 'Fail'
echo $@
exit 1

View file

@ -0,0 +1,4 @@
- id: failing_hook
name: Failing hook
entry: bin/hook.sh
language: script

View file

@ -1,5 +0,0 @@
import os
def func():
print os.getcwd()
return 0

View file

@ -6,18 +6,21 @@ from pre_commit.clientlib.validate_config import run
from pre_commit.clientlib.validate_config import validate_config_extra
from pre_commit.jsonschema_extensions import apply_defaults
from testing.util import is_valid_according_to_schema
from testing.util import get_resource_path
def test_returns_0_for_valid_config():
assert run(['example_pre-commit-config.yaml']) == 0
def test_returns_0_for_out_manifest():
assert run([]) == 0
def test_returns_1_for_failing():
assert run(['tests/data/valid_yaml_but_invalid_config.yaml']) == 1
@pytest.mark.parametrize(
('input', 'expected_output'),
(
(['example_pre-commit-config.yaml'], 0),
(['.pre-commit-config.yaml'], 0),
(['non_existent_file.yaml'], 1),
([get_resource_path('valid_yaml_but_invalid_config.yaml')], 1),
([get_resource_path('non_parseable_yaml_file.notyaml')], 1),
),
)
def test_run(input, expected_output):
assert run(input) == expected_output
@pytest.mark.parametrize(('manifest_obj', 'expected'), (

View file

@ -5,18 +5,21 @@ from pre_commit.clientlib.validate_manifest import InvalidManifestError
from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA
from pre_commit.clientlib.validate_manifest import run
from testing.util import is_valid_according_to_schema
from testing.util import get_resource_path
def test_returns_0_for_valid_manifest():
assert run(['example_hooks.yaml']) == 0
def test_returns_0_for_our_manifest():
assert run([]) == 0
def test_returns_1_for_failing():
assert run(['tests/data/valid_yaml_but_invalid_manifest.yaml']) == 1
@pytest.mark.parametrize(
('input', 'expected_output'),
(
(['example_hooks.yaml'], 0),
(['hooks.yaml'], 0),
(['non_existent_file.yaml'], 1),
([get_resource_path('valid_yaml_but_invalid_manifest.yaml')], 1),
([get_resource_path('non_parseable_yaml_file.notyaml')], 1),
),
)
def test_run(input, expected_output):
assert run(input) == expected_output
def test_additional_manifest_check_raises_for_bad_language():

View file

@ -1,3 +1,4 @@
import mock
import os
import os.path
import pkg_resources
@ -10,15 +11,10 @@ from plumbum import local
import pre_commit.constants as C
from pre_commit import commands
from pre_commit import git
from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
from pre_commit.clientlib.validate_config import validate_config_extra
from pre_commit.commands import autoupdate
from pre_commit.commands import clean
from pre_commit.commands import install
from pre_commit.commands import RepositoryCannotBeUpdatedError
from pre_commit.commands import uninstall
from pre_commit.commands import _update_repository
from pre_commit.jsonschema_extensions import apply_defaults
from pre_commit.runner import Runner
from testing.auto_namedtuple import auto_namedtuple
@ -27,7 +23,7 @@ from testing.util import get_resource_path
def test_install_pre_commit(empty_git_dir):
runner = Runner(empty_git_dir)
ret = install(runner)
ret = commands.install(runner)
assert ret == 0
assert os.path.exists(runner.pre_commit_path)
pre_commit_contents = open(runner.pre_commit_path).read()
@ -40,16 +36,16 @@ def test_install_pre_commit(empty_git_dir):
def test_uninstall_pre_commit_does_not_blow_up_when_not_there(empty_git_dir):
runner = Runner(empty_git_dir)
ret = uninstall(runner)
ret = commands.uninstall(runner)
assert ret == 0
def test_uninstall(empty_git_dir):
runner = Runner(empty_git_dir)
assert not os.path.exists(runner.pre_commit_path)
install(runner)
commands.install(runner)
assert os.path.exists(runner.pre_commit_path)
uninstall(runner)
commands.uninstall(runner)
assert not os.path.exists(runner.pre_commit_path)
@ -77,14 +73,14 @@ def up_to_date_repo(python_hooks_repo):
def test_up_to_date_repo(up_to_date_repo):
input_sha = up_to_date_repo.repo_config['sha']
ret = _update_repository(up_to_date_repo.repo_config)
ret = commands._update_repository(up_to_date_repo.repo_config)
assert ret['sha'] == input_sha
def test_autoupdate_up_to_date_repo(up_to_date_repo):
before = open(C.CONFIG_FILE).read()
runner = Runner(up_to_date_repo.python_hooks_repo)
ret = autoupdate(runner)
ret = commands.autoupdate(runner)
after = open(C.CONFIG_FILE).read()
assert ret == 0
assert before == after
@ -116,12 +112,12 @@ def out_of_date_repo(python_hooks_repo):
def test_out_of_date_repo(out_of_date_repo):
ret = _update_repository(out_of_date_repo.repo_config)
ret = commands._update_repository(out_of_date_repo.repo_config)
assert ret['sha'] == out_of_date_repo.head_sha
def test_removes_defaults(out_of_date_repo):
ret = _update_repository(out_of_date_repo.repo_config)
ret = commands._update_repository(out_of_date_repo.repo_config)
assert 'args' not in ret['hooks'][0]
assert 'expected_return_value' not in ret['hooks'][0]
@ -129,7 +125,7 @@ def test_removes_defaults(out_of_date_repo):
def test_autoupdate_out_of_date_repo(out_of_date_repo):
before = open(C.CONFIG_FILE).read()
runner = Runner(out_of_date_repo.python_hooks_repo)
ret = autoupdate(runner)
ret = commands.autoupdate(runner)
after = open(C.CONFIG_FILE).read()
assert ret == 0
assert before != after
@ -162,14 +158,14 @@ def hook_disappearing_repo(python_hooks_repo):
def test_hook_disppearing_repo_raises(hook_disappearing_repo):
with pytest.raises(RepositoryCannotBeUpdatedError):
_update_repository(hook_disappearing_repo.repo_config)
with pytest.raises(commands.RepositoryCannotBeUpdatedError):
commands._update_repository(hook_disappearing_repo.repo_config)
def test_autoupdate_hook_disappearing_repo(hook_disappearing_repo):
before = open(C.CONFIG_FILE).read()
runner = Runner(hook_disappearing_repo.python_hooks_repo)
ret = autoupdate(runner)
ret = commands.autoupdate(runner)
after = open(C.CONFIG_FILE).read()
assert ret == 1
assert before == after
@ -177,11 +173,105 @@ def test_autoupdate_hook_disappearing_repo(hook_disappearing_repo):
def test_clean(empty_git_dir):
os.mkdir(C.HOOKS_WORKSPACE)
clean(Runner(empty_git_dir))
commands.clean(Runner(empty_git_dir))
assert not os.path.exists(C.HOOKS_WORKSPACE)
def test_clean_empty(empty_git_dir):
assert not os.path.exists(C.HOOKS_WORKSPACE)
clean(Runner(empty_git_dir))
commands.clean(Runner(empty_git_dir))
assert not os.path.exists(C.HOOKS_WORKSPACE)
def stage_a_file():
local['touch']['foo.py']()
local['git']['add', 'foo.py']()
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)
args = auto_namedtuple(
all_files=False, color=False, verbose=False, 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 '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
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,
)
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',
)
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

View file

@ -2,8 +2,10 @@ from __future__ import absolute_import
import pytest
import time
import yaml
from plumbum import local
import pre_commit.constants as C
from pre_commit import git
from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
from pre_commit.clientlib.validate_config import validate_config_extra
@ -68,6 +70,11 @@ def script_hooks_repo(dummy_git_repo):
yield _make_repo(dummy_git_repo, 'script_hooks_repo')
@pytest.yield_fixture
def failing_hook_repo(dummy_git_repo):
yield _make_repo(dummy_git_repo, 'failing_hook_repo')
def _make_config(path, hook_id, file_regex):
config = {
'repo': path,
@ -96,4 +103,26 @@ def config_for_prints_cwd_repo(prints_cwd_repo):
@pytest.yield_fixture
def config_for_script_hooks_repo(script_hooks_repo):
yield _make_config(script_hooks_repo, 'bash_hook', '^$')
yield _make_config(script_hooks_repo, 'bash_hook', '')
def _make_repo_from_configs(*configs):
with open(C.CONFIG_FILE, 'w') as config_file:
yaml.dump(
configs,
stream=config_file,
Dumper=yaml.SafeDumper,
**C.YAML_DUMP_KWARGS
)
@pytest.yield_fixture
def repo_with_passing_hook(config_for_script_hooks_repo, empty_git_dir):
_make_repo_from_configs(config_for_script_hooks_repo)
yield empty_git_dir
@pytest.yield_fixture
def repo_with_failing_hook(failing_hook_repo, empty_git_dir):
_make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook', ''))
yield empty_git_dir

View file

@ -1,3 +1,6 @@
import jsonschema.exceptions
import pytest
from pre_commit.jsonschema_extensions import apply_defaults
from pre_commit.jsonschema_extensions import remove_defaults
@ -78,3 +81,9 @@ def test_remove_defaults_removes_default():
{'properties': {'foo': {'default': 'bar'}}},
)
assert ret == {}
@pytest.mark.parametrize('func', (apply_defaults, remove_defaults))
def test_still_validates_schema(func):
with pytest.raises(jsonschema.exceptions.ValidationError):
func({}, {'properties': {'foo': {}}, 'required': ['foo']})

View file

@ -21,7 +21,7 @@ def test_CalledProcessError_str():
@pytest.fixture
def popen_mock():
popen = mock.Mock(spec=subprocess.Popen)
popen.return_value.communicate.return_value = (mock.Mock(), mock.Mock())
popen.return_value.communicate.return_value = (b'stdout', b'stderr')
return popen
@ -64,11 +64,7 @@ def test_run_substitutes_prefix(popen_mock, makedirs_mock):
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
assert ret == (
popen_mock.return_value.returncode,
popen_mock.return_value.communicate.return_value[0],
popen_mock.return_value.communicate.return_value[1],
)
assert ret == (popen_mock.return_value.returncode, 'stdout', 'stderr')
PATH_TESTS = (

View file

@ -55,7 +55,7 @@ def test_run_a_python_hook(config_for_python_hooks_repo):
)
assert ret[0] == 0
assert ret[1] == b"['/dev/null']\nHello World\n"
assert ret[1] == "['/dev/null']\nHello World\n"
@pytest.mark.integration
@ -77,7 +77,7 @@ def test_cwd_of_hook(config_for_prints_cwd_repo):
)
assert ret[0] == 0
assert ret[1] == repo.repo_url.encode('utf-8') + b'\n'
assert ret[1] == repo.repo_url + '\n'
@pytest.mark.skipif(
@ -90,7 +90,7 @@ def test_run_a_node_hook(config_for_node_hooks_repo):
ret = repo.run_hook(PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', [])
assert ret[0] == 0
assert ret[1] == b'Hello World\n'
assert ret[1] == 'Hello World\n'
@pytest.mark.integration
@ -101,7 +101,7 @@ def test_run_a_script_hook(config_for_script_hooks_repo):
)
assert ret[0] == 0
assert ret[1] == b'bar\nHello World\n'
assert ret[1] == 'bar\nHello World\n'
@pytest.fixture

View file

@ -9,7 +9,7 @@ deps = -rrequirements_dev.txt
commands =
coverage erase
coverage run -m pytest {posargs:tests}
coverage report --show-missing --fail-under 88
coverage report --show-missing --fail-under 93
flake8 {[tox]project} tests setup.py
# pylint {[tox]project} tests setup.py