Launchpad itself

Merge lp:~jml/launchpad/subject-and-attachment into lp:launchpad

  1. subject-and-attachment
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 11433
Proposed branch: lp:~jml/launchpad/subject-and-attachment
Merge into: lp:launchpad
Diff against target: 203 lines (+58/-26)
2 files modified
lib/devscripts/ec2test/remote.py (+19/-11)
lib/devscripts/ec2test/tests/test_remote.py (+39/-15)
To merge this branch: bzr merge lp:~jml/launchpad/subject-and-attachment
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+33544@code.launchpad.net

Commit message

Change the subject and attachment of EC2 test mails.

Description of the change

This branch changes the subject and attachment name of the EC2 email.

For most email, the subject will become:

  Test Results: branch-name -> devel: SUCCESS
  Test Results: branch-name -> db-devel: FAILURE

For test runs against a trunk:

  Test Results: devel r12345: SUCCESS

The reasoning is that it's nice to be able to quickly distinguish between newly incoming mail. I thought about dropping the "SUCCESS" bit in order to preserve threading, but not sure it's worth it.

The attachment then becomes:

  branch-name.subunit.gz

In anticipation of better support for just clicking on the damn attachment.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/devscripts/ec2test/remote.py'
2--- lib/devscripts/ec2test/remote.py 2010-08-18 18:04:38 +0000
3+++ lib/devscripts/ec2test/remote.py 2010-08-24 16:22:46 +0000
4@@ -314,12 +314,12 @@
5 conn = bzrlib.smtp_connection.SMTPConnection(self._bzr_config)
6 conn.send_email(message)
7
8- def get_trunk_details(self):
9+ def get_target_details(self):
10 """Return (branch_url, revno) for trunk."""
11 branch = bzrlib.branch.Branch.open(self._local_branch_path)
12 return branch.get_parent().encode('utf-8'), branch.revno()
13
14- def get_branch_details(self):
15+ def get_source_details(self):
16 """Return (branch_url, revno) for the branch we're merging in.
17
18 If we're not merging in a branch, but instead just testing a trunk,
19@@ -337,12 +337,18 @@
20
21 def get_nick(self):
22 """Get the nick of the branch we are testing."""
23- details = self.get_branch_details()
24+ details = self.get_source_details()
25 if not details:
26- details = self.get_trunk_details()
27+ details = self.get_target_details()
28 url, revno = details
29 return self._last_segment(url)
30
31+ def get_revno(self):
32+ """Get the revno of the branch we are testing."""
33+ if self._revno is not None:
34+ return self._revno
35+ return bzrlib.branch.Branch.open(self._local_branch_path).revno()
36+
37 def get_merge_description(self):
38 """Get a description of the merge request.
39
40@@ -353,10 +359,10 @@
41 # XXX: JonathanLange 2010-08-17: Not actually used yet. I think it
42 # would be a great thing to have in the subject of the emails we
43 # receive.
44- source = self.get_branch_details()
45+ source = self.get_source_details()
46 if not source:
47- return self.get_nick()
48- target = self.get_trunk_details()
49+ return '%s r%s' % (self.get_nick(), self.get_revno())
50+ target = self.get_target_details()
51 return '%s => %s' % (
52 self._last_segment(source[0]), self._last_segment(target[0]))
53
54@@ -391,7 +397,8 @@
55 status = 'SUCCESS'
56 else:
57 status = 'FAILURE'
58- subject = 'Test results: %s' % (status,)
59+ subject = 'Test results: %s: %s' % (
60+ self.get_merge_description(), status)
61 message['Subject'] = subject
62
63 # Make the body.
64@@ -403,7 +410,8 @@
65 zipped_log = MIMEApplication(full_log_gz, 'x-gzip')
66 zipped_log.add_header(
67 'Content-Disposition', 'attachment',
68- filename='%s.log.gz' % self.get_nick())
69+ filename='%s-r%s.subunit.gz' % (
70+ self.get_nick(), self.get_revno()))
71 message.attach(zipped_log)
72 return message
73
74@@ -609,14 +617,14 @@
75 ''')
76
77 # Describe the trunk branch.
78- trunk, trunk_revno = self._request.get_trunk_details()
79+ trunk, trunk_revno = self._request.get_target_details()
80 msg = '%s, revision %d\n' % (trunk, trunk_revno)
81 add_to_html('''\
82 <p><strong>%s</strong></p>
83 ''' % (escape(msg),))
84 log(msg)
85
86- branch_details = self._request.get_branch_details()
87+ branch_details = self._request.get_source_details()
88 if not branch_details:
89 add_to_html('<p>(no merged branch)</p>\n')
90 log('(no merged branch)')
91
92=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
93--- lib/devscripts/ec2test/tests/test_remote.py 2010-08-18 17:36:12 +0000
94+++ lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 16:22:46 +0000
95@@ -235,31 +235,49 @@
96 req = self.make_request(emails=['foo@example.com'])
97 self.assertEqual(True, req.wants_email)
98
99- def test_get_trunk_details(self):
100+ def test_get_target_details(self):
101 parent = 'http://example.com/bzr/branch'
102 tree = self.make_trunk(parent)
103 req = self.make_request(trunk=tree)
104 self.assertEqual(
105- (parent, tree.branch.revno()), req.get_trunk_details())
106-
107- def test_get_branch_details_no_commits(self):
108+ (parent, tree.branch.revno()), req.get_target_details())
109+
110+ def test_get_revno_target_only(self):
111+ # If there's only a target branch, then the revno is the revno of that
112+ # branch.
113+ parent = 'http://example.com/bzr/branch'
114+ tree = self.make_trunk(parent)
115+ req = self.make_request(trunk=tree)
116+ self.assertEqual(tree.branch.revno(), req.get_revno())
117+
118+ def test_get_revno_source_and_target(self):
119+ # If we're merging in a branch, then the revno is the revno of the
120+ # branch we're merging in.
121+ tree = self.make_trunk()
122+ # Fake a merge, giving silly revision ids.
123+ tree.add_pending_merge('foo', 'bar')
124+ req = self.make_request(
125+ branch_url='https://example.com/bzr/thing', revno=42, trunk=tree)
126+ self.assertEqual(42, req.get_revno())
127+
128+ def test_get_source_details_no_commits(self):
129 req = self.make_request(trunk=self.make_trunk())
130- self.assertEqual(None, req.get_branch_details())
131+ self.assertEqual(None, req.get_source_details())
132
133- def test_get_branch_details_no_merge(self):
134+ def test_get_source_details_no_merge(self):
135 tree = self.make_trunk()
136 tree.commit(message='foo')
137 req = self.make_request(trunk=tree)
138- self.assertEqual(None, req.get_branch_details())
139+ self.assertEqual(None, req.get_source_details())
140
141- def test_get_branch_details_merge(self):
142+ def test_get_source_details_merge(self):
143 tree = self.make_trunk()
144 # Fake a merge, giving silly revision ids.
145 tree.add_pending_merge('foo', 'bar')
146 req = self.make_request(
147 branch_url='https://example.com/bzr/thing', revno=42, trunk=tree)
148 self.assertEqual(
149- ('https://example.com/bzr/thing', 42), req.get_branch_details())
150+ ('https://example.com/bzr/thing', 42), req.get_source_details())
151
152 def test_get_nick_trunk_only(self):
153 tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel')
154@@ -277,7 +295,8 @@
155 def test_get_merge_description_trunk_only(self):
156 tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel')
157 req = self.make_request(trunk=tree)
158- self.assertEqual('db-devel', req.get_merge_description())
159+ self.assertEqual(
160+ 'db-devel r%s' % req.get_revno(), req.get_merge_description())
161
162 def test_get_merge_description_merge(self):
163 tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel/')
164@@ -355,12 +374,16 @@
165 def test_report_email_subject_success(self):
166 req = self.make_request(emails=['foo@example.com'])
167 email = req._build_report_email(True, 'foo', 'gobbledygook')
168- self.assertEqual('Test results: SUCCESS', email['Subject'])
169+ self.assertEqual(
170+ 'Test results: %s: SUCCESS' % req.get_merge_description(),
171+ email['Subject'])
172
173 def test_report_email_subject_failure(self):
174 req = self.make_request(emails=['foo@example.com'])
175 email = req._build_report_email(False, 'foo', 'gobbledygook')
176- self.assertEqual('Test results: FAILURE', email['Subject'])
177+ self.assertEqual(
178+ 'Test results: %s: FAILURE' % req.get_merge_description(),
179+ email['Subject'])
180
181 def test_report_email_recipients(self):
182 req = self.make_request(emails=['foo@example.com', 'bar@example.com'])
183@@ -388,7 +411,8 @@
184 self.assertIsInstance(attachment, MIMEApplication)
185 self.assertEqual('application/x-gzip', attachment['Content-Type'])
186 self.assertEqual(
187- 'attachment; filename="%s.log.gz"' % req.get_nick(),
188+ 'attachment; filename="%s-r%s.subunit.gz"' % (
189+ req.get_nick(), req.get_revno()),
190 attachment['Content-Disposition'])
191 self.assertEqual(
192 "gobbledygook", attachment.get_payload().decode('base64'))
193@@ -623,8 +647,8 @@
194 [body, attachment] = user_message.get_payload()
195 self.assertEqual('application/x-gzip', attachment['Content-Type'])
196 self.assertEqual(
197- 'attachment; filename="%s.log.gz"' % request.get_nick(),
198- attachment['Content-Disposition'])
199+ 'attachment;',
200+ attachment['Content-Disposition'][:len('attachment;')])
201 attachment_contents = attachment.get_payload().decode('base64')
202 uncompressed = gunzip_data(attachment_contents)
203 self.assertEqual(