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

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
=== 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 12:56:33 +0000
@@ -40,11 +40,12 @@
4040
41import bzrlib.branch41import bzrlib.branch
42import bzrlib.config42import bzrlib.config
43import bzrlib.email_message
44import bzrlib.errors43import bzrlib.errors
45import bzrlib.smtp_connection
46import bzrlib.workingtree44import bzrlib.workingtree
4745
46from bzrlib.email_message import EmailMessage
47from bzrlib.smtp_connection import SMTPConnection
48
48import subunit49import subunit
4950
5051
@@ -134,19 +135,24 @@
134 SHUTDOWN_DELAY = 60135 SHUTDOWN_DELAY = 60
135136
136 def __init__(self, daemonize, pid_filename, shutdown_when_done,137 def __init__(self, daemonize, pid_filename, shutdown_when_done,
137 emails=None):138 smtp_connection=None, emails=None):
138 """Make an EC2Runner.139 """Make an EC2Runner.
139140
140 :param daemonize: Whether or not we will daemonize.141 :param daemonize: Whether or not we will daemonize.
141 :param pid_filename: The filename to store the pid in.142 :param pid_filename: The filename to store the pid in.
142 :param shutdown_when_done: Whether or not to shut down when the tests143 :param shutdown_when_done: Whether or not to shut down when the tests
143 are done.144 are done.
145 :param smtp_connection: The `SMTPConnection` to use to send email.
144 :param emails: The email address(es) to send catastrophic failure146 :param emails: The email address(es) to send catastrophic failure
145 messages to. If not provided, the error disappears into the ether.147 messages to. If not provided, the error disappears into the ether.
146 """148 """
147 self._should_daemonize = daemonize149 self._should_daemonize = daemonize
148 self._pid_filename = pid_filename150 self._pid_filename = pid_filename
149 self._shutdown_when_done = shutdown_when_done151 self._shutdown_when_done = shutdown_when_done
152 if smtp_connection is None:
153 config = bzrlib.config.GlobalConfig()
154 smtp_connection = SMTPConnection(config)
155 self._smtp_connection = smtp_connection
150 self._emails = emails156 self._emails = emails
151 self._daemonized = False157 self._daemonized = False
152158
@@ -188,10 +194,12 @@
188 config = bzrlib.config.GlobalConfig()194 config = bzrlib.config.GlobalConfig()
189 # Handle exceptions thrown by the test() or daemonize() methods.195 # Handle exceptions thrown by the test() or daemonize() methods.
190 if self._emails:196 if self._emails:
191 bzrlib.email_message.EmailMessage.send(197 msg = EmailMessage(
192 config, config.username(),198 from_address=config.username(),
193 self._emails, '%s FAILED' % (name,),199 to_address=self._emails,
194 traceback.format_exc())200 subject='%s FAILED' % (name,),
201 body=traceback.format_exc())
202 self._smtp_connection.send_email(msg)
195 raise203 raise
196 finally:204 finally:
197 # When everything is over, if we've been ask to shut down, then205 # When everything is over, if we've been ask to shut down, then
@@ -267,9 +275,7 @@
267 exit_status = popen.wait()275 exit_status = popen.wait()
268 except:276 except:
269 self._logger.error_in_testrunner(sys.exc_info())277 self._logger.error_in_testrunner(sys.exc_info())
270 exit_status = 1278 else:
271 raise
272 finally:
273 self._logger.got_result(not exit_status)279 self._logger.got_result(not exit_status)
274280
275 def _gather_test_output(self, input_stream, logger):281 def _gather_test_output(self, input_stream, logger):
@@ -287,7 +293,7 @@
287 """A request to have a branch tested and maybe landed."""293 """A request to have a branch tested and maybe landed."""
288294
289 def __init__(self, branch_url, revno, local_branch_path, sourcecode_path,295 def __init__(self, branch_url, revno, local_branch_path, sourcecode_path,
290 emails=None, pqm_message=None):296 emails=None, pqm_message=None, smtp_connection=None):
291 """Construct a `Request`.297 """Construct a `Request`.
292298
293 :param branch_url: The public URL to the Launchpad branch we are299 :param branch_url: The public URL to the Launchpad branch we are
@@ -303,6 +309,7 @@
303 provided, no emails are sent.309 provided, no emails are sent.
304 :param pqm_message: The message to submit to PQM. If not provided, we310 :param pqm_message: The message to submit to PQM. If not provided, we
305 don't submit to PQM.311 don't submit to PQM.
312 :param smtp_connection: The `SMTPConnection` to use to send email.
306 """313 """
307 self._branch_url = branch_url314 self._branch_url = branch_url
308 self._revno = revno315 self._revno = revno
@@ -312,11 +319,13 @@
312 self._pqm_message = pqm_message319 self._pqm_message = pqm_message
313 # Used for figuring out how to send emails.320 # Used for figuring out how to send emails.
314 self._bzr_config = bzrlib.config.GlobalConfig()321 self._bzr_config = bzrlib.config.GlobalConfig()
322 if smtp_connection is None:
323 smtp_connection = SMTPConnection(self._bzr_config)
324 self._smtp_connection = smtp_connection
315325
316 def _send_email(self, message):326 def _send_email(self, message):
317 """Actually send 'message'."""327 """Actually send 'message'."""
318 conn = bzrlib.smtp_connection.SMTPConnection(self._bzr_config)328 self._smtp_connection.send_email(message)
319 conn.send_email(message)
320329
321 def get_target_details(self):330 def get_target_details(self):
322 """Return (branch_url, revno) for trunk."""331 """Return (branch_url, revno) for trunk."""
@@ -510,6 +519,13 @@
510 summary.write('\n\nERROR IN TESTRUNNER\n\n')519 summary.write('\n\nERROR IN TESTRUNNER\n\n')
511 traceback.print_exception(exc_type, exc_value, exc_tb, file=summary)520 traceback.print_exception(exc_type, exc_value, exc_tb, file=summary)
512 summary.flush()521 summary.flush()
522 if self._request.wants_email:
523 self._write_to_filename(
524 self._summary_filename,
525 '\n(See the attached file for the complete log)\n')
526 summary = self.get_summary_contents()
527 full_log_gz = gzip_data(self.get_full_log_contents())
528 self._request.send_report_email(False, summary, full_log_gz)
513529
514 def get_index_contents(self):530 def get_index_contents(self):
515 """Return the contents of the index.html page."""531 """Return the contents of the index.html page."""
@@ -805,16 +821,19 @@
805821
806 pid_filename = os.path.join(LAUNCHPAD_DIR, 'ec2test-remote.pid')822 pid_filename = os.path.join(LAUNCHPAD_DIR, 'ec2test-remote.pid')
807823
824 smtp_connection = SMTPConnection(bzrlib.config.GlobalConfig())
825
808 request = Request(826 request = Request(
809 options.public_branch, options.public_branch_revno, TEST_DIR,827 options.public_branch, options.public_branch_revno, TEST_DIR,
810 SOURCECODE_DIR, options.email, pqm_message)828 SOURCECODE_DIR, options.email, pqm_message, smtp_connection)
811 # Only write to stdout if we are running as the foreground process.829 # Only write to stdout if we are running as the foreground process.
812 echo_to_stdout = not options.daemon830 echo_to_stdout = not options.daemon
813 logger = WebTestLogger.make_in_directory(831 logger = WebTestLogger.make_in_directory(
814 '/var/www', request, echo_to_stdout)832 '/var/www', request, echo_to_stdout)
815833
816 runner = EC2Runner(834 runner = EC2Runner(
817 options.daemon, pid_filename, options.shutdown, options.email)835 options.daemon, pid_filename, options.shutdown,
836 smtp_connection, options.email)
818837
819 tester = LaunchpadTester(logger, TEST_DIR, test_options=args[1:])838 tester = LaunchpadTester(logger, TEST_DIR, test_options=args[1:])
820 runner.run("Test runner", tester.test)839 runner.run("Test runner", tester.test)
821840
=== 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 12:56:33 +0000
@@ -26,6 +26,7 @@
26from testtools.content_type import ContentType26from testtools.content_type import ContentType
2727
28from devscripts.ec2test.remote import (28from devscripts.ec2test.remote import (
29 EC2Runner,
29 FlagFallStream,30 FlagFallStream,
30 gunzip_data,31 gunzip_data,
31 gzip_data,32 gzip_data,
@@ -38,6 +39,16 @@
38 )39 )
3940
4041
42class LoggingSMTPConnection(object):
43 """An SMTPConnection double that logs sent email."""
44
45 def __init__(self, log):
46 self._log = log
47
48 def send_email(self, message):
49 self._log.append(message)
50
51
41class RequestHelpers:52class RequestHelpers:
4253
43 def patch(self, obj, name, value):54 def patch(self, obj, name, value):
@@ -60,7 +71,7 @@
6071
61 def make_request(self, branch_url=None, revno=None,72 def make_request(self, branch_url=None, revno=None,
62 trunk=None, sourcecode_path=None,73 trunk=None, sourcecode_path=None,
63 emails=None, pqm_message=None):74 emails=None, pqm_message=None, emails_sent=None):
64 """Make a request to test, specifying only things we care about.75 """Make a request to test, specifying only things we care about.
6576
66 Note that the returned request object will not ever send email, but77 Note that the returned request object will not ever send email, but
@@ -73,11 +84,12 @@
73 [('a', 'http://example.com/bzr/a', 2),84 [('a', 'http://example.com/bzr/a', 2),
74 ('b', 'http://example.com/bzr/b', 3),85 ('b', 'http://example.com/bzr/b', 3),
75 ('c', 'http://example.com/bzr/c', 5)])86 ('c', 'http://example.com/bzr/c', 5)])
87 if emails_sent is None:
88 emails_sent = []
89 smtp_connection = LoggingSMTPConnection(emails_sent)
76 request = Request(90 request = Request(
77 branch_url, revno, trunk.basedir, sourcecode_path, emails,91 branch_url, revno, trunk.basedir, sourcecode_path, emails,
78 pqm_message)92 pqm_message, smtp_connection)
79 request.emails_sent = []
80 request._send_email = request.emails_sent.append
81 return request93 return request
8294
83 def make_sourcecode(self, branches):95 def make_sourcecode(self, branches):
@@ -94,6 +106,13 @@
94 tree.commit(message=str(i))106 tree.commit(message=str(i))
95 return 'sourcecode/'107 return 'sourcecode/'
96108
109 def make_tester(self, logger=None, test_directory=None, test_options=()):
110 if not logger:
111 logger = self.make_logger()
112 if not test_directory:
113 test_directory = 'unspecified-test-directory'
114 return LaunchpadTester(logger, test_directory, test_options)
115
97 def make_logger(self, request=None, echo_to_stdout=False):116 def make_logger(self, request=None, echo_to_stdout=False):
98 if request is None:117 if request is None:
99 request = self.make_request()118 request = self.make_request()
@@ -199,13 +218,6 @@
199218
200class TestLaunchpadTester(TestCaseWithTransport, RequestHelpers):219class TestLaunchpadTester(TestCaseWithTransport, RequestHelpers):
201220
202 def make_tester(self, logger=None, test_directory=None, test_options=()):
203 if not logger:
204 logger = self.make_logger()
205 if not test_directory:
206 test_directory = 'unspecified-test-directory'
207 return LaunchpadTester(logger, test_directory, test_options)
208
209 def test_build_test_command_no_options(self):221 def test_build_test_command_no_options(self):
210 # The LaunchpadTester runs "make check" if given no options.222 # The LaunchpadTester runs "make check" if given no options.
211 tester = self.make_tester()223 tester = self.make_tester()
@@ -243,14 +255,15 @@
243 # got_result with the result. This test is more of a smoke test to255 # got_result with the result. This test is more of a smoke test to
244 # make sure that everything integrates well.256 # make sure that everything integrates well.
245 message = {'Subject': "One Crowded Hour"}257 message = {'Subject': "One Crowded Hour"}
246 request = self.make_request(pqm_message=message)258 log = []
259 request = self.make_request(pqm_message=message, emails_sent=log)
247 logger = self.make_logger(request=request)260 logger = self.make_logger(request=request)
248 tester = self.make_tester(logger=logger)261 tester = self.make_tester(logger=logger)
249 output = "test output\n"262 output = "test output\n"
250 tester._spawn_test_process = lambda: FakePopen(output, 0)263 tester._spawn_test_process = lambda: FakePopen(output, 0)
251 tester.test()264 tester.test()
252 # Message being sent implies got_result thought it got a success.265 # Message being sent implies got_result thought it got a success.
253 self.assertEqual([message], request.emails_sent)266 self.assertEqual([message], log)
254267
255 def test_error_in_testrunner(self):268 def test_error_in_testrunner(self):
256 # Any exception is raised within LaunchpadTester.test() is an error in269 # Any exception is raised within LaunchpadTester.test() is an error in
@@ -260,28 +273,30 @@
260 # failure.273 # failure.
261 # 3. Re-raise the error. In the script, this triggers an email.274 # 3. Re-raise the error. In the script, this triggers an email.
262 message = {'Subject': "One Crowded Hour"}275 message = {'Subject': "One Crowded Hour"}
263 request = self.make_request(pqm_message=message)276 log = []
277 request = self.make_request(pqm_message=message, emails_sent=log)
264 logger = self.make_logger(request=request)278 logger = self.make_logger(request=request)
265 tester = self.make_tester(logger=logger)279 tester = self.make_tester(logger=logger)
266 # Break the test runner deliberately. In production, this is more280 # Break the test runner deliberately. In production, this is more
267 # likely to be a system error than a programming error.281 # likely to be a system error than a programming error.
268 tester._spawn_test_process = lambda: 1/0282 tester._spawn_test_process = lambda: 1/0
269 self.assertRaises(ZeroDivisionError, tester.test)283 tester.test()
270 # Message not being sent implies got_result thought it got a failure.284 # Message not being sent implies got_result thought it got a failure.
271 self.assertEqual([], request.emails_sent)285 self.assertEqual([], log)
272 self.assertIn("ERROR IN TESTRUNNER", logger.get_summary_contents())286 self.assertIn("ERROR IN TESTRUNNER", logger.get_summary_contents())
273 self.assertIn("ZeroDivisionError", logger.get_summary_contents())287 self.assertIn("ZeroDivisionError", logger.get_summary_contents())
274288
275 def test_nonzero_exit_code(self):289 def test_nonzero_exit_code(self):
276 message = {'Subject': "One Crowded Hour"}290 message = {'Subject': "One Crowded Hour"}
277 request = self.make_request(pqm_message=message)291 log = []
292 request = self.make_request(pqm_message=message, emails_sent=log)
278 logger = self.make_logger(request=request)293 logger = self.make_logger(request=request)
279 tester = self.make_tester(logger=logger)294 tester = self.make_tester(logger=logger)
280 output = "test output\n"295 output = "test output\n"
281 tester._spawn_test_process = lambda: FakePopen(output, 10)296 tester._spawn_test_process = lambda: FakePopen(output, 10)
282 tester.test()297 tester.test()
283 # Message not being sent implies got_result thought it got a failure.298 # Message not being sent implies got_result thought it got a failure.
284 self.assertEqual([], request.emails_sent)299 self.assertEqual([], log)
285300
286301
287class TestPidfileHelpers(TestCase):302class TestPidfileHelpers(TestCase):
@@ -449,9 +464,10 @@
449464
450 def test_submit_to_pqm_no_message_doesnt_send(self):465 def test_submit_to_pqm_no_message_doesnt_send(self):
451 # If there's no PQM message, then 'submit_to_pqm' returns None.466 # If there's no PQM message, then 'submit_to_pqm' returns None.
452 req = self.make_request(pqm_message=None)467 log = []
468 req = self.make_request(pqm_message=None, emails_sent=log)
453 req.submit_to_pqm(successful=True)469 req.submit_to_pqm(successful=True)
454 self.assertEqual([], req.emails_sent)470 self.assertEqual([], log)
455471
456 def test_submit_to_pqm_unsuccessful(self):472 def test_submit_to_pqm_unsuccessful(self):
457 # submit_to_pqm returns the subject of the PQM mail even if it's473 # submit_to_pqm returns the subject of the PQM mail even if it's
@@ -464,17 +480,19 @@
464 def test_submit_to_pqm_unsuccessful_no_email(self):480 def test_submit_to_pqm_unsuccessful_no_email(self):
465 # submit_to_pqm doesn't send any email if the run was unsuccessful.481 # submit_to_pqm doesn't send any email if the run was unsuccessful.
466 message = {'Subject:': 'My PQM message'}482 message = {'Subject:': 'My PQM message'}
467 req = self.make_request(pqm_message=message)483 log = []
484 req = self.make_request(pqm_message=message, emails_sent=log)
468 req.submit_to_pqm(successful=False)485 req.submit_to_pqm(successful=False)
469 self.assertEqual([], req.emails_sent)486 self.assertEqual([], log)
470487
471 def test_submit_to_pqm_successful(self):488 def test_submit_to_pqm_successful(self):
472 # submit_to_pqm returns the subject of the PQM mail.489 # submit_to_pqm returns the subject of the PQM mail.
473 message = {'Subject:': 'My PQM message'}490 message = {'Subject:': 'My PQM message'}
474 req = self.make_request(pqm_message=message)491 log = []
492 req = self.make_request(pqm_message=message, emails_sent=log)
475 subject = req.submit_to_pqm(successful=True)493 subject = req.submit_to_pqm(successful=True)
476 self.assertIs(message.get('Subject'), subject)494 self.assertIs(message.get('Subject'), subject)
477 self.assertEqual([message], req.emails_sent)495 self.assertEqual([message], log)
478496
479 def test_report_email_subject_success(self):497 def test_report_email_subject_success(self):
480 req = self.make_request(emails=['foo@example.com'])498 req = self.make_request(emails=['foo@example.com'])
@@ -523,10 +541,11 @@
523 "gobbledygook", attachment.get_payload().decode('base64'))541 "gobbledygook", attachment.get_payload().decode('base64'))
524542
525 def test_send_report_email_sends_email(self):543 def test_send_report_email_sends_email(self):
526 req = self.make_request(emails=['foo@example.com'])544 log = []
545 req = self.make_request(emails=['foo@example.com'], emails_sent=log)
527 expected = req._build_report_email(False, "foo", "gobbledygook")546 expected = req._build_report_email(False, "foo", "gobbledygook")
528 req.send_report_email(False, "foo", "gobbledygook")547 req.send_report_email(False, "foo", "gobbledygook")
529 [observed] = req.emails_sent548 [observed] = log
530 # The standard library sucks. None of the MIME objects have __eq__549 # The standard library sucks. None of the MIME objects have __eq__
531 # implementations.550 # implementations.
532 for expected_part, observed_part in izip(551 for expected_part, observed_part in izip(
@@ -597,7 +616,7 @@
597 self.assertEqual('foo\n', logger.get_full_log_contents())616 self.assertEqual('foo\n', logger.get_full_log_contents())
598 self.assertEqual('foo\n', logger.get_summary_contents())617 self.assertEqual('foo\n', logger.get_summary_contents())
599618
600 def test_error_in_testrunner(self):619 def test_error_in_testrunner_logs_to_summary(self):
601 # error_in_testrunner logs the traceback to the summary log in a very620 # error_in_testrunner logs the traceback to the summary log in a very
602 # prominent way.621 # prominent way.
603 try:622 try:
@@ -611,6 +630,93 @@
611 "\n\nERROR IN TESTRUNNER\n\n%s" % (stack,),630 "\n\nERROR IN TESTRUNNER\n\n%s" % (stack,),
612 logger.get_summary_contents())631 logger.get_summary_contents())
613632
633 def test_error_in_testrunner_sends_email(self):
634 # If email addresses are configurd, error_in_testrunner emails them
635 # with the failure and the full log.
636 try:
637 1/0
638 except ZeroDivisionError:
639 exc_info = sys.exc_info()
640 log = []
641 request = self.make_request(
642 emails=['foo@example.com'], emails_sent=log)
643 logger = self.make_logger(request=request)
644 logger.error_in_testrunner(exc_info)
645 [email] = log
646 self.assertEqual(
647 'Test results: %s: FAILURE' % request.get_merge_description(),
648 email['Subject'])
649 [body, attachment] = email.get_payload()
650 self.assertIsInstance(body, MIMEText)
651 self.assertEqual('inline', body['Content-Disposition'])
652 self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
653 self.assertEqual(
654 logger.get_summary_contents(), body.get_payload(decode=True))
655 self.assertIsInstance(attachment, MIMEApplication)
656 self.assertEqual('application/x-gzip', attachment['Content-Type'])
657 self.assertEqual(
658 'attachment;',
659 attachment['Content-Disposition'][:len('attachment;')])
660 self.assertEqual(
661 logger.get_full_log_contents(),
662 gunzip_data(attachment.get_payload().decode('base64')))
663
664
665class TestEC2Runner(TestCaseWithTransport, RequestHelpers):
666
667 def make_ec2runner(self, emails=None, email_log=None):
668 if email_log is None:
669 email_log = []
670 smtp_connection = LoggingSMTPConnection(email_log)
671 return EC2Runner(
672 False, "who-cares.pid", False, smtp_connection, emails=emails)
673
674 def test_run(self):
675 calls = []
676 runner = self.make_ec2runner()
677 runner.run(
678 "boring test method",
679 lambda *a, **kw: calls.append((a, kw)), "foo", "bar", baz="qux")
680 self.assertEqual([(("foo", "bar"), {'baz': 'qux'})], calls)
681
682 def test_email_on_failure_no_emails(self):
683 # If no emails are specified, then no email is sent on failure.
684 log = []
685 runner = self.make_ec2runner(email_log=log)
686 self.assertRaises(
687 ZeroDivisionError, runner.run, "failing method", lambda: 1/0)
688 self.assertEqual([], log)
689
690 def test_email_on_failure_some_emails(self):
691 # If no emails are specified, then no email is sent on failure.
692 log = []
693 runner = self.make_ec2runner(
694 email_log=log, emails=["foo@example.com"])
695 self.assertRaises(
696 ZeroDivisionError, runner.run, "failing method", lambda: 1/0)
697 # XXX: Expect this to fail. Fix the test to be more correct.
698 [message] = log
699 self.assertEqual('failing method FAILED', message['Subject'])
700 self.assertEqual('foo@example.com', message['To'])
701 self.assertIn('ZeroDivisionError', str(message))
702
703 def test_email_with_launchpad_tester_failure(self):
704 email_log = []
705 to_emails = ['foo@example.com']
706 request = self.make_request(emails=to_emails, emails_sent=email_log)
707 logger = self.make_logger(request=request)
708 tester = self.make_tester(logger=logger)
709 # Deliberately break 'tester'. A likely failure in production is not
710 # being able to spawn the child process.
711 tester._spawn_test_process = lambda: 1/0
712 runner = self.make_ec2runner(emails=to_emails, email_log=email_log)
713 runner.run("launchpad tester", tester.test)
714 # The primary thing we care about is that email *was* sent.
715 self.assertNotEqual([], email_log)
716 [tester_msg] = email_log
717 self.assertEqual('foo@example.com', tester_msg['To'])
718 self.assertIn('ZeroDivisionError', str(tester_msg))
719
614720
615class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):721class TestDaemonizationInteraction(TestCaseWithTransport, RequestHelpers):
616722
@@ -662,59 +768,69 @@
662 return email.get_payload()[0].get_payload()768 return email.get_payload()[0].get_payload()
663769
664 def test_success_no_emails(self):770 def test_success_no_emails(self):
665 request = self.make_request(emails=[])771 log = []
772 request = self.make_request(emails=[], emails_sent=log)
666 logger = self.make_logger(request=request)773 logger = self.make_logger(request=request)
667 logger.got_result(True)774 logger.got_result(True)
668 self.assertEqual([], request.emails_sent)775 self.assertEqual([], log)
669776
670 def test_failure_no_emails(self):777 def test_failure_no_emails(self):
671 request = self.make_request(emails=[])778 log = []
779 request = self.make_request(emails=[], emails_sent=log)
672 logger = self.make_logger(request=request)780 logger = self.make_logger(request=request)
673 logger.got_result(False)781 logger.got_result(False)
674 self.assertEqual([], request.emails_sent)782 self.assertEqual([], log)
675783
676 def test_submits_to_pqm_on_success(self):784 def test_submits_to_pqm_on_success(self):
785 log = []
677 message = {'Subject': 'foo'}786 message = {'Subject': 'foo'}
678 request = self.make_request(emails=[], pqm_message=message)787 request = self.make_request(
788 emails=[], pqm_message=message, emails_sent=log)
679 logger = self.make_logger(request=request)789 logger = self.make_logger(request=request)
680 logger.got_result(True)790 logger.got_result(True)
681 self.assertEqual([message], request.emails_sent)791 self.assertEqual([message], log)
682792
683 def test_records_pqm_submission_in_email(self):793 def test_records_pqm_submission_in_email(self):
794 log = []
684 message = {'Subject': 'foo'}795 message = {'Subject': 'foo'}
685 request = self.make_request(796 request = self.make_request(
686 emails=['foo@example.com'], pqm_message=message)797 emails=['foo@example.com'], pqm_message=message, emails_sent=log)
687 logger = self.make_logger(request=request)798 logger = self.make_logger(request=request)
688 logger.got_result(True)799 logger.got_result(True)
689 [pqm_message, user_message] = request.emails_sent800 [pqm_message, user_message] = log
690 self.assertEqual(message, pqm_message)801 self.assertEqual(message, pqm_message)
691 self.assertIn(802 self.assertIn(
692 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),803 'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
693 self.get_body_text(user_message))804 self.get_body_text(user_message))
694805
695 def test_doesnt_submit_to_pqm_no_failure(self):806 def test_doesnt_submit_to_pqm_no_failure(self):
807 log = []
696 message = {'Subject': 'foo'}808 message = {'Subject': 'foo'}
697 request = self.make_request(emails=[], pqm_message=message)809 request = self.make_request(
810 emails=[], pqm_message=message, emails_sent=log)
698 logger = self.make_logger(request=request)811 logger = self.make_logger(request=request)
699 logger.got_result(False)812 logger.got_result(False)
700 self.assertEqual([], request.emails_sent)813 self.assertEqual([], log)
701814
702 def test_records_non_pqm_submission_in_email(self):815 def test_records_non_pqm_submission_in_email(self):
816 log = []
703 message = {'Subject': 'foo'}817 message = {'Subject': 'foo'}
704 request = self.make_request(818 request = self.make_request(
705 emails=['foo@example.com'], pqm_message=message)819 emails=['foo@example.com'], pqm_message=message, emails_sent=log)
706 logger = self.make_logger(request=request)820 logger = self.make_logger(request=request)
707 logger.got_result(False)821 logger.got_result(False)
708 [user_message] = request.emails_sent822 [user_message] = log
709 self.assertIn(823 self.assertIn(
710 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),824 '**NOT** submitted to PQM:\n%s' % (message['Subject'],),
711 self.get_body_text(user_message))825 self.get_body_text(user_message))
712826
713 def test_email_refers_to_attached_log(self):827 def test_email_refers_to_attached_log(self):
714 request = self.make_request(emails=['foo@example.com'])828 log = []
829 request = self.make_request(
830 emails=['foo@example.com'], emails_sent=log)
715 logger = self.make_logger(request=request)831 logger = self.make_logger(request=request)
716 logger.got_result(False)832 logger.got_result(False)
717 [user_message] = request.emails_sent833 [user_message] = log
718 self.assertIn(834 self.assertIn(
719 '(See the attached file for the complete log)\n',835 '(See the attached file for the complete log)\n',
720 self.get_body_text(user_message))836 self.get_body_text(user_message))
@@ -726,23 +842,27 @@
726 # now and can vary independently from the contents of the sent842 # now and can vary independently from the contents of the sent
727 # email. We probably just want the contents of the email to be a list843 # email. We probably just want the contents of the email to be a list
728 # of failing tests.844 # of failing tests.
729 request = self.make_request(emails=['foo@example.com'])845 log = []
846 request = self.make_request(
847 emails=['foo@example.com'], emails_sent=log)
730 logger = self.make_logger(request=request)848 logger = self.make_logger(request=request)
731 logger.get_summary_stream().write('bar\nbaz\nqux\n')849 logger.get_summary_stream().write('bar\nbaz\nqux\n')
732 logger.got_result(False)850 logger.got_result(False)
733 [user_message] = request.emails_sent851 [user_message] = log
734 self.assertEqual(852 self.assertEqual(
735 'bar\nbaz\nqux\n\n(See the attached file for the complete log)\n',853 'bar\nbaz\nqux\n\n(See the attached file for the complete log)\n',
736 self.get_body_text(user_message))854 self.get_body_text(user_message))
737855
738 def test_gzip_of_full_log_attached(self):856 def test_gzip_of_full_log_attached(self):
739 # The full log is attached to the email.857 # The full log is attached to the email.
740 request = self.make_request(emails=['foo@example.com'])858 log = []
859 request = self.make_request(
860 emails=['foo@example.com'], emails_sent=log)
741 logger = self.make_logger(request=request)861 logger = self.make_logger(request=request)
742 logger.got_line("output from test process\n")862 logger.got_line("output from test process\n")
743 logger.got_line("more output\n")863 logger.got_line("more output\n")
744 logger.got_result(False)864 logger.got_result(False)
745 [user_message] = request.emails_sent865 [user_message] = log
746 [body, attachment] = user_message.get_payload()866 [body, attachment] = user_message.get_payload()
747 self.assertEqual('application/x-gzip', attachment['Content-Type'])867 self.assertEqual('application/x-gzip', attachment['Content-Type'])
748 self.assertEqual(868 self.assertEqual(