Merge ~jslarraz/review-tools:unify-schema-validation into review-tools:master

[Jorge Sancho Larraz]

rt/{sr_lint,tests/schema}: unify how jsonschema is validated

[Alex Murray (alexmurray)]

LGTM!

review: Approve

diff --git a/reviewtools/schemas/schema_validator.py b/reviewtools/schemas/schema_validator.py
new file mode 100644
index 0000000..d9803c3
--- /dev/null
+++ b/reviewtools/schemas/schema_validator.py
@@ -0,0 +1,27 @@
+import os
+import json
+import jsonschema
+from pkg_resources import resource_filename
+
+
+def validate(yaml, schema_name):
+    # prefer local copy if it exists, otherwise, use one shipped in the
+    # package
+    schema_fn = "./reviewtools/schemas/%s" % schema_name
+    if not os.path.exists(schema_fn):
+        schema_fn = resource_filename("reviewtools", "schemas/%s" % schema_name)
+        if not os.path.exists(schema_fn):
+            return "Cannot find the schema file: %s" % schema_fn
+
+    with open(schema_fn) as fd:
+        schema = json.loads(fd.read())
+
+    resolver = jsonschema.RefResolver(
+        "file://%s/" % os.path.abspath(os.path.dirname(__file__)), None
+    )
+
+    try:
+        jsonschema.validate(yaml, schema, resolver=resolver)
+        return None
+    except jsonschema.ValidationError as e:
+        return e
diff --git a/reviewtools/sr_lint.py b/reviewtools/sr_lint.py
index 5c7d53f..40990ad 100644
--- a/reviewtools/sr_lint.py
+++ b/reviewtools/sr_lint.py
@@ -15,10 +15,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import print_function
+from reviewtools.schemas.schema_validator import validate
 from reviewtools.sr_common import SnapReview
 from reviewtools.common import (
     find_external_symlinks,
-    error,
 )
 from reviewtools.overrides import (
     redflagged_snap_types_overrides,
@@ -32,9 +32,6 @@ import os
 import re
 import shlex
 import unicodedata
-import jsonschema
-import json
-from pkg_resources import resource_filename
 
 
 class SnapReviewLint(SnapReview):
@@ -79,25 +76,11 @@ class SnapReviewLint(SnapReview):
 
     def check_schema(self):
         """Check snap.yaml against the schema"""
-        t = "info"
         n = self._get_check_name("snap_schema")
-        s = "OK"
         manual_review = False
 
-        # prefer local copy if it exists, otherwise, use one shipped in the
-        # package
-        schema_fn = "./reviewtools/schemas/snap.json"
-        if not os.path.exists(schema_fn):
-            schema_fn = resource_filename("reviewtools", "schemas/snap.json")
-            if not os.path.exists(schema_fn):
-                error("could not find '%s'" % schema_fn)
-
-        with open(schema_fn) as fd:
-            schema = json.loads(fd.read())
-
-        try:
-            jsonschema.validate(self.snap_yaml, schema)
-        except jsonschema.ValidationError as e:
+        e = validate(self.snap_yaml, "snap.json")
+        if e is not None:
             e.absolute_path.appendleft("$")
             t = "error"
             s = "Error found in snap.yaml while validating %s: %s" % (
diff --git a/reviewtools/tests/schemas/test_schema_against_store.py b/reviewtools/tests/schemas/test_schema_against_store.py
index 16259e2..d6fd208 100644
--- a/reviewtools/tests/schemas/test_schema_against_store.py
+++ b/reviewtools/tests/schemas/test_schema_against_store.py
@@ -3,10 +3,9 @@ import json
 import yaml
 import os
 import sys
-import jsonschema
 import unittest
+from reviewtools.schemas.schema_validator import validate
 
-SNAP_SCHEMA = "reviewtools/schemas/snap.json"
 SNAP_YAML_CACHE = os.path.expanduser("~/.cache/review-tools")
 
 
@@ -22,10 +21,6 @@ def test_store():
     except OSError:
         pass
 
-    # Load schema
-    with open(SNAP_SCHEMA) as fd:
-        snap_schema = json.loads(fd.read())
-
     # Get list of snaps in the store
     with open("/var/cache/snapd/names", "r") as fd:
         snaps = fd.readlines()
@@ -60,9 +55,8 @@ def test_store():
             continue
 
         # Validate the snap.yaml against the schema
-        try:
-            jsonschema.validate(snap_yaml, snap_schema)
-        except jsonschema.ValidationError as e:
+        e = validate(snap_yaml, "snap.json")
+        if e is not None:
             print(
                 "Error to validate %s for snap %s: %s" % (e.json_path, snap, e.message)
             )
diff --git a/reviewtools/tests/schemas/test_schema_base.py b/reviewtools/tests/schemas/test_schema_base.py
index b41f5dd..d6c4536 100644
--- a/reviewtools/tests/schemas/test_schema_base.py
+++ b/reviewtools/tests/schemas/test_schema_base.py
@@ -1,30 +1,18 @@
 import json
-import jsonschema
 import copy
 import unittest
+from reviewtools.schemas.schema_validator import validate
 
 
 class TestSchemaBase(unittest.TestCase):
     yaml = {}
-    schema_file = "reviewtools/schemas/****.json"
+    schema_name = "****.json"
 
     def setUp(self):
-        # Load the relevant schema (defined in subclass)
-        with open(self.schema_file) as fd:
-            self.schema = json.loads(fd.read())
-
         # Validate base yaml against the schema
-        error = self._validate(self.yaml, self.schema)
+        error = validate(self.yaml, self.schema_name)
         self.assertEqual(None, error)
 
-    def _validate(self, yaml, schema):
-        # Validate yaml against the schema
-        try:
-            jsonschema.validate(yaml, schema)
-            return None
-        except jsonschema.ValidationError as e:
-            return e.message
-
     def _test_value(self, key, value, expected_error):
         # Prepare yaml, make a copy so we use the pristine yaml on each subtest
         yaml = copy.deepcopy(self.yaml)
@@ -34,11 +22,11 @@ class TestSchemaBase(unittest.TestCase):
         else:
             yaml[key] = value
 
-        # Validate the schema
-        error = self._validate(yaml, self.schema)
+        # Validate yaml against the schema
+        e = validate(yaml, self.schema_name)
 
         # Compare the errors against the expected ones
         if expected_error is None:
-            self.assertIsNone(error)
+            self.assertIsNone(e.message)
         else:
-            self.assertIn(expected_error, error)
+            self.assertIn(expected_error, e.message)
diff --git a/reviewtools/tests/schemas/test_schema_snap.py b/reviewtools/tests/schemas/test_schema_snap.py
index bcc55da..23a7098 100644
--- a/reviewtools/tests/schemas/test_schema_snap.py
+++ b/reviewtools/tests/schemas/test_schema_snap.py
@@ -2,7 +2,7 @@ from reviewtools.tests.schemas.test_schema_base import TestSchemaBase
 
 
 class TestSchemaSnap(TestSchemaBase):
-    schema_file = "reviewtools/schemas/snap.json"
+    schema_name = "snap.json"
 
     def setUp(self):
         self.yaml = {