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

Proposed by Michael Vogt
Status: Merged
Merged at revision: 567
Proposed branch: lp:~mvo/click-reviewers-tools/minimal-snap-yaml
Merge into: lp:click-reviewers-tools
Diff against target: 201 lines (+89/-7)
4 files modified
clickreviews/cr_common.py (+27/-7)
clickreviews/cr_lint.py (+9/-0)
clickreviews/cr_tests.py (+29/-0)
clickreviews/tests/test_cr_lint.py (+24/-0)
To merge this branch: bzr merge lp:~mvo/click-reviewers-tools/minimal-snap-yaml
Reviewer Review Type Date Requested Status
Jamie Strandboge (community) Approve
Martin Albisetti (community) Approve
Review via email: mp+284562@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Martin Albisetti (beuno) wrote :

PEP8 has a small complaint:

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

Other than that, +1

review: Approve
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
161161
162 # Parse and store the package.yaml, if it exists162 # Parse and store the package.yaml, if it exists
163 pkg_yaml = self._extract_package_yaml()163 pkg_yaml = self._extract_package_yaml()
164 snap_yaml = self._extract_snap_yaml()
164 self.is_snap = False165 self.is_snap = False
165 if pkg_yaml is None:166 if pkg_yaml is None and snap_yaml is None:
166 self.pkgfmt["type"] = "click"167 self.pkgfmt["type"] = "click"
167 else:168 else:
168 self.pkgfmt["type"] = "snap"169 self.pkgfmt["type"] = "snap"
@@ -176,11 +177,23 @@
176 else:177 else:
177 self.pkgfmt["version"] = "15.04"178 self.pkgfmt["version"] = "15.04"
178179
179 try:180 if snap_yaml:
180 self.pkg_yaml = yaml.safe_load(pkg_yaml)181 try:
181 except Exception:182 self.snap_yaml = yaml.safe_load(snap_yaml)
182 error("Could not load package.yaml. Is it properly formatted?")183 except Exception:
183 self._verify_package_yaml_structure()184 error("Could not load snap.yaml. Is it properly formatted?")
185 # convert enough of snap_yaml too pkg.yaml to make
186 # the basics of the review tools
187 if self.snap_yaml:
188 self.pkg_yaml = self.snap_yaml
189 self.pkg_yaml["version"] = str(self.snap_yaml["version"])
190
191 if pkg_yaml:
192 try:
193 self.pkg_yaml = yaml.safe_load(pkg_yaml)
194 except Exception:
195 error("Could not load package.yaml. Is it properly formatted?")
196 self._verify_package_yaml_structure()
184 self.is_snap = True197 self.is_snap = True
185198
186 # default to 'app'199 # default to 'app'
@@ -272,12 +285,19 @@
272 return open_file_read(m)285 return open_file_read(m)
273286
274 def _extract_package_yaml(self):287 def _extract_package_yaml(self):
275 '''Extract and read the snappy package.yaml'''288 '''Extract and read the snappy 15.04 package.yaml'''
276 y = os.path.join(self.unpack_dir, "meta/package.yaml")289 y = os.path.join(self.unpack_dir, "meta/package.yaml")
277 if not os.path.isfile(y):290 if not os.path.isfile(y):
278 return None # snappy packaging is still optional291 return None # snappy packaging is still optional
279 return open_file_read(y)292 return open_file_read(y)
280293
294 def _extract_snap_yaml(self):
295 '''Extract and read the snappy 16.04 snap.yaml'''
296 y = os.path.join(self.unpack_dir, "meta/snap.yaml")
297 if not os.path.isfile(y):
298 return None # snappy packaging is still optional
299 return open_file_read(y)
300
281 def _extract_hashes_yaml(self):301 def _extract_hashes_yaml(self):
282 '''Extract and read the snappy hashes.yaml'''302 '''Extract and read the snappy hashes.yaml'''
283 y = os.path.join(self.unpack_dir, "DEBIAN/hashes.yaml")303 y = os.path.join(self.unpack_dir, "DEBIAN/hashes.yaml")
284304
=== 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 @@
1222 manual_review = True1222 manual_review = True
1223 self._add_result(t, n, s, manual_review=manual_review)1223 self._add_result(t, n, s, manual_review=manual_review)
12241224
1225 def check_squashfs_uses_snap_yaml(self):
1226 '''Ensure that squashfs uses 16.04'''
1227 if is_squashfs(self.pkg_filename) and not getattr(self, "snap_yaml"):
1228 t = 'error'
1229 n = self._get_check_name('check_squashfs_uses_snap_yaml')
1230 s = "squashfs snaps must have a meta/snap.yaml"
1231 manual_review = False
1232 self._add_result(t, n, s, manual_review=manual_review)
1233
1225 def check_snappy_hashes(self):1234 def check_snappy_hashes(self):
1226 '''Check snappy hashes.yaml'''1235 '''Check snappy hashes.yaml'''
1227 if self._pkgfmt_type() == "snap" and \1236 if self._pkgfmt_type() == "snap" and \
12281237
=== 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 @@
31TEST_CONTROL = ""31TEST_CONTROL = ""
32TEST_MANIFEST = ""32TEST_MANIFEST = ""
33TEST_PKG_YAML = ""33TEST_PKG_YAML = ""
34TEST_SNAP_YAML = ""
34TEST_HASHES_YAML = ""35TEST_HASHES_YAML = ""
35TEST_README_MD = ""36TEST_README_MD = ""
36TEST_SECURITY = dict()37TEST_SECURITY = dict()
@@ -77,6 +78,11 @@
77 return io.StringIO(TEST_PKG_YAML)78 return io.StringIO(TEST_PKG_YAML)
7879
7980
81def _extract_snap_yaml(self):
82 '''Pretend we read the package.yaml file'''
83 return io.StringIO(TEST_SNAP_YAML)
84
85
80def _extract_hashes_yaml(self):86def _extract_hashes_yaml(self):
81 '''Pretend we read the hashes.yaml file'''87 '''Pretend we read the hashes.yaml file'''
82 return io.StringIO(TEST_HASHES_YAML)88 return io.StringIO(TEST_HASHES_YAML)
@@ -281,6 +287,9 @@
281 'clickreviews.cr_common.ClickReview._extract_package_yaml',287 'clickreviews.cr_common.ClickReview._extract_package_yaml',
282 _extract_package_yaml))288 _extract_package_yaml))
283 patches.append(patch(289 patches.append(patch(
290 'clickreviews.cr_common.ClickReview._extract_snap_yaml',
291 _extract_snap_yaml))
292 patches.append(patch(
284 'clickreviews.cr_common.ClickReview._extract_hashes_yaml',293 'clickreviews.cr_common.ClickReview._extract_hashes_yaml',
285 _extract_hashes_yaml))294 _extract_hashes_yaml))
286 patches.append(patch(295 patches.append(patch(
@@ -448,6 +457,8 @@
448 [self.test_control['Architecture']])457 [self.test_control['Architecture']])
449 self._update_test_pkg_yaml()458 self._update_test_pkg_yaml()
450459
460 self.test_snap_yaml = dict()
461
451 self.test_hashes_yaml = dict()462 self.test_hashes_yaml = dict()
452 self._update_test_hashes_yaml()463 self._update_test_hashes_yaml()
453464
@@ -566,6 +577,12 @@
566 default_flow_style=False,577 default_flow_style=False,
567 indent=4)578 indent=4)
568579
580 def _update_test_snap_yaml(self):
581 global TEST_SNAP_YAML
582 TEST_SNAP_YAML = yaml.dump(self.test_snap_yaml,
583 default_flow_style=False,
584 indent=4)
585
569 def _update_test_hashes_yaml(self):586 def _update_test_hashes_yaml(self):
570 global TEST_HASHES_YAML587 global TEST_HASHES_YAML
571 TEST_HASHES_YAML = yaml.dump(self.test_hashes_yaml,588 TEST_HASHES_YAML = yaml.dump(self.test_hashes_yaml,
@@ -794,6 +811,16 @@
794 self.test_pkg_yaml[key] = value811 self.test_pkg_yaml[key] = value
795 self._update_test_pkg_yaml()812 self._update_test_pkg_yaml()
796813
814 def set_test_snap_yaml(self, key, value):
815 '''Set key in meta/snap.yaml to value. If value is None, remove
816 key'''
817 if value is None:
818 if key in self.test_snap_yaml:
819 self.test_snap_yaml.pop(key, None)
820 else:
821 self.test_snap_yaml[key] = value
822 self._update_test_snap_yaml()
823
797 def set_test_hashes_yaml(self, yaml):824 def set_test_hashes_yaml(self, yaml):
798 '''Set hashes.yaml to yaml'''825 '''Set hashes.yaml to yaml'''
799 self.test_hashes_yaml = yaml826 self.test_hashes_yaml = yaml
@@ -1137,6 +1164,8 @@
1137 TEST_MANIFEST = ""1164 TEST_MANIFEST = ""
1138 global TEST_PKG_YAML1165 global TEST_PKG_YAML
1139 TEST_PKG_YAML = ""1166 TEST_PKG_YAML = ""
1167 global TEST_SNAP_YAML
1168 TEST_SNAP_YAML = ""
1140 global TEST_HASHES_YAML1169 global TEST_HASHES_YAML
1141 TEST_HASHES_YAML = ""1170 TEST_HASHES_YAML = ""
1142 global TEST_README_MD1171 global TEST_README_MD
11431172
=== 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 @@
2190 expected_counts = {'info': None, 'warn': 0, 'error': 1}2190 expected_counts = {'info': None, 'warn': 0, 'error': 1}
2191 self.check_results(r, expected_counts)2191 self.check_results(r, expected_counts)
21922192
2193 def test_squashfs_needs_snap_yaml(self):
2194 '''Test that squashfs snaps have a snap.yaml'''
2195 c = ClickReviewLint(self.test_name)
2196 c.unpack_dir = "/nonexistent.nonexec"
2197 with patch("clickreviews.cr_lint.is_squashfs") as mock_is_squashfs:
2198 mock_is_squashfs.return_value = True
2199 c.check_squashfs_uses_snap_yaml()
2200 r = c.click_report
2201 expected_counts = {'info': None, 'warn': 0, 'error': 1}
2202 self.assertEqual(r["error"]["lint:check_squashfs_uses_snap_yaml"]["text"], "squashfs snaps must have a meta/snap.yaml")
2203 self.check_results(r, expected_counts)
2204
2205 def test_squashfs_needs_snap_yaml_ok(self):
2206 '''Test that squashfs snaps have a snap.yaml'''
2207 c = ClickReviewLint(self.test_name)
2208 c.unpack_dir = "/nonexistent.nonexec"
2209 c.snap_yaml = {"name": "foo"}
2210 with patch("clickreviews.cr_lint.is_squashfs") as mock_is_squashfs:
2211 mock_is_squashfs.return_value = True
2212 c.check_squashfs_uses_snap_yaml()
2213 r = c.click_report
2214 expected_counts = {'info': None, 'warn': 0, 'error': 0}
2215 self.check_results(r, expected_counts)
2216
21932217
2194class ClickReviewLintTestCase(TestCase):2218class ClickReviewLintTestCase(TestCase):
2195 """Tests without mocks where they are not needed."""2219 """Tests without mocks where they are not needed."""

Subscribers

People subscribed via source and target branches