Skip to content

Commit 3b0f03d

Browse files
struysstruys
committed
Merge branch 'master' of github.com:pre-commit/pre-commit
Conflicts: tests/git_test.py
2 parents 6498321 + fdf05b0 commit 3b0f03d

11 files changed

Lines changed: 236 additions & 153 deletions

‎example_manifest.yaml‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,29 @@
11

22
# Hooks are set up as follows
3-
# hooks:
4-
# -
5-
# id: hook_id
6-
# name: 'Readable name'
7-
# entry: my_hook_executable
3+
# -
4+
# id: hook_id
5+
# name: 'Readable name'
6+
# entry: my_hook_executable
87
#
9-
# # Optional
10-
# description: 'Longer description of the hook'
8+
# # Optional
9+
# description: 'Longer description of the hook'
1110
#
12-
# # Optional, for now 'python[optional version]', 'ruby #.#.#', 'node'
13-
# language: 'python'
11+
# # Optional, for now 'python[optional version]', 'ruby #.#.#', 'node'
12+
# language: 'python'
1413
#
15-
# # Optional, defaults to zero
16-
# expected_return_value: 0
14+
# # Optional, defaults to zero
15+
# expected_return_value: 0
1716

18-
hooks:
19-
-
20-
id: my_hook
21-
name: My Simple Hook
22-
description: This is my simple hook that does blah
23-
entry: my-simple-hook.py
24-
language: python
25-
expected_return_value: 0
26-
-
27-
id: my_grep_based_hook
28-
name: My Bash Based Hook
29-
description: This is a hook that uses grep to validate some stuff
30-
entry: ./my_grep_based_hook.sh
31-
expected_return_value: 1
17+
-
18+
id: my_hook
19+
name: My Simple Hook
20+
description: This is my simple hook that does blah
21+
entry: my-simple-hook.py
22+
language: python
23+
expected_return_value: 0
24+
-
25+
id: my_grep_based_hook
26+
name: My Bash Based Hook
27+
description: This is a hook that uses grep to validate some stuff
28+
entry: ./my_grep_based_hook.sh
29+
expected_return_value: 1

‎manifest.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
-
3+
id: validate_manifest
4+
name: Validate Manifest
5+
description: This validator validates a pre-commit hooks manifest file
6+
entry: validate-manifest
7+
language: python

‎pre_commit/clientlib/validate_base.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
2+
import jsonschema
3+
import jsonschema.exceptions
4+
import os.path
5+
import yaml
6+
7+
from pre_commit import git
8+
9+
10+
def get_validator(
11+
default_filename,
12+
json_schema,
13+
exception_type,
14+
additional_validation_strategy=lambda obj: None,
15+
):
16+
"""Returns a function which will validate a yaml file for correctness
17+
18+
Args:
19+
default_filename - Default filename to look for if none is specified
20+
json_schema - JSON schema to validate file with
21+
exception_type - Error type to raise on failure
22+
additional_validation_strategy - Strategy for additional validation of
23+
the object read from the file. The function should either raise
24+
exception_type on failure.
25+
"""
26+
27+
def validate(filename=None):
28+
filename = filename or os.path.join(git.get_root(), default_filename)
29+
30+
if not os.path.exists(filename):
31+
raise exception_type('File {0} does not exist'.format(filename))
32+
33+
file_contents = open(filename, 'r').read()
34+
35+
try:
36+
obj = yaml.load(file_contents)
37+
except Exception as e:
38+
raise exception_type(
39+
'File {0} is not a valid yaml file'.format(filename), e,
40+
)
41+
42+
try:
43+
jsonschema.validate(obj, json_schema)
44+
except jsonschema.exceptions.ValidationError as e:
45+
raise exception_type(
46+
'File {0} is not a valid file'.format(filename), e,
47+
)
48+
49+
additional_validation_strategy(obj)
50+
51+
return validate

‎pre_commit/clientlib/validate_manifest.py‎

Lines changed: 32 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,34 @@
22
from __future__ import print_function
33

44
import argparse
5-
import jsonschema
6-
import jsonschema.exceptions
7-
import os.path
8-
import yaml
95

106
import pre_commit.constants as C
7+
from pre_commit.clientlib.validate_base import get_validator
118

129

1310
class InvalidManifestError(ValueError): pass
1411

1512

1613
MANIFEST_JSON_SCHEMA = {
17-
'type': 'object',
18-
'properties': {
19-
'hooks': {
20-
'type': 'array',
21-
'minItems': 1,
22-
'items': {
23-
'type': 'object',
24-
'properties': {
25-
'id': {'type': 'string'},
26-
'name': {'type': 'string'},
27-
'description': {'type': 'string'},
28-
'entry': {'type': 'string'},
29-
'language': {'type': 'string'},
30-
'expected_return_value': {'type': 'number'},
31-
},
32-
'required': ['id', 'name', 'entry'],
33-
},
14+
'type': 'array',
15+
'minItems': 1,
16+
'items': {
17+
'type': 'object',
18+
'properties': {
19+
'id': {'type': 'string'},
20+
'name': {'type': 'string'},
21+
'description': {'type': 'string'},
22+
'entry': {'type': 'string'},
23+
'language': {'type': 'string'},
24+
'expected_return_value': {'type': 'number'},
3425
},
26+
'required': ['id', 'name', 'entry'],
3527
},
36-
'required': ['hooks'],
3728
}
3829

3930

40-
def check_is_valid_manifest(file_contents):
41-
file_objects = yaml.load(file_contents)
42-
43-
jsonschema.validate(file_objects, MANIFEST_JSON_SCHEMA)
44-
45-
for hook_config in file_objects['hooks']:
31+
def additional_manifest_check(obj):
32+
for hook_config in obj:
4633
language = hook_config.get('language')
4734

4835
if language is not None and not any(
@@ -57,41 +44,33 @@ def check_is_valid_manifest(file_contents):
5744
)
5845

5946

47+
validate_manifest = get_validator(
48+
C.MANIFEST_FILE,
49+
MANIFEST_JSON_SCHEMA,
50+
InvalidManifestError,
51+
additional_manifest_check,
52+
)
53+
54+
6055
def run(argv):
6156
parser = argparse.ArgumentParser()
6257
parser.add_argument(
63-
'--filename',
64-
required=False, default=None,
58+
'filename',
59+
nargs='?', default=None,
6560
help='Manifest filename. Defaults to {0} at root of git repo'.format(
6661
C.MANIFEST_FILE,
6762
)
6863
)
6964
args = parser.parse_args(argv)
7065

71-
if args.filename is None:
72-
# TODO: filename = git.get_root() + C.MANIFEST_FILE
73-
raise NotImplementedError
74-
else:
75-
filename = args.filename
76-
77-
if not os.path.exists(filename):
78-
print('File {0} does not exist'.format(filename))
79-
return 1
80-
81-
file_contents = open(filename, 'r').read()
82-
83-
try:
84-
yaml.load(file_contents)
85-
except Exception as e:
86-
print('File {0} is not a valid yaml file'.format(filename))
87-
print(str(e))
88-
return 1
89-
9066
try:
91-
check_is_valid_manifest(file_contents)
92-
except (jsonschema.exceptions.ValidationError, InvalidManifestError) as e:
93-
print('File {0} is not a valid manifest file'.format(filename))
94-
print(str(e))
67+
validate_manifest(args.filename)
68+
except InvalidManifestError as e:
69+
print(e.args[0])
70+
# If we have more than one exception argument print the stringified
71+
# version
72+
if len(e.args) > 1:
73+
print(str(e.args[1]))
9574
return 1
9675

9776
return 0

‎pre_commit/constants.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11

2-
PRE_COMMIT_FILE = '.pre-commit-config.yaml'
2+
CONFIG_FILE = '.pre-commit-config.yaml'
33

44
PRE_COMMIT_DIR = '.pre-commit-files'
55

‎pre_commit/git.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ def install(self):
5454
with self.in_checkout():
5555
if local.path('setup.py').exists():
5656
local['virtualenv']['py_env']()
57-
local['bash'][local['pip']['install', '.']]
57+
local['bash']['-c', 'source py_env/bin/activate && pip install .']()
58+
print local.cwd.getpath()
5859

5960
def create_repo_in_env(git_repo_path, sha):
6061
project = PreCommitProject(git_repo_path, sha)

‎tests/clientlib/validate_base_test.py‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
2+
import __builtin__
3+
4+
import os.path
5+
import mock
6+
import pytest
7+
8+
from pre_commit import git
9+
from pre_commit.clientlib.validate_base import get_validator
10+
11+
12+
class AdditionalValidatorError(ValueError): pass
13+
14+
15+
@pytest.fixture
16+
def noop_validator():
17+
return get_validator('example_manifest.yaml', {}, ValueError)
18+
19+
20+
@pytest.fixture
21+
def array_validator():
22+
return get_validator('', {'type': 'array'}, ValueError)
23+
24+
25+
@pytest.fixture
26+
def additional_validator():
27+
def raises_always(obj):
28+
raise AdditionalValidatorError
29+
30+
return get_validator(
31+
'example_manifest.yaml',
32+
{},
33+
ValueError,
34+
additional_validation_strategy=raises_always,
35+
)
36+
37+
38+
def test_raises_for_non_existent_file(noop_validator):
39+
with pytest.raises(ValueError):
40+
noop_validator('file_that_does_not_exist.yaml')
41+
42+
43+
def test_raises_for_invalid_yaml_file(noop_validator):
44+
with pytest.raises(ValueError):
45+
noop_validator('tests/data/non_parseable_yaml_file.yaml')
46+
47+
48+
def test_defaults_to_backup_filename(noop_validator):
49+
with mock.patch.object(__builtin__, 'open', side_effect=open) as mock_open:
50+
noop_validator()
51+
mock_open.assert_called_once_with(
52+
os.path.join(git.get_root(), 'example_manifest.yaml'), 'r',
53+
)
54+
55+
56+
def test_raises_for_failing_schema(array_validator):
57+
with pytest.raises(ValueError):
58+
array_validator('tests/data/valid_yaml_but_invalid_manifest.yaml')
59+
60+
61+
def test_passes_array_schema(array_validator):
62+
array_validator('tests/data/array_yaml_file.yaml')
63+
64+
65+
def test_raises_when_additional_validation_fails(additional_validator):
66+
with pytest.raises(AdditionalValidatorError):
67+
additional_validator()

0 commit comments

Comments
 (0)