From cb8dd335f4271af0abbf5ada58976709e8931811 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 5 Mar 2017 18:06:11 -0800 Subject: [PATCH] Replace jsonschema with better error messages --- .coveragerc | 4 +- pre_commit/clientlib.py | 144 +++++++ pre_commit/clientlib/__init__.py | 0 pre_commit/clientlib/validate_base.py | 88 ---- pre_commit/clientlib/validate_config.py | 93 ----- pre_commit/clientlib/validate_manifest.py | 103 ----- pre_commit/commands/autoupdate.py | 20 +- pre_commit/five.py | 13 + pre_commit/jsonschema_extensions.py | 57 --- pre_commit/manifest.py | 2 +- pre_commit/repository.py | 24 +- pre_commit/runner.py | 2 +- pre_commit/schema.py | 279 +++++++++++++ setup.py | 5 +- testing/fixtures.py | 14 +- testing/resources/array_yaml_file.yaml | 2 - .../resources/non_parseable_yaml_file.notyaml | 1 - testing/resources/ordering_data_test.yaml | 2 - testing/util.py | 9 - tests/clientlib/__init__.py | 0 tests/clientlib/validate_base_test.py | 74 ---- tests/clientlib/validate_config_test.py | 195 --------- tests/clientlib/validate_manifest_test.py | 87 ---- tests/clientlib_test.py | 194 +++++++++ tests/commands/autoupdate_test.py | 2 +- tests/jsonschema_extensions_test.py | 91 ---- tests/manifest_test.py | 6 +- tests/repository_test.py | 2 +- tests/schema_test.py | 391 ++++++++++++++++++ tox.ini | 2 +- 30 files changed, 1064 insertions(+), 842 deletions(-) create mode 100644 pre_commit/clientlib.py delete mode 100644 pre_commit/clientlib/__init__.py delete mode 100644 pre_commit/clientlib/validate_base.py delete mode 100644 pre_commit/clientlib/validate_config.py delete mode 100644 pre_commit/clientlib/validate_manifest.py delete mode 100644 pre_commit/jsonschema_extensions.py create mode 100644 pre_commit/schema.py delete mode 100644 testing/resources/array_yaml_file.yaml delete mode 100644 testing/resources/non_parseable_yaml_file.notyaml delete mode 100644 testing/resources/ordering_data_test.yaml delete mode 100644 tests/clientlib/__init__.py delete mode 100644 tests/clientlib/validate_base_test.py delete mode 100644 tests/clientlib/validate_config_test.py delete mode 100644 tests/clientlib/validate_manifest_test.py create mode 100644 tests/clientlib_test.py delete mode 100644 tests/jsonschema_extensions_test.py create mode 100644 tests/schema_test.py diff --git a/.coveragerc b/.coveragerc index c6d704c6..f73b9e3d 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,7 +1,6 @@ [run] branch = True -source = - . +source = . omit = .tox/* /usr/* @@ -11,6 +10,7 @@ omit = */__main__.py [report] +show_missing = True exclude_lines = # Have to re-enable the standard pragma \#\s*pragma: no cover diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py new file mode 100644 index 00000000..58031e8d --- /dev/null +++ b/pre_commit/clientlib.py @@ -0,0 +1,144 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +import argparse +import functools + +from aspy.yaml import ordered_load + +import pre_commit.constants as C +from pre_commit import schema +from pre_commit.errors import FatalError +from pre_commit.languages.all import all_languages + + +def check_language(v): + if v not in all_languages: + raise schema.ValidationError( + 'Expected {} to be in {!r}'.format(v, all_languages), + ) + + +def _make_argparser(filenames_help): + parser = argparse.ArgumentParser() + parser.add_argument('filenames', nargs='*', help=filenames_help) + parser.add_argument('-V', '--version', action='version', version=C.VERSION) + return parser + + +MANIFEST_HOOK_DICT = schema.Map( + 'Hook', 'id', + + schema.Required('id', schema.check_string), + schema.Required('name', schema.check_string), + schema.Required('entry', schema.check_string), + schema.Required( + 'language', schema.check_and(schema.check_string, check_language), + ), + + schema.Conditional( + 'files', schema.check_and(schema.check_string, schema.check_regex), + condition_key='always_run', condition_value=False, ensure_absent=True, + ), + + schema.Optional( + 'additional_dependencies', schema.check_array(schema.check_string), [], + ), + schema.Optional('args', schema.check_array(schema.check_string), []), + schema.Optional('always_run', schema.check_bool, False), + schema.Optional('description', schema.check_string, ''), + schema.Optional( + 'exclude', + schema.check_and(schema.check_string, schema.check_regex), + '^$', + ), + schema.Optional('language_version', schema.check_string, 'default'), + schema.Optional('minimum_pre_commit_version', schema.check_string, '0'), + schema.Optional('stages', schema.check_array(schema.check_string), []), +) +MANIFEST_SCHEMA = schema.Array(MANIFEST_HOOK_DICT) + + +class InvalidManifestError(FatalError): + pass + + +load_manifest = functools.partial( + schema.load_from_filename, + schema=MANIFEST_SCHEMA, + load_strategy=ordered_load, + exc_tp=InvalidManifestError, +) + + +def validate_manifest_main(argv=None): + parser = _make_argparser('Manifest filenames.') + args = parser.parse_args(argv) + ret = 0 + for filename in args.filenames: + try: + load_manifest(filename) + except InvalidManifestError as e: + print(e) + ret = 1 + return ret + + +_LOCAL_SENTINEL = 'local' +CONFIG_HOOK_DICT = schema.Map( + 'Hook', 'id', + + schema.Required('id', schema.check_string), + + # All keys in manifest hook dict are valid in a config hook dict, but + # are optional. + # No defaults are provided here as the config is merged on top of the + # manifest. + *[ + schema.OptionalNoDefault(item.key, item.check_fn) + for item in MANIFEST_HOOK_DICT.items + if item.key != 'id' + ] +) +CONFIG_REPO_DICT = schema.Map( + 'Repository', 'repo', + + schema.Required('repo', schema.check_string), + schema.RequiredRecurse('hooks', schema.Array(CONFIG_HOOK_DICT)), + + schema.Conditional( + 'sha', schema.check_string, + condition_key='repo', condition_value=schema.Not(_LOCAL_SENTINEL), + ensure_absent=True, + ), +) +CONFIG_SCHEMA = schema.Array(CONFIG_REPO_DICT) + + +def is_local_repo(repo_entry): + return repo_entry['repo'] == _LOCAL_SENTINEL + + +class InvalidConfigError(FatalError): + pass + + +load_config = functools.partial( + schema.load_from_filename, + schema=CONFIG_SCHEMA, + load_strategy=ordered_load, + exc_tp=InvalidConfigError, +) + + +def validate_config_main(argv=None): + parser = _make_argparser('Config filenames.') + args = parser.parse_args(argv) + ret = 0 + for filename in args.filenames: + try: + load_config(filename) + except InvalidConfigError as e: + print(e) + ret = 1 + return ret diff --git a/pre_commit/clientlib/__init__.py b/pre_commit/clientlib/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/pre_commit/clientlib/validate_base.py b/pre_commit/clientlib/validate_base.py deleted file mode 100644 index 43b1f3f8..00000000 --- a/pre_commit/clientlib/validate_base.py +++ /dev/null @@ -1,88 +0,0 @@ -from __future__ import print_function -from __future__ import unicode_literals - -import argparse -import os.path -import re - -import jsonschema -import jsonschema.exceptions -import yaml - -import pre_commit.constants as C -from pre_commit import output -from pre_commit.jsonschema_extensions import apply_defaults - - -def is_regex_valid(regex): - try: - re.compile(regex) - return True - except re.error: - return False - - -def get_validator( - json_schema, - exception_type, - additional_validation_strategy=lambda obj: None, -): - """Returns a function which will validate a yaml file for correctness - - Args: - json_schema - JSON schema to validate file with - exception_type - Error type to raise on failure - additional_validation_strategy - Strategy for additional validation of - the object read from the file. The function should either raise - exception_type on failure. - """ - def validate(filename, load_strategy=yaml.load): - if not os.path.exists(filename): - raise exception_type('File {} does not exist'.format(filename)) - - file_contents = open(filename, 'r').read() - - try: - obj = load_strategy(file_contents) - except Exception as e: - raise exception_type( - 'Invalid yaml: {}\n{}'.format(os.path.relpath(filename), e), - ) - - try: - jsonschema.validate(obj, json_schema) - except jsonschema.exceptions.ValidationError as e: - raise exception_type( - 'Invalid content: {}\n{}'.format( - os.path.relpath(filename), e - ), - ) - - obj = apply_defaults(obj, json_schema) - - additional_validation_strategy(obj) - - return obj - - return validate - - -def get_run_function(filenames_help, validate_strategy, exception_cls): - def run(argv=None): - parser = argparse.ArgumentParser() - parser.add_argument('filenames', nargs='*', help=filenames_help) - parser.add_argument( - '-V', '--version', action='version', version=C.VERSION, - ) - - args = parser.parse_args(argv) - - retval = 0 - for filename in args.filenames: - try: - validate_strategy(filename) - except exception_cls as e: - output.write_line(e.args[0]) - retval = 1 - return retval - return run diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py deleted file mode 100644 index 3624e62b..00000000 --- a/pre_commit/clientlib/validate_config.py +++ /dev/null @@ -1,93 +0,0 @@ -from __future__ import unicode_literals - -from pre_commit.clientlib.validate_base import get_run_function -from pre_commit.clientlib.validate_base import get_validator -from pre_commit.clientlib.validate_base import is_regex_valid -from pre_commit.errors import FatalError - - -_LOCAL_HOOKS_MAGIC_REPO_STRING = 'local' - - -def is_local_hooks(repo_entry): - return repo_entry['repo'] == _LOCAL_HOOKS_MAGIC_REPO_STRING - - -class InvalidConfigError(FatalError): - pass - - -CONFIG_JSON_SCHEMA = { - 'type': 'array', - 'minItems': 1, - 'items': { - 'type': 'object', - 'properties': { - 'repo': {'type': 'string'}, - 'sha': {'type': 'string'}, - 'hooks': { - 'type': 'array', - 'minItems': 1, - 'items': { - 'type': 'object', - 'properties': { - 'id': {'type': 'string'}, - 'always_run': {'type': 'boolean'}, - 'files': {'type': 'string'}, - 'exclude': {'type': 'string'}, - 'language_version': {'type': 'string'}, - 'args': { - 'type': 'array', - 'items': {'type': 'string'}, - }, - 'additional_dependencies': { - 'type': 'array', - 'items': {'type': 'string'}, - }, - }, - 'required': ['id'], - } - } - }, - 'required': ['repo', 'hooks'], - } -} - - -def try_regex(repo, hook, value, field_name): - if not is_regex_valid(value): - raise InvalidConfigError( - 'Invalid {} regex at {}, {}: {}'.format( - field_name, repo, hook, value, - ) - ) - - -def validate_config_extra(config): - for repo in config: - if is_local_hooks(repo): - if 'sha' in repo: - raise InvalidConfigError( - '"sha" property provided for local hooks' - ) - elif 'sha' not in repo: - raise InvalidConfigError( - 'Missing "sha" field for repository {}'.format(repo['repo']) - ) - for hook in repo['hooks']: - try_regex(repo, hook['id'], hook.get('files', ''), 'files') - try_regex(repo, hook['id'], hook.get('exclude', ''), 'exclude') - - -load_config = get_validator( - CONFIG_JSON_SCHEMA, - InvalidConfigError, - additional_validation_strategy=validate_config_extra, -) - - -run = get_run_function('Config filenames.', load_config, InvalidConfigError) - - -if __name__ == '__main__': - exit(run()) diff --git a/pre_commit/clientlib/validate_manifest.py b/pre_commit/clientlib/validate_manifest.py deleted file mode 100644 index 10234556..00000000 --- a/pre_commit/clientlib/validate_manifest.py +++ /dev/null @@ -1,103 +0,0 @@ -from __future__ import unicode_literals - -from pre_commit.clientlib.validate_base import get_run_function -from pre_commit.clientlib.validate_base import get_validator -from pre_commit.clientlib.validate_base import is_regex_valid -from pre_commit.languages.all import all_languages - - -class InvalidManifestError(ValueError): - pass - - -MANIFEST_JSON_SCHEMA = { - 'type': 'array', - 'minItems': 1, - 'items': { - 'type': 'object', - 'properties': { - 'id': {'type': 'string'}, - 'always_run': {'type': 'boolean', 'default': False}, - 'name': {'type': 'string'}, - 'description': {'type': 'string', 'default': ''}, - 'entry': {'type': 'string'}, - 'exclude': {'type': 'string', 'default': '^$'}, - 'language': {'type': 'string'}, - 'language_version': {'type': 'string', 'default': 'default'}, - 'minimum_pre_commit_version': { - 'type': 'string', 'default': '0.0.0', - }, - 'files': {'type': 'string'}, - 'stages': { - 'type': 'array', - 'default': [], - 'items': { - 'type': 'string', - }, - }, - 'args': { - 'type': 'array', - 'default': [], - 'items': { - 'type': 'string', - }, - }, - 'additional_dependencies': { - 'type': 'array', - 'items': {'type': 'string'}, - }, - }, - 'required': ['id', 'name', 'entry', 'language', 'files'], - }, -} - - -def validate_languages(hook_config): - if hook_config['language'] not in all_languages: - raise InvalidManifestError( - 'Expected language {} for {} to be one of {!r}'.format( - hook_config['id'], - hook_config['language'], - all_languages, - ) - ) - - -def validate_files(hook_config): - if not is_regex_valid(hook_config['files']): - raise InvalidManifestError( - 'Invalid files regex at {}: {}'.format( - hook_config['id'], hook_config['files'], - ) - ) - - if not is_regex_valid(hook_config.get('exclude', '')): - raise InvalidManifestError( - 'Invalid exclude regex at {}: {}'.format( - hook_config['id'], hook_config['exclude'], - ) - ) - - -def additional_manifest_check(obj): - for hook_config in obj: - validate_languages(hook_config) - validate_files(hook_config) - - -load_manifest = get_validator( - MANIFEST_JSON_SCHEMA, - InvalidManifestError, - additional_manifest_check, -) - - -run = get_run_function( - 'Manifest filenames.', - load_manifest, - InvalidManifestError, -) - - -if __name__ == '__main__': - exit(run()) diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 01a361c8..99a5d62f 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -8,11 +8,11 @@ from aspy.yaml import ordered_load import pre_commit.constants as C from pre_commit import output -from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA -from pre_commit.clientlib.validate_config import is_local_hooks -from pre_commit.clientlib.validate_config import load_config -from pre_commit.jsonschema_extensions import remove_defaults +from pre_commit.clientlib import CONFIG_SCHEMA +from pre_commit.clientlib import is_local_repo +from pre_commit.clientlib import load_config from pre_commit.repository import Repository +from pre_commit.schema import remove_defaults from pre_commit.util import CalledProcessError from pre_commit.util import cmd_output from pre_commit.util import cwd @@ -77,7 +77,7 @@ def autoupdate(runner, tags_only): ) for repo_config in input_configs: - if is_local_hooks(repo_config): + if is_local_repo(repo_config): output_configs.append(repo_config) continue output.write('Updating {}...'.format(repo_config['repo'])) @@ -101,11 +101,9 @@ def autoupdate(runner, tags_only): if changed: with open(runner.config_file_path, 'w') as config_file: - config_file.write( - ordered_dump( - remove_defaults(output_configs, CONFIG_JSON_SCHEMA), - **C.YAML_DUMP_KWARGS - ) - ) + config_file.write(ordered_dump( + remove_defaults(output_configs, CONFIG_SCHEMA), + **C.YAML_DUMP_KWARGS + )) return retv diff --git a/pre_commit/five.py b/pre_commit/five.py index b7741460..5bab4e56 100644 --- a/pre_commit/five.py +++ b/pre_commit/five.py @@ -5,14 +5,20 @@ PY3 = str is not bytes if PY2: # pragma: no cover (PY2 only) text = unicode # flake8: noqa + string_types = (text, bytes) def n(s): if isinstance(s, bytes): return s else: return s.encode('UTF-8') + + exec("""def reraise(tp, value, tb=None): + raise tp, value, tb +""") else: # pragma: no cover (PY3 only) text = str + string_types = (text,) def n(s): if isinstance(s, text): @@ -20,6 +26,13 @@ else: # pragma: no cover (PY3 only) else: return s.decode('UTF-8') + def reraise(tp, value, tb=None): + if value is None: + value = tp() + if value.__traceback__ is not tb: + raise value.with_traceback(tb) + raise value + def to_text(s): return s if isinstance(s, text) else s.decode('UTF-8') diff --git a/pre_commit/jsonschema_extensions.py b/pre_commit/jsonschema_extensions.py deleted file mode 100644 index 0314e32e..00000000 --- a/pre_commit/jsonschema_extensions.py +++ /dev/null @@ -1,57 +0,0 @@ -from __future__ import unicode_literals - -import copy - -import jsonschema -import jsonschema.validators - - -# From https://github.com/Julian/jsonschema/blob/master/docs/faq.rst -def extend_validator_cls(validator_cls, modify): - validate_properties = validator_cls.VALIDATORS['properties'] - - def new_properties(validator, properties, instance, schema): - # Exhaust the validator - list(validate_properties(validator, properties, instance, schema)) - modify(properties, instance) - - return jsonschema.validators.extend( - validator_cls, {'properties': new_properties}, - ) - - -def default_values(properties, instance): - for prop, subschema in properties.items(): - if 'default' in subschema: - instance.setdefault( - prop, copy.deepcopy(subschema['default']), - ) - - -def remove_default_values(properties, instance): - for prop, subschema in properties.items(): - if ( - 'default' in subschema and - instance.get(prop) == subschema['default'] - ): - del instance[prop] - - -_AddDefaultsValidator = extend_validator_cls( - jsonschema.Draft4Validator, default_values, -) -_RemoveDefaultsValidator = extend_validator_cls( - jsonschema.Draft4Validator, remove_default_values, -) - - -def apply_defaults(obj, schema): - obj = copy.deepcopy(obj) - _AddDefaultsValidator(schema).validate(obj) - return obj - - -def remove_defaults(obj, schema): - obj = copy.deepcopy(obj) - _RemoveDefaultsValidator(schema).validate(obj) - return obj diff --git a/pre_commit/manifest.py b/pre_commit/manifest.py index 2b4614c7..888ad6dd 100644 --- a/pre_commit/manifest.py +++ b/pre_commit/manifest.py @@ -6,7 +6,7 @@ import os.path from cached_property import cached_property import pre_commit.constants as C -from pre_commit.clientlib.validate_manifest import load_manifest +from pre_commit.clientlib import load_manifest logger = logging.getLogger('pre_commit') diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 3bdeba9b..2c1eedb3 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -13,13 +13,14 @@ from cached_property import cached_property import pre_commit.constants as C from pre_commit import five from pre_commit import git -from pre_commit.clientlib.validate_config import is_local_hooks -from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA -from pre_commit.jsonschema_extensions import apply_defaults +from pre_commit.clientlib import is_local_repo +from pre_commit.clientlib import MANIFEST_HOOK_DICT from pre_commit.languages.all import languages from pre_commit.languages.helpers import environment_dir from pre_commit.manifest import Manifest from pre_commit.prefixed_command_runner import PrefixedCommandRunner +from pre_commit.schema import apply_defaults +from pre_commit.schema import validate logger = logging.getLogger('pre_commit') @@ -115,7 +116,7 @@ class Repository(object): @classmethod def create(cls, config, store): - if is_local_hooks(config): + if is_local_repo(config): return LocalRepository(config, store) else: return cls(config, store) @@ -162,7 +163,7 @@ class Repository(object): deps_dict = defaultdict(_UniqueList) for _, hook in self.hooks: deps_dict[(hook['language'], hook['language_version'])].update( - hook.get('additional_dependencies', []), + hook['additional_dependencies'], ) ret = [] for (language, version), deps in deps_dict.items(): @@ -182,7 +183,7 @@ class Repository(object): """ self.require_installed() language_name = hook['language'] - deps = hook.get('additional_dependencies', []) + deps = hook['additional_dependencies'] cmd_runner = self._cmd_runner_from_deps(language_name, deps) return languages[language_name].run_hook(cmd_runner, hook, file_args) @@ -207,9 +208,12 @@ class LocalRepository(Repository): return tuple( ( hook['id'], - _validate_minimum_version(apply_defaults( - hook, MANIFEST_JSON_SCHEMA['items'], - )), + _validate_minimum_version( + apply_defaults( + validate(hook, MANIFEST_HOOK_DICT), + MANIFEST_HOOK_DICT, + ), + ), ) for hook in self.repo_config['hooks'] ) @@ -220,7 +224,7 @@ class LocalRepository(Repository): for _, hook in self.hooks: language = hook['language'] version = hook['language_version'] - deps = hook.get('additional_dependencies', []) + deps = hook['additional_dependencies'] ret.append(( self._cmd_runner_from_deps(language, deps), language, version, deps, diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 985e6456..c7455d71 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -5,7 +5,7 @@ import os.path from cached_property import cached_property from pre_commit import git -from pre_commit.clientlib.validate_config import load_config +from pre_commit.clientlib import load_config from pre_commit.repository import Repository from pre_commit.store import Store diff --git a/pre_commit/schema.py b/pre_commit/schema.py new file mode 100644 index 00000000..60da2530 --- /dev/null +++ b/pre_commit/schema.py @@ -0,0 +1,279 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +import collections +import contextlib +import io +import os.path +import re +import sys + +from pre_commit import five + + +class ValidationError(ValueError): + def __init__(self, error_msg, ctx=None): + super(ValidationError, self).__init__(error_msg) + self.error_msg = error_msg + self.ctx = ctx + + def __str__(self): + out = '\n' + err = self + while err.ctx is not None: + out += '==> {}\n'.format(err.ctx) + err = err.error_msg + out += '=====> {}'.format(err.error_msg) + return out + + +MISSING = collections.namedtuple('Missing', ())() +type(MISSING).__repr__ = lambda self: 'MISSING' + + +@contextlib.contextmanager +def validate_context(msg): + try: + yield + except ValidationError as e: + _, _, tb = sys.exc_info() + five.reraise(ValidationError, ValidationError(e, ctx=msg), tb) + + +@contextlib.contextmanager +def reraise_as(tp): + try: + yield + except ValidationError as e: + _, _, tb = sys.exc_info() + five.reraise(tp, tp(e), tb) + + +def _dct_noop(self, dct): + pass + + +def _check_optional(self, dct): + if self.key not in dct: + return + with validate_context('At key: {}'.format(self.key)): + self.check_fn(dct[self.key]) + + +def _apply_default_optional(self, dct): + dct.setdefault(self.key, self.default) + + +def _remove_default_optional(self, dct): + if dct.get(self.key, MISSING) == self.default: + del dct[self.key] + + +def _require_key(self, dct): + if self.key not in dct: + raise ValidationError('Missing required key: {}'.format(self.key)) + + +def _check_required(self, dct): + _require_key(self, dct) + _check_optional(self, dct) + + +@property +def _check_fn_required_recurse(self): + def check_fn(val): + validate(val, self.schema) + return check_fn + + +def _apply_default_required_recurse(self, dct): + dct[self.key] = apply_defaults(dct[self.key], self.schema) + + +def _remove_default_required_recurse(self, dct): + dct[self.key] = remove_defaults(dct[self.key], self.schema) + + +def _check_conditional(self, dct): + if dct.get(self.condition_key, MISSING) == self.condition_value: + _check_required(self, dct) + elif self.condition_key in dct and self.ensure_absent and self.key in dct: + if isinstance(self.condition_value, Not): + op = 'is' + cond_val = self.condition_value.val + else: + op = 'is not' + cond_val = self.condition_value + raise ValidationError( + 'Expected {key} to be absent when {cond_key} {op} {cond_val!r}, ' + 'found {key}: {val!r}'.format( + key=self.key, + val=dct[self.key], + cond_key=self.condition_key, + op=op, + cond_val=cond_val, + ) + ) + + +Required = collections.namedtuple('Required', ('key', 'check_fn')) +Required.check = _check_required +Required.apply_default = _dct_noop +Required.remove_default = _dct_noop +RequiredRecurse = collections.namedtuple('RequiredRecurse', ('key', 'schema')) +RequiredRecurse.check = _check_required +RequiredRecurse.check_fn = _check_fn_required_recurse +RequiredRecurse.apply_default = _apply_default_required_recurse +RequiredRecurse.remove_default = _remove_default_required_recurse +Optional = collections.namedtuple('Optional', ('key', 'check_fn', 'default')) +Optional.check = _check_optional +Optional.apply_default = _apply_default_optional +Optional.remove_default = _remove_default_optional +OptionalNoDefault = collections.namedtuple( + 'OptionalNoDefault', ('key', 'check_fn'), +) +OptionalNoDefault.check = _check_optional +OptionalNoDefault.apply_default = _dct_noop +OptionalNoDefault.remove_default = _dct_noop +Conditional = collections.namedtuple( + 'Conditional', + ('key', 'check_fn', 'condition_key', 'condition_value', 'ensure_absent'), +) +Conditional.__new__.__defaults__ = (False,) +Conditional.check = _check_conditional +Conditional.apply_default = _dct_noop +Conditional.remove_default = _dct_noop + + +class Map(collections.namedtuple('Map', ('object_name', 'id_key', 'items'))): + __slots__ = () + + def __new__(cls, object_name, id_key, *items): + return super(Map, cls).__new__(cls, object_name, id_key, items) + + def check(self, v): + if not isinstance(v, dict): + raise ValidationError('Expected a {} map but got a {}'.format( + self.object_name, type(v).__name__, + )) + with validate_context('At {}({}={!r})'.format( + self.object_name, self.id_key, v.get(self.id_key, MISSING), + )): + for item in self.items: + item.check(v) + + def apply_defaults(self, v): + ret = v.copy() + for item in self.items: + item.apply_default(ret) + return ret + + def remove_defaults(self, v): + ret = v.copy() + for item in self.items: + item.remove_default(ret) + return ret + + +class Array(collections.namedtuple('Array', ('of',))): + __slots__ = () + + def check(self, v): + check_array(check_any)(v) + if not v: + raise ValidationError( + "Expected at least 1 '{}'".format(self.of.object_name), + ) + for val in v: + validate(val, self.of) + + def apply_defaults(self, v): + return [apply_defaults(val, self.of) for val in v] + + def remove_defaults(self, v): + return [remove_defaults(val, self.of) for val in v] + + +class Not(object): + def __init__(self, val): + self.val = val + + def __eq__(self, other): + return other is not MISSING and other != self.val + + +def check_any(_): + pass + + +def check_type(tp, typename=None): + def check_type_fn(v): + if not isinstance(v, tp): + raise ValidationError( + 'Expected {} got {}'.format( + typename or tp.__name__, type(v).__name__, + ), + ) + return check_type_fn + + +check_bool = check_type(bool) +check_string = check_type(five.string_types, typename='string') + + +def check_regex(v): + try: + re.compile(v) + except re.error: + raise ValidationError('{!r} is not a valid python regex'.format(v)) + + +def check_array(inner_check): + def check_array_fn(v): + if not isinstance(v, (list, tuple)): + raise ValidationError( + 'Expected array but got {!r}'.format(type(v).__name__), + ) + + for i, val in enumerate(v): + with validate_context('At index {}'.format(i)): + inner_check(val) + return check_array_fn + + +def check_and(*fns): + def check(v): + for fn in fns: + fn(v) + return check + + +def validate(v, schema): + schema.check(v) + return v + + +def apply_defaults(v, schema): + return schema.apply_defaults(v) + + +def remove_defaults(v, schema): + return schema.remove_defaults(v) + + +def load_from_filename(filename, schema, load_strategy, exc_tp): + with reraise_as(exc_tp): + if not os.path.exists(filename): + raise ValidationError('{} does not exist'.format(filename)) + + with io.open(filename) as f: + contents = f.read() + + with validate_context('File {}'.format(filename)): + try: + data = load_strategy(contents) + except Exception as e: + raise ValidationError(str(e)) + + validate(data, schema) + return apply_defaults(data, schema) diff --git a/setup.py b/setup.py index 0921a953..22eb717e 100644 --- a/setup.py +++ b/setup.py @@ -41,7 +41,6 @@ setup( install_requires=[ 'aspy.yaml', 'cached-property', - 'jsonschema', 'nodeenv>=0.11.1', 'pyyaml', 'virtualenv', @@ -49,8 +48,8 @@ setup( entry_points={ 'console_scripts': [ 'pre-commit = pre_commit.main:main', - 'pre-commit-validate-config = pre_commit.clientlib.validate_config:run', # noqa - 'pre-commit-validate-manifest = pre_commit.clientlib.validate_manifest:run', # noqa + 'pre-commit-validate-config = pre_commit.clientlib:validate_config_main', # noqa + 'pre-commit-validate-manifest = pre_commit.clientlib:validate_manifest_main', # noqa ], }, ) diff --git a/testing/fixtures.py b/testing/fixtures.py index 599558b8..dffff4ca 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -10,10 +10,10 @@ from aspy.yaml import ordered_dump from aspy.yaml import ordered_load import pre_commit.constants as C -from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA -from pre_commit.clientlib.validate_config import validate_config_extra -from pre_commit.clientlib.validate_manifest import load_manifest -from pre_commit.jsonschema_extensions import apply_defaults +from pre_commit.clientlib import CONFIG_SCHEMA +from pre_commit.clientlib import load_manifest +from pre_commit.schema import apply_defaults +from pre_commit.schema import validate from pre_commit.util import cmd_output from pre_commit.util import copy_tree_to_path from pre_commit.util import cwd @@ -94,9 +94,9 @@ def make_config_from_repo( )) if check: - wrapped_config = apply_defaults([config], CONFIG_JSON_SCHEMA) - validate_config_extra(wrapped_config) - return wrapped_config[0] + wrapped = validate([config], CONFIG_SCHEMA) + config, = apply_defaults(wrapped, CONFIG_SCHEMA) + return config else: return config diff --git a/testing/resources/array_yaml_file.yaml b/testing/resources/array_yaml_file.yaml deleted file mode 100644 index 59121da8..00000000 --- a/testing/resources/array_yaml_file.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- foo -- bar diff --git a/testing/resources/non_parseable_yaml_file.notyaml b/testing/resources/non_parseable_yaml_file.notyaml deleted file mode 100644 index ea185188..00000000 --- a/testing/resources/non_parseable_yaml_file.notyaml +++ /dev/null @@ -1 +0,0 @@ -foo: " diff --git a/testing/resources/ordering_data_test.yaml b/testing/resources/ordering_data_test.yaml deleted file mode 100644 index 3d606686..00000000 --- a/testing/resources/ordering_data_test.yaml +++ /dev/null @@ -1,2 +0,0 @@ -foo: bar -bar: baz diff --git a/testing/util.py b/testing/util.py index 4cb6f0d8..4d752f3e 100644 --- a/testing/util.py +++ b/testing/util.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals import os.path -import jsonschema import pytest from pre_commit import parse_shebang @@ -24,14 +23,6 @@ def get_head_sha(dir): return cmd_output('git', 'rev-parse', 'HEAD')[1].strip() -def is_valid_according_to_schema(obj, schema): - try: - jsonschema.validate(obj, schema) - return True - except jsonschema.exceptions.ValidationError: - return False - - def cmd_output_mocked_pre_commit_home(*args, **kwargs): # keyword-only argument tempdir_factory = kwargs.pop('tempdir_factory') diff --git a/tests/clientlib/__init__.py b/tests/clientlib/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/clientlib/validate_base_test.py b/tests/clientlib/validate_base_test.py deleted file mode 100644 index 7cbcada2..00000000 --- a/tests/clientlib/validate_base_test.py +++ /dev/null @@ -1,74 +0,0 @@ -from __future__ import unicode_literals - -from collections import OrderedDict - -import pytest -from aspy.yaml import ordered_load - -from pre_commit.clientlib.validate_base import get_validator -from testing.util import get_resource_path - - -class AdditionalValidatorError(ValueError): - pass - - -@pytest.fixture -def noop_validator(): - return get_validator({}, ValueError) - - -@pytest.fixture -def array_validator(): - return get_validator({'type': 'array'}, ValueError) - - -@pytest.fixture -def additional_validator(): - def raises_always(_): - raise AdditionalValidatorError - - return get_validator( - {}, - ValueError, - additional_validation_strategy=raises_always, - ) - - -def test_raises_for_non_existent_file(noop_validator): - with pytest.raises(ValueError): - noop_validator('file_that_does_not_exist.yaml') - - -def test_raises_for_invalid_yaml_file(noop_validator): - with pytest.raises(ValueError): - noop_validator(get_resource_path('non_parseable_yaml_file.notyaml')) - - -def test_raises_for_failing_schema(array_validator): - with pytest.raises(ValueError): - array_validator( - get_resource_path('valid_yaml_but_invalid_manifest.yaml') - ) - - -def test_passes_array_schema(array_validator): - array_validator(get_resource_path('array_yaml_file.yaml')) - - -def test_raises_when_additional_validation_fails(additional_validator): - with pytest.raises(AdditionalValidatorError): - additional_validator(get_resource_path('array_yaml_file.yaml')) - - -def test_returns_object_after_validating(noop_validator): - ret = noop_validator(get_resource_path('array_yaml_file.yaml')) - assert ret == ['foo', 'bar'] - - -def test_load_strategy(noop_validator): - ret = noop_validator( - get_resource_path('ordering_data_test.yaml'), - load_strategy=ordered_load, - ) - assert type(ret) is OrderedDict diff --git a/tests/clientlib/validate_config_test.py b/tests/clientlib/validate_config_test.py deleted file mode 100644 index d3e0a737..00000000 --- a/tests/clientlib/validate_config_test.py +++ /dev/null @@ -1,195 +0,0 @@ -from __future__ import unicode_literals - -import jsonschema -import pytest - -from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA -from pre_commit.clientlib.validate_config import InvalidConfigError -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 get_resource_path -from testing.util import is_valid_according_to_schema - - -@pytest.mark.parametrize( - ('input', 'expected_output'), - ( - (['.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(('config_obj', 'expected'), ( - ([], False), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}], - }], - True, - ), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '\\.py$', - 'args': ['foo', 'bar', 'baz'], - }, - ], - }], - True, - ), - ( - [{ - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', - 'hooks': [ - { - 'id': 'pyflakes', - 'files': '\\.py$', - # Exclude pattern must be a string - 'exclude': 0, - 'args': ['foo', 'bar', 'baz'], - }, - ], - }], - False, - ), -)) -def test_is_valid_according_to_schema(config_obj, expected): - ret = is_valid_according_to_schema(config_obj, CONFIG_JSON_SCHEMA) - assert ret is expected - - -def test_config_with_failing_regexes_fails(): - with pytest.raises(InvalidConfigError): - # Note the regex '(' is invalid (unbalanced parens) - config = apply_defaults( - [{ - 'repo': 'foo', - 'sha': 'foo', - 'hooks': [{'id': 'hook_id', 'files': '('}], - }], - CONFIG_JSON_SCHEMA, - ) - validate_config_extra(config) - - -def test_config_with_ok_regexes_passes(): - config = apply_defaults( - [{ - 'repo': 'foo', - 'sha': 'foo', - 'hooks': [{'id': 'hook_id', 'files': '\\.py$'}], - }], - CONFIG_JSON_SCHEMA, - ) - validate_config_extra(config) - - -def test_config_with_invalid_exclude_regex_fails(): - with pytest.raises(InvalidConfigError): - # Note the regex '(' is invalid (unbalanced parens) - config = apply_defaults( - [{ - 'repo': 'foo', - 'sha': 'foo', - 'hooks': [{'id': 'hook_id', 'files': '', 'exclude': '('}], - }], - CONFIG_JSON_SCHEMA, - ) - validate_config_extra(config) - - -def test_config_with_ok_exclude_regex_passes(): - config = apply_defaults( - [{ - 'repo': 'foo', - 'sha': 'foo', - 'hooks': [{'id': 'hook_id', 'files': '', 'exclude': '^vendor/'}], - }], - CONFIG_JSON_SCHEMA, - ) - validate_config_extra(config) - - -@pytest.mark.parametrize('config_obj', ( - [{ - 'repo': 'local', - 'sha': 'foo', - 'hooks': [{ - 'id': 'do_not_commit', - 'name': 'Block if "DO NOT COMMIT" is found', - 'entry': 'DO NOT COMMIT', - 'language': 'pcre', - 'files': '^(.*)$', - }], - }], -)) -def test_config_with_local_hooks_definition_fails(config_obj): - with pytest.raises(( - jsonschema.exceptions.ValidationError, InvalidConfigError - )): - jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA) - config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA) - validate_config_extra(config) - - -@pytest.mark.parametrize('config_obj', ( - [{ - 'repo': 'local', - 'hooks': [{ - 'id': 'arg-per-line', - 'name': 'Args per line hook', - 'entry': 'bin/hook.sh', - 'language': 'script', - 'files': '', - 'args': ['hello', 'world'], - }], - }], - [{ - 'repo': 'local', - 'hooks': [{ - 'id': 'arg-per-line', - 'name': 'Args per line hook', - 'entry': 'bin/hook.sh', - 'language': 'script', - 'files': '', - 'args': ['hello', 'world'], - }] - }], -)) -def test_config_with_local_hooks_definition_passes(config_obj): - jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA) - config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA) - validate_config_extra(config) - - -def test_does_not_contain_defaults(): - """Due to the way our merging works, if this schema has any defaults they - will clobber potentially useful values in the backing manifest. #227 - """ - to_process = [(CONFIG_JSON_SCHEMA, ())] - while to_process: - schema, route = to_process.pop() - # Check this value - if isinstance(schema, dict): - if 'default' in schema: - raise AssertionError( - 'Unexpected default in schema at {}'.format( - ' => '.join(route), - ) - ) - - for key, value in schema.items(): - to_process.append((value, route + (key,))) diff --git a/tests/clientlib/validate_manifest_test.py b/tests/clientlib/validate_manifest_test.py deleted file mode 100644 index 97cfd6b0..00000000 --- a/tests/clientlib/validate_manifest_test.py +++ /dev/null @@ -1,87 +0,0 @@ -from __future__ import unicode_literals - -import pytest - -from pre_commit.clientlib.validate_manifest import additional_manifest_check -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 get_resource_path -from testing.util import is_valid_according_to_schema - - -@pytest.mark.parametrize( - ('input', 'expected_output'), - ( - (['.pre-commit-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(): - with pytest.raises(InvalidManifestError): - additional_manifest_check([{'id': 'foo', 'language': 'not valid'}]) - - -@pytest.mark.parametrize( - 'obj', - ( - [{'language': 'python', 'files': ''}], - [{'language': 'ruby', 'files': ''}] - ), -) -def test_additional_manifest_check_passing(obj): - additional_manifest_check(obj) - - -@pytest.mark.parametrize( - 'obj', - ( - [{'id': 'a', 'language': 'not a language', 'files': ''}], - [{'id': 'a', 'language': 'python3', 'files': ''}], - [{'id': 'a', 'language': 'python', 'files': 'invalid regex('}], - [{'id': 'a', 'language': 'not a language', 'files': ''}], - [{'id': 'a', 'language': 'python3', 'files': ''}], - [{'id': 'a', 'language': 'python', 'files': '', 'exclude': '('}], - ), -) -def test_additional_manifest_failing(obj): - with pytest.raises(InvalidManifestError): - additional_manifest_check(obj) - - -@pytest.mark.parametrize( - ('manifest_obj', 'expected'), - ( - ([], False), - ( - [{ - 'id': 'a', - 'name': 'b', - 'entry': 'c', - 'language': 'python', - 'files': r'\.py$' - }], - True, - ), - ( - [{ - 'id': 'a', - 'name': 'b', - 'entry': 'c', - 'language': 'python', - 'language_version': 'python3.4', - 'files': r'\.py$', - }], - True, - ), - ) -) -def test_is_valid_according_to_schema(manifest_obj, expected): - ret = is_valid_according_to_schema(manifest_obj, MANIFEST_JSON_SCHEMA) - assert ret is expected diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py new file mode 100644 index 00000000..deaa7e9e --- /dev/null +++ b/tests/clientlib_test.py @@ -0,0 +1,194 @@ +from __future__ import unicode_literals + +import pytest + +from pre_commit import schema +from pre_commit.clientlib import check_language +from pre_commit.clientlib import CONFIG_HOOK_DICT +from pre_commit.clientlib import CONFIG_SCHEMA +from pre_commit.clientlib import is_local_repo +from pre_commit.clientlib import MANIFEST_SCHEMA +from pre_commit.clientlib import validate_config_main +from pre_commit.clientlib import validate_manifest_main +from testing.util import get_resource_path + + +def is_valid_according_to_schema(obj, obj_schema): + try: + schema.validate(obj, obj_schema) + return True + except schema.ValidationError: + return False + + +@pytest.mark.parametrize('value', ('not a language', 'python3')) +def test_check_language_failures(value): + with pytest.raises(schema.ValidationError): + check_language(value) + + +@pytest.mark.parametrize('value', ('python', 'node', 'pcre')) +def test_check_language_ok(value): + check_language(value) + + +def test_is_local_repo(): + assert is_local_repo({'repo': 'local'}) + + +@pytest.mark.parametrize( + ('args', 'expected_output'), + ( + (['.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_validate_config_main(args, expected_output): + assert validate_config_main(args) == expected_output + + +@pytest.mark.parametrize(('config_obj', 'expected'), ( + ([], False), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}], + }], + True, + ), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\\.py$', + 'args': ['foo', 'bar', 'baz'], + }, + ], + }], + True, + ), + ( + [{ + 'repo': 'git@github.com:pre-commit/pre-commit-hooks', + 'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37', + 'hooks': [ + { + 'id': 'pyflakes', + 'files': '\\.py$', + # Exclude pattern must be a string + 'exclude': 0, + 'args': ['foo', 'bar', 'baz'], + }, + ], + }], + False, + ), +)) +def test_config_valid(config_obj, expected): + ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA) + assert ret is expected + + +@pytest.mark.parametrize('config_obj', ( + [{ + 'repo': 'local', + 'sha': 'foo', + 'hooks': [{ + 'id': 'do_not_commit', + 'name': 'Block if "DO NOT COMMIT" is found', + 'entry': 'DO NOT COMMIT', + 'language': 'pcre', + 'files': '^(.*)$', + }], + }], +)) +def test_config_with_local_hooks_definition_fails(config_obj): + with pytest.raises(schema.ValidationError): + schema.validate(config_obj, CONFIG_SCHEMA) + + +@pytest.mark.parametrize('config_obj', ( + [{ + 'repo': 'local', + 'hooks': [{ + 'id': 'arg-per-line', + 'name': 'Args per line hook', + 'entry': 'bin/hook.sh', + 'language': 'script', + 'files': '', + 'args': ['hello', 'world'], + }], + }], + [{ + 'repo': 'local', + 'hooks': [{ + 'id': 'arg-per-line', + 'name': 'Args per line hook', + 'entry': 'bin/hook.sh', + 'language': 'script', + 'files': '', + 'args': ['hello', 'world'], + }] + }], +)) +def test_config_with_local_hooks_definition_passes(config_obj): + schema.validate(config_obj, CONFIG_SCHEMA) + + +def test_config_schema_does_not_contain_defaults(): + """Due to the way our merging works, if this schema has any defaults they + will clobber potentially useful values in the backing manifest. #227 + """ + for item in CONFIG_HOOK_DICT.items: + assert not isinstance(item, schema.Optional) + + +@pytest.mark.parametrize( + ('args', 'expected_output'), + ( + (['.pre-commit-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_validate_manifest_main(args, expected_output): + assert validate_manifest_main(args) == expected_output + + +@pytest.mark.parametrize( + ('manifest_obj', 'expected'), + ( + ([], False), + ( + [{ + 'id': 'a', + 'name': 'b', + 'entry': 'c', + 'language': 'python', + 'files': r'\.py$' + }], + True, + ), + ( + [{ + 'id': 'a', + 'name': 'b', + 'entry': 'c', + 'language': 'python', + 'language_version': 'python3.4', + 'files': r'\.py$', + }], + True, + ), + ) +) +def test_valid_manifests(manifest_obj, expected): + ret = is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA) + assert ret is expected diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py index b5858ff0..550946b6 100644 --- a/tests/commands/autoupdate_test.py +++ b/tests/commands/autoupdate_test.py @@ -6,7 +6,7 @@ from collections import OrderedDict import pytest import pre_commit.constants as C -from pre_commit.clientlib.validate_config import load_config +from pre_commit.clientlib import load_config from pre_commit.commands.autoupdate import _update_repo from pre_commit.commands.autoupdate import autoupdate from pre_commit.commands.autoupdate import RepositoryCannotBeUpdatedError diff --git a/tests/jsonschema_extensions_test.py b/tests/jsonschema_extensions_test.py deleted file mode 100644 index 65948564..00000000 --- a/tests/jsonschema_extensions_test.py +++ /dev/null @@ -1,91 +0,0 @@ -from __future__ import unicode_literals - -import jsonschema.exceptions -import pytest - -from pre_commit.jsonschema_extensions import apply_defaults -from pre_commit.jsonschema_extensions import remove_defaults - - -def test_apply_defaults_copies_object(): - input = {} - ret = apply_defaults(input, {}) - assert ret is not input - - -def test_apply_default_does_not_touch_schema_without_defaults(): - ret = apply_defaults( - {'foo': 'bar'}, - {'type': 'object', 'properties': {'foo': {}, 'baz': {}}}, - ) - assert ret == {'foo': 'bar'} - - -def test_apply_defaults_applies_defaults(): - ret = apply_defaults( - {'foo': 'bar'}, - { - 'type': 'object', - 'properties': { - 'foo': {'default': 'biz'}, - 'baz': {'default': 'herp'}, - } - } - ) - assert ret == {'foo': 'bar', 'baz': 'herp'} - - -def test_apply_defaults_deep(): - ret = apply_defaults( - {'foo': {'bar': {}}}, - { - 'type': 'object', - 'properties': { - 'foo': { - 'type': 'object', - 'properties': { - 'bar': { - 'type': 'object', - 'properties': {'baz': {'default': 'herp'}}, - }, - }, - }, - }, - }, - ) - assert ret == {'foo': {'bar': {'baz': 'herp'}}} - - -def test_apply_defaults_copies(): - schema = {'properties': {'foo': {'default': []}}} - ret1 = apply_defaults({}, schema) - ret2 = apply_defaults({}, schema) - assert ret1['foo'] is not ret2['foo'] - - -def test_remove_defaults_copies_object(): - input = {} - ret = remove_defaults(input, {}) - assert ret is not input - - -def test_remove_defaults_does_not_remove_non_default(): - ret = remove_defaults( - {'foo': 'bar'}, - {'properties': {'foo': {'default': 'baz'}}}, - ) - assert ret == {'foo': 'bar'} - - -def test_remove_defaults_removes_default(): - ret = remove_defaults( - {'foo': 'bar'}, - {'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']}) diff --git a/tests/manifest_test.py b/tests/manifest_test.py index 58242214..658210da 100644 --- a/tests/manifest_test.py +++ b/tests/manifest_test.py @@ -20,6 +20,7 @@ def test_manifest_contents(manifest): # Should just retrieve the manifest contents assert manifest.manifest_contents == [{ 'always_run': False, + 'additional_dependencies': [], 'args': [], 'description': '', 'entry': 'bin/hook.sh', @@ -28,7 +29,7 @@ def test_manifest_contents(manifest): 'id': 'bash_hook', 'language': 'script', 'language_version': 'default', - 'minimum_pre_commit_version': '0.0.0', + 'minimum_pre_commit_version': '0', 'name': 'Bash hook', 'stages': [], }] @@ -37,6 +38,7 @@ def test_manifest_contents(manifest): def test_hooks(manifest): assert manifest.hooks['bash_hook'] == { 'always_run': False, + 'additional_dependencies': [], 'args': [], 'description': '', 'entry': 'bin/hook.sh', @@ -45,7 +47,7 @@ def test_hooks(manifest): 'id': 'bash_hook', 'language': 'script', 'language_version': 'default', - 'minimum_pre_commit_version': '0.0.0', + 'minimum_pre_commit_version': '0', 'name': 'Bash hook', 'stages': [], } diff --git a/tests/repository_test.py b/tests/repository_test.py index 653b1a10..4cb2e4ea 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -13,7 +13,7 @@ import pytest import pre_commit.constants as C from pre_commit import five from pre_commit import parse_shebang -from pre_commit.clientlib.validate_manifest import load_manifest +from pre_commit.clientlib import load_manifest from pre_commit.languages import golang from pre_commit.languages import helpers from pre_commit.languages import node diff --git a/tests/schema_test.py b/tests/schema_test.py new file mode 100644 index 00000000..914e6097 --- /dev/null +++ b/tests/schema_test.py @@ -0,0 +1,391 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +import json + +import mock +import pytest + +from pre_commit.schema import apply_defaults +from pre_commit.schema import Array +from pre_commit.schema import check_and +from pre_commit.schema import check_any +from pre_commit.schema import check_array +from pre_commit.schema import check_bool +from pre_commit.schema import check_regex +from pre_commit.schema import check_type +from pre_commit.schema import Conditional +from pre_commit.schema import load_from_filename +from pre_commit.schema import Map +from pre_commit.schema import MISSING +from pre_commit.schema import Not +from pre_commit.schema import Optional +from pre_commit.schema import OptionalNoDefault +from pre_commit.schema import remove_defaults +from pre_commit.schema import Required +from pre_commit.schema import RequiredRecurse +from pre_commit.schema import validate +from pre_commit.schema import ValidationError + + +def _assert_exception_trace(e, trace): + inner = e + for ctx in trace[:-1]: + assert inner.ctx == ctx + inner = inner.error_msg + assert inner.error_msg == trace[-1] + + +def test_ValidationError_simple_str(): + assert str(ValidationError('error msg')) == ( + '\n' + '=====> error msg' + ) + + +def test_ValidationError_nested(): + error = ValidationError( + ValidationError( + ValidationError('error msg'), + ctx='At line 1', + ), + ctx='In file foo', + ) + assert str(error) == ( + '\n' + '==> In file foo\n' + '==> At line 1\n' + '=====> error msg' + ) + + +def test_check_regex(): + with pytest.raises(ValidationError) as excinfo: + check_regex(str('(')) + assert excinfo.value.error_msg == "'(' is not a valid python regex" + + +def test_check_regex_ok(): + check_regex('^$') + + +def test_check_array_failed_inner_check(): + check = check_array(check_bool) + with pytest.raises(ValidationError) as excinfo: + check([True, False, 5]) + _assert_exception_trace( + excinfo.value, ('At index 2', 'Expected bool got int'), + ) + + +def test_check_array_ok(): + check_array(check_bool)([True, False]) + + +def test_check_and(): + check = check_and(check_type(str), check_regex) + with pytest.raises(ValidationError) as excinfo: + check(True) + assert excinfo.value.error_msg == 'Expected str got bool' + with pytest.raises(ValidationError) as excinfo: + check(str('(')) + assert excinfo.value.error_msg == "'(' is not a valid python regex" + + +def test_check_and_ok(): + check = check_and(check_type(str), check_regex) + check(str('^$')) + + +@pytest.mark.parametrize( + ('val', 'expected'), + (('bar', True), ('foo', False), (MISSING, False)), +) +def test_not(val, expected): + compared = Not('foo') + assert (val == compared) is expected + assert (compared == val) is expected + + +trivial_array_schema = Array(Map('foo', 'id')) + + +def test_validate_top_level_array_not_an_array(): + with pytest.raises(ValidationError) as excinfo: + validate({}, trivial_array_schema) + assert excinfo.value.error_msg == "Expected array but got 'dict'" + + +def test_validate_top_level_array_no_objects(): + with pytest.raises(ValidationError) as excinfo: + validate([], trivial_array_schema) + assert excinfo.value.error_msg == "Expected at least 1 'foo'" + + +@pytest.mark.parametrize('v', (({},), [{}])) +def test_ok_both_types(v): + validate(v, trivial_array_schema) + + +map_required = Map('foo', 'key', Required('key', check_bool)) +map_optional = Map('foo', 'key', Optional('key', check_bool, False)) +map_no_default = Map('foo', 'key', OptionalNoDefault('key', check_bool)) + + +def test_map_wrong_type(): + with pytest.raises(ValidationError) as excinfo: + validate([], map_required) + assert excinfo.value.error_msg == 'Expected a foo map but got a list' + + +def test_required_missing_key(): + with pytest.raises(ValidationError) as excinfo: + validate({}, map_required) + _assert_exception_trace( + excinfo.value, ('At foo(key=MISSING)', 'Missing required key: key'), + ) + + +@pytest.mark.parametrize( + 'schema', (map_required, map_optional, map_no_default), +) +def test_map_value_wrong_type(schema): + with pytest.raises(ValidationError) as excinfo: + validate({'key': 5}, schema) + _assert_exception_trace( + excinfo.value, + ('At foo(key=5)', 'At key: key', 'Expected bool got int'), + ) + + +@pytest.mark.parametrize( + 'schema', (map_required, map_optional, map_no_default), +) +def test_map_value_correct_type(schema): + validate({'key': True}, schema) + + +@pytest.mark.parametrize('schema', (map_optional, map_no_default)) +def test_optional_key_missing(schema): + validate({}, schema) + + +map_conditional = Map( + 'foo', 'key', + Conditional( + 'key2', check_bool, condition_key='key', condition_value=True, + ), +) +map_conditional_not = Map( + 'foo', 'key', + Conditional( + 'key2', check_bool, condition_key='key', condition_value=Not(False), + ), +) +map_conditional_absent = Map( + 'foo', 'key', + Conditional( + 'key2', check_bool, + condition_key='key', condition_value=True, ensure_absent=True, + ), +) +map_conditional_absent_not = Map( + 'foo', 'key', + Conditional( + 'key2', check_bool, + condition_key='key', condition_value=Not(True), ensure_absent=True, + ), +) + + +@pytest.mark.parametrize('schema', (map_conditional, map_conditional_not)) +@pytest.mark.parametrize( + 'v', + ( + # Conditional check passes, key2 is checked and passes + {'key': True, 'key2': True}, + # Conditional check fails, key2 is not checked + {'key': False, 'key2': 'ohai'}, + ), +) +def test_ok_conditional_schemas(v, schema): + validate(v, schema) + + +@pytest.mark.parametrize('schema', (map_conditional, map_conditional_not)) +def test_not_ok_conditional_schemas(schema): + with pytest.raises(ValidationError) as excinfo: + validate({'key': True, 'key2': 5}, schema) + _assert_exception_trace( + excinfo.value, + ('At foo(key=True)', 'At key: key2', 'Expected bool got int'), + ) + + +def test_ensure_absent_conditional(): + with pytest.raises(ValidationError) as excinfo: + validate({'key': False, 'key2': True}, map_conditional_absent) + _assert_exception_trace( + excinfo.value, + ( + 'At foo(key=False)', + 'Expected key2 to be absent when key is not True, ' + 'found key2: True', + ), + ) + + +def test_ensure_absent_conditional_not(): + with pytest.raises(ValidationError) as excinfo: + validate({'key': True, 'key2': True}, map_conditional_absent_not) + _assert_exception_trace( + excinfo.value, + ( + 'At foo(key=True)', + 'Expected key2 to be absent when key is True, ' + 'found key2: True', + ), + ) + + +def test_no_error_conditional_absent(): + validate({}, map_conditional_absent) + validate({}, map_conditional_absent_not) + validate({'key2': True}, map_conditional_absent) + validate({'key2': True}, map_conditional_absent_not) + + +def test_apply_defaults_copies_object(): + val = {} + ret = apply_defaults(val, map_optional) + assert ret is not val + + +def test_apply_defaults_sets_default(): + ret = apply_defaults({}, map_optional) + assert ret == {'key': False} + + +def test_apply_defaults_does_not_change_non_default(): + ret = apply_defaults({'key': True}, map_optional) + assert ret == {'key': True} + + +def test_apply_defaults_does_nothing_on_non_optional(): + ret = apply_defaults({}, map_required) + assert ret == {} + + +def test_apply_defaults_map_in_list(): + ret = apply_defaults([{}], Array(map_optional)) + assert ret == [{'key': False}] + + +def test_remove_defaults_copies_object(): + val = {'key': False} + ret = remove_defaults(val, map_optional) + assert ret is not val + + +def test_remove_defaults_removes_defaults(): + ret = remove_defaults({'key': False}, map_optional) + assert ret == {} + + +def test_remove_defaults_nothing_to_remove(): + ret = remove_defaults({}, map_optional) + assert ret == {} + + +def test_remove_defaults_does_not_change_non_default(): + ret = remove_defaults({'key': True}, map_optional) + assert ret == {'key': True} + + +def test_remove_defaults_map_in_list(): + ret = remove_defaults([{'key': False}], Array(map_optional)) + assert ret == [{}] + + +def test_remove_defaults_does_nothing_on_non_optional(): + ret = remove_defaults({'key': True}, map_required) + assert ret == {'key': True} + + +nested_schema_required = Map( + 'Repository', 'repo', + Required('repo', check_any), + RequiredRecurse('hooks', Array(map_required)), +) +nested_schema_optional = Map( + 'Repository', 'repo', + Required('repo', check_any), + RequiredRecurse('hooks', Array(map_optional)), +) + + +def test_validate_failure_nested(): + with pytest.raises(ValidationError) as excinfo: + validate({'repo': 1, 'hooks': [{}]}, nested_schema_required) + _assert_exception_trace( + excinfo.value, + ( + 'At Repository(repo=1)', 'At key: hooks', 'At foo(key=MISSING)', + 'Missing required key: key', + ), + ) + + +def test_apply_defaults_nested(): + val = {'repo': 'repo1', 'hooks': [{}]} + ret = apply_defaults(val, nested_schema_optional) + assert ret == {'repo': 'repo1', 'hooks': [{'key': False}]} + + +def test_remove_defaults_nested(): + val = {'repo': 'repo1', 'hooks': [{'key': False}]} + ret = remove_defaults(val, nested_schema_optional) + assert ret == {'repo': 'repo1', 'hooks': [{}]} + + +class Error(Exception): + pass + + +def test_load_from_filename_file_does_not_exist(): + with pytest.raises(Error) as excinfo: + load_from_filename('does_not_exist', map_required, json.loads, Error) + assert excinfo.value.args[0].error_msg == 'does_not_exist does not exist' + + +def test_load_from_filename_fails_load_strategy(tmpdir): + f = tmpdir.join('foo.notjson') + f.write('totes not json') + with pytest.raises(Error) as excinfo: + load_from_filename(f.strpath, map_required, json.loads, Error) + _assert_exception_trace( + excinfo.value.args[0], + # ANY is json's error message + ('File {}'.format(f.strpath), mock.ANY) + ) + + +def test_load_from_filename_validation_error(tmpdir): + f = tmpdir.join('foo.json') + f.write('{}') + with pytest.raises(Error) as excinfo: + load_from_filename(f.strpath, map_required, json.loads, Error) + _assert_exception_trace( + excinfo.value.args[0], + ( + 'File {}'.format(f.strpath), 'At foo(key=MISSING)', + 'Missing required key: key', + ), + ) + + +def test_load_from_filename_applies_defaults(tmpdir): + f = tmpdir.join('foo.json') + f.write('{}') + ret = load_from_filename(f.strpath, map_optional, json.loads, Error) + assert ret == {'key': False} diff --git a/tox.ini b/tox.ini index 1efecce5..872b4c35 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,7 @@ commands = coverage erase coverage run -m pytest {posargs:tests} # TODO: change to 100 - coverage report --show-missing --fail-under 99 + coverage report --fail-under 99 pre-commit run --all-files [testenv:venv]