Launchpad itself

Merge lp:~abentley/launchpad/detailed-build-emails into lp:launchpad

  1. detailed-build-emails
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Merged at revision: 11270
Proposed branch: lp:~abentley/launchpad/detailed-build-emails
Merge into: lp:launchpad
Diff against target: 315 lines (+116/-29)
6 files modified
lib/canonical/launchpad/emailtemplates/build-request.txt (+7/-1)
lib/lp/code/mail/sourcepackagerecipebuild.py (+16/-1)
lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py (+69/-10)
lib/lp/code/model/sourcepackagerecipebuild.py (+3/-2)
lib/lp/code/model/tests/test_sourcepackagerecipebuild.py (+16/-9)
lib/lp/testing/factory.py (+5/-6)
To merge this branch: bzr merge lp:~abentley/launchpad/detailed-build-emails
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+31413@code.launchpad.net

Commit message

Make recipe build emails more like binary build ones

Description of the change

= Summary =
Fix bug #603606: Recipe Build failure emails aren't as binary package build failures

== Proposed fix ==
Update recipe build emails to use similar formatting to bianary build emails.

== Pre-implementation notes ==
Mid-implementation with Tim Penhey

== Implementation details ==
The formatting now matches. The fields supplied are:
State, Recipe, Archive, Distroseries, Duration, Build Log and Builder.

Similarly, the subject line contains recipe build id, recipe, distroseries
and status.

Component and pocket are not provided as these do not vary.

== Tests ==
bin/test test_sourcepackagerecipebuild -t notif -t generateEmail.

== Demo and Q/A ==
Request a build. When is processed, the resulting mail should be as described.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/tests/test_sourcepackagerecipebuild.py
  lib/lp/code/mail/sourcepackagerecipebuild.py
  lib/lp/testing/factory.py
  lib/lp/code/model/sourcepackagerecipebuild.py
  lib/canonical/launchpad/emailtemplates/build-request.txt
  lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py

./lib/lp/code/mail/sourcepackagerecipebuild.py
       5: E303 too many blank lines (3)
      71: Line exceeds 78 characters.
      46: E231 missing whitespace after ','
./lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py
      15: 'IStore' imported but unused
       5: E303 too many blank lines (3)
      37: W291 trailing whitespace
      38: W291 trailing whitespace
      39: W291 trailing whitespace
      42: E302 expected 2 blank lines, found 1
      37: Line has trailing whitespace.
      38: Line has trailing whitespace.
      39: Line has trailing whitespace.

To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

Looks fine to me.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/emailtemplates/build-request.txt'
2--- lib/canonical/launchpad/emailtemplates/build-request.txt 2010-06-08 18:31:56 +0000
3+++ lib/canonical/launchpad/emailtemplates/build-request.txt 2010-08-01 04:36:51 +0000
4@@ -1,1 +1,7 @@
5-Build %(recipe_owner)s/%(recipe)s into %(archive)s for %(distroseries)s: %(status)s.
6+ * State: %(status)s
7+ * Recipe: %(recipe_owner)s/%(recipe)s
8+ * Archive: %(archive_owner)s/%(archive)s
9+ * Distroseries: %(distroseries)s
10+ * Duration: %(duration)s
11+ * Build Log: %(log_url)s
12+ * Builder: %(builder_url)s
13
14=== modified file 'lib/lp/code/mail/sourcepackagerecipebuild.py'
15--- lib/lp/code/mail/sourcepackagerecipebuild.py 2010-06-09 20:24:11 +0000
16+++ lib/lp/code/mail/sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
17@@ -11,6 +11,7 @@
18
19 from canonical.config import config
20 from canonical.launchpad.webapp import canonical_url
21+from canonical.launchpad.webapp.tales import DurationFormatterAPI
22 from lp.services.mail.basemailer import BaseMailer, RecipientReason
23
24
25@@ -25,7 +26,8 @@
26 requester = build.requester
27 recipients = {requester: RecipientReason.forBuildRequester(requester)}
28 return cls(
29- '%(status)s: %(recipe)s for %(distroseries)s',
30+ '[recipe build #%(build_id)d] of ~%(recipe_owner)s %(recipe)s in'
31+ ' %(distroseries)s: %(status)s',
32 'build-request.txt', recipients,
33 config.canonical.noreply_from_address, build)
34
35@@ -50,13 +52,26 @@
36 params = super(
37 SourcePackageRecipeBuildMailer, self)._getTemplateParams(email)
38 params.update({
39+ 'build_id': self.build.id,
40 'status': self.build.buildstate.title,
41 'distroseries': self.build.distroseries.name,
42 'recipe': self.build.recipe.name,
43 'recipe_owner': self.build.recipe.owner.name,
44 'archive': self.build.archive.name,
45+ 'archive_owner': self.build.archive.owner.name,
46+ 'log_url': '',
47+ 'component': self.build.current_component.name,
48+ 'duration': '',
49+ 'builder_url': '',
50 'build_url': canonical_url(self.build),
51 })
52+ if self.build.builder is not None:
53+ params['builder_url'] = canonical_url(self.build.builder)
54+ if self.build.buildduration is not None:
55+ duration_formatter = DurationFormatterAPI(self.build.buildduration)
56+ params['duration'] = duration_formatter.approximateduration()
57+ if self.build.build_log_url is not None:
58+ params['log_url'] = self.build.build_log_url
59 return params
60
61 def _getFooter(self, params):
62
63=== modified file 'lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py'
64--- lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2010-06-09 20:24:11 +0000
65+++ lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
66@@ -5,42 +5,68 @@
67 __metaclass__ = type
68
69
70+from datetime import timedelta
71 from unittest import TestLoader
72
73+from storm.locals import Store
74+from zope.security.proxy import removeSecurityProxy
75+
76 from canonical.config import config
77 from canonical.launchpad.interfaces.lpstorm import IStore
78-from canonical.testing import DatabaseFunctionalLayer
79+from canonical.testing import LaunchpadFunctionalLayer
80 from lp.buildmaster.interfaces.buildbase import BuildStatus
81 from lp.code.mail.sourcepackagerecipebuild import (
82 SourcePackageRecipeBuildMailer)
83 from lp.testing import TestCaseWithFactory
84
85+expected_body = u"""\
86+ * State: Successfully built
87+ * Recipe: person/recipe
88+ * Archive: archiveowner/ppa
89+ * Distroseries: distroseries
90+ * Duration: five minutes
91+ * Build Log: %s
92+ * Builder: http://launchpad.dev/builders/bob
93+"""
94+
95+superseded_body = u"""\
96+ * State: Build for superseded Source
97+ * Recipe: person/recipe
98+ * Archive: archiveowner/ppa
99+ * Distroseries: distroseries
100+ * Duration:
101+ * Build Log:
102+ * Builder:
103+"""
104
105 class TestSourcePackageRecipeBuildMailer(TestCaseWithFactory):
106
107- layer = DatabaseFunctionalLayer
108+ layer = LaunchpadFunctionalLayer
109
110 def test_generateEmail(self):
111 """GenerateEmail produces the right headers and body."""
112 person = self.factory.makePerson(name='person')
113 cake = self.factory.makeSourcePackageRecipe(
114 name=u'recipe', owner=person)
115- pantry = self.factory.makeArchive(name='ppa')
116+ pantry_owner = self.factory.makePerson(name='archiveowner')
117+ pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
118 secret = self.factory.makeDistroSeries(name=u'distroseries')
119 build = self.factory.makeSourcePackageRecipeBuild(
120 recipe=cake, distroseries=secret, archive=pantry,
121- status=BuildStatus.FULLYBUILT)
122- IStore(build).flush()
123+ status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=5))
124+ naked_build = removeSecurityProxy(build)
125+ naked_build.builder = self.factory.makeBuilder(name='bob')
126+ naked_build.buildlog = self.factory.makeLibraryFileAlias()
127+ Store.of(build).flush()
128 mailer = SourcePackageRecipeBuildMailer.forStatus(build)
129 email = build.requester.preferredemail.email
130 ctrl = mailer.generateEmail(email, build.requester)
131- self.assertEqual('Successfully built: recipe for distroseries',
132- ctrl.subject)
133+ self.assertEqual(
134+ u'[recipe build #%d] of ~person recipe in distroseries: '
135+ 'Successfully built' % (build.id), ctrl.subject)
136 body, footer = ctrl.body.split('\n-- \n')
137 self.assertEqual(
138- 'Build person/recipe into ppa for distroseries: Successfully'
139- ' built.\n', body
140- )
141+ expected_body % build.build_log_url, body)
142 self.assertEqual(
143 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
144 'You are the requester of the build.\n', footer)
145@@ -54,6 +80,39 @@
146 self.assertEqual(
147 'FULLYBUILT', ctrl.headers['X-Launchpad-Build-State'])
148
149+ def test_generateEmail_with_null_fields(self):
150+ """GenerateEmail works when many fields are NULL."""
151+ person = self.factory.makePerson(name='person')
152+ cake = self.factory.makeSourcePackageRecipe(
153+ name=u'recipe', owner=person)
154+ pantry_owner = self.factory.makePerson(name='archiveowner')
155+ pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
156+ secret = self.factory.makeDistroSeries(name=u'distroseries')
157+ build = self.factory.makeSourcePackageRecipeBuild(
158+ recipe=cake, distroseries=secret, archive=pantry,
159+ status=BuildStatus.SUPERSEDED)
160+ Store.of(build).flush()
161+ mailer = SourcePackageRecipeBuildMailer.forStatus(build)
162+ email = build.requester.preferredemail.email
163+ ctrl = mailer.generateEmail(email, build.requester)
164+ self.assertEqual(
165+ u'[recipe build #%d] of ~person recipe in distroseries: '
166+ 'Build for superseded Source' % (build.id), ctrl.subject)
167+ body, footer = ctrl.body.split('\n-- \n')
168+ self.assertEqual(superseded_body, body)
169+ self.assertEqual(
170+ 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
171+ 'You are the requester of the build.\n', footer)
172+ self.assertEqual(
173+ config.canonical.noreply_from_address, ctrl.from_addr)
174+ self.assertEqual(
175+ 'Requester', ctrl.headers['X-Launchpad-Message-Rationale'])
176+ self.assertEqual(
177+ 'recipe-build-status',
178+ ctrl.headers['X-Launchpad-Notification-Type'])
179+ self.assertEqual(
180+ 'SUPERSEDED', ctrl.headers['X-Launchpad-Build-State'])
181+
182
183 def test_suite():
184 return TestLoader().loadTestsFromName(__name__)
185
186=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
187--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-07-28 16:44:00 +0000
188+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
189@@ -202,7 +202,7 @@
190
191 @classmethod
192 def new(cls, distroseries, recipe, requester, archive, pocket=None,
193- date_created=None):
194+ date_created=None, duration=None):
195 """See `ISourcePackageRecipeBuildSource`."""
196 store = IMasterStore(SourcePackageRecipeBuild)
197 if pocket is None:
198@@ -215,7 +215,8 @@
199 requester,
200 archive,
201 pocket,
202- date_created=date_created)
203+ date_created=date_created,
204+ build_duration=duration)
205 store.add(spbuild)
206 return spbuild
207
208
209=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
210--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-30 14:04:15 +0000
211+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
212@@ -30,6 +30,8 @@
213 from lp.code.interfaces.sourcepackagerecipebuild import (
214 ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
215 ISourcePackageRecipeBuildSource)
216+from lp.code.mail.sourcepackagerecipebuild import (
217+ SourcePackageRecipeBuildMailer)
218 from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild
219 from lp.registry.interfaces.pocket import PackagePublishingPocket
220 from lp.services.mail.sendmail import format_address
221@@ -320,7 +322,8 @@
222 pantry = self.factory.makeArchive(name='ppa')
223 secret = self.factory.makeDistroSeries(name=u'distroseries')
224 build = self.factory.makeSourcePackageRecipeBuild(
225- recipe=cake, distroseries=secret, archive=pantry)
226+ recipe=cake, distroseries=secret, archive=pantry,
227+ duration=datetime.timedelta(minutes=5))
228 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
229 IStore(build).flush()
230 build.notify()
231@@ -328,21 +331,24 @@
232 requester = build.requester
233 requester_address = format_address(
234 requester.displayname, requester.preferredemail.email)
235+ mailer = SourcePackageRecipeBuildMailer.forStatus(build)
236+ expected = mailer.generateEmail(
237+ requester.preferredemail.email, requester)
238 self.assertEqual(
239 requester_address, re.sub(r'\n\t+', ' ', message['To']))
240- self.assertEqual('Successfully built: recipe for distroseries',
241- message['Subject'])
242- body, footer = message.get_payload(decode=True).split('\n-- \n')
243+ self.assertEqual(expected.subject, message['Subject'].replace(
244+ '\n\t', ' '))
245 self.assertEqual(
246- 'Build person/recipe into ppa for distroseries: Successfully'
247- ' built.\n', body)
248+ expected.body, message.get_payload(decode=True))
249
250 def test_handleStatusNotifies(self):
251 """"handleStatus causes notification, even if OK."""
252
253 def prepare_build():
254- queue_record = self.factory.makeSourcePackageRecipeBuildJob()
255- build = queue_record.specific_job.build
256+ build = self.factory.makeSourcePackageRecipeBuild(
257+ duration=datetime.timedelta(minutes=5))
258+ queue_record = self.factory.makeSourcePackageRecipeBuildJob(
259+ recipe_build=build)
260 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
261 queue_record.builder = self.factory.makeBuilder()
262 slave = WaitingSlave('BuildStatus.OK')
263@@ -373,7 +379,8 @@
264 distroseries.nominatedarchindep = distroseries_i386
265 build = self.factory.makeSourcePackageRecipeBuild(
266 distroseries=distroseries,
267- status=BuildStatus.FULLYBUILT)
268+ status=BuildStatus.FULLYBUILT,
269+ duration=datetime.timedelta(minutes=5))
270 build.queueBuild(build)
271 return build
272
273
274=== modified file 'lib/lp/testing/factory.py'
275--- lib/lp/testing/factory.py 2010-07-30 20:33:47 +0000
276+++ lib/lp/testing/factory.py 2010-08-01 04:36:51 +0000
277@@ -167,8 +167,6 @@
278 time_counter,
279 )
280 from lp.translations.interfaces.potemplate import IPOTemplateSet
281-from lp.translations.interfaces.translationimportqueue import (
282- RosettaImportStatus)
283 from lp.translations.interfaces.translationgroup import (
284 ITranslationGroupSet)
285 from lp.translations.interfaces.translationimportqueue import (
286@@ -1871,7 +1869,8 @@
287 requester=None, archive=None,
288 sourcename=None, distroseries=None,
289 pocket=None, date_created=None,
290- status=BuildStatus.NEEDSBUILD):
291+ status=BuildStatus.NEEDSBUILD,
292+ duration=None):
293 """Make a new SourcePackageRecipeBuild."""
294 if recipe is None:
295 recipe = self.makeSourcePackageRecipe(name=sourcename)
296@@ -1888,7 +1887,8 @@
297 archive=archive,
298 requester=requester,
299 pocket=pocket,
300- date_created=date_created)
301+ date_created=date_created,
302+ duration=duration)
303 removeSecurityProxy(spr_build).buildstate = status
304 return spr_build
305
306@@ -2773,8 +2773,7 @@
307
308
309 def is_security_proxied_or_harmless(obj):
310- """Check that the given object is security wrapped or a harmless object.
311- """
312+ """Check that the object is security wrapped or a harmless object."""
313 if obj is None:
314 return True
315 if builtin_isinstance(obj, Proxy):