launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19306
[Merge] lp:~cjwatson/launchpad/more-pop-notifications into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/more-pop-notifications into lp:launchpad.
Commit message:
Use pop_notifications in more places to avoid fragile patterns with stub.test_emails, and introduce an assertEmailQueueLength helper.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/more-pop-notifications/+merge/270371
This is kind of yak-shaving. I've been making some progress on https://bugs.launchpad.net/launchpad/+bug/29744, but in order to turn off immediate mail delivery I need to make sure that doing so isn't going to hide test problems. A number of tests do something like "code_under_test(); self.assertEqual([], stub.test_emails);", and not all of them make sure to commit beforehand. With immediate mail delivery disabled, this could hide bugs because test_emails could be empty until the transaction is committed. The pop_notifications interface is in most situations better (it commits the transaction first by default, and it saves you having to clear test_emails yourself).
I've gone through the cases where test_emails was used directly and converted most of them to pop_notifications or something based on it. This branch contains only changes to files that contained unsafe uses of test_emails; the rest can come later.
Checking the length of the mail queue is a common pattern in tests, so I introduced an assertEmailQueueLength helper for this.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/more-pop-notifications into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py'
--- lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2014-10-29 06:04:09 +0000
+++ lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2015-09-08 09:46:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the generate_ppa_htaccess.py script. """
@@ -33,7 +33,6 @@
from lp.registry.interfaces.teammembership import TeamMembershipStatus
from lp.services.config import config
from lp.services.log.logger import BufferLogger
-from lp.services.mail import stub
from lp.services.osutils import (
ensure_directory_exists,
remove_if_exists,
@@ -281,9 +280,7 @@
self.assertNotDeactivated(tokens[person])
# Ensure that a cancellation email was sent.
- num_emails = len(stub.test_emails)
- self.assertEqual(
- num_emails, 1, "Expected 1 email, got %s" % num_emails)
+ self.assertEmailQueueLength(1)
# Promiscuous_person now leaves team1, but does not lose his
# token because he's also in team2. No other tokens are
@@ -298,9 +295,7 @@
self.assertNotDeactivated(tokens[person])
# Ensure that a cancellation email was not sent.
- num_emails = len(stub.test_emails)
- self.assertEqual(
- num_emails, 0, "Expected no emails, got %s" % num_emails)
+ self.assertEmailQueueLength(0)
# Team 2 now leaves parent_team, and all its members lose their
# tokens.
@@ -486,10 +481,6 @@
script.sendCancellationEmail(tokens[0])
- num_emails = len(stub.test_emails)
- self.assertEqual(
- num_emails, 1, "Expected 1 email, got %s" % num_emails)
-
[email] = pop_notifications()
self.assertEqual(
email['Subject'],
@@ -537,9 +528,7 @@
script.sendCancellationEmail(token)
- num_emails = len(stub.test_emails)
- self.assertEqual(
- num_emails, 0, "Expected 0 emails, got %s" % num_emails)
+ self.assertEmailQueueLength(0)
def test_getTimeToSyncFrom(self):
# Sync from 1s before previous start to catch anything made during the
=== modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2015-08-26 13:41:21 +0000
+++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2015-09-08 09:46:54 +0000
@@ -9,7 +9,7 @@
__metaclass__ = type
-from email import message_from_string
+from operator import itemgetter
import os
import shutil
@@ -27,7 +27,6 @@
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.librarian.interfaces import ILibraryFileAliasSet
-from lp.services.mail import stub
from lp.soyuz.enums import (
PackagePublishingStatus,
PackageUploadStatus,
@@ -43,6 +42,7 @@
from lp.soyuz.tests.fakepackager import FakePackager
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing.dbuser import switch_dbuser
+from lp.testing.mail_helpers import pop_notifications
class TestPPAUploadProcessorBase(TestUploadProcessorBase):
@@ -87,20 +87,19 @@
"X-Launchpad-PPA" header; it defaults to "name16" and should be
explicitly set to None for non-PPA or rejection notifications.
"""
- for item in expected:
+ notifications = self.assertEmailQueueLength(
+ len(expected), sort_key=itemgetter('X-Envelope-To'))
+
+ for item, msg in zip(expected, notifications):
recipient = item.get("recipient", self.name16_recipient)
contents = item.get("contents", [])
ppa_header = item.get("ppa_header", "name16")
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
-
# This is now a non-multipart message.
self.assertFalse(msg.is_multipart())
body = msg.get_payload(decode=True)
- clean_recipients = [r.strip() for r in to_addrs]
- self.assertContentEqual([recipient], clean_recipients)
+ self.assertEqual(recipient, msg['X-Envelope-To'])
subject = "Subject: %s\n" % msg['Subject']
body = subject + body
@@ -112,10 +111,6 @@
self.assertIn('X-Launchpad-PPA', msg.keys())
self.assertEqual(msg['X-Launchpad-PPA'], ppa_header)
- self.assertEqual(
- [], stub.test_emails,
- "%d emails left over" % len(stub.test_emails))
-
def checkFilesRestrictedInLibrarian(self, queue_item, condition):
"""Check the libraryfilealias restricted flag.
@@ -300,8 +295,8 @@
self.assertEqual(
self.uploadprocessor.last_processed_upload.queue_root.status,
PackageUploadStatus.DONE)
- # Consume the test email so the assertion futher down does not fail.
- _from_addr, _to_addrs, _raw_msg = stub.test_emails.pop()
+ # Consume the test email so the assertion further down does not fail.
+ pop_notifications()
# The SourcePackageRelease still has a component of universe:
pub_foo = self.name16.archive.getPublishedSources(name=u"bar").one()
@@ -320,8 +315,7 @@
self.build_uploadprocessor, upload_dir, build=build)
# No mails are sent for successful binary uploads.
- self.assertEqual(len(stub.test_emails), 0,
- "Unexpected email generated on binary upload.")
+ self.assertEmailQueueLength(0)
# Publish the binary.
[queue_item] = self.breezy.getPackageUploads(
@@ -477,7 +471,7 @@
name12.preferredemail.email)
self.assertEmails([
{"ppa_header": "cprov", "recipient": expected_recipient}
- for expected_recipient in reversed(sorted(expected_recipients))])
+ for expected_recipient in sorted(expected_recipients)])
def testPPADistroSeriesOverrides(self):
"""It's possible to override target distroseries of PPA uploads.
@@ -850,9 +844,7 @@
# Please note: this upload goes to the Ubuntu main archive.
upload_dir = self.queueUpload("bar_1.0-10")
self.processUpload(self.uploadprocessor, upload_dir)
- # Discard the announcement email and check the acceptance message
- # content.
- stub.test_emails.pop()
+ pop_notifications()
self.assertEqual(
self.uploadprocessor.last_processed_upload.queue_root.status,
@@ -991,9 +983,9 @@
u'Files specified in DSC are broken or missing, skipping package '
u'unpack verification.')
- # Also, the email generated should be sane.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
+ # Also, the email generated should be sane. Any of the multiple
+ # notifications will do.
+ msg = pop_notifications()[-1]
body = msg.get_payload(decode=True)
self.assertTrue(
@@ -1015,8 +1007,7 @@
self.processUpload(self.uploadprocessor, upload_dir)
# The email generated should be sane.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
+ [msg] = pop_notifications()
body = msg.get_payload(decode=True)
self.assertTrue(
@@ -1148,7 +1139,7 @@
queue_item.setAccepted()
queue_item.realiseUpload()
transaction.commit()
- stub.test_emails.pop()
+ pop_notifications()
# Now upload a 3.0 (quilt) source with missing orig*.tar.* to a
# PPA. All of the missing files will be retrieved from the
=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2015-08-25 14:05:24 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2015-09-08 09:46:54 +0000
@@ -9,7 +9,6 @@
"TestUploadProcessorBase",
]
-from email import message_from_string
import os
import shutil
from StringIO import StringIO
@@ -65,7 +64,6 @@
BufferLogger,
DevNullLogger,
)
-from lp.services.mail import stub
from lp.soyuz.enums import (
ArchivePermissionType,
ArchivePurpose,
@@ -357,12 +355,16 @@
over after checking the ones in expected.
"""
- for item in expected:
+ if allow_leftover:
+ notifications = pop_notifications()
+ self.assertTrue(len(notifications) >= len(expected))
+ else:
+ notifications = self.assertEmailQueueLength(len(expected))
+
+ for item, msg in zip(expected, notifications):
recipient = item.get("recipient", self.name16_recipient)
contents = item.get("contents", [])
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
# This is now a MIMEMultipart message.
body = msg.get_payload(0)
body = body.get_payload(decode=True)
@@ -370,8 +372,7 @@
# Only check the recipient if the caller didn't explicitly pass
# "recipient": None.
if recipient is not None:
- clean_recipients = [r.strip() for r in to_addrs]
- self.assertContentEqual([recipient], clean_recipients)
+ self.assertEqual(recipient, msg['X-Envelope-To'])
subject = "Subject: %s\n" % msg['Subject']
body = subject + body
@@ -381,11 +382,6 @@
content in body,
"Expect: '%s'\nGot:\n%s" % (content, body))
- if not allow_leftover:
- self.assertEqual(
- [], stub.test_emails,
- "%d emails left over" % len(stub.test_emails))
-
def PGPSignatureNotPreserved(self, archive=None):
"""PGP signatures should be removed from .changes files.
@@ -423,13 +419,11 @@
def _checkPartnerUploadEmailSuccess(self):
"""Ensure partner uploads generate the right email."""
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- foo_bar = "foo.bar@xxxxxxxxxxxxx"
- self.assertEqual([e.strip() for e in to_addrs], [foo_bar])
+ [msg] = pop_notifications()
+ self.assertEqual("foo.bar@xxxxxxxxxxxxx", msg["X-Envelope-To"])
self.assertTrue(
- "rejected" not in raw_msg,
- "Expected acceptance email not rejection. Actually Got:\n%s"
- % raw_msg)
+ "rejected" not in str(msg),
+ "Expected acceptance email not rejection. Actually Got:\n%s" % msg)
def testInstantiate(self):
"""UploadProcessor should instantiate"""
@@ -563,13 +557,13 @@
self.processUpload(uploadprocessor, upload_dir)
# Check the mailer stub has a rejection email for Daniel
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+ [msg] = pop_notifications()
# This is now a MIMEMultipart message.
- msg = message_from_string(raw_msg)
body = msg.get_payload(0)
body = body.get_payload(decode=True)
- self.assertEqual(["daniel.silverstone@xxxxxxxxxxxxx"], to_addrs)
+ self.assertEqual(
+ "daniel.silverstone@xxxxxxxxxxxxx", msg["X-Envelope-To"])
self.assertTrue("Unhandled exception processing upload: Exception "
"raised by BrokenUploadPolicy for testing."
in body)
@@ -599,15 +593,14 @@
self.processUpload(uploadprocessor, upload_dir)
# Check it went ok to the NEW queue and all is going well so far.
- foo_bar = "foo.bar@xxxxxxxxxxxxx"
daniel = "daniel.silverstone@xxxxxxxxxxxxx"
- for expected_to_addr in foo_bar, daniel:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- to_addrs = [e.strip() for e in to_addrs]
- self.assertContentEqual(to_addrs, [expected_to_addr])
+ foo_bar = "foo.bar@xxxxxxxxxxxxx"
+ notifications = self.assertEmailQueueLength(2)
+ for expected_to_addr, msg in zip((daniel, foo_bar), notifications):
+ self.assertEqual(expected_to_addr, msg["X-Envelope-To"])
self.assertTrue(
- "NEW" in raw_msg, "Expected email containing 'NEW', got:\n%s"
- % raw_msg)
+ "NEW" in str(msg),
+ "Expected email containing 'NEW', got:\n%s" % msg)
# Accept and publish the upload.
# This is required so that the next upload of a later version of
@@ -635,13 +628,12 @@
self.processUpload(uploadprocessor, upload_dir)
# Verify we get an email talking about awaiting approval.
- for expected_to_addr in foo_bar, daniel:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- to_addrs = [e.strip() for e in to_addrs]
- self.assertContentEqual(to_addrs, [expected_to_addr])
- self.assertTrue("Waiting for approval" in raw_msg,
- "Expected an 'upload awaits approval' email.\n"
- "Got:\n%s" % raw_msg)
+ notifications = self.assertEmailQueueLength(2)
+ for expected_to_addr, msg in zip((daniel, foo_bar), notifications):
+ self.assertEqual(expected_to_addr, msg["X-Envelope-To"])
+ self.assertTrue(
+ "Waiting for approval" in str(msg),
+ "Expected an 'upload awaits approval' email.\nGot:\n%s" % msg)
# And verify that the queue item is in the unapproved state.
queue_items = self.breezy.getPackageUploads(
@@ -918,10 +910,10 @@
self.processUpload(uploadprocessor, upload_dir)
# Check that it was rejected appropriately.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "Partner archive for distro '%s' not found" % self.ubuntu.name
- in raw_msg)
+ [msg] = pop_notifications()
+ self.assertIn(
+ "Partner archive for distro '%s' not found" % self.ubuntu.name,
+ str(msg))
def testMixedPartnerUploadFails(self):
"""Uploads with partner and non-partner files are rejected.
@@ -937,13 +929,12 @@
self.processUpload(uploadprocessor, upload_dir)
# Check that it was rejected.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- foo_bar = "foo.bar@xxxxxxxxxxxxx"
- self.assertEqual([e.strip() for e in to_addrs], [foo_bar])
+ [msg] = pop_notifications()
+ self.assertEqual("foo.bar@xxxxxxxxxxxxx", msg["X-Envelope-To"])
self.assertTrue(
- "Cannot mix partner files with non-partner." in raw_msg,
+ "Cannot mix partner files with non-partner." in str(msg),
"Expected email containing 'Cannot mix partner files with "
- "non-partner.', got:\n%s" % raw_msg)
+ "non-partner.', got:\n%s" % msg)
def testPartnerReusingOrigFromPartner(self):
"""Partner uploads reuse 'orig.tar.gz' from the partner archive."""
@@ -977,8 +968,7 @@
self.processUpload(uploadprocessor, upload_dir)
# Discard the announcement email and check the acceptance message
# content.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
+ msg = pop_notifications()[-1]
# This is now a MIMEMultipart message.
body = msg.get_payload(0)
body = body.get_payload(decode=True)
@@ -1095,11 +1085,10 @@
self.processUpload(uploadprocessor, upload_dir)
# Check it went ok to the NEW queue and all is going well so far.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+ [msg] = pop_notifications()
self.assertTrue(
- "NEW" in raw_msg,
- "Expected email containing 'NEW', got:\n%s"
- % raw_msg)
+ "NEW" in str(msg),
+ "Expected email containing 'NEW', got:\n%s" % msg)
# Accept and publish the upload.
partner_archive = getUtility(IArchiveSet).getByDistroPurpose(
@@ -1198,10 +1187,10 @@
# Check it is rejected.
expect_msg = ("Partner uploads must be for the RELEASE or "
"PROPOSED pocket.")
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+ [msg] = pop_notifications()
self.assertTrue(
- expect_msg in raw_msg,
- "Expected email with %s, got:\n%s" % (expect_msg, raw_msg))
+ expect_msg in str(msg),
+ "Expected email with %s, got:\n%s" % (expect_msg, msg))
# And an oops should be filed for the error.
error_report = self.oopses[new_index]
@@ -1428,10 +1417,9 @@
upload_dir = self.queueUpload("bar_1.0-1_lzma")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it went ok:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "rejected" not in raw_msg,
- "Failed to upload bar source:\n%s" % raw_msg)
+ [msg] = pop_notifications()
+ self.assertFalse(
+ "rejected" in str(msg), "Failed to upload bar source:\n%s" % msg)
self.publishPackage("bar", "1.0-1")
# Clear out emails generated during upload.
pop_notifications()
@@ -1441,11 +1429,7 @@
self.processUpload(uploadprocessor, upload_dir)
# Successful binary uploads won't generate any email.
- if len(stub.test_emails) != 0:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertEqual(
- len(stub.test_emails), 0,
- "Expected no emails! Actually got:\n%s" % raw_msg)
+ self.assertEmailQueueLength(0)
# Check in the queue to see if it really made it:
queue_items = self.breezy.getPackageUploads(
@@ -1472,10 +1456,9 @@
upload_dir = self.queueUpload("bar_1.0-1_xz")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it went ok:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "rejected" not in raw_msg,
- "Failed to upload bar source:\n%s" % raw_msg)
+ [msg] = pop_notifications()
+ self.assertFalse(
+ "rejected" in str(msg), "Failed to upload bar source:\n%s" % msg)
self.publishPackage("bar", "1.0-1")
# Clear out emails generated during upload.
pop_notifications()
@@ -1485,11 +1468,7 @@
self.processUpload(uploadprocessor, upload_dir)
# Successful binary uploads won't generate any email.
- if len(stub.test_emails) != 0:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertEqual(
- len(stub.test_emails), 0,
- "Expected no emails! Actually got:\n%s" % raw_msg)
+ self.assertEmailQueueLength(0)
# Check in the queue to see if it really made it:
queue_items = self.breezy.getPackageUploads(
@@ -1683,8 +1662,8 @@
self.processUpload(uploadprocessor, upload_dir)
# Check that it failed (permissions were granted for wrong series).
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- msg = message_from_string(raw_msg)
+ # Any of the multiple notifications will do.
+ msg = pop_notifications()[-1]
self.assertEqual(
msg['Subject'], '[ubuntu] bar_1.0-2_source.changes (Rejected)')
@@ -1747,16 +1726,16 @@
expected = []
expected.append({
"contents": base_contents + [
- "You are receiving this email because you made this upload."],
- "recipient": "foo.bar@xxxxxxxxxxxxx",
- })
- expected.append({
- "contents": base_contents + [
"You are receiving this email because you are the most "
"recent person",
"listed in this package's changelog."],
"recipient": "daniel.silverstone@xxxxxxxxxxxxx",
})
+ expected.append({
+ "contents": base_contents + [
+ "You are receiving this email because you made this upload."],
+ "recipient": "foo.bar@xxxxxxxxxxxxx",
+ })
self.assertEmails(expected)
def test30QuiltUploadToUnsupportingSeriesIsRejected(self):
@@ -1774,11 +1753,11 @@
upload_dir = self.queueUpload("bar_1.0-1_3.0-quilt")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it was rejected.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+ [msg] = pop_notifications()
self.assertTrue(
"bar_1.0-1.dsc: format '3.0 (quilt)' is not permitted in "
- "breezy." in raw_msg,
- "Source was not rejected properly:\n%s" % raw_msg)
+ "breezy." in str(msg),
+ "Source was not rejected properly:\n%s" % msg)
def test30QuiltUpload(self):
"""Ensure that 3.0 (quilt) uploads work properly. """
@@ -1792,10 +1771,9 @@
upload_dir = self.queueUpload("bar_1.0-1_3.0-quilt")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it went ok:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "rejected" not in raw_msg,
- "Failed to upload bar source:\n%s" % raw_msg)
+ [msg] = pop_notifications()
+ self.assertFalse(
+ "rejected" in str(msg), "Failed to upload bar source:\n%s" % msg)
spph = self.publishPackage("bar", "1.0-1")
self.assertEquals(
@@ -1827,10 +1805,9 @@
upload_dir = self.queueUpload("bar_1.0-1_3.0-quilt")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it went ok:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "rejected" not in raw_msg,
- "Failed to upload bar source:\n%s" % raw_msg)
+ [msg] = pop_notifications()
+ self.assertFalse(
+ "rejected" in str(msg), "Failed to upload bar source:\n%s" % msg)
self.publishPackage("bar", "1.0-1")
# Upload another source sharing the same (component) orig.
@@ -1865,10 +1842,9 @@
upload_dir = self.queueUpload("bar_1.0_3.0-native")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it went ok:
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
- self.assertTrue(
- "rejected" not in raw_msg,
- "Failed to upload bar source:\n%s" % raw_msg)
+ [msg] = pop_notifications()
+ self.assertFalse(
+ "rejected" in str(msg), "Failed to upload bar source:\n%s" % msg)
spph = self.publishPackage("bar", "1.0")
self.assertEquals(
@@ -1890,11 +1866,11 @@
upload_dir = self.queueUpload("bar_1.0-1_1.0-bzip2")
self.processUpload(uploadprocessor, upload_dir)
# Make sure it was rejected.
- from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+ [msg] = pop_notifications()
self.assertTrue(
"bar_1.0-1.dsc: is format 1.0 but uses bzip2 compression."
- in raw_msg,
- "Source was not rejected properly:\n%s" % raw_msg)
+ in str(msg),
+ "Source was not rejected properly:\n%s" % msg)
def testUploadToWrongPocketIsRejected(self):
# Uploads to the wrong pocket are rejected.
@@ -1922,16 +1898,16 @@
expected = []
expected.append({
"contents": base_contents + [
- "You are receiving this email because you made this upload."],
- "recipient": "foo.bar@xxxxxxxxxxxxx",
- })
- expected.append({
- "contents": base_contents + [
"You are receiving this email because you are the most "
"recent person",
"listed in this package's changelog."],
"recipient": "daniel.silverstone@xxxxxxxxxxxxx",
})
+ expected.append({
+ "contents": base_contents + [
+ "You are receiving this email because you made this upload."],
+ "recipient": "foo.bar@xxxxxxxxxxxxx",
+ })
self.assertEmails(expected)
def testPGPSignatureNotPreserved(self):
@@ -1969,7 +1945,7 @@
self.assertEqual(UploadStatusEnum.REJECTED, result)
self.assertLogContains(
"INFO Failed to parse changes file")
- self.assertEqual(len(stub.test_emails), 0)
+ self.assertEmailQueueLength(0)
self.assertEqual([], self.oopses)
def test_ddeb_upload_overrides(self):
@@ -2137,13 +2113,13 @@
queue_entry=leaf_name)
self.options.context = 'buildd'
self.options.builds = True
- last_stub_mail_count = len(stub.test_emails)
+ pop_notifications()
BuildUploadHandler(self.uploadprocessor, self.incoming_folder,
leaf_name).process()
self.layer.txn.commit()
# No emails are sent on success
- self.assertEquals(len(stub.test_emails), last_stub_mail_count)
- self.assertEquals(BuildStatus.FULLYBUILT, build.status)
+ self.assertEmailQueueLength(0)
+ self.assertEqual(BuildStatus.FULLYBUILT, build.status)
# Upon full build the upload log is unset.
self.assertIs(None, build.upload_log)
@@ -2192,7 +2168,7 @@
See bug 778437.
"""
self.doSuccessRecipeBuild()
- self.assertEquals(len(pop_notifications()), 0, pop_notifications)
+ self.assertEmailQueueLength(0)
def doFailureRecipeBuild(self):
# A source package recipe build will fail if no files are present.
@@ -2284,7 +2260,7 @@
status=PackageUploadStatus.ACCEPTED,
version=u"1.0-1", name=u"bar")
queue_item.setDone()
- stub.test_emails = []
+ pop_notifications()
build.buildqueue_record.markAsBuilding(self.factory.makeBuilder())
build.updateStatus(status)
@@ -2300,7 +2276,7 @@
"bar_1.0-1_binary", queue_entry=leaf_name)
self.options.context = 'buildd'
self.options.builds = True
- self.assertEqual([], stub.test_emails)
+ self.assertEmailQueueLength(0)
BuildUploadHandler(self.uploadprocessor, self.incoming_folder,
leaf_name).process()
self.layer.txn.commit()
=== modified file 'lib/lp/bugs/doc/externalbugtracker-debbugs.txt'
--- lib/lp/bugs/doc/externalbugtracker-debbugs.txt 2015-03-13 19:05:50 +0000
+++ lib/lp/bugs/doc/externalbugtracker-debbugs.txt 2015-09-08 09:46:54 +0000
@@ -596,15 +596,15 @@
... '<123456@xxxxxxxxxxxxx>')
'<123456@xxxxxxxxxxxxx>'
-We can look in the test_emails list for the mail that would have been
-sent.
-
- >>> from lp.services.mail import stub
- >>> recipent, to_addrs, comment_email = stub.test_emails.pop()
- >>> print to_addrs
- ['1234@xxxxxxxxxxx']
-
- >>> print comment_email
+We can look for the mail that would have been sent.
+
+ >>> from lp.testing.mail_helpers import pop_notifications
+ >>> [msg] = pop_notifications()
+ >>> print msg['X-Envelope-To']
+ 1234@xxxxxxxxxxx
+
+ >>> print str(msg)
+ From ...
Content-Type...
Message-Id: <123456@xxxxxxxxxxxxx>
To: 1234@xxxxxxxxxxx
=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
--- lib/lp/bugs/mail/tests/test_handler.py 2012-09-17 16:13:40 +0000
+++ lib/lp/bugs/mail/tests/test_handler.py 2015-09-08 09:46:54 +0000
@@ -1,11 +1,10 @@
-# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test MaloneHandler."""
__metaclass__ = type
-import email
import time
import transaction
@@ -32,7 +31,6 @@
from lp.registry.enums import BugSharingPolicy
from lp.services.config import config
from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
-from lp.services.mail import stub
from lp.services.webapp.authorization import LaunchpadSecurityPolicy
from lp.testing import (
celebrity_logged_in,
@@ -124,12 +122,11 @@
response = handler.extractAndAuthenticateCommands(
message, 'help@xxxxxxxxxxxxxxxxxx')
mail_handled, add_comment_to_bug, commands = response
- self.assertEquals(mail_handled, True)
- emails = self.getSentMail()
- self.assertEquals(1, len(emails))
- self.assertEquals(['non@xxxxxx'], emails[0][1])
- self.assertTrue(
- 'Subject: Launchpad Bug Tracker Email Interface' in emails[0][2])
+ self.assertTrue(mail_handled)
+ emails = self.assertEmailQueueLength(1)
+ self.assertEqual('non@xxxxxx', emails[0]['X-Envelope-To'])
+ self.assertEqual(
+ 'Launchpad Bug Tracker Email Interface Help', emails[0]['Subject'])
def test_mailToHelpFromUnknownUser(self):
"""Mail from people of no account to help@ is simply dropped.
@@ -140,8 +137,8 @@
mail_handled, add_comment_to_bug, commands = \
handler.extractAndAuthenticateCommands(message,
'help@xxxxxxxxxxxxxxxxxx')
- self.assertEquals(mail_handled, True)
- self.assertEquals(self.getSentMail(), [])
+ self.assertTrue(mail_handled)
+ self.assertEmailQueueLength(0)
def test_mailToHelp(self):
"""Mail to help@ generates a help command."""
@@ -152,19 +149,11 @@
mail_handled, add_comment_to_bug, commands = \
handler.extractAndAuthenticateCommands(message,
'help@xxxxxxxxxxxxxxxxxx')
- self.assertEquals(mail_handled, True)
- emails = self.getSentMail()
- self.assertEquals(1, len(emails))
- self.assertEquals([message['From']], emails[0][1])
- self.assertTrue(
- 'Subject: Launchpad Bug Tracker Email Interface' in emails[0][2])
-
- def getSentMail(self):
- # Sending mail is (unfortunately) a side effect of parsing the
- # commands, and unfortunately you must commit the transaction to get
- # them sent.
- transaction.commit()
- return stub.test_emails[:]
+ self.assertEqual(mail_handled, True)
+ emails = self.assertEmailQueueLength(1)
+ self.assertEqual(message['From'], emails[0]['X-Envelope-To'])
+ self.assertEqual(
+ 'Launchpad Bug Tracker Email Interface Help', emails[0]['Subject'])
def getFailureForMessage(self, to_address, from_address=None, body=None):
mail = self.factory.makeSignedMessage(
@@ -683,22 +672,6 @@
self.timestamp = timestamp
-def get_last_email():
- from_addr, to_addrs, raw_message = stub.test_emails[-1]
- sent_msg = email.message_from_string(raw_message)
- error_mail, original_mail = sent_msg.get_payload()
- # clear the emails so we don't accidentally get one from a previous test
- return dict(
- subject=sent_msg['Subject'],
- body=error_mail.get_payload(decode=True))
-
-
-BAD_SIGNATURE_TIMESTAMP_MESSAGE = (
- 'The message you sent included commands to modify the bug '
- 'report, but the\nsignature was (apparently) generated too far '
- 'in the past or future.')
-
-
class TestSignatureTimestampValidation(TestCaseWithFactory):
"""GPG signature timestamps are checked for emails containing commands."""
@@ -715,10 +688,9 @@
handler = MaloneHandler()
with person_logged_in(self.factory.makePerson()):
handler.process(msg, msg['To'])
- transaction.commit()
# Since there were no commands in the poorly-timestamped message, no
# error emails were generated.
- self.assertEqual(stub.test_emails, [])
+ self.assertEmailQueueLength(0)
def test_bad_timestamp_but_no_commands(self):
# If an email message's GPG signature's timestamp is too far in the
@@ -732,10 +704,9 @@
msg.signature = FakeSignature(timestamp=now + one_week)
handler = MaloneHandler()
# Clear old emails before potentially generating more.
- del stub.test_emails[:]
+ pop_notifications()
with person_logged_in(self.factory.makePerson()):
handler.process(msg, msg['To'])
- transaction.commit()
# Since there were no commands in the poorly-timestamped message, no
# error emails were generated.
- self.assertEqual(stub.test_emails, [])
+ self.assertEmailQueueLength(0)
=== modified file 'lib/lp/services/mail/tests/incomingmail.txt'
--- lib/lp/services/mail/tests/incomingmail.txt 2015-07-08 16:05:11 +0000
+++ lib/lp/services/mail/tests/incomingmail.txt 2015-09-08 09:46:54 +0000
@@ -121,8 +121,9 @@
wasn't a handler registered for that domain, an OOPS was recorded with
a link to the original message.
- >>> from lp.services.mail import stub
- >>> print stub.test_emails[-1][2]
+ >>> from lp.testing.mail_helpers import pop_notifications
+ >>> print str(pop_notifications()[-1])
+ From ...
Content-Type: multipart/mixed...
...
To: Sample Person <test@xxxxxxxxxxxxx>
@@ -140,8 +141,6 @@
Subject: Signed Email
...
- >>> stub.test_emails = []
-
---------------------------------------------
Mail from Persons not registered in Launchpad
---------------------------------------------
@@ -156,7 +155,7 @@
>>> msgid not in foo_handler.handledMails
True
- >>> stub.test_emails = []
+ >>> _ = pop_notifications()
However, bar_handler specifies that it can handle such emails:
@@ -171,7 +170,7 @@
>>> msgid in bar_handler.handledMails
True
- >>> stub.test_emails = []
+ >>> _ = pop_notifications()
---------------------------------------------------------
Mail from Persons with with an inactive Launchpad account
@@ -260,7 +259,8 @@
An exception is raised, an OOPS is recorded, and an email is sent back
to the user, citing the OOPS ID, with the original message attached.
- >>> print stub.test_emails[-1][2]
+ >>> print str(pop_notifications()[-1])
+ From ...
Content-Type: multipart/mixed...
...
To: Foo Bar <foo.bar@xxxxxxxxxxxxx>
@@ -279,8 +279,6 @@
Subject: doesn't matter
...
- >>> stub.test_emails = []
-
Unauthorized exceptions, which are ignored for the purpose of OOPS
reporting in the web interface, are not ignored in the email interface.
@@ -305,7 +303,8 @@
...
Unauthorized
- >>> print stub.test_emails[-1][2]
+ >>> print str(pop_notifications()[-1])
+ From ...
Content-Type: multipart/mixed...
...
To: Foo Bar <foo.bar@xxxxxxxxxxxxx>
@@ -324,8 +323,6 @@
Subject: doesn't matter
...
- >>> stub.test_emails = []
-
-------------
DB exceptions
-------------
@@ -382,6 +379,7 @@
There is only one mail left in the mail box - the one sent back to
the user reporting the error:
+ >>> from lp.services.mail import stub
>>> len(stub.test_emails)
1
@@ -412,7 +410,7 @@
2
>>> LibrarianLayer.reveal()
- >>> stub.test_emails = []
+ >>> _ = pop_notifications()
----------------
Handling bounces
@@ -425,7 +423,6 @@
Emails with an empty Return-Path header should be dropped:
- >>> stub.test_emails = []
>>> msg = read_test_message('signed_detached.txt')
>>> msg.replace_header('To', '123@xxxxxxx')
>>> msg['Return-Path'] = '<>'
@@ -437,8 +434,8 @@
Since this happens way too often, as we seem to get more spam than
legitimate email, an email is not sent about it to the errors-list.
- >>> len(stub.test_emails)
- 0
+ >>> pop_notifications()
+ []
If the content type is multipart/report, it's most likely a DSN
(RFC 3464), so those get dropped as well. Normally a DSN should have an
@@ -455,8 +452,8 @@
>>> msgid in foo_handler.handledMails
False
- >>> len(stub.test_emails)
- 0
+ >>> pop_notifications()
+ []
Email with the Precedence header are probably from an auto-responder or
another robot. We also drop those.
@@ -470,8 +467,8 @@
>>> msgid in foo_handler.handledMails
False
- >>> len(stub.test_emails)
- 0
+ >>> pop_notifications()
+ []
.. Doctest cleanup
=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py 2015-09-04 12:19:07 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py 2015-09-08 09:46:54 +0000
@@ -29,7 +29,6 @@
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.runner import JobRunner
from lp.services.librarian.browser import ProxiedLibraryFileAlias
-from lp.services.mail import stub
from lp.soyuz.adapters.overrides import SourceOverride
from lp.soyuz.enums import (
PackagePublishingStatus,
@@ -192,10 +191,11 @@
with dbuser(config.IPackageUploadNotificationJobSource.dbuser):
JobRunner([job]).runAll()
- def assertEmail(self, expected_to_addrs):
- """Pop an email from the stub queue and check its recipients."""
- _, to_addrs, _ = stub.test_emails.pop()
- self.assertEqual(expected_to_addrs, to_addrs)
+ def assertEmails(self, expected_to_addrs):
+ """Pop emails from the stub queue and check their recipients."""
+ notifications = self.assertEmailQueueLength(len(expected_to_addrs))
+ for expected_to_addr, msg in zip(expected_to_addrs, notifications):
+ self.assertEqual(expected_to_addr, msg["X-Envelope-To"])
def test_acceptFromQueue_source_sends_email(self):
# Accepting a source package sends emails to the announcement list
@@ -204,10 +204,11 @@
upload, uploader = self.makeSourcePackageUpload()
upload.acceptFromQueue()
self.runPackageUploadNotificationJob()
- self.assertEqual(2, len(stub.test_emails))
# Emails sent are the uploader's notification and the announcement:
- self.assertEmail([uploader.preferredemail.email])
- self.assertEmail(["autotest_changes@xxxxxxxxxx"])
+ self.assertEmails([
+ uploader.preferredemail.email,
+ "autotest_changes@xxxxxxxxxx",
+ ])
def test_acceptFromQueue_source_backports_sends_no_announcement(self):
# Accepting a source package into BACKPORTS does not send an
@@ -219,10 +220,9 @@
pocket=PackagePublishingPocket.BACKPORTS)
upload.acceptFromQueue()
self.runPackageUploadNotificationJob()
- self.assertEqual(1, len(stub.test_emails))
# Only one email is sent, to the person in the changed-by field. No
# announcement email is sent.
- self.assertEmail([uploader.preferredemail.email])
+ self.assertEmails([uploader.preferredemail.email])
def test_acceptFromQueue_source_translations_sends_no_email(self):
# Accepting source packages in the "translations" section (i.e.
@@ -235,7 +235,7 @@
upload.acceptFromQueue()
self.runPackageUploadNotificationJob()
self.assertEqual("DONE", upload.status.name)
- self.assertEqual(0, len(stub.test_emails))
+ self.assertEmailQueueLength(0)
def test_acceptFromQueue_source_creates_builds(self):
# Accepting a source package creates build records.
@@ -281,7 +281,7 @@
upload, _ = self.makeBuildPackageUpload()
upload.acceptFromQueue()
self.runPackageUploadNotificationJob()
- self.assertEqual(0, len(stub.test_emails))
+ self.assertEmailQueueLength(0)
def test_acceptFromQueue_handles_duplicates(self):
# Duplicate queue entries are handled sensibly.
@@ -329,8 +329,7 @@
upload, uploader = self.makeSourcePackageUpload()
upload.rejectFromQueue(self.factory.makePerson())
self.runPackageUploadNotificationJob()
- self.assertEqual(1, len(stub.test_emails))
- self.assertEmail([uploader.preferredemail.email])
+ self.assertEmails([uploader.preferredemail.email])
def test_rejectFromQueue_binary_sends_email(self):
# Rejecting a binary package sends an email to the uploader.
@@ -338,8 +337,7 @@
upload, uploader = self.makeBuildPackageUpload()
upload.rejectFromQueue(self.factory.makePerson())
self.runPackageUploadNotificationJob()
- self.assertEqual(1, len(stub.test_emails))
- self.assertEmail([uploader.preferredemail.email])
+ self.assertEmails([uploader.preferredemail.email])
def test_rejectFromQueue_source_translations_sends_no_email(self):
# Rejecting a language pack sends no email.
@@ -350,7 +348,7 @@
section_name="translations")
upload.rejectFromQueue(self.factory.makePerson())
self.runPackageUploadNotificationJob()
- self.assertEqual(0, len(stub.test_emails))
+ self.assertEmailQueueLength(0)
def test_rejectFromQueue_source_with_reason(self):
# Rejecting a source package with a reason includes it in the email to
@@ -360,10 +358,10 @@
person = self.factory.makePerson()
upload.rejectFromQueue(user=person, comment='Because.')
self.runPackageUploadNotificationJob()
- self.assertEqual(1, len(stub.test_emails))
+ [msg] = self.assertEmailQueueLength(1)
self.assertIn(
'Rejected:\nRejected by %s: Because.' % person.displayname,
- stub.test_emails[0][-1])
+ str(msg))
class TestPackageUploadSecurity(TestCaseWithFactory):
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2015-08-03 12:59:18 +0000
+++ lib/lp/testing/__init__.py 2015-09-08 09:46:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from __future__ import absolute_import
@@ -184,6 +184,7 @@
from lp.testing.dbuser import switch_dbuser
from lp.testing.fixture import CaptureOops
from lp.testing.karma import KarmaRecorder
+from lp.testing.mail_helpers import pop_notifications
# The following names have been imported for the purpose of being
# exported. They are referred to here to silence lint warnings.
@@ -744,6 +745,17 @@
sorted(
expected_permissions[permission] - attribute_names)))
+ def assertEmailQueueLength(self, length, sort_key=None):
+ """Pop the email queue, assert its length, and return it.
+
+ This commits the transaction as part of pop_notifications.
+ """
+ notifications = pop_notifications(sort_key=sort_key)
+ self.assertEqual(
+ length, len(notifications),
+ "Expected %d emails, got %d" % (length, len(notifications)))
+ return notifications
+
class TestCaseWithFactory(TestCase):
Follow ups
