Codecov showed text/html followed by text/plain not tested. Fixed bug

rouilj · rouilj · commit de728899b402 · 2017-10-13T22:56:42.000-04:00
discovered.

The html part was double attached. Removed an unneeded attachment
call. Added some more test cases. Reworked test code to make selecting
html filter tool easier.
diff --git a/roundup/mailgw.py b/roundup/mailgw.py
@@ -425,8 +425,8 @@ def extract_content(self, parent_type=None, ignore_alternatives=False,
                         attachments.append(part.text_as_attachment())
                     elif html_part_found:
                         # text/plain part found after html
-                        # save html as attachment
-                        attachments.append(cpart.as_attachment())
+                        # text/html already stored as attachment,
+                        # so just use the text as the content.
                         content = new_content
                         cpart   = part
                     else:
diff --git a/test/test_multipart.py b/test/test_multipart.py
@@ -160,11 +160,15 @@ def testMultipart(self):
         p = m.getpart()
         self.assert_(p is None)
 
-    def TestExtraction(self, spec, expected):
-        from roundup.dehtml import dehtml
+    def TestExtraction(self, spec, expected, convert_html_with=False):
+        if convert_html_with:
+            from roundup.dehtml import dehtml
+            html2text=dehtml(convert_html_with).html2text
+        else:
+            html2text=None
 
         self.assertEqual(ExampleMessage(spec).extract_content(
-            html2text=dehtml('dhtml').html2text), expected)
+            html2text=html2text), expected)
 
     def testTextPlain(self):
         self.TestExtraction('text/plain', ('foo\n', [], False))
@@ -186,14 +190,27 @@ def testMultipartMixed(self):
                    [('foo.pdf', 'application/pdf', 'foo\n')], False))
 
     def testMultipartMixedHtml(self):
+        # test with html conversion enabled
         self.TestExtraction("""
 multipart/mixed
     text/html
     application/pdf""",
                   ('bar\n',
                    [('bar.html', 'text/html',
                       '<html><body>bar</body></html>\n'),
-                   ('foo.pdf', 'application/pdf', 'foo\n')], False))
+                   ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with='dehtml')
+
+        # test with html conversion disabled
+        self.TestExtraction("""
+multipart/mixed
+    text/html
+    application/pdf""",
+                  (None,
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with=False)
 
     def testMultipartAlternative(self):
         self.TestExtraction("""
@@ -210,7 +227,69 @@ def testMultipartAlternativeHtml(self):
                   ('bar\n',
                    [('bar.html', 'text/html',
                       '<html><body>bar</body></html>\n'),
-                   ('foo.pdf', 'application/pdf', 'foo\n')], False))
+                   ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with='dehtml')
+
+        self.TestExtraction("""
+multipart/alternative
+    text/html
+    application/pdf""",
+                  (None,
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with=False)
+
+    def testMultipartAlternativeHtmlText(self):
+        # text should take priority over html when html is first
+        self.TestExtraction("""
+multipart/alternative
+    text/html
+    text/plain
+    application/pdf""",
+                  ('foo\n',
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with='dehtml')
+
+        # text should take priority over html when text is first
+        self.TestExtraction("""
+multipart/alternative
+    text/plain
+    text/html
+    application/pdf""",
+                  ('foo\n',
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with='dehtml')
+
+        # text should take priority over html when text is second and
+        # html is disabled
+        self.TestExtraction("""
+multipart/alternative
+    text/html
+    text/plain
+    application/pdf""",
+                  ('foo\n',
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with=False)
+
+        # text should take priority over html when text is first and
+        # html is disabled
+        self.TestExtraction("""
+multipart/alternative
+    text/plain
+    text/html
+    application/pdf""",
+                  ('foo\n',
+                   [('bar.html', 'text/html',
+                      '<html><body>bar</body></html>\n'),
+                    ('foo.pdf', 'application/pdf', 'foo\n')], False),
+                            convert_html_with=False)
 
     def testDeepMultipartAlternative(self):
         self.TestExtraction("""
