Merge pull request #312 from blink1073/jsonformat-for-errors

nbformat/validator.py

@@ -223,6 +223,8 @@ def better_validation_error(error, version, version_minor):
     if it's a cell type or output_type error,
     try validating directly based on the type for a better error message
     """
+    if not len(error.schema_path):
+        return error
     key = error.schema_path[-1]
     ref = None
     if key.endswith("Of"):
@@ -503,6 +505,26 @@ def validate(
         raise error
 
 
+def _get_errors(
+    nbdict: Any, version: int, version_minor: int, relax_add_props: bool, *args: Any
+) -> Any:
+    validator = get_validator(version, version_minor, relax_add_props=relax_add_props)
+    if not validator:
+        raise ValidationError(f"No schema for validating v{version}.{version_minor} notebooks")
+    iter_errors = validator.iter_errors(nbdict, *args)
+    errors = list(iter_errors)
+    # jsonschema gives the best error messages.
+    if len(errors) and validator.name != "jsonschema":
+        validator = get_validator(
+            version=version,
+            version_minor=version_minor,
+            relax_add_props=relax_add_props,
+            name="jsonschema",
+        )
+        return validator.iter_errors(nbdict, *args)
+    return iter(errors)
+
+
 def _strip_invalida_metadata(
     nbdict: Any, version: int, version_minor: int, relax_add_props: bool
 ) -> int:
@@ -527,22 +549,21 @@ def _strip_invalida_metadata(
         number of modifications
 
     """
-    validator = get_validator(version, version_minor, relax_add_props=relax_add_props)
-    if not validator:
-        raise ValidationError(f"No schema for validating v{version}.{version_minor} notebooks")
-    errors = [e for e in validator.iter_errors(nbdict)]
-
+    errors = _get_errors(nbdict, version, version_minor, relax_add_props)
     changes = 0
-    if len(errors) > 0:
-        if validator.name == "fastjsonschema":
-            validator = get_validator(
-                version=version,
-                version_minor=version_minor,
-                relax_add_props=relax_add_props,
-                name="jsonschema",
+    if len(list(errors)) > 0:
+        # jsonschema gives a better error tree.
+        validator = get_validator(
+            version=version,
+            version_minor=version_minor,
+            relax_add_props=relax_add_props,
+            name="jsonschema",
+        )
+        if not validator:
+            raise ValidationError(
+                f"No jsonschema for validating v{version}.{version_minor} notebooks"
             )
-            errors = list(validator.iter_errors(nbdict))
-
+        errors = validator.iter_errors(nbdict)
         error_tree = validator.error_tree(errors)
         if "metadata" in error_tree:
             for key in error_tree["metadata"]:
@@ -609,15 +630,15 @@ def iter_validate(
     if version is None:
         version, version_minor = get_version(nbdict)
 
-    validator = get_validator(version, version_minor, relax_add_props=relax_add_props)
-
-    if validator is None:
-        # no validator
-        yield ValidationError("No schema for validating v%s notebooks" % version)
-        return
-
     if ref:
-        errors = validator.iter_errors(nbdict, {"$ref": "#/definitions/%s" % ref})
+        try:
+            errors = _get_errors(
+                nbdict, version, version_minor, relax_add_props, {"$ref": "#/definitions/%s" % ref}
+            )
+        except ValidationError as e:
+            yield e
+            return
+
     else:
         if strip_invalid_metadata:
             _strip_invalida_metadata(nbdict, version, version_minor, relax_add_props)
@@ -626,7 +647,11 @@ def iter_validate(
         # didn't cause another complex validation issue in the schema.
         # Also to ensure that higher-level errors produced by individual metadata validation
         # failures are removed.
-        errors = validator.iter_errors(nbdict)
+        try:
+            errors = _get_errors(nbdict, version, version_minor, relax_add_props)
+        except ValidationError as e:
+            yield e
+            return
 
     for error in errors:
         yield better_validation_error(error, version, version_minor)

tests/test_validator.py

@@ -213,8 +213,7 @@ def test_future(validator_name):
     assert isvalid(nb)
 
 
-# This is only a valid test for the default validator, jsonschema
-@pytest.mark.parametrize("validator_name", ["jsonschema"])
+@pytest.mark.parametrize("validator_name", VALIDATORS)
 def test_validation_error(validator_name):
     set_validator(validator_name)
     with TestsBase.fopen("invalid.ipynb", "r") as f:
@@ -229,8 +228,7 @@ def test_validation_error(validator_name):
     assert len(s.splitlines()) < 10
 
 
-# This is only a valid test for the default validator, jsonschema
-@pytest.mark.parametrize("validator_name", ["jsonschema"])
+@pytest.mark.parametrize("validator_name", VALIDATORS)
 def test_iter_validation_error(validator_name):
     set_validator(validator_name)
     with TestsBase.fopen("invalid.ipynb", "r") as f:
@@ -246,7 +244,7 @@ def test_iter_validation_empty(validator_name):
     """Test that an empty notebook (invalid) fails validation via iter_validate"""
     set_validator(validator_name)
     errors = list(iter_validate({}))
-    assert len(errors) == 1
+    assert len(errors)
     assert type(errors[0]) == ValidationError