Launchpad itself

Merge lp:~mars/launchpad/fix-ec2test-utf-in-devel into lp:launchpad

  1. fix-ec2test-utf-in-devel
  2. Merge into devel
Proposed by Māris Fogels
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11683
Proposed branch: lp:~mars/launchpad/fix-ec2test-utf-in-devel
Merge into: lp:launchpad
Diff against target: 101 lines (+21/-13)
1 file modified
lib/devscripts/ec2test/tests/test_remote.py (+21/-13)
To merge this branch: bzr merge lp:~mars/launchpad/fix-ec2test-utf-in-devel
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Leonard Richardson Pending
Jonathan Lange Pending
Review via email: mp+37799@code.launchpad.net

This proposal supersedes a proposal from 2010-10-06.

Commit message

Fix a number of errors related to the utf-8 charset in the ec2 test suite.

Description of the change

Hi,

This branch fixes a problem on production-devel and Maverick local developer stations where a test would fail with a UTF-8 decoding error. This patch has already landed in production (see https://code.edge.launchpad.net/~mars/launchpad/fix-ec2test-on-production-devel/+merge/37526).

In this new proposal I have applied the technique more widely to catch a number of similar errors identified in the Launchpad stable branch. I reproduced these errors using devel on Maverick, and have patched them accordingly. All tests in 'bin/test -cv devscripts.ec2test.tests.test_remote' now pass.

Maris

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote : Posted in a previous version of this proposal

Hey Mars,

I think the right approach is to change get_body_text() to use decode=True itself. The point of the helper is to get the text.

Also, strangely, these tests are failing for me in *stable*.

jml

review: Needs Fixing
Revision history for this message
Leonard Richardson (leonardr) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Māris Fogels (mars) wrote : Posted in a previous version of this proposal

> Hey Mars,
>
> I think the right approach is to change get_body_text() to use decode=True
> itself. The point of the helper is to get the text.
>
> Also, strangely, these tests are failing for me in *stable*.
>
> jml

I saw this test failing for me in devel/Maverick, and in production-devel/Lucid. I don't know how it spread so widely.

I can fix the helper, but I also found that the reported TestResult changed if I did not do the calculations before the assertEquals() call. If the assertEquals() failed, then *its* TestResult would be used instead, to the great confusion of the developer.

I can change the helper.

Revision history for this message
Māris Fogels (mars) wrote : Posted in a previous version of this proposal

Jono, does this fix look better?

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Looks plausible. I'm taking your word that it fixes the tests :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-06 22:07:04 +0000
@@ -122,6 +122,16 @@
122 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)122 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
123123
124124
125def get_body_text(email):
126 """A helper to retrieve the text body of a test run mail.
127
128 :param email: An email.mime.MultiPartMime object.
129 """
130 # Stringify the utf8-encoded MIME text message part containing the
131 # test run summary.
132 return email.get_payload(0).get_payload(decode=True)
133
134
125class TestFlagFallStream(TestCase):135class TestFlagFallStream(TestCase):
126 """Tests for `FlagFallStream`."""136 """Tests for `FlagFallStream`."""
127137
@@ -526,8 +536,8 @@
526 [body, attachment] = email.get_payload()536 [body, attachment] = email.get_payload()
527 self.assertIsInstance(body, MIMEText)537 self.assertIsInstance(body, MIMEText)
528 self.assertEqual('inline', body['Content-Disposition'])538 self.assertEqual('inline', body['Content-Disposition'])
529 self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])539 self.assertEqual('text/plain; charset="utf8"', body['Content-Type'])
530 self.assertEqual("foo", body.get_payload())540 self.assertEqual("foo", get_body_text(email))
531541
532 def test_report_email_attachment(self):542 def test_report_email_attachment(self):
533 req = self.make_request(emails=['foo@example.com'])543 req = self.make_request(emails=['foo@example.com'])
@@ -769,7 +779,7 @@
769 [body, attachment] = email.get_payload()779 [body, attachment] = email.get_payload()
770 self.assertIsInstance(body, MIMEText)780 self.assertIsInstance(body, MIMEText)
771 self.assertEqual('inline', body['Content-Disposition'])781 self.assertEqual('inline', body['Content-Disposition'])
772 self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])782 self.assertEqual('text/plain; charset="utf8"', body['Content-Type'])
773 self.assertEqual(783 self.assertEqual(
774 logger.get_summary_contents(), body.get_payload(decode=True))784 logger.get_summary_contents(), body.get_payload(decode=True))
775 self.assertIsInstance(attachment, MIMEApplication)785 self.assertIsInstance(attachment, MIMEApplication)
@@ -838,7 +848,7 @@
838 self.assertNotEqual([], email_log)848 self.assertNotEqual([], email_log)
839 [tester_msg] = email_log849 [tester_msg] = email_log
840 self.assertEqual('foo@example.com', tester_msg['To'])850 self.assertEqual('foo@example.com', tester_msg['To'])
841 self.assertIn('ZeroDivisionError', str(tester_msg))851 self.assertIn('ZeroDivisionError', get_body_text(tester_msg))
842852
843853
844class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):854class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):
@@ -887,9 +897,6 @@
887class TestResultHandling(TestCaseWithTransport, RequestHelpers):897class TestResultHandling(TestCaseWithTransport, RequestHelpers):
888 """Tests for how we handle the result at the end of the test suite."""898 """Tests for how we handle the result at the end of the test suite."""
889899
890 def get_body_text(self, email):
891 return email.get_payload()[0].get_payload()
892
893 def make_empty_result(self):900 def make_empty_result(self):
894 return TestResult()901 return TestResult()
895902
@@ -943,7 +950,7 @@
943 self.assertEqual(message, pqm_message)950 self.assertEqual(message, pqm_message)
944 self.assertIn(951 self.assertIn(
945 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),952 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
946 self.get_body_text(user_message))953 get_body_text(user_message))
947954
948 def test_doesnt_submit_to_pqm_on_failure(self):955 def test_doesnt_submit_to_pqm_on_failure(self):
949 log = []956 log = []
@@ -964,7 +971,7 @@
964 [user_message] = log971 [user_message] = log
965 self.assertIn(972 self.assertIn(
966 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),973 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),
967 self.get_body_text(user_message))974 get_body_text(user_message))
968975
969 def test_email_refers_to_attached_log(self):976 def test_email_refers_to_attached_log(self):
970 log = []977 log = []
@@ -975,7 +982,7 @@
975 [user_message] = log982 [user_message] = log
976 self.assertIn(983 self.assertIn(
977 '(See the attached file for the complete log)\n',984 '(See the attached file for the complete log)\n',
978 self.get_body_text(user_message))985 get_body_text(user_message))
979986
980 def test_email_body_is_format_result(self):987 def test_email_body_is_format_result(self):
981 # The body of the email sent to the user is the summary file.988 # The body of the email sent to the user is the summary file.
@@ -986,10 +993,11 @@
986 result = self.make_failing_result()993 result = self.make_failing_result()
987 logger.got_result(result)994 logger.got_result(result)
988 [user_message] = log995 [user_message] = log
996 error_result_string = request.format_result(
997 result, logger._start_time, logger._end_time)
989 self.assertEqual(998 self.assertEqual(
990 request.format_result(999 error_result_string,
991 result, logger._start_time, logger._end_time),1000 get_body_text(user_message))
992 self.get_body_text(user_message).decode('quoted-printable'))
9931001
994 def test_gzip_of_full_log_attached(self):1002 def test_gzip_of_full_log_attached(self):
995 # The full log is attached to the email.1003 # The full log is attached to the email.