Launchpad itself

Merge lp:~jml/launchpad/one-email-on-crash into lp:launchpad

  1. one-email-on-crash
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Jonathan Lange
Approved revision: no longer in the source branch.
Merged at revision: 11455
Proposed branch: lp:~jml/launchpad/one-email-on-crash
Merge into: lp:launchpad
Prerequisite: lp:~jml/launchpad/launchpad-tester
Diff against target: 547 lines (+199/-60)
2 files modified
lib/devscripts/ec2test/remote.py (+34/-15)
lib/devscripts/ec2test/tests/test_remote.py (+165/-45)
To merge this branch: bzr merge lp:~jml/launchpad/one-email-on-crash
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+33768@code.launchpad.net

Commit message

Only send one email when "ec2 test" fails catastrophically.

Description of the change

ec2 test has a fair bit of stuff in it to make sure that unexpected failures
don't screw things up too badly, and that appropriate folk get told about it.

Unfortunately, in the code currently in Launchpad, there's perhaps a bit too
much redundancy and safety. This code changes things so that you get exactly
one email if ec2 test fails surprisingly.

The reason I wanted to make this change is to:
 a) clarify, test and document the way we send email on failure.
 b) create separate interfaces for sending email on catastrophe vs normal
    test results
 c) reduce the amount of email I receive ever so slightly

Landing this branch will make it possible, finally, to free the normal
test email from the shackles of displaying the actual output of the test
runner.

In terms of code changes:
 * Refactor EC2Runner & LaunchpadTester to take an SMTPConnection object
   and use that for sending mail, thus allowing for easier testing of
   what mail gets sent.

 * Change WebTestLogger.error_in_testrunner to send the email on catastrophe,
   rather than got_result.

 * Change LaunchpadTester.test not to re-raise exceptions it receives from
   running the tests. Note: I'm not sure what the reason was for doing so
   in the first place.

Thanks,
jml

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (6.1 KiB)

It would have been nice if the email handling could simply be a logging
configuration using:

http://docs.python.org/library/logging.html#smtphandler

but afaics, that doesn't handle attachments for emails.

Anyway, r=me, with some typos fixed below.

> === modified file 'lib/devscripts/ec2test/remote.py'
> --- lib/devscripts/ec2test/remote.py 2010-08-24 17:35:55 +0000
> +++ lib/devscripts/ec2test/remote.py 2010-08-26 13:19:57 +0000
> @@ -510,6 +519,13 @@
> summary.write('\n\nERROR IN TESTRUNNER\n\n')
> traceback.print_exception(exc_type, exc_value, exc_tb, file=summary)
> summary.flush()
> + if self._request.wants_email:
> + self._write_to_filename(
> + self._summary_filename,
> + '\n(See the attached file for the complete log)\n')
> + summary = self.get_summary_contents()
> + full_log_gz = gzip_data(self.get_full_log_contents())
> + self._request.send_report_email(False, summary, full_log_gz)

Is it worth reordering the args to send_report_email, so you could instead
write:
               self._request.send_report_email(
                   summary, full_log_gz, successful=False)

Probably not. I just had to look back to see what was False, but it's obvious
in retrospect :)

> === modified file 'lib/devscripts/ec2test/tests/test_remote.py'
> --- lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:44:22 +0000
> +++ lib/devscripts/ec2test/tests/test_remote.py 2010-08-26 13:19:57 +0000
> @@ -60,7 +71,7 @@
>
> def make_request(self, branch_url=None, revno=None,
> trunk=None, sourcecode_path=None,
> - emails=None, pqm_message=None):
> + emails=None, pqm_message=None, emails_sent=None):
> """Make a request to test, specifying only things we care about.
>
> Note that the returned request object will not ever send email, but

The above comment needs updating with your change.

> @@ -94,6 +106,13 @@
> tree.commit(message=str(i))
> return 'sourcecode/'
>
> + def make_tester(self, logger=None, test_directory=None, test_options=()):
> + if not logger:
> + logger = self.make_logger()
> + if not test_directory:
> + test_directory = 'unspecified-test-directory'

why is test_directory optional? If set it like this, I would have assumed
that Popen() would fail when it tries to change into this directory?

I haven't tried it, but from the docs:

If cwd is not None, the child’s current directory will be changed to cwd
before it is executed.

[1] http://docs.python.org/library/subprocess.html#module-subprocess

We discussed this:

15:51 < noodles775> jml: why do you set test_directory when it's passed as None?
15:52 < noodles775> (that is, why do you set it to a non-existent directory)
15:52 < jml> noodles775, umm, not sure. maybe because stuff explodes.
15:52 * jml tries.
15:53 < noodles775> According to the docs, I'd assume it wouldn't explode when its None, but will in its current state?
15:53 < noodles775> But yes, science is the best answer :)
15:54 < salgado> thanks for the reviews, noodles775
1...

Read more...

review: Approve (code)
Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (5.9 KiB)

On Thu, Aug 26, 2010 at 3:13 PM, Michael Nelson
<email address hidden> wrote:
> Review: Approve code
> It would have been nice if the email handling could simply be a logging
> configuration using:
>
> http://docs.python.org/library/logging.html#smtphandler
>
> but afaics, that doesn't handle attachments for emails.
>
> Anyway, r=me, with some typos fixed below.
>
>> === modified file 'lib/devscripts/ec2test/remote.py'
>> --- lib/devscripts/ec2test/remote.py  2010-08-24 17:35:55 +0000
>> +++ lib/devscripts/ec2test/remote.py  2010-08-26 13:19:57 +0000
>> @@ -510,6 +519,13 @@
>>          summary.write('\n\nERROR IN TESTRUNNER\n\n')
>>          traceback.print_exception(exc_type, exc_value, exc_tb, file=summary)
>>          summary.flush()
>> +        if self._request.wants_email:
>> +            self._write_to_filename(
>> +                self._summary_filename,
>> +                '\n(See the attached file for the complete log)\n')
>> +            summary = self.get_summary_contents()
>> +            full_log_gz = gzip_data(self.get_full_log_contents())
>> +            self._request.send_report_email(False, summary, full_log_gz)
>
> Is it worth reordering the args to send_report_email, so you could instead
> write:
>               self._request.send_report_email(
>                   summary, full_log_gz, successful=False)
>
> Probably not. I just had to look back to see what was False, but it's obvious
> in retrospect :)
>

I won't do it in this branch, for the reasons you give here. I plan a
follow-up branch that will probably change the API & behaviour a bit,
since it would be nice if the subject for "failed tests" is noticeably
different to the subject for "something went horribly wrong".

>> === modified file 'lib/devscripts/ec2test/tests/test_remote.py'
>> --- lib/devscripts/ec2test/tests/test_remote.py       2010-08-24 17:44:22 +0000
>> +++ lib/devscripts/ec2test/tests/test_remote.py       2010-08-26 13:19:57 +0000
>> @@ -60,7 +71,7 @@
>>
>>      def make_request(self, branch_url=None, revno=None,
>>                       trunk=None, sourcecode_path=None,
>> -                     emails=None, pqm_message=None):
>> +                     emails=None, pqm_message=None, emails_sent=None):
>>          """Make a request to test, specifying only things we care about.
>>
>>          Note that the returned request object will not ever send email, but
>
> The above comment needs updating with your change.
>

Done.

>> @@ -94,6 +106,13 @@
>>                  tree.commit(message=str(i))
>>          return 'sourcecode/'
>>
>> +    def make_tester(self, logger=None, test_directory=None, test_options=()):
>> +        if not logger:
>> +            logger = self.make_logger()
>> +        if not test_directory:
>> +            test_directory = 'unspecified-test-directory'
>
> why is test_directory optional? If set it like this, I would have assumed
> that Popen() would fail when it tries to change into this directory?
>
> I haven't tried it, but from the docs:
>
> If cwd is not None, the child’s current directory will be changed to cwd
> before it is executed.
>
> [1] http://docs.python.org/library/subprocess.html#module-subprocess
>
> We discus...

Read more...

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-24 17:35:55 +0000
3+++ lib/devscripts/ec2test/remote.py 2010-08-26 12:56:33 +0000
4@@ -40,11 +40,12 @@
5
6 import bzrlib.branch
7 import bzrlib.config
8-import bzrlib.email_message
9 import bzrlib.errors
10-import bzrlib.smtp_connection
11 import bzrlib.workingtree
12
13+from bzrlib.email_message import EmailMessage
14+from bzrlib.smtp_connection import SMTPConnection
15+
16 import subunit
17
18
19@@ -134,19 +135,24 @@
20 SHUTDOWN_DELAY = 60
21
22 def __init__(self, daemonize, pid_filename, shutdown_when_done,
23- emails=None):
24+ smtp_connection=None, emails=None):
25 """Make an EC2Runner.
26
27 :param daemonize: Whether or not we will daemonize.
28 :param pid_filename: The filename to store the pid in.
29 :param shutdown_when_done: Whether or not to shut down when the tests
30 are done.
31+ :param smtp_connection: The `SMTPConnection` to use to send email.
32 :param emails: The email address(es) to send catastrophic failure
33 messages to. If not provided, the error disappears into the ether.
34 """
35 self._should_daemonize = daemonize
36 self._pid_filename = pid_filename
37 self._shutdown_when_done = shutdown_when_done
38+ if smtp_connection is None:
39+ config = bzrlib.config.GlobalConfig()
40+ smtp_connection = SMTPConnection(config)
41+ self._smtp_connection = smtp_connection
42 self._emails = emails
43 self._daemonized = False
44
45@@ -188,10 +194,12 @@
46 config = bzrlib.config.GlobalConfig()
47 # Handle exceptions thrown by the test() or daemonize() methods.
48 if self._emails:
49- bzrlib.email_message.EmailMessage.send(
50- config, config.username(),
51- self._emails, '%s FAILED' % (name,),
52- traceback.format_exc())
53+ msg = EmailMessage(
54+ from_address=config.username(),
55+ to_address=self._emails,
56+ subject='%s FAILED' % (name,),
57+ body=traceback.format_exc())
58+ self._smtp_connection.send_email(msg)
59 raise
60 finally:
61 # When everything is over, if we've been ask to shut down, then
62@@ -267,9 +275,7 @@
63 exit_status = popen.wait()
64 except:
65 self._logger.error_in_testrunner(sys.exc_info())
66- exit_status = 1
67- raise
68- finally:
69+ else:
70 self._logger.got_result(not exit_status)
71
72 def _gather_test_output(self, input_stream, logger):
73@@ -287,7 +293,7 @@
74 """A request to have a branch tested and maybe landed."""
75
76 def __init__(self, branch_url, revno, local_branch_path, sourcecode_path,
77- emails=None, pqm_message=None):
78+ emails=None, pqm_message=None, smtp_connection=None):
79 """Construct a `Request`.
80
81 :param branch_url: The public URL to the Launchpad branch we are
82@@ -303,6 +309,7 @@
83 provided, no emails are sent.
84 :param pqm_message: The message to submit to PQM. If not provided, we
85 don't submit to PQM.
86+ :param smtp_connection: The `SMTPConnection` to use to send email.
87 """
88 self._branch_url = branch_url
89 self._revno = revno
90@@ -312,11 +319,13 @@
91 self._pqm_message = pqm_message
92 # Used for figuring out how to send emails.
93 self._bzr_config = bzrlib.config.GlobalConfig()
94+ if smtp_connection is None:
95+ smtp_connection = SMTPConnection(self._bzr_config)
96+ self._smtp_connection = smtp_connection
97
98 def _send_email(self, message):
99 """Actually send 'message'."""
100- conn = bzrlib.smtp_connection.SMTPConnection(self._bzr_config)
101- conn.send_email(message)
102+ self._smtp_connection.send_email(message)
103
104 def get_target_details(self):
105 """Return (branch_url, revno) for trunk."""
106@@ -510,6 +519,13 @@
107 summary.write('\n\nERROR IN TESTRUNNER\n\n')
108 traceback.print_exception(exc_type, exc_value, exc_tb, file=summary)
109 summary.flush()
110+ if self._request.wants_email:
111+ self._write_to_filename(
112+ self._summary_filename,
113+ '\n(See the attached file for the complete log)\n')
114+ summary = self.get_summary_contents()
115+ full_log_gz = gzip_data(self.get_full_log_contents())
116+ self._request.send_report_email(False, summary, full_log_gz)
117
118 def get_index_contents(self):
119 """Return the contents of the index.html page."""
120@@ -805,16 +821,19 @@
121
122 pid_filename = os.path.join(LAUNCHPAD_DIR, 'ec2test-remote.pid')
123
124+ smtp_connection = SMTPConnection(bzrlib.config.GlobalConfig())
125+
126 request = Request(
127 options.public_branch, options.public_branch_revno, TEST_DIR,
128- SOURCECODE_DIR, options.email, pqm_message)
129+ SOURCECODE_DIR, options.email, pqm_message, smtp_connection)
130 # Only write to stdout if we are running as the foreground process.
131 echo_to_stdout = not options.daemon
132 logger = WebTestLogger.make_in_directory(
133 '/var/www', request, echo_to_stdout)
134
135 runner = EC2Runner(
136- options.daemon, pid_filename, options.shutdown, options.email)
137+ options.daemon, pid_filename, options.shutdown,
138+ smtp_connection, options.email)
139
140 tester = LaunchpadTester(logger, TEST_DIR, test_options=args[1:])
141 runner.run("Test runner", tester.test)
142
143=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
144--- lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:44:22 +0000
145+++ lib/devscripts/ec2test/tests/test_remote.py 2010-08-26 12:56:33 +0000
146@@ -26,6 +26,7 @@
147 from testtools.content_type import ContentType
148
149 from devscripts.ec2test.remote import (
150+ EC2Runner,
151 FlagFallStream,
152 gunzip_data,
153 gzip_data,
154@@ -38,6 +39,16 @@
155 )
156
157
158+class LoggingSMTPConnection(object):
159+ """An SMTPConnection double that logs sent email."""
160+
161+ def __init__(self, log):
162+ self._log = log
163+
164+ def send_email(self, message):
165+ self._log.append(message)
166+
167+
168 class RequestHelpers:
169
170 def patch(self, obj, name, value):
171@@ -60,7 +71,7 @@
172
173 def make_request(self, branch_url=None, revno=None,
174 trunk=None, sourcecode_path=None,
175- emails=None, pqm_message=None):
176+ emails=None, pqm_message=None, emails_sent=None):
177 """Make a request to test, specifying only things we care about.
178
179 Note that the returned request object will not ever send email, but
180@@ -73,11 +84,12 @@
181 [('a', 'http://example.com/bzr/a', 2),
182 ('b', 'http://example.com/bzr/b', 3),
183 ('c', 'http://example.com/bzr/c', 5)])
184+ if emails_sent is None:
185+ emails_sent = []
186+ smtp_connection = LoggingSMTPConnection(emails_sent)
187 request = Request(
188 branch_url, revno, trunk.basedir, sourcecode_path, emails,
189- pqm_message)
190- request.emails_sent = []
191- request._send_email = request.emails_sent.append
192+ pqm_message, smtp_connection)
193 return request
194
195 def make_sourcecode(self, branches):
196@@ -94,6 +106,13 @@
197 tree.commit(message=str(i))
198 return 'sourcecode/'
199
200+ def make_tester(self, logger=None, test_directory=None, test_options=()):
201+ if not logger:
202+ logger = self.make_logger()
203+ if not test_directory:
204+ test_directory = 'unspecified-test-directory'
205+ return LaunchpadTester(logger, test_directory, test_options)
206+
207 def make_logger(self, request=None, echo_to_stdout=False):
208 if request is None:
209 request = self.make_request()
210@@ -199,13 +218,6 @@
211
212 class TestLaunchpadTester(TestCaseWithTransport, RequestHelpers):
213
214- def make_tester(self, logger=None, test_directory=None, test_options=()):
215- if not logger:
216- logger = self.make_logger()
217- if not test_directory:
218- test_directory = 'unspecified-test-directory'
219- return LaunchpadTester(logger, test_directory, test_options)
220-
221 def test_build_test_command_no_options(self):
222 # The LaunchpadTester runs "make check" if given no options.
223 tester = self.make_tester()
224@@ -243,14 +255,15 @@
225 # got_result with the result. This test is more of a smoke test to
226 # make sure that everything integrates well.
227 message = {'Subject': "One Crowded Hour"}
228- request = self.make_request(pqm_message=message)
229+ log = []
230+ request = self.make_request(pqm_message=message, emails_sent=log)
231 logger = self.make_logger(request=request)
232 tester = self.make_tester(logger=logger)
233 output = "test output\n"
234 tester._spawn_test_process = lambda: FakePopen(output, 0)
235 tester.test()
236 # Message being sent implies got_result thought it got a success.
237- self.assertEqual([message], request.emails_sent)
238+ self.assertEqual([message], log)
239
240 def test_error_in_testrunner(self):
241 # Any exception is raised within LaunchpadTester.test() is an error in
242@@ -260,28 +273,30 @@
243 # failure.
244 # 3. Re-raise the error. In the script, this triggers an email.
245 message = {'Subject': "One Crowded Hour"}
246- request = self.make_request(pqm_message=message)
247+ log = []
248+ request = self.make_request(pqm_message=message, emails_sent=log)
249 logger = self.make_logger(request=request)
250 tester = self.make_tester(logger=logger)
251 # Break the test runner deliberately. In production, this is more
252 # likely to be a system error than a programming error.
253 tester._spawn_test_process = lambda: 1/0
254- self.assertRaises(ZeroDivisionError, tester.test)
255+ tester.test()
256 # Message not being sent implies got_result thought it got a failure.
257- self.assertEqual([], request.emails_sent)
258+ self.assertEqual([], log)
259 self.assertIn("ERROR IN TESTRUNNER", logger.get_summary_contents())
260 self.assertIn("ZeroDivisionError", logger.get_summary_contents())
261
262 def test_nonzero_exit_code(self):
263 message = {'Subject': "One Crowded Hour"}
264- request = self.make_request(pqm_message=message)
265+ log = []
266+ request = self.make_request(pqm_message=message, emails_sent=log)
267 logger = self.make_logger(request=request)
268 tester = self.make_tester(logger=logger)
269 output = "test output\n"
270 tester._spawn_test_process = lambda: FakePopen(output, 10)
271 tester.test()
272 # Message not being sent implies got_result thought it got a failure.
273- self.assertEqual([], request.emails_sent)
274+ self.assertEqual([], log)
275
276
277 class TestPidfileHelpers(TestCase):
278@@ -449,9 +464,10 @@
279
280 def test_submit_to_pqm_no_message_doesnt_send(self):
281 # If there's no PQM message, then 'submit_to_pqm' returns None.
282- req = self.make_request(pqm_message=None)
283+ log = []
284+ req = self.make_request(pqm_message=None, emails_sent=log)
285 req.submit_to_pqm(successful=True)
286- self.assertEqual([], req.emails_sent)
287+ self.assertEqual([], log)
288
289 def test_submit_to_pqm_unsuccessful(self):
290 # submit_to_pqm returns the subject of the PQM mail even if it's
291@@ -464,17 +480,19 @@
292 def test_submit_to_pqm_unsuccessful_no_email(self):
293 # submit_to_pqm doesn't send any email if the run was unsuccessful.
294 message = {'Subject:': 'My PQM message'}
295- req = self.make_request(pqm_message=message)
296+ log = []
297+ req = self.make_request(pqm_message=message, emails_sent=log)
298 req.submit_to_pqm(successful=False)
299- self.assertEqual([], req.emails_sent)
300+ self.assertEqual([], log)
301
302 def test_submit_to_pqm_successful(self):
303 # submit_to_pqm returns the subject of the PQM mail.
304 message = {'Subject:': 'My PQM message'}
305- req = self.make_request(pqm_message=message)
306+ log = []
307+ req = self.make_request(pqm_message=message, emails_sent=log)
308 subject = req.submit_to_pqm(successful=True)
309 self.assertIs(message.get('Subject'), subject)
310- self.assertEqual([message], req.emails_sent)
311+ self.assertEqual([message], log)
312
313 def test_report_email_subject_success(self):
314 req = self.make_request(emails=['foo@example.com'])
315@@ -523,10 +541,11 @@
316 "gobbledygook", attachment.get_payload().decode('base64'))
317
318 def test_send_report_email_sends_email(self):
319- req = self.make_request(emails=['foo@example.com'])
320+ log = []
321+ req = self.make_request(emails=['foo@example.com'], emails_sent=log)
322 expected = req._build_report_email(False, "foo", "gobbledygook")
323 req.send_report_email(False, "foo", "gobbledygook")
324- [observed] = req.emails_sent
325+ [observed] = log
326 # The standard library sucks. None of the MIME objects have __eq__
327 # implementations.
328 for expected_part, observed_part in izip(
329@@ -597,7 +616,7 @@
330 self.assertEqual('foo\n', logger.get_full_log_contents())
331 self.assertEqual('foo\n', logger.get_summary_contents())
332
333- def test_error_in_testrunner(self):
334+ def test_error_in_testrunner_logs_to_summary(self):
335 # error_in_testrunner logs the traceback to the summary log in a very
336 # prominent way.
337 try:
338@@ -611,6 +630,93 @@
339 "\n\nERROR IN TESTRUNNER\n\n%s" % (stack,),
340 logger.get_summary_contents())
341
342+ def test_error_in_testrunner_sends_email(self):
343+ # If email addresses are configurd, error_in_testrunner emails them
344+ # with the failure and the full log.
345+ try:
346+ 1/0
347+ except ZeroDivisionError:
348+ exc_info = sys.exc_info()
349+ log = []
350+ request = self.make_request(
351+ emails=['foo@example.com'], emails_sent=log)
352+ logger = self.make_logger(request=request)
353+ logger.error_in_testrunner(exc_info)
354+ [email] = log
355+ self.assertEqual(
356+ 'Test results: %s: FAILURE' % request.get_merge_description(),
357+ email['Subject'])
358+ [body, attachment] = email.get_payload()
359+ self.assertIsInstance(body, MIMEText)
360+ self.assertEqual('inline', body['Content-Disposition'])
361+ self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
362+ self.assertEqual(
363+ logger.get_summary_contents(), body.get_payload(decode=True))
364+ self.assertIsInstance(attachment, MIMEApplication)
365+ self.assertEqual('application/x-gzip', attachment['Content-Type'])
366+ self.assertEqual(
367+ 'attachment;',
368+ attachment['Content-Disposition'][:len('attachment;')])
369+ self.assertEqual(
370+ logger.get_full_log_contents(),
371+ gunzip_data(attachment.get_payload().decode('base64')))
372+
373+
374+class TestEC2Runner(TestCaseWithTransport, RequestHelpers):
375+
376+ def make_ec2runner(self, emails=None, email_log=None):
377+ if email_log is None:
378+ email_log = []
379+ smtp_connection = LoggingSMTPConnection(email_log)
380+ return EC2Runner(
381+ False, "who-cares.pid", False, smtp_connection, emails=emails)
382+
383+ def test_run(self):
384+ calls = []
385+ runner = self.make_ec2runner()
386+ runner.run(
387+ "boring test method",
388+ lambda *a, **kw: calls.append((a, kw)), "foo", "bar", baz="qux")
389+ self.assertEqual([(("foo", "bar"), {'baz': 'qux'})], calls)
390+
391+ def test_email_on_failure_no_emails(self):
392+ # If no emails are specified, then no email is sent on failure.
393+ log = []
394+ runner = self.make_ec2runner(email_log=log)
395+ self.assertRaises(
396+ ZeroDivisionError, runner.run, "failing method", lambda: 1/0)
397+ self.assertEqual([], log)
398+
399+ def test_email_on_failure_some_emails(self):
400+ # If no emails are specified, then no email is sent on failure.
401+ log = []
402+ runner = self.make_ec2runner(
403+ email_log=log, emails=["foo@example.com"])
404+ self.assertRaises(
405+ ZeroDivisionError, runner.run, "failing method", lambda: 1/0)
406+ # XXX: Expect this to fail. Fix the test to be more correct.
407+ [message] = log
408+ self.assertEqual('failing method FAILED', message['Subject'])
409+ self.assertEqual('foo@example.com', message['To'])
410+ self.assertIn('ZeroDivisionError', str(message))
411+
412+ def test_email_with_launchpad_tester_failure(self):
413+ email_log = []
414+ to_emails = ['foo@example.com']
415+ request = self.make_request(emails=to_emails, emails_sent=email_log)
416+ logger = self.make_logger(request=request)
417+ tester = self.make_tester(logger=logger)
418+ # Deliberately break 'tester'. A likely failure in production is not
419+ # being able to spawn the child process.
420+ tester._spawn_test_process = lambda: 1/0
421+ runner = self.make_ec2runner(emails=to_emails, email_log=email_log)
422+ runner.run("launchpad tester", tester.test)
423+ # The primary thing we care about is that email *was* sent.
424+ self.assertNotEqual([], email_log)
425+ [tester_msg] = email_log
426+ self.assertEqual('foo@example.com', tester_msg['To'])
427+ self.assertIn('ZeroDivisionError', str(tester_msg))
428+
429
430 class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):
431
432@@ -662,59 +768,69 @@
433 return email.get_payload()[0].get_payload()
434
435 def test_success_no_emails(self):
436- request = self.make_request(emails=[])
437+ log = []
438+ request = self.make_request(emails=[], emails_sent=log)
439 logger = self.make_logger(request=request)
440 logger.got_result(True)
441- self.assertEqual([], request.emails_sent)
442+ self.assertEqual([], log)
443
444 def test_failure_no_emails(self):
445- request = self.make_request(emails=[])
446+ log = []
447+ request = self.make_request(emails=[], emails_sent=log)
448 logger = self.make_logger(request=request)
449 logger.got_result(False)
450- self.assertEqual([], request.emails_sent)
451+ self.assertEqual([], log)
452
453 def test_submits_to_pqm_on_success(self):
454+ log = []
455 message = {'Subject': 'foo'}
456- request = self.make_request(emails=[], pqm_message=message)
457+ request = self.make_request(
458+ emails=[], pqm_message=message, emails_sent=log)
459 logger = self.make_logger(request=request)
460 logger.got_result(True)
461- self.assertEqual([message], request.emails_sent)
462+ self.assertEqual([message], log)
463
464 def test_records_pqm_submission_in_email(self):
465+ log = []
466 message = {'Subject': 'foo'}
467 request = self.make_request(
468- emails=['foo@example.com'], pqm_message=message)
469+ emails=['foo@example.com'], pqm_message=message, emails_sent=log)
470 logger = self.make_logger(request=request)
471 logger.got_result(True)
472- [pqm_message, user_message] = request.emails_sent
473+ [pqm_message, user_message] = log
474 self.assertEqual(message, pqm_message)
475 self.assertIn(
476 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
477 self.get_body_text(user_message))
478
479 def test_doesnt_submit_to_pqm_no_failure(self):
480+ log = []
481 message = {'Subject': 'foo'}
482- request = self.make_request(emails=[], pqm_message=message)
483+ request = self.make_request(
484+ emails=[], pqm_message=message, emails_sent=log)
485 logger = self.make_logger(request=request)
486 logger.got_result(False)
487- self.assertEqual([], request.emails_sent)
488+ self.assertEqual([], log)
489
490 def test_records_non_pqm_submission_in_email(self):
491+ log = []
492 message = {'Subject': 'foo'}
493 request = self.make_request(
494- emails=['foo@example.com'], pqm_message=message)
495+ emails=['foo@example.com'], pqm_message=message, emails_sent=log)
496 logger = self.make_logger(request=request)
497 logger.got_result(False)
498- [user_message] = request.emails_sent
499+ [user_message] = log
500 self.assertIn(
501 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),
502 self.get_body_text(user_message))
503
504 def test_email_refers_to_attached_log(self):
505- request = self.make_request(emails=['foo@example.com'])
506+ log = []
507+ request = self.make_request(
508+ emails=['foo@example.com'], emails_sent=log)
509 logger = self.make_logger(request=request)
510 logger.got_result(False)
511- [user_message] = request.emails_sent
512+ [user_message] = log
513 self.assertIn(
514 '(See the attached file for the complete log)\n',
515 self.get_body_text(user_message))
516@@ -726,23 +842,27 @@
517 # now and can vary independently from the contents of the sent
518 # email. We probably just want the contents of the email to be a list
519 # of failing tests.
520- request = self.make_request(emails=['foo@example.com'])
521+ log = []
522+ request = self.make_request(
523+ emails=['foo@example.com'], emails_sent=log)
524 logger = self.make_logger(request=request)
525 logger.get_summary_stream().write('bar\nbaz\nqux\n')
526 logger.got_result(False)
527- [user_message] = request.emails_sent
528+ [user_message] = log
529 self.assertEqual(
530 'bar\nbaz\nqux\n\n(See the attached file for the complete log)\n',
531 self.get_body_text(user_message))
532
533 def test_gzip_of_full_log_attached(self):
534 # The full log is attached to the email.
535- request = self.make_request(emails=['foo@example.com'])
536+ log = []
537+ request = self.make_request(
538+ emails=['foo@example.com'], emails_sent=log)
539 logger = self.make_logger(request=request)
540 logger.got_line("output from test process\n")
541 logger.got_line("more output\n")
542 logger.got_result(False)
543- [user_message] = request.emails_sent
544+ [user_message] = log
545 [body, attachment] = user_message.get_payload()
546 self.assertEqual('application/x-gzip', attachment['Content-Type'])
547 self.assertEqual(