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

[Barry Warsaw]

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!

[Barry Warsaw (barry)]

With tests! :)

[Michael Vogt (mvo)]

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

=== modified file 'click/build.py'
--- click/build.py	2014-12-09 16:30:57 +0000
+++ click/build.py	2014-12-09 23:25:25 +0000
@@ -131,9 +131,18 @@
         self.manifest["version"] = str(self.manifest["version"])
         readme = os.path.join(os.path.dirname(manifest_path), "readme.md")
         with io.open(readme) as fp:
-            self.manifest["title"] = fp.readline().strip()
+            title = fp.readline().strip()
             # FIXME: read paragraph here instead of just the second line
-            self.manifest["description"] = fp.readline()
+            description = fp.readline()
+        # fp.readline() returns the empty string at EOF; check that condition
+        # now.  Downstream consumers of the manifest require some non-empty
+        # text in these fields.
+        if title == '' or description == '':
+            raise ClickBuildError(
+                '{} must contain at least two non-empty lines'.format(
+                    manifest_path))
+        self.manifest["title"] = title
+        self.manifest["description"] = description
         # a ".snap" package always implicitly depends on a core framework
         # the user does not have to specify this
         if "frameworks" not in self.manifest:

=== modified file 'click/tests/test_build.py'
--- click/tests/test_build.py	2014-12-09 16:30:57 +0000
+++ click/tests/test_build.py	2014-12-09 23:25:25 +0000
@@ -19,7 +19,9 @@
 
 __metaclass__ = type
 __all__ = [
+    'TestClickBuildYaml',
     'TestClickBuilder',
+    'TestClickFrameworkValidation',
     'TestClickSourceBuilder',
     ]
 
@@ -493,17 +495,19 @@
         super(TestClickBuildYaml, self).setUp()
         self.builder = ClickBuilder()
 
-    def _make_metadata(self, scratch, template=SIMPLE_TEMPLATE):
+    def _make_metadata(self, scratch, template=SIMPLE_TEMPLATE, readme=None):
         with mkfile(os.path.join(scratch, "meta", "package.yaml")) as f:
             f.write(dedent(template))
-        with mkfile(os.path.join(scratch, "meta", "readme.md")) as f:
-            f.write(dedent("""test title
+        if readme is None:
+            readme = dedent("""test title
 
             Long description that spawns
             two lines.
 
             Some more random data.
-            """))
+            """)
+        with mkfile(os.path.join(scratch, "meta", "readme.md")) as f:
+            f.write(readme)
 
     def test_build_valid_pkg(self):
         self.use_temp_dir()
@@ -875,3 +879,33 @@
                 "policy_version": 1.3
             }
             """))
+
+    def test_empty_readme(self):
+        # LP: #1400308, case 1
+        self.use_temp_dir()
+        self._make_metadata(self.temp_dir, readme='')
+        with self.assertRaises(ClickBuildError) as cm:
+            self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
+            self.assertEqual(
+                str(cm.exception)[-59:],
+                'meta/package.yaml must contain at least two non-empty lines')
+
+    def test_readme_with_nonempty_blank_first_line(self):
+        # LP: #1400308, case 1; first line is .strip()'d
+        self.use_temp_dir()
+        self._make_metadata(self.temp_dir, readme='          ')
+        with self.assertRaises(ClickBuildError) as cm:
+            self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
+            self.assertEqual(
+                str(cm.exception)[-59:],
+                'meta/package.yaml must contain at least two non-empty lines')
+
+    def test_single_line_readme(self):
+        # LP: #1400308, case 2
+        self.use_temp_dir()
+        self._make_metadata(self.temp_dir, readme='a title\n')
+        with self.assertRaises(ClickBuildError) as cm:
+            self.builder.read_manifest(self.temp_dir + "/meta/package.yaml")
+            self.assertEqual(
+                str(cm.exception)[-59:],
+                'meta/package.yaml must contain at least two non-empty lines')