launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-more-email-bytes into launchpad:master.

Commit message:
Handle email messages as bytes in more places

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/398901
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-more-email-bytes into launchpad:master.

diff --git a/lib/lp/bugs/mail/tests/test_bug_subscribe.py b/lib/lp/bugs/mail/tests/test_bug_subscribe.py
index 69ebb12..09dfb94 100644
--- a/lib/lp/bugs/mail/tests/test_bug_subscribe.py
+++ b/lib/lp/bugs/mail/tests/test_bug_subscribe.py
@@ -32,9 +32,9 @@ class TestSubscribedBySomeoneElseNotification(TestCaseWithFactory):
             person_subscribed, person_subscribing, suppress_notify=False)
         transaction.commit()
         self.assertEqual(len(stub.test_emails), 1)
-        rationale = 'You have been subscribed to a public bug by Foo Suber'
+        rationale = b'You have been subscribed to a public bug by Foo Suber'
         msg = stub.test_emails[-1][2]
-        self.assertTrue(rationale in msg)
+        self.assertIn(rationale, msg)
 
     def test_suppress_notify_true_does_not_notify(self):
         """Test notifications aren't sent when suppress_notify is True."""
diff --git a/lib/lp/bugs/mail/tests/test_bug_task_assignment.py b/lib/lp/bugs/mail/tests/test_bug_task_assignment.py
index 3e34cdd..cec7c15 100644
--- a/lib/lp/bugs/mail/tests/test_bug_task_assignment.py
+++ b/lib/lp/bugs/mail/tests/test_bug_task_assignment.py
@@ -54,10 +54,9 @@ class TestAssignmentNotification(TestCaseWithFactory):
         transaction.commit()
         self.assertEqual(len(stub.test_emails), 1, 'email not sent')
         rationale = (
-            'Sample Person (name12) has assigned this bug to you for Rebirth')
+            b'Sample Person (name12) has assigned this bug to you for Rebirth')
         msg = stub.test_emails[-1][2]
-        self.assertTrue(rationale in msg,
-                        '%s not in\n%s\n' % (rationale, msg))
+        self.assertIn(rationale, msg)
 
     def test_self_assignee_notification_message(self):
         """Test notification string when a person is assigned a task by
@@ -68,7 +67,7 @@ class TestAssignmentNotification(TestCaseWithFactory):
         transaction.commit()
         self.assertEqual(1, len(stub.test_emails))
         rationale = (
-            'You have assigned this bug to yourself for Rebirth')
+            b'You have assigned this bug to yourself for Rebirth')
         [email] = stub.test_emails
         # Actual message is part 2 of the email.
         msg = email[2]
@@ -82,10 +81,9 @@ class TestAssignmentNotification(TestCaseWithFactory):
             self.bug_task.transitionToAssignee(self.person_assigned)
         transaction.commit()
         self.assertEqual(len(stub.test_emails), 1, 'email not sent')
-        new_message = '[NEW]'
+        new_message = b'[NEW]'
         msg = stub.test_emails[-1][2]
-        self.assertTrue(new_message in msg,
-                        '%s not in \n%s\n' % (new_message, msg))
+        self.assertIn(new_message, msg)
 
     def test_assignee_new_subscriber(self):
         """Build a list of people who will receive emails about the bug
diff --git a/lib/lp/bugs/stories/bugs/bug-add-subscriber.txt b/lib/lp/bugs/stories/bugs/bug-add-subscriber.txt
index 31783eb..752ccb9 100644
--- a/lib/lp/bugs/stories/bugs/bug-add-subscriber.txt
+++ b/lib/lp/bugs/stories/bugs/bug-add-subscriber.txt
@@ -76,7 +76,7 @@ David got a notification, saying that he was subscribed to the bug.
     >>> from lp.services.mail import stub
     >>> len(stub.test_emails)
     1
-    >>> print(stub.test_emails[0][2])
+    >>> print(six.ensure_text(stub.test_emails[0][2]))
     MIME-Version: 1.0
     ...
     To: david.allouche@xxxxxxxxxxxxx
diff --git a/lib/lp/registry/browser/tests/test_peoplemerge.py b/lib/lp/registry/browser/tests/test_peoplemerge.py
index a2ea2b6..3eec0d6 100644
--- a/lib/lp/registry/browser/tests/test_peoplemerge.py
+++ b/lib/lp/registry/browser/tests/test_peoplemerge.py
@@ -4,6 +4,7 @@
 
 __metaclass__ = type
 
+import six
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -105,8 +106,10 @@ class TestRequestPeopleMergeMultipleEmails(RequestPeopleMergeMixin):
         self.assertEqual('bounces@xxxxxxxxxxxxx', from_addr2)
         self.assertEqual(['foo@xxxxxxx'], to_addrs1)
         self.assertEqual(['bar.foo@xxxxxxxxxxxxx'], to_addrs2)
-        self.assertIn('Launchpad: request to merge accounts', raw_msg1)
-        self.assertIn('Launchpad: request to merge accounts', raw_msg2)
+        self.assertIn(
+            'Launchpad: request to merge accounts', six.ensure_text(raw_msg1))
+        self.assertIn(
+            'Launchpad: request to merge accounts', six.ensure_text(raw_msg2))
 
     def _assert_validation_email_confirm(self):
         # Test that the user can go to the page we sent a link via email to
diff --git a/lib/lp/registry/stories/person/xx-approve-members.txt b/lib/lp/registry/stories/person/xx-approve-members.txt
index 2dd23c6..c88e3ec 100644
--- a/lib/lp/registry/stories/person/xx-approve-members.txt
+++ b/lib/lp/registry/stories/person/xx-approve-members.txt
@@ -42,7 +42,7 @@ Person's. A comment is also sent to the applying users and the team admins.
     ['mark@xxxxxxxxxxx']
     ['mark@xxxxxxxxxxx']
     ['test@xxxxxxxxxxxxx']
-    >>> print(raw_msg)
+    >>> print(six.ensure_text(raw_msg))
     Content-Type: text/plain; charset="utf-8"
     ...
     Mark Shuttleworth said:
diff --git a/lib/lp/registry/stories/person/xx-user-to-user.txt b/lib/lp/registry/stories/person/xx-user-to-user.txt
index c035e1d..cb595cc 100644
--- a/lib/lp/registry/stories/person/xx-user-to-user.txt
+++ b/lib/lp/registry/stories/person/xx-user-to-user.txt
@@ -31,7 +31,7 @@ Salgado receives the email message from No Priv.
     1
     >>> print(to_addrs[0])
     Guilherme Salgado <guilherme.salgado@xxxxxxxxxxxxx>
-    >>> print(raw_msg)
+    >>> print(six.ensure_text(raw_msg))
     Content-Type: text/plain; charset="us-ascii"
     MIME-Version: 1.0
     Content-Transfer-Encoding: 7bit
@@ -74,7 +74,7 @@ their alternative addresses.
     >>> browser.getControl('Send').click()
 
     >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop()
-    >>> print(raw_msg)
+    >>> print(six.ensure_text(raw_msg))
     Content-Type: text/plain; charset="us-ascii"
     MIME-Version: 1.0
     Content-Transfer-Encoding: 7bit
diff --git a/lib/lp/services/mail/mailbox.py b/lib/lp/services/mail/mailbox.py
index b6650ad..7bc3c4b 100644
--- a/lib/lp/services/mail/mailbox.py
+++ b/lib/lp/services/mail/mailbox.py
@@ -142,7 +142,7 @@ class POP3MailBox:
 
         for msg_id in range(1, count + 1):
             response, msg_lines, size = popbox.retr(msg_id)
-            yield (msg_id, '\n'.join(msg_lines))
+            yield (msg_id, b'\n'.join(msg_lines))
 
     def delete(self, id):
         """See IMailBox."""
@@ -171,7 +171,7 @@ class DirectoryMailBox:
         """See IMailBox."""
         for entry in scandir.scandir(self.mail_dir):
             if entry.is_file():
-                with open(entry.path) as mail_file:
+                with open(entry.path, "rb") as mail_file:
                     yield (entry.path, mail_file.read())
 
     def delete(self, id):
diff --git a/lib/lp/services/mail/tests/test_mailbox.py b/lib/lp/services/mail/tests/test_mailbox.py
index 753ca7a..645508a 100644
--- a/lib/lp/services/mail/tests/test_mailbox.py
+++ b/lib/lp/services/mail/tests/test_mailbox.py
@@ -50,7 +50,7 @@ class TestDirectoryMailBox(TestCase):
         box = DirectoryMailBox(self.email_dir)
         [mail] = list(box.items())
         self.assertEqual('foo', os.path.basename(mail[0]))
-        self.assertEqual('This is the content', mail[1])
+        self.assertEqual(b'This is the content', mail[1])
 
     def test_delete(self):
         # Deleting the item removes the file from the mail dir.