Merge pull request #231 from pre-commit/no_defaults_in_config_227

Remove defaults from pre-commit config schema.  Resolves #227
This commit is contained in:
Anthony Sottile
2015-05-18 15:14:29 -07:00
5 changed files with 36 additions and 4 deletions

View File

@@ -33,7 +33,7 @@ CONFIG_JSON_SCHEMA = {
'properties': {
'id': {'type': 'string'},
'files': {'type': 'string'},
'exclude': {'type': 'string', 'default': '^$'},
'exclude': {'type': 'string'},
'language_version': {'type': 'string'},
'args': {
'type': 'array',
@@ -71,7 +71,7 @@ def validate_config_extra(config):
)
for hook in repo['hooks']:
try_regex(repo, hook['id'], hook.get('files', ''), 'files')
try_regex(repo, hook['id'], hook['exclude'], 'exclude')
try_regex(repo, hook['id'], hook.get('exclude', ''), 'exclude')
load_config = get_validator(

View File

@@ -20,6 +20,7 @@ MANIFEST_JSON_SCHEMA = {
'name': {'type': 'string'},
'description': {'type': 'string', 'default': ''},
'entry': {'type': 'string'},
'exclude': {'type': 'string', 'default': '^$'},
'language': {'type': 'string'},
'language_version': {'type': 'string', 'default': 'default'},
'files': {'type': 'string'},
@@ -52,8 +53,14 @@ def validate_files(hook_config):
if not is_regex_valid(hook_config['files']):
raise InvalidManifestError(
'Invalid files regex at {0}: {1}'.format(
hook_config['id'],
hook_config['files'],
hook_config['id'], hook_config['files'],
)
)
if not is_regex_valid(hook_config.get('exclude', '')):
raise InvalidManifestError(
'Invalid exclude regex at {0}: {1}'.format(
hook_config['id'], hook_config['exclude'],
)
)

View File

@@ -174,3 +174,23 @@ 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 {0}'.format(
' => '.join(route),
)
)
for key, value in schema.items():
to_process.append((value, route + (key,)))

View File

@@ -46,6 +46,9 @@ def test_additional_manifest_check_passing(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):

View File

@@ -22,6 +22,7 @@ def test_manifest_contents(manifest):
'args': [],
'description': '',
'entry': 'bin/hook.sh',
'exclude': '^$',
'expected_return_value': 0,
'files': '',
'id': 'bash_hook',
@@ -36,6 +37,7 @@ def test_hooks(manifest):
'args': [],
'description': '',
'entry': 'bin/hook.sh',
'exclude': '^$',
'expected_return_value': 0,
'files': '',
'id': 'bash_hook',