Consolidated file validation

pre_commit/clientlib/validate_base.py

@@ -0,0 +1,51 @@
+
+import jsonschema
+import jsonschema.exceptions
+import os.path
+import yaml
+
+from pre_commit import git
+
+
+def get_validator(
+    default_filename,
+    json_schema,
+    exception_type,
+    additional_validation_strategy=lambda obj: None,
+):
+    """Returns a function which will validate a yaml file for correctness
+
+    Args:
+        default_filename - Default filename to look for if none is specified
+        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=None):
+        filename = filename or os.path.join(git.get_root(), default_filename)
+
+        if not os.path.exists(filename):
+            raise exception_type('File {0} does not exist'.format(filename))
+
+        file_contents = open(filename, 'r').read()
+
+        try:
+            obj = yaml.load(file_contents)
+        except Exception as e:
+            raise exception_type(
+                'File {0} is not a valid yaml file'.format(filename), e,
+            )
+
+        try:
+            jsonschema.validate(obj, json_schema)
+        except jsonschema.exceptions.ValidationError as e:
+            raise exception_type(
+                'File {0} is not a valid file'.format(filename), e,
+            )
+
+        additional_validation_strategy(obj)
+
+    return validate

pre_commit/clientlib/validate_manifest.py

@@ -2,13 +2,9 @@
 from __future__ import print_function
 
 import argparse
-import jsonschema
-import jsonschema.exceptions
-import os.path
-import yaml
 
 import pre_commit.constants as C
-from pre_commit import git
+from pre_commit.clientlib.validate_base import get_validator
 
 
 class InvalidManifestError(ValueError): pass
@@ -38,12 +34,8 @@ MANIFEST_JSON_SCHEMA = {
 }
 
 
-def check_is_valid_manifest(file_contents):
+def additional_manifest_check(obj):
-    file_objects = yaml.load(file_contents)
+    for hook_config in obj['hooks']:
-
-    jsonschema.validate(file_objects, MANIFEST_JSON_SCHEMA)
-
-    for hook_config in file_objects['hooks']:
         language = hook_config.get('language')
 
         if language is not None and not any(
@@ -58,6 +50,14 @@ def check_is_valid_manifest(file_contents):
             )
 
 
+validate_manifest = get_validator(
+    C.MANIFEST_FILE,
+    MANIFEST_JSON_SCHEMA,
+    InvalidManifestError,
+    additional_manifest_check,
+)
+
+
 def run(argv):
     parser = argparse.ArgumentParser()
     parser.add_argument(
@@ -69,29 +69,14 @@ def run(argv):
     )
     args = parser.parse_args(argv)
 
-    if args.filename is None:
-        filename = os.path.join(git.get_root(), C.MANIFEST_FILE)
-    else:
-        filename = args.filename
-
-    if not os.path.exists(filename):
-        print('File {0} does not exist'.format(filename))
-        return 1
-
-    file_contents = open(filename, 'r').read()
-
     try:
-        yaml.load(file_contents)
+        validate_manifest(args.filename)
-    except Exception as e:
+    except InvalidManifestError as e:
-        print('File {0} is not a valid yaml file'.format(filename))
+        print(e.args[0])
-        print(str(e))
+        # If we have more than one exception argument print the stringified
-        return 1
+        # version
-
+        if len(e.args) > 1:
-    try:
+            print(str(e.args[1]))
-        check_is_valid_manifest(file_contents)
-    except (jsonschema.exceptions.ValidationError, InvalidManifestError) as e:
-        print('File {0} is not a valid manifest file'.format(filename))
-        print(str(e))
         return 1
 
     return 0

pre_commit/constants.py

@@ -1,5 +1,5 @@
 
-PRE_COMMIT_FILE = '.pre-commit-config.yaml'
+CONFIG_FILE = '.pre-commit-config.yaml'
 
 PRE_COMMIT_DIR = '.pre-commit-files'
 

tests/clientlib/validate_base_test.py

@@ -0,0 +1,63 @@
+
+import __builtin__
+
+import os.path
+import mock
+import pytest
+
+from pre_commit import git
+from pre_commit.clientlib.validate_base import get_validator
+
+
+class AdditionalValidatorError(ValueError): pass
+
+
+@pytest.fixture
+def noop_validator():
+    return get_validator('example_manifest.yaml', {}, ValueError)
+
+
+@pytest.fixture
+def array_validator():
+    return get_validator('', {'type': 'array'}, ValueError)
+
+
+@pytest.fixture
+def additional_validator():
+    def raises_always(obj):
+        raise AdditionalValidatorError
+
+    return get_validator(
+        'example_manifest.yaml',
+        {},
+        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('tests/data/non_parseable_yaml_file.yaml')
+
+
+def test_defaults_to_backup_filename(noop_validator):
+    with mock.patch.object(__builtin__, 'open', side_effect=open) as mock_open:
+        noop_validator()
+        mock_open.assert_called_once_with(
+            os.path.join(git.get_root(), 'example_manifest.yaml'), 'r',
+        )
+
+
+def test_raises_for_failing_schema(array_validator):
+    with pytest.raises(ValueError):
+        array_validator('tests/data/non_parseable_yaml_file.yaml')
+
+
+def test_raises_when_additional_validation_fails(additional_validator):
+    with pytest.raises(AdditionalValidatorError):
+        additional_validator()

tests/clientlib/validate_manifest_test.py

@@ -1,14 +1,12 @@
 
 import __builtin__
-import jsonschema
 import pytest
 import mock
 from plumbum import local
 
 import pre_commit.constants as C
-from pre_commit.clientlib.validate_manifest import check_is_valid_manifest
+from pre_commit.clientlib.validate_manifest import run, InvalidManifestError, \
-from pre_commit.clientlib.validate_manifest import InvalidManifestError
+    additional_manifest_check
-from pre_commit.clientlib.validate_manifest import run
 
 
 @pytest.yield_fixture
@@ -40,7 +38,7 @@ def test_returns_1_for_valid_yaml_file_but_invalid_manifest(print_mock):
     ret = run(['--filename', invalid_manifest])
     assert ret == 1
     print_mock.assert_any_call(
-        'File {0} is not a valid manifest file'.format(invalid_manifest)
+        'File {0} is not a valid file'.format(invalid_manifest)
     )
 
 
@@ -62,39 +60,17 @@ hooks:
     assert ret == 0
 
 
-@pytest.mark.parametrize(('manifest', 'expected_exception_type'), (
+def test_additional_manifest_check_raises_for_bad_language():
-    (
+    with pytest.raises(InvalidManifestError):
-        """
+        additional_manifest_check(
-hooks:
+            {'hooks': [{'id': 'foo', 'language': 'not valid'}]}
-    -
+        )
-        id: foo
+
-        entry: foo
+
-        """,
+@pytest.mark.parametrize(('obj'), (
-        jsonschema.exceptions.ValidationError,
+    {'hooks': [{}]},
-    ),
+    {'hooks': [{'language': 'python'}]},
-    (
+    {'hooks': [{'language': 'python>2.6'}]},
-        """
-hooks:
-    -
-        id: foo
-        name: Foo
-        language: Not a Language lol
-        entry: foo
-        """,
-        InvalidManifestError,
-    ),
 ))
-def test_check_invalid_manifests(manifest, expected_exception_type):
+def test_additional_manifest_check_is_ok_with_missing_language(obj):
-    with pytest.raises(expected_exception_type):
+    additional_manifest_check(obj)
-        check_is_valid_manifest(manifest)
-
-
-def test_valid_manifest_is_valid():
-    check_is_valid_manifest("""
-hooks:
-    -
-        id: foo
-        name: Foo
-        entry: foo
-        language: python>2.6
-    """)