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
1=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
2--- lib/devscripts/ec2test/tests/test_remote.py 2010-09-20 22:44:49 +0000
3+++ lib/devscripts/ec2test/tests/test_remote.py 2010-10-06 22:07:04 +0000
4@@ -122,6 +122,16 @@
5 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
6
7
8+def get_body_text(email):
9+ """A helper to retrieve the text body of a test run mail.
10+
11+ :param email: An email.mime.MultiPartMime object.
12+ """
13+ # Stringify the utf8-encoded MIME text message part containing the
14+ # test run summary.
15+ return email.get_payload(0).get_payload(decode=True)
16+
17+
18 class TestFlagFallStream(TestCase):
19 """Tests for `FlagFallStream`."""
20
21@@ -526,8 +536,8 @@
22 [body, attachment] = email.get_payload()
23 self.assertIsInstance(body, MIMEText)
24 self.assertEqual('inline', body['Content-Disposition'])
25- self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
26- self.assertEqual("foo", body.get_payload())
27+ self.assertEqual('text/plain; charset="utf8"', body['Content-Type'])
28+ self.assertEqual("foo", get_body_text(email))
29
30 def test_report_email_attachment(self):
31 req = self.make_request(emails=['foo@example.com'])
32@@ -769,7 +779,7 @@
33 [body, attachment] = email.get_payload()
34 self.assertIsInstance(body, MIMEText)
35 self.assertEqual('inline', body['Content-Disposition'])
36- self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
37+ self.assertEqual('text/plain; charset="utf8"', body['Content-Type'])
38 self.assertEqual(
39 logger.get_summary_contents(), body.get_payload(decode=True))
40 self.assertIsInstance(attachment, MIMEApplication)
41@@ -838,7 +848,7 @@
42 self.assertNotEqual([], email_log)
43 [tester_msg] = email_log
44 self.assertEqual('foo@example.com', tester_msg['To'])
45- self.assertIn('ZeroDivisionError', str(tester_msg))
46+ self.assertIn('ZeroDivisionError', get_body_text(tester_msg))
47
48
49 class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):
50@@ -887,9 +897,6 @@
51 class TestResultHandling(TestCaseWithTransport, RequestHelpers):
52 """Tests for how we handle the result at the end of the test suite."""
53
54- def get_body_text(self, email):
55- return email.get_payload()[0].get_payload()
56-
57 def make_empty_result(self):
58 return TestResult()
59
60@@ -943,7 +950,7 @@
61 self.assertEqual(message, pqm_message)
62 self.assertIn(
63 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
64- self.get_body_text(user_message))
65+ get_body_text(user_message))
66
67 def test_doesnt_submit_to_pqm_on_failure(self):
68 log = []
69@@ -964,7 +971,7 @@
70 [user_message] = log
71 self.assertIn(
72 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),
73- self.get_body_text(user_message))
74+ get_body_text(user_message))
75
76 def test_email_refers_to_attached_log(self):
77 log = []
78@@ -975,7 +982,7 @@
79 [user_message] = log
80 self.assertIn(
81 '(See the attached file for the complete log)\n',
82- self.get_body_text(user_message))
83+ get_body_text(user_message))
84
85 def test_email_body_is_format_result(self):
86 # The body of the email sent to the user is the summary file.
87@@ -986,10 +993,11 @@
88 result = self.make_failing_result()
89 logger.got_result(result)
90 [user_message] = log
91+ error_result_string = request.format_result(
92+ result, logger._start_time, logger._end_time)
93 self.assertEqual(
94- request.format_result(
95- result, logger._start_time, logger._end_time),
96- self.get_body_text(user_message).decode('quoted-printable'))
97+ error_result_string,
98+ get_body_text(user_message))
99
100 def test_gzip_of_full_log_attached(self):
101 # The full log is attached to the email.