launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01428
[Merge] lp:~stevenk/launchpad/fix-ec2test-utf8 into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/fix-ec2test-utf8 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch fixes the two failures in the ec2test tests.
--
https://code.launchpad.net/~stevenk/launchpad/fix-ec2test-utf8/+merge/37831
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/fix-ec2test-utf8 into lp:launchpad.
=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
--- lib/devscripts/ec2test/tests/test_remote.py 2010-09-20 22:44:49 +0000
+++ lib/devscripts/ec2test/tests/test_remote.py 2010-10-07 09:31:10 +0000
@@ -122,6 +122,16 @@
'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
+def get_body_text(email):
+ """A helper to retrieve the text body of a test run mail.
+
+ :param email: An email.mime.MultiPartMime object.
+ """
+ # Stringify the utf8-encoded MIME text message part containing the
+ # test run summary.
+ return email.get_payload(0).get_payload(decode=True)
+
+
class TestFlagFallStream(TestCase):
"""Tests for `FlagFallStream`."""
@@ -527,7 +537,7 @@
self.assertIsInstance(body, MIMEText)
self.assertEqual('inline', body['Content-Disposition'])
self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
- self.assertEqual("foo", body.get_payload())
+ self.assertEqual("foo", get_body_text(email))
def test_report_email_attachment(self):
req = self.make_request(emails=['foo@xxxxxxxxxxx'])
@@ -838,7 +848,7 @@
self.assertNotEqual([], email_log)
[tester_msg] = email_log
self.assertEqual('foo@xxxxxxxxxxx', tester_msg['To'])
- self.assertIn('ZeroDivisionError', str(tester_msg))
+ self.assertIn('ZeroDivisionError', get_body_text(tester_msg))
class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):
@@ -887,9 +897,6 @@
class TestResultHandling(TestCaseWithTransport, RequestHelpers):
"""Tests for how we handle the result at the end of the test suite."""
- def get_body_text(self, email):
- return email.get_payload()[0].get_payload()
-
def make_empty_result(self):
return TestResult()
@@ -943,7 +950,7 @@
self.assertEqual(message, pqm_message)
self.assertIn(
'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
- self.get_body_text(user_message))
+ get_body_text(user_message))
def test_doesnt_submit_to_pqm_on_failure(self):
log = []
@@ -964,7 +971,7 @@
[user_message] = log
self.assertIn(
'**NOT** submitted to PQM:\n%s' % (message['Subject'],),
- self.get_body_text(user_message))
+ get_body_text(user_message))
def test_email_refers_to_attached_log(self):
log = []
@@ -975,7 +982,7 @@
[user_message] = log
self.assertIn(
'(See the attached file for the complete log)\n',
- self.get_body_text(user_message))
+ get_body_text(user_message))
def test_email_body_is_format_result(self):
# The body of the email sent to the user is the summary file.
@@ -986,10 +993,11 @@
result = self.make_failing_result()
logger.got_result(result)
[user_message] = log
+ error_result_string = request.format_result(
+ result, logger._start_time, logger._end_time)
self.assertEqual(
- request.format_result(
- result, logger._start_time, logger._end_time),
- self.get_body_text(user_message).decode('quoted-printable'))
+ error_result_string,
+ get_body_text(user_message))
def test_gzip_of_full_log_attached(self):
# The full log is attached to the email.
=== modified file 'lib/lp/services/mailman/doc/postings.txt'
--- lib/lp/services/mailman/doc/postings.txt 2010-10-03 15:30:06 +0000
+++ lib/lp/services/mailman/doc/postings.txt 2010-10-07 09:31:10 +0000
@@ -832,79 +832,3 @@
>>> vette_watcher.wait_for_discard('narwhale')
>>> len(list(smtpd))
0
-
-
-Large messages
-==============
-
-Only messages which are less than about 40k in size are allowed straight
-through on the mailing list. A message bigger than that will be held for
-moderator approval.
-
- >>> from canonical.config import config
- >>> config.mailman.soft_max_size
- 40000
-
- >>> from email.mime.multipart import MIMEMultipart
- >>> from email.mime.application import MIMEApplication
- >>> from email.mime.text import MIMEText
-
- >>> big_message = MIMEMultipart()
- >>> big_message['From'] = 'anne.person@xxxxxxxxxxx'
- >>> big_message['To'] = 'itest-one@xxxxxxxxxxxxxxxxxxx'
- >>> big_message['Subject'] = 'A huge message'
- >>> big_message['Message-ID'] = '<puma>'
- >>> big_message.attach(
- ... MIMEText('look at this pdf.', 'plain'))
- >>> big_message.attach(
- ... MIMEApplication('\n'.join(['x' * 50] * 1000), 'octet-stream'))
- >>> inject('itest-one', big_message.as_string())
-
- >>> vette_watcher.wait_for_hold('itest-one', 'puma')
- >>> print_message_summaries()
- Number of messages: 1
- bounces@xxxxxxxxxxxxx
- ...
- Itest One <noreply@xxxxxxxxxxxxx>
- New mailing list message requiring approval for Itest One
-
-Once this message is approved, it is posted through to the mailing list.
-
- >>> browser.open(
- ... 'http://launchpad.dev:8085/~itest-one/+mailinglist-moderate')
- >>> browser.getControl(name='field.%3Cpuma%3E').value = ['approve']
- >>> browser.getControl('Moderate').click()
- >>> smtpd_watcher.wait_for_mbox_delivery('puma')
- >>> smtpd_watcher.wait_for_mbox_delivery('puma')
-
- >>> print_message_summaries()
- Number of messages: 2
- itest-one-bounces+anne.person=example.com@xxxxxxxxxxxxxxxxxxx
- <puma>
- anne.person@xxxxxxxxxxx
- [Itest-one] A huge message
- itest-one-bounces+archive=mail-archive.dev@xxxxxxxxxxxxxxxxxxx
- <puma>
- anne.person@xxxxxxxxxxx
- [Itest-one] A huge message
-
-There is a hard limit of 1MB, over which the message is summarily logged and
-discarded.
-
- >>> config.mailman.hard_max_size
- 1000000
-
- >>> huge_message = MIMEMultipart()
- >>> huge_message['From'] = 'anne.person@xxxxxxxxxxx'
- >>> huge_message['To'] = 'itest-one@xxxxxxxxxxxxxxxxxxx'
- >>> huge_message['Subject'] = 'A huge message'
- >>> huge_message['Message-ID'] = '<quahog>'
- >>> huge_message.attach(
- ... MIMEText('look at this huge pdf.', 'plain'))
- >>> huge_message.attach(
- ... MIMEApplication('\n'.join(['x' * 50] * 20000), 'octet-stream'))
- >>> inject('itest-one', huge_message.as_string())
-
- >>> vette_watcher.wait_for_discard('quahog')
- >>> print_message_summaries()
- Number of messages: 0
=== modified file 'lib/lp/services/mailman/monkeypatches/lpsize.py'
--- lib/lp/services/mailman/monkeypatches/lpsize.py 2010-08-20 20:31:18 +0000
+++ lib/lp/services/mailman/monkeypatches/lpsize.py 2010-10-07 09:31:10 +0000
@@ -1,17 +1,45 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""A pipeline handler for checking message sizes."""
-# pylint: disable-msg=F0401
+import email
+
from Mailman import (
Errors,
+ Message,
mm_cfg,
)
from Mailman.Handlers.LPModerate import hold
from Mailman.Logging.Syslog import syslog
+def truncated_message(original_message, limit=10000):
+ """Create a smaller version of the message for moderation."""
+ message = email.message_from_string(
+ original_message.as_string(), Message.Message)
+ for part in email.iterators.typed_subpart_iterator(message, 'multipart'):
+ subparts = part.get_payload()
+ removeable = []
+ for subpart in subparts:
+ if subpart.get_content_maintype() == 'multipart':
+ # This part is handled in the outer loop.
+ continue
+ elif subpart.get_content_type() == 'text/plain':
+ # Truncate the message at 10kb so that there is enough
+ # information for the moderator to make a decision.
+ content = subpart.get_payload().strip()
+ if len(content) > limit:
+ subpart.set_payload(
+ content[:limit] + '\n[truncated for moderation]',
+ subpart.get_charset())
+ else:
+ removeable.append(subpart)
+ for subpart in removeable:
+ subparts.remove(subpart)
+ return message
+
+
def process(mlist, msg, msgdata):
"""Check the message size (approximately) against hard and soft limits.
@@ -42,7 +70,10 @@
if message_size < mm_cfg.LAUNCHPAD_HARD_MAX_SIZE:
# Hold the message in Mailman. See lpmoderate.py for similar
# algorithm.
- hold(mlist, msg, msgdata, 'Too big')
+ # There is a limit to the size that can be stored in Lp. Send
+ # a trucated copy of the message that has enough information for
+ # the moderator to make a decision.
+ hold(mlist, truncated_message(msg), msgdata, 'Too big')
# The message is larger than the hard limit, so log and discard.
syslog('vette', 'Discarding message w/size > hard limit: %s',
msg.get('message-id', 'n/a'))
=== modified file 'lib/lp/services/mailman/testing/__init__.py'
--- lib/lp/services/mailman/testing/__init__.py 2010-10-04 19:50:45 +0000
+++ lib/lp/services/mailman/testing/__init__.py 2010-10-07 09:31:10 +0000
@@ -7,6 +7,7 @@
from contextlib import contextmanager
import email
+from email.mime.multipart import MIMEMultipart
from email.mime.text import MIMEText
import os
import shutil
@@ -87,13 +88,16 @@
# Make a Mailman Message.Message.
if isinstance(sender, (list, tuple)):
sender = ', '.join(sender)
- message = MIMEText(content, mime_type)
+ message = MIMEMultipart()
message['from'] = sender
message['to'] = mm_list.getListAddress()
message['subject'] = subject
message['message-id'] = self.getUniqueString()
+ message.attach(MIMEText(content, mime_type))
+ if attachment is not None:
+ # Rewrap the text message in a multipart message and add the
+ # attachment.
+ message.attach(attachment)
mm_message = email.message_from_string(
message.as_string(), Message.Message)
- if attachment is not None:
- mm_message.attach(attachment, 'octet-stream')
return mm_message
=== modified file 'lib/lp/services/mailman/tests/test_lpmoderate.py'
--- lib/lp/services/mailman/tests/test_lpmoderate.py 2010-10-04 19:50:45 +0000
+++ lib/lp/services/mailman/tests/test_lpmoderate.py 2010-10-07 09:31:10 +0000
@@ -18,11 +18,11 @@
"""Test lpmoderate.
Mailman process() methods quietly return. They may set msg_data key-values
- or raise an error to end processing. This group of tests tests often check
- for errors, but that does not mean there is an error condition, it only
- means message processing has reached a final decision. Messages that do
- not cause a final decision pass-through and the process() methods ends
- without a return.
+ or raise an error to end processing. These tests often check for errors,
+ but that does not mean there is an error condition, it only means message
+ processing has reached a final decision. Messages that do not cause a
+ final decision pass through, and the process() methods ends without a
+ return.
"""
layer = LaunchpadFunctionalLayer
=== added file 'lib/lp/services/mailman/tests/test_lpsize.py'
--- lib/lp/services/mailman/tests/test_lpsize.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/mailman/tests/test_lpsize.py 2010-10-07 09:31:10 +0000
@@ -0,0 +1,129 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Test the lpsize monekypatches"""
+
+from __future__ import with_statement
+
+__metaclass__ = type
+__all__ = []
+
+from email.mime.application import MIMEApplication
+
+from Mailman import Errors
+from Mailman.Handlers import LPSize
+
+from canonical.config import config
+from canonical.testing import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
+from lp.services.mailman.testing import MailmanTestCase
+
+
+class TestLPSizeTestCase(MailmanTestCase):
+ """Test LPSize.
+
+ Mailman process() methods quietly return. They may set msg_data key-values
+ or raise an error to end processing. These tests often check for errors,
+ but that does not mean there is an error condition, it only means message
+ processing has reached a final decision. Messages that do not cause a
+ final decision pass through, and the process() methods ends without a
+ return.
+ """
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(TestLPSizeTestCase, self).setUp()
+ self.team, self.mailing_list = self.factory.makeTeamAndMailingList(
+ 'team-1', 'team-1-owner')
+ self.mm_list = self.makeMailmanList(self.mailing_list)
+ self.subscriber_email = self.team.teamowner.preferredemail.email
+
+ def tearDown(self):
+ super(TestLPSizeTestCase, self).tearDown()
+ self.cleanMailmanList(self.mm_list)
+
+ def test_process_size_under_soft_limit(self):
+ # Any message under 40kb is sent to the list.
+ attachment = MIMEApplication(
+ '\n'.join(['x' * 20] * 1000), 'octet-stream')
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content',
+ attachment=attachment)
+ msg_data = {}
+ silence = LPSize.process(self.mm_list, message, msg_data)
+ self.assertEqual(None, silence)
+
+ def test_process_size_over_soft_limit_held(self):
+ # Messages over 40kb held for moderation.
+ self.assertEqual(40000, config.mailman.soft_max_size)
+ attachment = MIMEApplication(
+ '\n'.join(['x' * 40] * 1000), 'octet-stream')
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content',
+ attachment=attachment)
+ msg_data = {}
+ args = (self.mm_list, message, msg_data)
+ self.assertRaises(
+ Errors.HoldMessage, LPSize.process, *args)
+ self.assertEqual(1, self.mailing_list.getReviewableMessages().count())
+
+ def test_process_size_over_hard_limit_discarded(self):
+ # Messages over 1MB are discarded.
+ self.assertEqual(1000000, config.mailman.hard_max_size)
+ attachment = MIMEApplication(
+ '\n'.join(['x' * 1000] * 1000), 'octet-stream')
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content',
+ attachment=attachment)
+ msg_data = {}
+ args = (self.mm_list, message, msg_data)
+ self.assertRaises(
+ Errors.DiscardMessage, LPSize.process, *args)
+ self.assertEqual(0, self.mailing_list.getReviewableMessages().count())
+
+
+class TestTruncatedMessage(MailmanTestCase):
+ """Test truncated_message helper."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestTruncatedMessage, self).setUp()
+ self.team, self.mailing_list = self.factory.makeTeamAndMailingList(
+ 'team-1', 'team-1-owner')
+ self.mm_list = self.makeMailmanList(self.mailing_list)
+ self.subscriber_email = self.team.teamowner.preferredemail.email
+
+ def test_attchments_are_removed(self):
+ # Plain-text and multipart are preserved, everything else is removed.
+ attachment = MIMEApplication('binary gibberish', 'octet-stream')
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content',
+ attachment=attachment)
+ moderated_message = LPSize.truncated_message(message)
+ parts = [part for part in moderated_message.walk()]
+ types = [part.get_content_type() for part in parts]
+ self.assertEqual(['multipart/mixed', 'text/plain'], types)
+
+ def test_small_text_is_preserved(self):
+ # Text parts below the limit are unchanged.
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content')
+ moderated_message = LPSize.truncated_message(message, limit=1000)
+ parts = [part for part in moderated_message.walk()]
+ types = [part.get_content_type() for part in parts]
+ self.assertEqual(['multipart/mixed', 'text/plain'], types)
+ self.assertEqual('content', parts[1].get_payload())
+
+ def test_large_text_is_truncated(self):
+ # Text parts above the limit are truncated.
+ message = self.makeMailmanMessage(
+ self.mm_list, self.subscriber_email, 'subject', 'content excess')
+ moderated_message = LPSize.truncated_message(message, limit=7)
+ parts = [part for part in moderated_message.walk()]
+ types = [part.get_content_type() for part in parts]
+ self.assertEqual(['multipart/mixed', 'text/plain'], types)
+ self.assertEqual(
+ 'content\n[truncated for moderation]', parts[1].get_payload())
=== modified file 'versions.cfg'
--- versions.cfg 2010-09-18 08:00:27 +0000
+++ versions.cfg 2010-10-07 09:31:10 +0000
@@ -76,10 +76,11 @@
van.testing = 2.0.1
wadllib = 1.1.5
webunit = 1.3.8
-# r1440 of lp:~bjornt/windmill/1.3-lp. It includes our patches to make test
-# setup and tear down more robust, which didn't make it into the 1.3 release.
-# Windmill 1.5 will include our patches.
-windmill = 1.3beta3-lp-r1440
+# r1544 of lp:windmill (the tip revision at the time of packaging).
+# We need to use this revison rather than the official 1.3 release since
+# there is a bug fix for WindmillTestClient.asserts.assertProperty() which
+# makes this api call work property on html disabled and readonly attributes.
+windmill = 1.3r1544
wsgi-fileserver = 0.2.7
wsgi-intercept = 0.4
wsgi-jsonrpc = 0.2.8
