Merge lp:~matsubara/tarmac/bundle-lander into lp:~launchpad/tarmac/lp-tarmac

Proposed by Diogo Matsubara on 2010-12-10
Status: Merged
Approved by: Diogo Matsubara on 2010-12-15
Approved revision: 398
Merged at revision: 384
Proposed branch: lp:~matsubara/tarmac/bundle-lander
Merge into: lp:~launchpad/tarmac/lp-tarmac
Diff against target: 834 lines (+661/-35)
7 files modified
tarmac/bin/commands.py (+20/-0)
tarmac/pidfile.py (+77/-0)
tarmac/plugins/bundlecommand.py (+215/-30)
tarmac/plugins/tests/test_bundlecommand.py (+280/-0)
tarmac/tests/mock.py (+17/-0)
tarmac/tests/test_commands.py (+21/-5)
tarmac/tests/test_pidfile.py (+31/-0)
To merge this branch: bzr merge lp:~matsubara/tarmac/bundle-lander
Reviewer Review Type Date Requested Status
Gary Poster (community) 2010-12-10 Approve on 2010-12-15
Review via email: mp+43396@code.launchpad.net

Commit message

Implements a lock mechanism and a way to properly format the test results similar to the way they are formatted in ec2test and email those results to merge proposal subscribers.

Description of the change

This branch implements a simple lock mechanism for the bundle merge command and implements a way to properly format the test resuls similar to the way they are formatted in ec2test and email those results to merge proposal subscribers.
All tests pass.

To post a comment you must log in.
Gary Poster (gary) wrote :

Lock file: here are some things that would be nice, in decreasing order of importance in my mind.
 1 lock file is actually a pid file. pid is reported in error message. This gives SAs a chance to quickly see if it is dead or not.
 2 if lock file exists and has been around for fewer than N seconds, exit with status code 0. If lock file has been around for greater than or equal to N seconds, exit with a non-0 status code. N should be configurable. This gives SAs a chance to care about serious problems, and ignore typical problems.
 3 if lock file exists, error includes how long it has been around. That's easy enough to do without (just look at the file yourself), but a nice convenience.
 4 if lock file exists, with pid, and identified pid doesn't exist, delete the pid file, mention what you are doing, and proceed to run tarmac.

FWIW, lib/canonical/lazr/pidfile.py has some simple code to steal for #1.

I think I'll say that I want #1 unless you convince me otherwise. I'd be happy if you threw in #2 and #3, but I'm fine with ignoring it for now. I don't think you should pursue #4 right now.

Email: cool. A small note is that "(See the attached file for the complete log)" will be included in the merge proposal output, and that will be incorrect. Might be mildly confusing. Would be nice to omit it in the merge proposal, and only include it when sending the mail.

Nice tests. It test_run_failure, is it possible/reasonable to show that the merge proposal was changed, and that emails were sent? That seems like it would be nice.

Gary Poster (gary) wrote :

And by the way, very nice branch. :-) Thank you.

Diogo Matsubara (matsubara) wrote :

On Mon, Dec 13, 2010 at 3:03 PM, Gary Poster <email address hidden> wrote:
> Lock file: here are some things that would be nice, in decreasing order of importance in my mind.
>  1 lock file is actually a pid file.  pid is reported in error message.  This gives SAs a chance to quickly see if it is dead or not.

I did this one by reusing Tarmac's PID_FILE in the config.

>  2 if lock file exists and has been around for fewer than N seconds, exit with status code 0.  If lock file has been around for greater than or equal to N seconds, exit with a non-0 status code.  N should be configurable.  This gives SAs a chance to care about serious problems, and ignore typical problems.

Done. SAs need to change the tarmac config file to look like:

allowed_lock_period = X where X is the amount of seconds allowed for
the lock to be considered OK.

If the lock file is in place for less than the amount configured,
tarmac will return and log the amount. If the lock file is in place
for more than the amount configured, tarmac will return an error and
show the PID and the lock file age.

>  3 if lock file exists, error includes how long it has been around.  That's easy enough to do without (just look at the file yourself), but a nice convenience.

Done. It'll log the lock file age and pid as explained above.

>  4 if lock file exists, with pid, and identified pid doesn't exist, delete the pid file, mention what you are doing, and proceed to run tarmac.

Didn't do this one.

>
> FWIW, lib/canonical/lazr/pidfile.py has some simple code to steal for #1.
>
> I think I'll say that I want #1 unless you convince me otherwise.  I'd be happy if you threw in #2 and #3, but I'm fine with ignoring it for now.  I don't think you should pursue #4 right now.
>
> Email: cool.  A small note is that "(See the attached file for the complete log)" will be included in the merge proposal output, and that will be incorrect.  Might be mildly confusing.  Would be nice to omit it in the merge proposal, and only include it when sending the mail.

Done.

>
> Nice tests.  It test_run_failure, is it possible/reasonable to show that the merge proposal was changed, and that emails were sent?  That seems like it would be nice.

Done.

Thanks for the review.
--
Diogo M. Matsubara

Diogo Matsubara (matsubara) wrote :

Hi Gary,

based in our conversation on IRC I added tests to the lock file mechanism and to the functions in the pidfile.py module. I removed an XXX in the bundle merge command test to make it more complete.

Let me know what you think and if this is ok to be merged.

Thanks for the detailed review and help.

Gary Poster (gary) wrote :

Hi Diogo. We talked about a couple of things that will probably be important for merging this into tarmac trunk, but since we are not doing that now, I'm not worrying about it yet. Thank you!

Here are the notes I have that might be pertinent when we consider merging this to tarmac trunk.

 - We are reusing some sort of tarmac pid configuration value that didn't seem to actually be used in tarmac. Therefore, we are not sure if what we are doing makes sense in the larger context.

 - We are only using the lock for certain commands, not help. Does that make sense, or should we make the lock pervasive, or pervasive for everything other than help? I don't know, and it's not necessary to figure out the answer yet, but it will be when a merge is possible.

review: Approve
Diogo Matsubara (matsubara) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'tarmac/bin/commands.py'
--- tarmac/bin/commands.py 2010-12-15 16:32:58 +0000
+++ tarmac/bin/commands.py 2010-12-15 16:55:13 +0000
@@ -2,6 +2,7 @@
2import httplib22import httplib2
3import logging3import logging
4import os4import os
5import time
56
6from bzrlib.commands import Command7from bzrlib.commands import Command
7from bzrlib.help import help_commands8from bzrlib.help import help_commands
@@ -14,6 +15,7 @@
14from tarmac.log import set_up_debug_logging, set_up_logging15from tarmac.log import set_up_debug_logging, set_up_logging
15from tarmac.exceptions import TarmacCommandError, TarmacMergeError16from tarmac.exceptions import TarmacCommandError, TarmacMergeError
16from tarmac.plugin import load_plugins17from tarmac.plugin import load_plugins
18from tarmac.pidfile import get_pid, make_pidfile
1719
1820
19class TarmacCommand(Command):21class TarmacCommand(Command):
@@ -235,6 +237,24 @@
235 """237 """
236238
237 def _do_merges(self, branch_url, imply_commit_message):239 def _do_merges(self, branch_url, imply_commit_message):
240 existing_pid = get_pid()
241 if existing_pid is None:
242 make_pidfile()
243 else:
244 # Check how long the lock has been there.
245 lock_creation_time = os.stat(self.config.PID_FILE)[-1]
246 now = time.time()
247 locked_period = now - lock_creation_time
248 # If the locked_period is greater than the allowed, it probably
249 # means the test suite is stuck.
250 message = 'Tarmac lock file in place for %d seconds, PID: %d'
251 if locked_period > int(self.config.allowed_lock_period):
252 self.logger.info(message % (locked_period, get_pid()))
253 raise TarmacCommandError(message % (locked_period, get_pid()))
254 else:
255 self.logger.info(message % (locked_period, get_pid()))
256 return
257
238 target = Branch.create(258 target = Branch.create(
239 self.launchpad.branches.getByUrl(url=branch_url), self.config,259 self.launchpad.branches.getByUrl(url=branch_url), self.config,
240 imply_commit_message=imply_commit_message)260 imply_commit_message=imply_commit_message)
241261
=== added file 'tarmac/pidfile.py'
--- tarmac/pidfile.py 1970-01-01 00:00:00 +0000
+++ tarmac/pidfile.py 2010-12-15 16:55:13 +0000
@@ -0,0 +1,77 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4# Code from lib/canonical/lazr/pidfile.py modified to be used here.
5
6__metaclass__ = type
7
8import tempfile
9import os
10import atexit
11import sys
12from signal import signal, SIGTERM
13
14from tarmac.config import TarmacConfig
15
16
17def make_pidfile():
18 """Write the current process id to a PID file.
19
20 Also installs an atexit handler to remove the file on process termination.
21
22 Also installs a SIGTERM signal handler to remove the file on SIGTERM.
23 If you install your own handler, you will want to call remove_pidfile
24 inside it.
25 """
26 pidfile = TarmacConfig().PID_FILE
27 if os.path.exists(pidfile):
28 raise RuntimeError("PID file %s already exists. Already running?" %
29 pidfile)
30
31 atexit.register(remove_pidfile)
32 def remove_pidfile_handler(*ignored):
33 sys.exit(-1 * SIGTERM)
34 signal(SIGTERM, remove_pidfile_handler)
35
36 fd, tempname = tempfile.mkstemp(dir=os.path.dirname(pidfile))
37 outf = os.fdopen(fd, 'w')
38 outf.write(str(os.getpid())+'\n')
39 outf.flush()
40 outf.close()
41 os.rename(tempname, pidfile)
42
43
44def remove_pidfile():
45 """Remove the PID file.
46
47 This should only be needed if you are overriding the default SIGTERM
48 signal handler.
49 """
50 pidfile = TarmacConfig().PID_FILE
51 pid = get_pid()
52 if pid is None:
53 return
54 if pid == os.getpid():
55 os.unlink(pidfile)
56
57
58def get_pid():
59 """Return the PID as an integer, or None
60
61 May raise a ValueError if the PID file is corrupt.
62
63 This method will only be needed by service or monitoring scripts.
64
65 Currently no checking is done to ensure that the process is actually
66 running, is healthy, or died horribly a while ago and its PID being
67 used by something else. What we have is probably good enough.
68 """
69 pidfile = TarmacConfig().PID_FILE
70 try:
71 pid = open(pidfile).read()
72 return int(pid)
73 except IOError:
74 return None
75 except ValueError:
76 raise ValueError("Invalid PID %s" % repr(pid))
77
078
=== modified file 'tarmac/plugins/bundlecommand.py'
--- tarmac/plugins/bundlecommand.py 2010-11-29 13:51:42 +0000
+++ tarmac/plugins/bundlecommand.py 2010-12-15 16:55:13 +0000
@@ -20,16 +20,101 @@
20os = None20os = None
21subprocess = None21subprocess = None
2222
23import datetime
24from email import MIMEMultipart, MIMEText
25from email.mime.application import MIMEApplication
26import gzip
27from StringIO import StringIO
28import tempfile
29import unittest
30
31import bzrlib.config
32
23from bzrlib.lazy_import import lazy_import33from bzrlib.lazy_import import lazy_import
34from bzrlib.smtp_connection import SMTPConnection
24lazy_import(globals(), '''35lazy_import(globals(), '''
25 import os36 import os
26 import subprocess37 import subprocess
27 ''')38 ''')
2839
40import subunit
41
29from tarmac.exceptions import TarmacMergeError42from tarmac.exceptions import TarmacMergeError
30from tarmac.hooks import TarmacHookPoint, tarmac_hooks43from tarmac.hooks import TarmacHookPoint, tarmac_hooks
31from tarmac.plugins import TarmacPlugin44from tarmac.plugins import TarmacPlugin
3245
46_double_line = '=' * 70
47_single_line = '-' * 70
48
49def format_error(flavor, test, error):
50 return '\n'.join(
51 [_double_line,
52 '%s: %s' % (flavor, test),
53 _single_line,
54 error,
55 ''])
56
57
58class TestResultFormatter:
59
60 def __init__(self, start_time, end_time, result, proposal):
61 self.result = result
62 self.start_time = start_time
63 self.source_branch = proposal.source_branch.display_name
64 self.source_revno = proposal.source_branch.revision_count
65 self.target_branch = proposal.target_branch.display_name
66 self.target_revno = proposal.target_branch.revision_count
67 self.num_tests = self.result.testsRun
68 self.duration = end_time - start_time
69 self.num_failures = len(self.result.failures)
70 self.num_errors = len(self.result.errors)
71
72 def _format_test_list(self, header, tests):
73 if not tests:
74 return []
75 tests = [' ' + test.id() for test, error in tests]
76 return [header, '-' * len(header)] + tests + ['']
77
78 def get_full_result(self):
79 return '\n'.join(self.full_result)
80
81 def format_result(self, attached_log=False):
82 output = [
83 'Tests started at approximately %s' % self.start_time,
84 ]
85 output.append('Source: %s r%s' % (
86 self.source_branch, self.source_revno))
87 output.append('Target: %s r%s' % (
88 self.target_branch, self.target_revno))
89 output.extend([
90 '',
91 '%s tests run in %s, %s failures, %s errors' % (
92 self.num_tests, self.duration, self.num_failures,
93 self.num_errors),
94 '',
95 ])
96
97 bad_tests = (
98 self._format_test_list('Failing tests', self.result.failures) +
99 self._format_test_list('Tests with errors', self.result.errors))
100 output.extend(bad_tests)
101
102 if bad_tests:
103 email_summary = StringIO()
104 for test, error in self.result.failures:
105 email_summary.write(
106 format_error('FAILURE', test, error))
107 for test, error in self.result.errors:
108 email_summary.write(
109 format_error('ERROR', test, error))
110 output.append(email_summary.getvalue())
111
112 # Include notification about the attached log. Useful when the output
113 # is sent as an email.
114 if attached_log:
115 output.extend(['(See the attached file for the complete log)', ''])
116 return '\n'.join(output)
117
33118
34class VerifyCommandFailed(TarmacMergeError):119class VerifyCommandFailed(TarmacMergeError):
35 """Running the verify_command failed."""120 """Running the verify_command failed."""
@@ -37,6 +122,12 @@
37122
38class BundleCommand(TarmacPlugin):123class BundleCommand(TarmacPlugin):
39124
125 def __init__(self, smtp_connection=None):
126 super(BundleCommand, self).__init__()
127 if smtp_connection is None:
128 smtp_connection = SMTPConnection(bzrlib.config.GlobalConfig())
129 self._smtp_connection = smtp_connection
130
40 def run(self, command, target, collected_proposals):131 def run(self, command, target, collected_proposals):
41 """Plugin for running the registered bundle verify command on target.132 """Plugin for running the registered bundle verify command on target.
42133
@@ -58,45 +149,84 @@
58 except AttributeError:149 except AttributeError:
59 return150 return
60151
152 start_time = datetime.datetime.utcnow()
61 self.logger.debug('Running test command: %s' % self.verify_command)153 self.logger.debug('Running test command: %s' % self.verify_command)
62 cwd = os.getcwd()154 cwd = os.getcwd()
63 os.chdir(target.tree.abspath(''))155 os.chdir(target.tree.abspath(''))
64 proc = subprocess.Popen(156 output = tempfile.TemporaryFile()
65 self.verify_command,157 try:
66 shell=True,158 proc = subprocess.Popen(
67 stdout=subprocess.PIPE,159 self.verify_command,
68 stderr=subprocess.PIPE)160 shell=True,
69 stdout_value, stderr_value = proc.communicate()161 stdout=output,
70 return_code = proc.wait()162 stderr=subprocess.STDOUT)
71 os.chdir(cwd)163 return_code = proc.wait()
72 self.logger.debug('Completed test command: %s' % self.verify_command)164 output.seek(0)
73165 os.chdir(cwd)
74 if return_code != 0:166 self.logger.debug('Completed test command: %s' % self.verify_command)
75 for source, proposal in collected_proposals:167 end_time = datetime.datetime.utcnow()
76 self.do_failed(proposal, stdout_value, stderr_value)168
77 message = u'Test command "%s" failed.' % self.verify_command169 if return_code != 0:
78 comment = (170 for source, proposal in collected_proposals:
79 u'Bundle lander failed to land proposals: %s' %171 self.do_failed(
80 collected_proposals)172 start_time, end_time, proposal, output)
81 raise VerifyCommandFailed(message, comment)173 message = u'Test command "%s" failed.' % self.verify_command
82 else:174 comment = (
83 self.logger.info('==========')175 u'Bundle lander failed to land proposals: %s' %
84 self.logger.info(stdout_value)176 collected_proposals)
85177 raise VerifyCommandFailed(message, comment)
86 def do_failed(self, proposal, stdout_value, stderr_value):178 else:
179 self.logger.info('==========')
180 self.logger.info(output.read())
181 finally:
182 output.close()
183
184 def do_failed(self, start_time, end_time, proposal, output):
87 '''Perform failure tests.185 '''Perform failure tests.
88186
89 In this case, the output of the test command is posted as a comment,187 In this case, the full failure is emailed to the proposal subscribers,
90 and the merge proposal is then set to "Needs review" so that Tarmac188 a summary of the test failure is posted as a comment and the
91 doesn't attempt to merge it again without human interaction.189 proposal is then set to "Needs Review" so that Tarmac doesn't attempt
190 to merge it again without human interaction.
92 '''191 '''
192 # Make output into a Result object, and create a tgz of the
193 # full log.
194 # We will use these for the gzip.
195 fd, path = tempfile.mkstemp()
196 os.close(fd)
197 gz = gzip.open(path, 'wb')
198 # These are for creating the test result object.
199 result = unittest.TestResult()
200 discard = tempfile.TemporaryFile()
201 # The temporary file gets things that subunit doesn't understand.
202 # It prints them to stdout by default. We don't want the output on
203 # stdout, so we get it and discard it. (Note that the things subunit
204 # doesn't understand are still put in the full_result, so that the
205 # email will include them.)
206 subunit_server = subunit.TestProtocolServer(result, discard)
207 for line in output:
208 subunit_server.lineReceived(line)
209 gz.write(line)
210 gz.close()
211 try:
212 gzipped_log = open(path).read()
213 finally:
214 os.unlink(path)
215 formatter = TestResultFormatter(
216 start_time, end_time, result, proposal)
217 output.seek(0)
218
219 emails = self.get_subscribers_emails(proposal)
220 self.send_report_email(emails, formatter, gzipped_log)
221
222 summary = formatter.format_result()
93 message = u'Test command "%s" failed.' % self.verify_command223 message = u'Test command "%s" failed.' % self.verify_command
94 comment = (u'The attempt to merge %(source)s into %(target)s failed. '224 comment = (u'The attempt to merge %(source)s into %(target)s failed. '
95 u'Below is the output from the failed tests.\n\n'225 u'Below is a summary from the failed tests.\n\n'
96 u'%(output)s') % {226 u'%(summary)s') % {
97 'source': proposal.source_branch.display_name,227 'source': proposal.source_branch.display_name,
98 'target': proposal.target_branch.display_name,228 'target': proposal.target_branch.display_name,
99 'output': u'\n'.join([stdout_value, stderr_value]),229 'summary': summary,
100 }230 }
101231
102 self.logger.warn(232 self.logger.warn(
@@ -111,9 +241,64 @@
111241
112 proposal.createComment(subject=subject,242 proposal.createComment(subject=subject,
113 content=comment)243 content=comment)
114 proposal.setStatus(status=u'Needs review')244 proposal.setStatus(status=u'Needs Review')
115 proposal.lp_save()245 proposal.lp_save()
116246
247 def get_subscribers_emails(self, proposal):
248 """Return a list of the proposal subscribers emails."""
249 subscribers_emails = set()
250 # Get all the reviewers.
251 for vote in proposal.votes:
252 subscribers_emails.add(
253 vote.reviewer.preferred_email_address.email)
254 # Get source branch subscribers.
255 for subscriber in proposal.source_branch.subscribers:
256 subscribers_emails.add(subscriber.preferred_email_address.email)
257 return subscribers_emails
258
259 def send_report_email(self, emails, formatter, full_log_gz):
260 """Send an email summarizing the test results.
261
262 :param emails: The list of emails to mail to.
263 :param formatter: The TestResultFormatter object.
264 :param full_test_result: The full log.
265 """
266 message = self._build_report_email(emails, formatter, full_log_gz)
267 self._send_email(message)
268
269 def _build_report_email(self, emails, formatter, full_log_gz):
270 """Build a MIME email summarizing the test results.
271
272 :param emails: The list of emails to mail to.
273 :param formatter: The TestResultFormatter object.
274 :param full_log_gz: A gzip of the full log.
275 """
276 message = MIMEMultipart.MIMEMultipart()
277 message['To'] = ', '.join(emails)
278 message['From'] = "Tarmac Bundle lander."
279 subject = 'Test results: %s => %s: %s' % (
280 formatter.source_branch, formatter.target_branch, "FAILURE")
281 message['Subject'] = subject
282
283 body_text = formatter.format_result(attached_log=True)
284 # Make the body.
285 body = MIMEText.MIMEText(body_text, 'plain', 'utf8')
286 body['Content-Disposition'] = 'inline'
287 message.attach(body)
288
289 # Attach the gzipped log.
290 zipped_log = MIMEApplication(full_log_gz, 'x-gzip')
291 zipped_log.add_header(
292 'Content-Disposition', 'attachment',
293 filename='%s-r%s.subunit.gz' % (
294 formatter.source_branch, formatter.source_revno))
295 message.attach(zipped_log)
296 return message
297
298 def _send_email(self, message):
299 """Actually send 'message'."""
300 self._smtp_connection.send_email(message)
301
117# Create the hook here so we don't change the tarmac_hooks API with this very302# Create the hook here so we don't change the tarmac_hooks API with this very
118# specific hook.303# specific hook.
119tarmac_hooks.create_hook(TarmacHookPoint('tarmac_bundle_merge',304tarmac_hooks.create_hook(TarmacHookPoint('tarmac_bundle_merge',
120305
=== added file 'tarmac/plugins/tests/test_bundlecommand.py'
--- tarmac/plugins/tests/test_bundlecommand.py 1970-01-01 00:00:00 +0000
+++ tarmac/plugins/tests/test_bundlecommand.py 2010-12-15 16:55:13 +0000
@@ -0,0 +1,280 @@
1"""Tests for the BundleCommmand plug-in."""
2
3from datetime import datetime, timedelta
4import doctest
5import os
6import unittest
7
8from testtools import TestCase, TestResult
9from testtools.matchers import DocTestMatches
10
11from tarmac.bin.registry import CommandRegistry
12from tarmac.plugins.bundlecommand import (
13 BundleCommand, TestResultFormatter, VerifyCommandFailed)
14from tarmac.tests import TarmacTestCase
15from tarmac.tests.test_commands import FakeCommand
16from tarmac.tests.mock import Thing, MockMergeProposal
17
18
19class LoggingSMTPConnection(object):
20 """An SMTPConnection double that logs sent email."""
21
22 def __init__(self, log):
23 self._log = log
24
25 def send_email(self, message):
26 self._log.append(message)
27
28
29class Helpers:
30
31 def make_formatter(self):
32 proposal = Thing(
33 source_branch=Thing(
34 revision_count=123,
35 display_name="lp:project/source"),
36 target_branch=Thing(
37 revision_count=120,
38 display_name="lp:project"))
39 start_time = datetime.utcnow()
40 end_time = start_time + timedelta(hours=1)
41
42 class SomeTest(TestCase):
43 def test_ok(self):
44 pass
45 def test_fail(self):
46 self.fail("oh no")
47 def test_error(self):
48 1/0
49 self.fail_test = SomeTest('test_fail')
50 self.error_test = SomeTest('test_error')
51 test = unittest.TestSuite(
52 [self.fail_test, self.error_test, SomeTest('test_ok')])
53 result = TestResult()
54 test.run(result)
55
56 return TestResultFormatter(start_time, end_time, result, proposal)
57
58 def lp_save(self, *args, **kwargs):
59 """Dummy lp_save method."""
60 pass
61
62
63class TestTestResultFormatter(TestCase, Helpers):
64 """Tests for `TestResultFormatter`."""
65
66 expected_failure_summary_email = ("""\
67Tests started at approximately %(start_time)s
68Source: %(source_branch)s r%(source_revno)s
69Target: %(target_branch)s r%(target_revno)s
70<BLANKLINE>
71%(num_tests)s tests run in %(duration)s, %(num_failures)s failures, %(num_errors)s errors
72<BLANKLINE>
73Failing tests
74-------------
75 %(fail_id)s
76<BLANKLINE>
77Tests with errors
78-----------------
79 %(error_id)s
80<BLANKLINE>
81======================================================================
82FAILURE: test_fail...
83----------------------------------------------------------------------
84Text attachment: traceback
85------------
86Traceback (most recent call last):
87...
88------------
89<BLANKLINE>
90======================================================================
91ERROR: test_error...
92----------------------------------------------------------------------
93Text attachment: traceback
94------------
95Traceback (most recent call last):
96...
97------------
98<BLANKLINE>
99<BLANKLINE>
100(See the attached file for the complete log)
101""")
102
103 expected_failure_summary = ("""\
104Tests started at approximately %(start_time)s
105Source: %(source_branch)s r%(source_revno)s
106Target: %(target_branch)s r%(target_revno)s
107<BLANKLINE>
108%(num_tests)s tests run in %(duration)s, %(num_failures)s failures, %(num_errors)s errors
109<BLANKLINE>
110Failing tests
111-------------
112 %(fail_id)s
113<BLANKLINE>
114Tests with errors
115-----------------
116 %(error_id)s
117<BLANKLINE>
118======================================================================
119FAILURE: test_fail...
120----------------------------------------------------------------------
121Text attachment: traceback
122------------
123Traceback (most recent call last):
124...
125------------
126<BLANKLINE>
127======================================================================
128ERROR: test_error...
129----------------------------------------------------------------------
130Text attachment: traceback
131------------
132Traceback (most recent call last):
133...
134------------
135<BLANKLINE>
136""")
137
138 def test_format_result_with_attached_log(self):
139 formatter = self.make_formatter()
140 summary = formatter.format_result(attached_log=True)
141 data = {
142 'start_time': formatter.start_time,
143 'source_branch': formatter.source_branch,
144 'source_revno': formatter.source_revno,
145 'target_branch': formatter.target_branch,
146 'target_revno': formatter.target_revno,
147 'num_tests': formatter.num_tests,
148 'duration': formatter.duration,
149 'num_failures': formatter.num_failures,
150 'num_errors': formatter.num_errors,
151 'fail_id': self.fail_test.id(),
152 'error_id': self.error_test.id()
153 }
154 self.assertThat(summary, DocTestMatches(
155 self.expected_failure_summary_email % data, doctest.REPORT_NDIFF|
156 doctest.ELLIPSIS))
157
158 def test_format_result_without_attached_log(self):
159 formatter = self.make_formatter()
160 summary = formatter.format_result(attached_log=False)
161 data = {
162 'start_time': formatter.start_time,
163 'source_branch': formatter.source_branch,
164 'source_revno': formatter.source_revno,
165 'target_branch': formatter.target_branch,
166 'target_revno': formatter.target_revno,
167 'num_tests': formatter.num_tests,
168 'duration': formatter.duration,
169 'num_failures': formatter.num_failures,
170 'num_errors': formatter.num_errors,
171 'fail_id': self.fail_test.id(),
172 'error_id': self.error_test.id()
173 }
174 self.assertThat(summary, DocTestMatches(
175 self.expected_failure_summary % data, doctest.REPORT_NDIFF|
176 doctest.ELLIPSIS))
177
178
179class BundleCommandTests(TarmacTestCase, Helpers):
180 """Test the BundleCommand."""
181
182 def setUp(self):
183 """Set up additional data we need for tests."""
184 super(BundleCommandTests, self).setUp()
185 self.collected_proposals = [
186 (Thing(),
187 MockMergeProposal(
188 Thing(
189 display_name='lp:project/source',
190 revision_count=123,
191 subscribers=[
192 Thing(preferred_email_address=Thing(
193 email='foo@example.com'))]),
194 Thing(
195 display_name='lp:project',
196 revision_count=120),
197 status="Approved",
198 votes=[
199 Thing(reviewer=Thing(
200 preferred_email_address=Thing(
201 email='foo.bar@example.com')))])),
202 (Thing(),
203 MockMergeProposal(
204 Thing(
205 display_name='lp:project/source2',
206 revision_count=124,
207 subscribers=[
208 Thing(preferred_email_address=Thing(
209 email='foo@example.com'))]),
210 Thing(
211 display_name='lp:project',
212 revision_count=120),
213 status="Approved",
214 votes=[
215 Thing(reviewer=Thing(
216 preferred_email_address=Thing(
217 email='foo.bar@example.com')))])),
218 ]
219 # Patch the plugin smtp_connection so emails are never actually sent.
220 email_log = []
221 smtp_connection = LoggingSMTPConnection(email_log)
222 self.plugin = BundleCommand(smtp_connection=smtp_connection)
223 registry = CommandRegistry(config=self.config)
224 self.command = FakeCommand(registry)
225
226 def test_run(self):
227 """Test that the plug-in run without errors."""
228 target = Thing(config=Thing(
229 bundle_verify_command="/bin/true"),
230 tree=Thing(abspath=os.path.abspath))
231 self.plugin.run(
232 command=self.command, target=target,
233 collected_proposals=self.collected_proposals)
234
235 def test_run_failure(self):
236 """Test that a failure raises the correct exception."""
237 # No email in the log.
238 self.assertEqual(self.plugin._smtp_connection._log, [])
239 # Proposal status are approved.
240 proposal1, proposal2 = self.collected_proposals
241 self.assertEqual(proposal1[1].queue_status, "Approved")
242 self.assertEqual(proposal2[1].queue_status, "Approved")
243
244 target = Thing(config=Thing(
245 bundle_verify_command="/bin/false"),
246 tree=Thing(abspath=os.path.abspath))
247 self.assertRaises(VerifyCommandFailed,
248 self.plugin.run,
249 command=self.command, target=target,
250 collected_proposals=self.collected_proposals)
251
252 # There are two emails in the log. One for each proposal.
253 email = self.plugin._smtp_connection._log.pop()
254 self.assertEqual(
255 email.get('Subject'),
256 'Test results: lp:project/source2 => lp:project: FAILURE')
257 email = self.plugin._smtp_connection._log.pop()
258 self.assertEqual(
259 email.get('Subject'),
260 'Test results: lp:project/source => lp:project: FAILURE')
261
262 # Proposals status is updated.
263 proposal1, proposal2 = self.collected_proposals
264 self.assertEqual(proposal1[1].queue_status, "Needs Review")
265 self.assertEqual(proposal2[1].queue_status, "Needs Review")
266
267 def test_send_report_email(self):
268 """Test send_report_email()."""
269 formatter = self.make_formatter()
270
271 emails = self.plugin.get_subscribers_emails(
272 self.collected_proposals[0][1])
273 self.plugin.send_report_email(
274 emails, formatter, 'some gzip content')
275 email = self.plugin._smtp_connection._log.pop()
276 self.assertEqual(
277 email.get('To'), 'foo@example.com, foo.bar@example.com')
278 self.assertEqual(
279 email.get('Subject'),
280 "Test results: lp:project/source => lp:project: FAILURE")
0281
=== modified file 'tarmac/tests/mock.py'
--- tarmac/tests/mock.py 2010-11-10 14:00:22 +0000
+++ tarmac/tests/mock.py 2010-12-15 16:55:13 +0000
@@ -58,6 +58,23 @@
58 self.landing_candidates = []58 self.landing_candidates = []
5959
6060
61class MockMergeProposal(object):
62 """A mock LP merge proposal."""
63
64 def __init__(self, source_branch, target_branch, status, votes=()):
65 self.createComment = self.lp_save
66 self.queue_status = status
67 self.source_branch = source_branch
68 self.target_branch = target_branch
69 self.votes = votes
70
71 def lp_save(self, *args, **kwargs):
72 pass
73
74 def setStatus(self, status="Needs Review"):
75 self.queue_status = status
76
77
61class cmd_mock(TarmacCommand):78class cmd_mock(TarmacCommand):
62 '''A mock command.'''79 '''A mock command.'''
6380
6481
=== modified file 'tarmac/tests/test_commands.py'
--- tarmac/tests/test_commands.py 2010-12-08 00:14:43 +0000
+++ tarmac/tests/test_commands.py 2010-12-15 16:55:13 +0000
@@ -1,18 +1,19 @@
1'''Tests for tarmac.bin.commands.py.'''1'''Tests for tarmac.bin.commands.py.'''
2from cStringIO import StringIO2from cStringIO import StringIO
3import os3import os
4import shutil
5import sys4import sys
65
7from tarmac.bin import commands6from tarmac.bin import commands
8from tarmac.bin.registry import CommandRegistry7from tarmac.bin.registry import CommandRegistry
9from tarmac.branch import Branch8from tarmac.branch import Branch
10from tarmac.config import TarmacConfig9from tarmac.config import TarmacConfig
11from tarmac.exceptions import TarmacMergeError, UnapprovedChanges10from tarmac.exceptions import (
11 TarmacCommandError, TarmacMergeError, UnapprovedChanges)
12from tarmac.hooks import TarmacHookPoint, tarmac_hooks12from tarmac.hooks import TarmacHookPoint, tarmac_hooks
13from tarmac.pidfile import make_pidfile
13from tarmac.plugins import TarmacPlugin14from tarmac.plugins import TarmacPlugin
14from tarmac.tests import TarmacTestCase, BranchTestCase15from tarmac.tests import TarmacTestCase, BranchTestCase
15from tarmac.tests.mock import MockLPBranch, Thing16from tarmac.tests.mock import Thing
1617
1718
18class FakeCommand(commands.TarmacCommand):19class FakeCommand(commands.TarmacCommand):
@@ -329,6 +330,8 @@
329330
330 self.launchpad = Thing(branches=Thing(getByUrl=self.getBranchByUrl))331 self.launchpad = Thing(branches=Thing(getByUrl=self.getBranchByUrl))
331 self.error = None332 self.error = None
333 # Add the allowed_lock_period option to the config.
334 self.config.set('Tarmac', 'allowed_lock_period', '10')
332 registry = CommandRegistry(config=self.config)335 registry = CommandRegistry(config=self.config)
333 registry.register_command('bundle_merge', commands.cmd_bundle_merge)336 registry.register_command('bundle_merge', commands.cmd_bundle_merge)
334337
@@ -412,8 +415,21 @@
412 self.assertEqual(415 self.assertEqual(
413 rev.properties.get('merge_url'),416 rev.properties.get('merge_url'),
414 u'http://code.launchpad.net/proposal2')417 u'http://code.launchpad.net/proposal2')
415 #XXX need to grab the previuous revision as well and do the418
416 # assertions.419 previous_to_last_revno = target.revno() - 1
420 previous_to_last_rev = target.get_rev_id(previous_to_last_revno)
421 rev = target.repository.get_revision(previous_to_last_rev)
422 self.assertEqual(rev.get_summary(), u"Commit this.")
423 self.assertEqual(rev.committer, "Tarmac")
424 self.assertEqual(
425 rev.properties.get('merge_url'),
426 u'http://code.launchpad.net/proposal1')
427
428 def test_run_locked_fails(self):
429 make_pidfile()
430 self.config.set('Tarmac', 'allowed_lock_period', '0')
431 self.assertRaises(
432 TarmacCommandError, self.command.run, launchpad=self.launchpad)
417433
418 def test_plugin_merge_errors_are_handled(self):434 def test_plugin_merge_errors_are_handled(self):
419 # Setup435 # Setup
420436
=== added file 'tarmac/tests/test_pidfile.py'
--- tarmac/tests/test_pidfile.py 1970-01-01 00:00:00 +0000
+++ tarmac/tests/test_pidfile.py 2010-12-15 16:55:13 +0000
@@ -0,0 +1,31 @@
1
2import os
3
4from tarmac.pidfile import make_pidfile, get_pid, remove_pidfile
5from tarmac.tests import TarmacTestCase
6
7
8class TestPidFile(TarmacTestCase):
9 """Tests for the functions in the pidfile.py module."""
10
11 def test_make_pidfile(self):
12 self.assertFalse(os.path.exists(self.config.PID_FILE))
13 make_pidfile()
14 self.assertTrue(os.path.exists(self.config.PID_FILE))
15
16 def test_make_pidfile_raises_if_exists(self):
17 self.assertFalse(os.path.exists(self.config.PID_FILE))
18 make_pidfile()
19 self.assertRaises(RuntimeError, make_pidfile)
20
21 def test_get_pid(self):
22 self.assertFalse(os.path.exists(self.config.PID_FILE))
23 make_pidfile()
24 self.assertEqual(os.getpid(), get_pid())
25
26 def test_remove_pidfile(self):
27 self.assertFalse(os.path.exists(self.config.PID_FILE))
28 make_pidfile()
29 self.assertTrue(os.path.exists(self.config.PID_FILE))
30 remove_pidfile()
31 self.assertFalse(os.path.exists(self.config.PID_FILE))

Subscribers

People subscribed via source and target branches

to all changes: