Merge lp:~snappy-dev/click/lp1400308 into lp:~snappy-dev/click/snappy

Proposed by Barry Warsaw on 2014-12-09
Status: Merged
Merge reported by: Michael Vogt
Merged at revision: not available
Proposed branch: lp:~snappy-dev/click/lp1400308
Merge into: lp:~snappy-dev/click/snappy
Diff against target: 96 lines (+49/-6)
2 files modified
click/build.py (+11/-2)
click/tests/test_build.py (+38/-4)
To merge this branch: bzr merge lp:~snappy-dev/click/lp1400308
Reviewer Review Type Date Requested Status
Michael Vogt 2014-12-09 Approve on 2014-12-10
James Hunt 2014-12-09 Pending
Snappy Developers 2014-12-09 Pending
Review via email: mp+244242@code.launchpad.net

Description of the change

Fixes LP: #1400308 by complaining loudly (i.e. raising ClickBuildError) if the meta/readme.md file isn't just so. Remember that fp.readline() returns the empty string at EOF!

To post a comment you must log in.
Barry Warsaw (barry) wrote :

With tests! :)

Michael Vogt (mvo) wrote :

Thanks, looks good. I merged it and also fixed the parsing to of the paragraph in a seperate commit so this bug is done :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click/build.py'
2--- click/build.py 2014-12-09 16:30:57 +0000
3+++ click/build.py 2014-12-09 23:25:25 +0000
4@@ -131,9 +131,18 @@
5 self.manifest["version"] = str(self.manifest["version"])
6 readme = os.path.join(os.path.dirname(manifest_path), "readme.md")
7 with io.open(readme) as fp:
8- self.manifest["title"] = fp.readline().strip()
9+ title = fp.readline().strip()
10 # FIXME: read paragraph here instead of just the second line
11- self.manifest["description"] = fp.readline()
12+ description = fp.readline()
13+ # fp.readline() returns the empty string at EOF; check that condition
14+ # now. Downstream consumers of the manifest require some non-empty
15+ # text in these fields.
16+ if title == '' or description == '':
17+ raise ClickBuildError(
18+ '{} must contain at least two non-empty lines'.format(
19+ manifest_path))
20+ self.manifest["title"] = title
21+ self.manifest["description"] = description
22 # a ".snap" package always implicitly depends on a core framework
23 # the user does not have to specify this
24 if "frameworks" not in self.manifest:
25
26=== modified file 'click/tests/test_build.py'
27--- click/tests/test_build.py 2014-12-09 16:30:57 +0000
28+++ click/tests/test_build.py 2014-12-09 23:25:25 +0000
29@@ -19,7 +19,9 @@
30
31 __metaclass__ = type
32 __all__ = [
33+ 'TestClickBuildYaml',
34 'TestClickBuilder',
35+ 'TestClickFrameworkValidation',
36 'TestClickSourceBuilder',
37 ]
38
39@@ -493,17 +495,19 @@
40 super(TestClickBuildYaml, self).setUp()
41 self.builder = ClickBuilder()
42
43- def _make_metadata(self, scratch, template=SIMPLE_TEMPLATE):
44+ def _make_metadata(self, scratch, template=SIMPLE_TEMPLATE, readme=None):
45 with mkfile(os.path.join(scratch, "meta", "package.yaml")) as f:
46 f.write(dedent(template))
47- with mkfile(os.path.join(scratch, "meta", "readme.md")) as f:
48- f.write(dedent("""test title
49+ if readme is None:
50+ readme = dedent("""test title
51
52 Long description that spawns
53 two lines.
54
55 Some more random data.
56- """))
57+ """)
58+ with mkfile(os.path.join(scratch, "meta", "readme.md")) as f:
59+ f.write(readme)
60
61 def test_build_valid_pkg(self):
62 self.use_temp_dir()
63@@ -875,3 +879,33 @@
64 "policy_version": 1.3
65 }
66 """))
67+
68+ def test_empty_readme(self):
69+ # LP: #1400308, case 1
70+ self.use_temp_dir()
71+ self._make_metadata(self.temp_dir, readme='')
72+ with self.assertRaises(ClickBuildError) as cm:
73+ self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
74+ self.assertEqual(
75+ str(cm.exception)[-59:],
76+ 'meta/package.yaml must contain at least two non-empty lines')
77+
78+ def test_readme_with_nonempty_blank_first_line(self):
79+ # LP: #1400308, case 1; first line is .strip()'d
80+ self.use_temp_dir()
81+ self._make_metadata(self.temp_dir, readme=' ')
82+ with self.assertRaises(ClickBuildError) as cm:
83+ self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
84+ self.assertEqual(
85+ str(cm.exception)[-59:],
86+ 'meta/package.yaml must contain at least two non-empty lines')
87+
88+ def test_single_line_readme(self):
89+ # LP: #1400308, case 2
90+ self.use_temp_dir()
91+ self._make_metadata(self.temp_dir, readme='a title\n')
92+ with self.assertRaises(ClickBuildError) as cm:
93+ self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
94+ self.assertEqual(
95+ str(cm.exception)[-59:],
96+ 'meta/package.yaml must contain at least two non-empty lines')

Subscribers

People subscribed via source and target branches

to all changes: