Merge lp:~mvo/click-reviewers-tools/minimal-snap-yaml into lp:click-reviewers-tools

[Michael Vogt]

This branch adds minimal support for meta/snap.yaml by converting the snap.yaml to pkg.yaml internally. This is a stop-gap to unblock the store so that we can push unit tests to the store again with snap.yaml.

[Martin Albisetti (beuno)]

PEP8 has a small complaint:

./clickreviews/cr_common.py:186:49: W291 trailing whitespace

Other than that, +1

review: Approve

[Jamie Strandboge (jdstrand)]

Yes, 'make check' also had another whitespace complaint. I fixed those.

review: Approve

=== modified file 'clickreviews/cr_common.py'
--- clickreviews/cr_common.py	2016-01-20 14:02:52 +0000
+++ clickreviews/cr_common.py	2016-02-01 09:02:56 +0000
@@ -161,8 +161,9 @@
 
         # Parse and store the package.yaml, if it exists
         pkg_yaml = self._extract_package_yaml()
+        snap_yaml = self._extract_snap_yaml()
         self.is_snap = False
-        if pkg_yaml is None:
+        if pkg_yaml is None and snap_yaml is None:
             self.pkgfmt["type"] = "click"
         else:
             self.pkgfmt["type"] = "snap"
@@ -176,11 +177,23 @@
             else:
                 self.pkgfmt["version"] = "15.04"
 
-            try:
-                self.pkg_yaml = yaml.safe_load(pkg_yaml)
-            except Exception:
-                error("Could not load package.yaml. Is it properly formatted?")
-            self._verify_package_yaml_structure()
+            if snap_yaml:
+                try:
+                    self.snap_yaml = yaml.safe_load(snap_yaml)
+                except Exception:
+                    error("Could not load snap.yaml. Is it properly formatted?")
+                # convert enough of snap_yaml too pkg.yaml to make
+                # the basics of the review tools 
+                if self.snap_yaml:
+                    self.pkg_yaml = self.snap_yaml
+                    self.pkg_yaml["version"] = str(self.snap_yaml["version"])
+
+            if pkg_yaml:
+                try:
+                    self.pkg_yaml = yaml.safe_load(pkg_yaml)
+                except Exception:
+                    error("Could not load package.yaml. Is it properly formatted?")
+                self._verify_package_yaml_structure()
             self.is_snap = True
 
             #  default to 'app'
@@ -272,12 +285,19 @@
         return open_file_read(m)
 
     def _extract_package_yaml(self):
-        '''Extract and read the snappy package.yaml'''
+        '''Extract and read the snappy 15.04 package.yaml'''
         y = os.path.join(self.unpack_dir, "meta/package.yaml")
         if not os.path.isfile(y):
             return None  # snappy packaging is still optional
         return open_file_read(y)
 
+    def _extract_snap_yaml(self):
+        '''Extract and read the snappy 16.04 snap.yaml'''
+        y = os.path.join(self.unpack_dir, "meta/snap.yaml")
+        if not os.path.isfile(y):
+            return None  # snappy packaging is still optional
+        return open_file_read(y)
+
     def _extract_hashes_yaml(self):
         '''Extract and read the snappy hashes.yaml'''
         y = os.path.join(self.unpack_dir, "DEBIAN/hashes.yaml")

=== modified file 'clickreviews/cr_lint.py'
--- clickreviews/cr_lint.py	2016-01-20 14:02:52 +0000
+++ clickreviews/cr_lint.py	2016-02-01 09:02:56 +0000
@@ -1222,6 +1222,15 @@
             manual_review = True
             self._add_result(t, n, s, manual_review=manual_review)
 
+    def check_squashfs_uses_snap_yaml(self):
+        '''Ensure that squashfs uses 16.04'''
+        if is_squashfs(self.pkg_filename) and not getattr(self, "snap_yaml"):
+            t = 'error'
+            n = self._get_check_name('check_squashfs_uses_snap_yaml')
+            s = "squashfs snaps must have a meta/snap.yaml"
+            manual_review = False
+            self._add_result(t, n, s, manual_review=manual_review)
+
     def check_snappy_hashes(self):
         '''Check snappy hashes.yaml'''
         if self._pkgfmt_type() == "snap" and \

=== modified file 'clickreviews/cr_tests.py'
--- clickreviews/cr_tests.py	2015-11-20 21:03:02 +0000
+++ clickreviews/cr_tests.py	2016-02-01 09:02:56 +0000
@@ -31,6 +31,7 @@
 TEST_CONTROL = ""
 TEST_MANIFEST = ""
 TEST_PKG_YAML = ""
+TEST_SNAP_YAML = ""
 TEST_HASHES_YAML = ""
 TEST_README_MD = ""
 TEST_SECURITY = dict()
@@ -77,6 +78,11 @@
     return io.StringIO(TEST_PKG_YAML)
 
 
+def _extract_snap_yaml(self):
+    '''Pretend we read the package.yaml file'''
+    return io.StringIO(TEST_SNAP_YAML)
+
+
 def _extract_hashes_yaml(self):
     '''Pretend we read the hashes.yaml file'''
     return io.StringIO(TEST_HASHES_YAML)
@@ -281,6 +287,9 @@
         'clickreviews.cr_common.ClickReview._extract_package_yaml',
         _extract_package_yaml))
     patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_snap_yaml',
+        _extract_snap_yaml))
+    patches.append(patch(
         'clickreviews.cr_common.ClickReview._extract_hashes_yaml',
         _extract_hashes_yaml))
     patches.append(patch(
@@ -448,6 +457,8 @@
                                [self.test_control['Architecture']])
         self._update_test_pkg_yaml()
 
+        self.test_snap_yaml = dict()
+        
         self.test_hashes_yaml = dict()
         self._update_test_hashes_yaml()
 
@@ -566,6 +577,12 @@
                                   default_flow_style=False,
                                   indent=4)
 
+    def _update_test_snap_yaml(self):
+        global TEST_SNAP_YAML
+        TEST_SNAP_YAML = yaml.dump(self.test_snap_yaml,
+                                   default_flow_style=False,
+                                   indent=4)
+
     def _update_test_hashes_yaml(self):
         global TEST_HASHES_YAML
         TEST_HASHES_YAML = yaml.dump(self.test_hashes_yaml,
@@ -794,6 +811,16 @@
             self.test_pkg_yaml[key] = value
         self._update_test_pkg_yaml()
 
+    def set_test_snap_yaml(self, key, value):
+        '''Set key in meta/snap.yaml to value. If value is None, remove
+           key'''
+        if value is None:
+            if key in self.test_snap_yaml:
+                self.test_snap_yaml.pop(key, None)
+        else:
+            self.test_snap_yaml[key] = value
+        self._update_test_snap_yaml()
+
     def set_test_hashes_yaml(self, yaml):
         '''Set hashes.yaml to yaml'''
         self.test_hashes_yaml = yaml
@@ -1137,6 +1164,8 @@
         TEST_MANIFEST = ""
         global TEST_PKG_YAML
         TEST_PKG_YAML = ""
+        global TEST_SNAP_YAML
+        TEST_SNAP_YAML = ""
         global TEST_HASHES_YAML
         TEST_HASHES_YAML = ""
         global TEST_README_MD

=== modified file 'clickreviews/tests/test_cr_lint.py'
--- clickreviews/tests/test_cr_lint.py	2016-01-20 17:27:51 +0000
+++ clickreviews/tests/test_cr_lint.py	2016-02-01 09:02:56 +0000
@@ -2190,6 +2190,30 @@
             expected_counts = {'info': None, 'warn': 0, 'error': 1}
             self.check_results(r, expected_counts)
 
+    def test_squashfs_needs_snap_yaml(self):
+        '''Test that squashfs snaps have a snap.yaml'''
+        c = ClickReviewLint(self.test_name)
+        c.unpack_dir = "/nonexistent.nonexec"
+        with patch("clickreviews.cr_lint.is_squashfs") as mock_is_squashfs:
+            mock_is_squashfs.return_value = True
+            c.check_squashfs_uses_snap_yaml()
+            r = c.click_report
+            expected_counts = {'info': None, 'warn': 0, 'error': 1}
+            self.assertEqual(r["error"]["lint:check_squashfs_uses_snap_yaml"]["text"], "squashfs snaps must have a meta/snap.yaml")
+            self.check_results(r, expected_counts)
+
+    def test_squashfs_needs_snap_yaml_ok(self):
+        '''Test that squashfs snaps have a snap.yaml'''
+        c = ClickReviewLint(self.test_name)
+        c.unpack_dir = "/nonexistent.nonexec"
+        c.snap_yaml = {"name": "foo"}
+        with patch("clickreviews.cr_lint.is_squashfs") as mock_is_squashfs:
+            mock_is_squashfs.return_value = True
+            c.check_squashfs_uses_snap_yaml()
+            r = c.click_report
+            expected_counts = {'info': None, 'warn': 0, 'error': 0}
+            self.check_results(r, expected_counts)
+
 
 class ClickReviewLintTestCase(TestCase):
     """Tests without mocks where they are not needed."""