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
=== modified file 'lib/canonical/launchpad/emailtemplates/build-request.txt'
--- lib/canonical/launchpad/emailtemplates/build-request.txt 2010-06-08 18:31:56 +0000
+++ lib/canonical/launchpad/emailtemplates/build-request.txt 2010-08-01 04:36:51 +0000
@@ -1,1 +1,7 @@
1Build %(recipe_owner)s/%(recipe)s into %(archive)s for %(distroseries)s: %(status)s.1 * State: %(status)s
2 * Recipe: %(recipe_owner)s/%(recipe)s
3 * Archive: %(archive_owner)s/%(archive)s
4 * Distroseries: %(distroseries)s
5 * Duration: %(duration)s
6 * Build Log: %(log_url)s
7 * Builder: %(builder_url)s
28
=== modified file 'lib/lp/code/mail/sourcepackagerecipebuild.py'
--- lib/lp/code/mail/sourcepackagerecipebuild.py 2010-06-09 20:24:11 +0000
+++ lib/lp/code/mail/sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
@@ -11,6 +11,7 @@
1111
12from canonical.config import config12from canonical.config import config
13from canonical.launchpad.webapp import canonical_url13from canonical.launchpad.webapp import canonical_url
14from canonical.launchpad.webapp.tales import DurationFormatterAPI
14from lp.services.mail.basemailer import BaseMailer, RecipientReason15from lp.services.mail.basemailer import BaseMailer, RecipientReason
1516
1617
@@ -25,7 +26,8 @@
25 requester = build.requester26 requester = build.requester
26 recipients = {requester: RecipientReason.forBuildRequester(requester)}27 recipients = {requester: RecipientReason.forBuildRequester(requester)}
27 return cls(28 return cls(
28 '%(status)s: %(recipe)s for %(distroseries)s',29 '[recipe build #%(build_id)d] of ~%(recipe_owner)s %(recipe)s in'
30 ' %(distroseries)s: %(status)s',
29 'build-request.txt', recipients,31 'build-request.txt', recipients,
30 config.canonical.noreply_from_address, build)32 config.canonical.noreply_from_address, build)
3133
@@ -50,13 +52,26 @@
50 params = super(52 params = super(
51 SourcePackageRecipeBuildMailer, self)._getTemplateParams(email)53 SourcePackageRecipeBuildMailer, self)._getTemplateParams(email)
52 params.update({54 params.update({
55 'build_id': self.build.id,
53 'status': self.build.buildstate.title,56 'status': self.build.buildstate.title,
54 'distroseries': self.build.distroseries.name,57 'distroseries': self.build.distroseries.name,
55 'recipe': self.build.recipe.name,58 'recipe': self.build.recipe.name,
56 'recipe_owner': self.build.recipe.owner.name,59 'recipe_owner': self.build.recipe.owner.name,
57 'archive': self.build.archive.name,60 'archive': self.build.archive.name,
61 'archive_owner': self.build.archive.owner.name,
62 'log_url': '',
63 'component': self.build.current_component.name,
64 'duration': '',
65 'builder_url': '',
58 'build_url': canonical_url(self.build),66 'build_url': canonical_url(self.build),
59 })67 })
68 if self.build.builder is not None:
69 params['builder_url'] = canonical_url(self.build.builder)
70 if self.build.buildduration is not None:
71 duration_formatter = DurationFormatterAPI(self.build.buildduration)
72 params['duration'] = duration_formatter.approximateduration()
73 if self.build.build_log_url is not None:
74 params['log_url'] = self.build.build_log_url
60 return params75 return params
6176
62 def _getFooter(self, params):77 def _getFooter(self, params):
6378
=== modified file 'lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2010-06-09 20:24:11 +0000
+++ lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
@@ -5,42 +5,68 @@
5__metaclass__ = type5__metaclass__ = type
66
77
8from datetime import timedelta
8from unittest import TestLoader9from unittest import TestLoader
910
11from storm.locals import Store
12from zope.security.proxy import removeSecurityProxy
13
10from canonical.config import config14from canonical.config import config
11from canonical.launchpad.interfaces.lpstorm import IStore15from canonical.launchpad.interfaces.lpstorm import IStore
12from canonical.testing import DatabaseFunctionalLayer16from canonical.testing import LaunchpadFunctionalLayer
13from lp.buildmaster.interfaces.buildbase import BuildStatus17from lp.buildmaster.interfaces.buildbase import BuildStatus
14from lp.code.mail.sourcepackagerecipebuild import (18from lp.code.mail.sourcepackagerecipebuild import (
15 SourcePackageRecipeBuildMailer)19 SourcePackageRecipeBuildMailer)
16from lp.testing import TestCaseWithFactory20from lp.testing import TestCaseWithFactory
1721
22expected_body = u"""\
23 * State: Successfully built
24 * Recipe: person/recipe
25 * Archive: archiveowner/ppa
26 * Distroseries: distroseries
27 * Duration: five minutes
28 * Build Log: %s
29 * Builder: http://launchpad.dev/builders/bob
30"""
31
32superseded_body = u"""\
33 * State: Build for superseded Source
34 * Recipe: person/recipe
35 * Archive: archiveowner/ppa
36 * Distroseries: distroseries
37 * Duration:
38 * Build Log:
39 * Builder:
40"""
1841
19class TestSourcePackageRecipeBuildMailer(TestCaseWithFactory):42class TestSourcePackageRecipeBuildMailer(TestCaseWithFactory):
2043
21 layer = DatabaseFunctionalLayer44 layer = LaunchpadFunctionalLayer
2245
23 def test_generateEmail(self):46 def test_generateEmail(self):
24 """GenerateEmail produces the right headers and body."""47 """GenerateEmail produces the right headers and body."""
25 person = self.factory.makePerson(name='person')48 person = self.factory.makePerson(name='person')
26 cake = self.factory.makeSourcePackageRecipe(49 cake = self.factory.makeSourcePackageRecipe(
27 name=u'recipe', owner=person)50 name=u'recipe', owner=person)
28 pantry = self.factory.makeArchive(name='ppa')51 pantry_owner = self.factory.makePerson(name='archiveowner')
52 pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
29 secret = self.factory.makeDistroSeries(name=u'distroseries')53 secret = self.factory.makeDistroSeries(name=u'distroseries')
30 build = self.factory.makeSourcePackageRecipeBuild(54 build = self.factory.makeSourcePackageRecipeBuild(
31 recipe=cake, distroseries=secret, archive=pantry,55 recipe=cake, distroseries=secret, archive=pantry,
32 status=BuildStatus.FULLYBUILT)56 status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=5))
33 IStore(build).flush()57 naked_build = removeSecurityProxy(build)
58 naked_build.builder = self.factory.makeBuilder(name='bob')
59 naked_build.buildlog = self.factory.makeLibraryFileAlias()
60 Store.of(build).flush()
34 mailer = SourcePackageRecipeBuildMailer.forStatus(build)61 mailer = SourcePackageRecipeBuildMailer.forStatus(build)
35 email = build.requester.preferredemail.email62 email = build.requester.preferredemail.email
36 ctrl = mailer.generateEmail(email, build.requester)63 ctrl = mailer.generateEmail(email, build.requester)
37 self.assertEqual('Successfully built: recipe for distroseries',64 self.assertEqual(
38 ctrl.subject)65 u'[recipe build #%d] of ~person recipe in distroseries: '
66 'Successfully built' % (build.id), ctrl.subject)
39 body, footer = ctrl.body.split('\n-- \n')67 body, footer = ctrl.body.split('\n-- \n')
40 self.assertEqual(68 self.assertEqual(
41 'Build person/recipe into ppa for distroseries: Successfully'69 expected_body % build.build_log_url, body)
42 ' built.\n', body
43 )
44 self.assertEqual(70 self.assertEqual(
45 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'71 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
46 'You are the requester of the build.\n', footer)72 'You are the requester of the build.\n', footer)
@@ -54,6 +80,39 @@
54 self.assertEqual(80 self.assertEqual(
55 'FULLYBUILT', ctrl.headers['X-Launchpad-Build-State'])81 'FULLYBUILT', ctrl.headers['X-Launchpad-Build-State'])
5682
83 def test_generateEmail_with_null_fields(self):
84 """GenerateEmail works when many fields are NULL."""
85 person = self.factory.makePerson(name='person')
86 cake = self.factory.makeSourcePackageRecipe(
87 name=u'recipe', owner=person)
88 pantry_owner = self.factory.makePerson(name='archiveowner')
89 pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
90 secret = self.factory.makeDistroSeries(name=u'distroseries')
91 build = self.factory.makeSourcePackageRecipeBuild(
92 recipe=cake, distroseries=secret, archive=pantry,
93 status=BuildStatus.SUPERSEDED)
94 Store.of(build).flush()
95 mailer = SourcePackageRecipeBuildMailer.forStatus(build)
96 email = build.requester.preferredemail.email
97 ctrl = mailer.generateEmail(email, build.requester)
98 self.assertEqual(
99 u'[recipe build #%d] of ~person recipe in distroseries: '
100 'Build for superseded Source' % (build.id), ctrl.subject)
101 body, footer = ctrl.body.split('\n-- \n')
102 self.assertEqual(superseded_body, body)
103 self.assertEqual(
104 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
105 'You are the requester of the build.\n', footer)
106 self.assertEqual(
107 config.canonical.noreply_from_address, ctrl.from_addr)
108 self.assertEqual(
109 'Requester', ctrl.headers['X-Launchpad-Message-Rationale'])
110 self.assertEqual(
111 'recipe-build-status',
112 ctrl.headers['X-Launchpad-Notification-Type'])
113 self.assertEqual(
114 'SUPERSEDED', ctrl.headers['X-Launchpad-Build-State'])
115
57116
58def test_suite():117def test_suite():
59 return TestLoader().loadTestsFromName(__name__)118 return TestLoader().loadTestsFromName(__name__)
60119
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-07-28 16:44:00 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
@@ -202,7 +202,7 @@
202202
203 @classmethod203 @classmethod
204 def new(cls, distroseries, recipe, requester, archive, pocket=None,204 def new(cls, distroseries, recipe, requester, archive, pocket=None,
205 date_created=None):205 date_created=None, duration=None):
206 """See `ISourcePackageRecipeBuildSource`."""206 """See `ISourcePackageRecipeBuildSource`."""
207 store = IMasterStore(SourcePackageRecipeBuild)207 store = IMasterStore(SourcePackageRecipeBuild)
208 if pocket is None:208 if pocket is None:
@@ -215,7 +215,8 @@
215 requester,215 requester,
216 archive,216 archive,
217 pocket,217 pocket,
218 date_created=date_created)218 date_created=date_created,
219 build_duration=duration)
219 store.add(spbuild)220 store.add(spbuild)
220 return spbuild221 return spbuild
221222
222223
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-30 14:04:15 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-08-01 04:36:51 +0000
@@ -30,6 +30,8 @@
30from lp.code.interfaces.sourcepackagerecipebuild import (30from lp.code.interfaces.sourcepackagerecipebuild import (
31 ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,31 ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
32 ISourcePackageRecipeBuildSource)32 ISourcePackageRecipeBuildSource)
33from lp.code.mail.sourcepackagerecipebuild import (
34 SourcePackageRecipeBuildMailer)
33from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild35from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild
34from lp.registry.interfaces.pocket import PackagePublishingPocket36from lp.registry.interfaces.pocket import PackagePublishingPocket
35from lp.services.mail.sendmail import format_address37from lp.services.mail.sendmail import format_address
@@ -320,7 +322,8 @@
320 pantry = self.factory.makeArchive(name='ppa')322 pantry = self.factory.makeArchive(name='ppa')
321 secret = self.factory.makeDistroSeries(name=u'distroseries')323 secret = self.factory.makeDistroSeries(name=u'distroseries')
322 build = self.factory.makeSourcePackageRecipeBuild(324 build = self.factory.makeSourcePackageRecipeBuild(
323 recipe=cake, distroseries=secret, archive=pantry)325 recipe=cake, distroseries=secret, archive=pantry,
326 duration=datetime.timedelta(minutes=5))
324 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT327 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
325 IStore(build).flush()328 IStore(build).flush()
326 build.notify()329 build.notify()
@@ -328,21 +331,24 @@
328 requester = build.requester331 requester = build.requester
329 requester_address = format_address(332 requester_address = format_address(
330 requester.displayname, requester.preferredemail.email)333 requester.displayname, requester.preferredemail.email)
334 mailer = SourcePackageRecipeBuildMailer.forStatus(build)
335 expected = mailer.generateEmail(
336 requester.preferredemail.email, requester)
331 self.assertEqual(337 self.assertEqual(
332 requester_address, re.sub(r'\n\t+', ' ', message['To']))338 requester_address, re.sub(r'\n\t+', ' ', message['To']))
333 self.assertEqual('Successfully built: recipe for distroseries',339 self.assertEqual(expected.subject, message['Subject'].replace(
334 message['Subject'])340 '\n\t', ' '))
335 body, footer = message.get_payload(decode=True).split('\n-- \n')
336 self.assertEqual(341 self.assertEqual(
337 'Build person/recipe into ppa for distroseries: Successfully'342 expected.body, message.get_payload(decode=True))
338 ' built.\n', body)
339343
340 def test_handleStatusNotifies(self):344 def test_handleStatusNotifies(self):
341 """"handleStatus causes notification, even if OK."""345 """"handleStatus causes notification, even if OK."""
342346
343 def prepare_build():347 def prepare_build():
344 queue_record = self.factory.makeSourcePackageRecipeBuildJob()348 build = self.factory.makeSourcePackageRecipeBuild(
345 build = queue_record.specific_job.build349 duration=datetime.timedelta(minutes=5))
350 queue_record = self.factory.makeSourcePackageRecipeBuildJob(
351 recipe_build=build)
346 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT352 removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
347 queue_record.builder = self.factory.makeBuilder()353 queue_record.builder = self.factory.makeBuilder()
348 slave = WaitingSlave('BuildStatus.OK')354 slave = WaitingSlave('BuildStatus.OK')
@@ -373,7 +379,8 @@
373 distroseries.nominatedarchindep = distroseries_i386379 distroseries.nominatedarchindep = distroseries_i386
374 build = self.factory.makeSourcePackageRecipeBuild(380 build = self.factory.makeSourcePackageRecipeBuild(
375 distroseries=distroseries,381 distroseries=distroseries,
376 status=BuildStatus.FULLYBUILT)382 status=BuildStatus.FULLYBUILT,
383 duration=datetime.timedelta(minutes=5))
377 build.queueBuild(build)384 build.queueBuild(build)
378 return build385 return build
379386
380387
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-07-30 20:33:47 +0000
+++ lib/lp/testing/factory.py 2010-08-01 04:36:51 +0000
@@ -167,8 +167,6 @@
167 time_counter,167 time_counter,
168 )168 )
169from lp.translations.interfaces.potemplate import IPOTemplateSet169from lp.translations.interfaces.potemplate import IPOTemplateSet
170from lp.translations.interfaces.translationimportqueue import (
171 RosettaImportStatus)
172from lp.translations.interfaces.translationgroup import (170from lp.translations.interfaces.translationgroup import (
173 ITranslationGroupSet)171 ITranslationGroupSet)
174from lp.translations.interfaces.translationimportqueue import (172from lp.translations.interfaces.translationimportqueue import (
@@ -1871,7 +1869,8 @@
1871 requester=None, archive=None,1869 requester=None, archive=None,
1872 sourcename=None, distroseries=None,1870 sourcename=None, distroseries=None,
1873 pocket=None, date_created=None,1871 pocket=None, date_created=None,
1874 status=BuildStatus.NEEDSBUILD):1872 status=BuildStatus.NEEDSBUILD,
1873 duration=None):
1875 """Make a new SourcePackageRecipeBuild."""1874 """Make a new SourcePackageRecipeBuild."""
1876 if recipe is None:1875 if recipe is None:
1877 recipe = self.makeSourcePackageRecipe(name=sourcename)1876 recipe = self.makeSourcePackageRecipe(name=sourcename)
@@ -1888,7 +1887,8 @@
1888 archive=archive,1887 archive=archive,
1889 requester=requester,1888 requester=requester,
1890 pocket=pocket,1889 pocket=pocket,
1891 date_created=date_created)1890 date_created=date_created,
1891 duration=duration)
1892 removeSecurityProxy(spr_build).buildstate = status1892 removeSecurityProxy(spr_build).buildstate = status
1893 return spr_build1893 return spr_build
18941894
@@ -2773,8 +2773,7 @@
27732773
27742774
2775def is_security_proxied_or_harmless(obj):2775def is_security_proxied_or_harmless(obj):
2776 """Check that the given object is security wrapped or a harmless object.2776 """Check that the object is security wrapped or a harmless object."""
2777 """
2778 if obj is None:2777 if obj is None:
2779 return True2778 return True
2780 if builtin_isinstance(obj, Proxy):2779 if builtin_isinstance(obj, Proxy):