launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00354
[Merge] lp:~abentley/launchpad/detailed-build-emails into lp:launchpad/devel
Aaron Bentley has proposed merging lp:~abentley/launchpad/detailed-build-emails into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#603606 Recipe Build failure emails aren't as binary package build failures
https://bugs.launchpad.net/bugs/603606
= 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.
--
https://code.launchpad.net/~abentley/launchpad/detailed-build-emails/+merge/31413
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/detailed-build-emails into lp:launchpad/devel.
=== 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-07-30 19:24:53 +0000
@@ -1,1 +1,7 @@
-Build %(recipe_owner)s/%(recipe)s into %(archive)s for %(distroseries)s: %(status)s.
+ * State: %(status)s
+ * Recipe: %(recipe_owner)s/%(recipe)s
+ * Archive: %(archive_owner)s/%(archive)s
+ * Distroseries: %(distroseries)s
+ * Duration: %(duration)s
+ * Build Log: %(log_url)s
+ * Builder: %(builder_url)s
=== 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-07-30 19:24:53 +0000
@@ -11,6 +11,7 @@
from canonical.config import config
from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.tales import DurationFormatterAPI
from lp.services.mail.basemailer import BaseMailer, RecipientReason
@@ -25,7 +26,8 @@
requester = build.requester
recipients = {requester: RecipientReason.forBuildRequester(requester)}
return cls(
- '%(status)s: %(recipe)s for %(distroseries)s',
+ '[recipe build #%(build_id)d] of ~%(recipe_owner)s %(recipe)s in'
+ ' %(distroseries)s: %(status)s',
'build-request.txt', recipients,
config.canonical.noreply_from_address, build)
@@ -50,13 +52,26 @@
params = super(
SourcePackageRecipeBuildMailer, self)._getTemplateParams(email)
params.update({
+ 'build_id': self.build.id,
'status': self.build.buildstate.title,
'distroseries': self.build.distroseries.name,
'recipe': self.build.recipe.name,
'recipe_owner': self.build.recipe.owner.name,
'archive': self.build.archive.name,
+ 'archive_owner': self.build.archive.owner.name,
+ 'log_url': '',
+ 'component': self.build.current_component.name,
+ 'duration': '',
+ 'builder_url': '',
'build_url': canonical_url(self.build),
})
+ if self.build.builder is not None:
+ params['builder_url'] = canonical_url(self.build.builder)
+ if self.build.buildduration is not None:
+ duration_formatter = DurationFormatterAPI(self.build.buildduration)
+ params['duration'] = duration_formatter.approximateduration()
+ if self.build.build_log_url is not None:
+ params['log_url'] = self.build.build_log_url
return params
def _getFooter(self, params):
=== 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-07-30 19:24:53 +0000
@@ -5,42 +5,68 @@
__metaclass__ = type
+from datetime import timedelta
from unittest import TestLoader
+from storm.locals import Store
+from zope.security.proxy import removeSecurityProxy
+
from canonical.config import config
from canonical.launchpad.interfaces.lpstorm import IStore
-from canonical.testing import DatabaseFunctionalLayer
+from canonical.testing import LaunchpadFunctionalLayer
from lp.buildmaster.interfaces.buildbase import BuildStatus
from lp.code.mail.sourcepackagerecipebuild import (
SourcePackageRecipeBuildMailer)
from lp.testing import TestCaseWithFactory
+expected_body = u"""\
+ * State: Successfully built
+ * Recipe: person/recipe
+ * Archive: archiveowner/ppa
+ * Distroseries: distroseries
+ * Duration: five minutes
+ * Build Log: %s
+ * Builder: http://launchpad.dev/builders/bob
+"""
+
+superseded_body = u"""\
+ * State: Build for superseded Source
+ * Recipe: person/recipe
+ * Archive: archiveowner/ppa
+ * Distroseries: distroseries
+ * Duration:
+ * Build Log:
+ * Builder:
+"""
class TestSourcePackageRecipeBuildMailer(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def test_generateEmail(self):
"""GenerateEmail produces the right headers and body."""
person = self.factory.makePerson(name='person')
cake = self.factory.makeSourcePackageRecipe(
name=u'recipe', owner=person)
- pantry = self.factory.makeArchive(name='ppa')
+ pantry_owner = self.factory.makePerson(name='archiveowner')
+ pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
secret = self.factory.makeDistroSeries(name=u'distroseries')
build = self.factory.makeSourcePackageRecipeBuild(
recipe=cake, distroseries=secret, archive=pantry,
- status=BuildStatus.FULLYBUILT)
- IStore(build).flush()
+ status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=5))
+ naked_build = removeSecurityProxy(build)
+ naked_build.builder = self.factory.makeBuilder(name='bob')
+ naked_build.buildlog = self.factory.makeLibraryFileAlias()
+ Store.of(build).flush()
mailer = SourcePackageRecipeBuildMailer.forStatus(build)
email = build.requester.preferredemail.email
ctrl = mailer.generateEmail(email, build.requester)
- self.assertEqual('Successfully built: recipe for distroseries',
- ctrl.subject)
+ self.assertEqual(
+ u'[recipe build #%d] of ~person recipe in distroseries: '
+ 'Successfully built' % (build.id), ctrl.subject)
body, footer = ctrl.body.split('\n-- \n')
self.assertEqual(
- 'Build person/recipe into ppa for distroseries: Successfully'
- ' built.\n', body
- )
+ expected_body % build.build_log_url, body)
self.assertEqual(
'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
'You are the requester of the build.\n', footer)
@@ -54,6 +80,39 @@
self.assertEqual(
'FULLYBUILT', ctrl.headers['X-Launchpad-Build-State'])
+ def test_generateEmail_with_null_fields(self):
+ """GenerateEmail works when many fields are NULL."""
+ person = self.factory.makePerson(name='person')
+ cake = self.factory.makeSourcePackageRecipe(
+ name=u'recipe', owner=person)
+ pantry_owner = self.factory.makePerson(name='archiveowner')
+ pantry = self.factory.makeArchive(name='ppa', owner=pantry_owner)
+ secret = self.factory.makeDistroSeries(name=u'distroseries')
+ build = self.factory.makeSourcePackageRecipeBuild(
+ recipe=cake, distroseries=secret, archive=pantry,
+ status=BuildStatus.SUPERSEDED)
+ Store.of(build).flush()
+ mailer = SourcePackageRecipeBuildMailer.forStatus(build)
+ email = build.requester.preferredemail.email
+ ctrl = mailer.generateEmail(email, build.requester)
+ self.assertEqual(
+ u'[recipe build #%d] of ~person recipe in distroseries: '
+ 'Build for superseded Source' % (build.id), ctrl.subject)
+ body, footer = ctrl.body.split('\n-- \n')
+ self.assertEqual(superseded_body, body)
+ self.assertEqual(
+ 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
+ 'You are the requester of the build.\n', footer)
+ self.assertEqual(
+ config.canonical.noreply_from_address, ctrl.from_addr)
+ self.assertEqual(
+ 'Requester', ctrl.headers['X-Launchpad-Message-Rationale'])
+ self.assertEqual(
+ 'recipe-build-status',
+ ctrl.headers['X-Launchpad-Notification-Type'])
+ self.assertEqual(
+ 'SUPERSEDED', ctrl.headers['X-Launchpad-Build-State'])
+
def test_suite():
return TestLoader().loadTestsFromName(__name__)
=== 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-07-30 19:24:53 +0000
@@ -202,7 +202,7 @@
@classmethod
def new(cls, distroseries, recipe, requester, archive, pocket=None,
- date_created=None):
+ date_created=None, duration=None):
"""See `ISourcePackageRecipeBuildSource`."""
store = IMasterStore(SourcePackageRecipeBuild)
if pocket is None:
@@ -215,7 +215,8 @@
requester,
archive,
pocket,
- date_created=date_created)
+ date_created=date_created,
+ build_duration=duration)
store.add(spbuild)
return spbuild
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-22 08:15:29 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-30 19:24:53 +0000
@@ -30,6 +30,8 @@
from lp.code.interfaces.sourcepackagerecipebuild import (
ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
ISourcePackageRecipeBuildSource)
+from lp.code.mail.sourcepackagerecipebuild import (
+ SourcePackageRecipeBuildMailer)
from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.mail.sendmail import format_address
@@ -319,7 +321,8 @@
pantry = self.factory.makeArchive(name='ppa')
secret = self.factory.makeDistroSeries(name=u'distroseries')
build = self.factory.makeSourcePackageRecipeBuild(
- recipe=cake, distroseries=secret, archive=pantry)
+ recipe=cake, distroseries=secret, archive=pantry,
+ duration=datetime.timedelta(minutes=5))
removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
IStore(build).flush()
build.notify()
@@ -327,21 +330,24 @@
requester = build.requester
requester_address = format_address(
requester.displayname, requester.preferredemail.email)
+ mailer = SourcePackageRecipeBuildMailer.forStatus(build)
+ expected = mailer.generateEmail(
+ requester.preferredemail.email, requester)
self.assertEqual(
requester_address, re.sub(r'\n\t+', ' ', message['To']))
- self.assertEqual('Successfully built: recipe for distroseries',
- message['Subject'])
- body, footer = message.get_payload(decode=True).split('\n-- \n')
+ self.assertEqual(expected.subject, message['Subject'].replace(
+ '\n\t', ' '))
self.assertEqual(
- 'Build person/recipe into ppa for distroseries: Successfully'
- ' built.\n', body)
+ expected.body, message.get_payload(decode=True))
def test_handleStatusNotifies(self):
""""handleStatus causes notification, even if OK."""
def prepare_build():
- queue_record = self.factory.makeSourcePackageRecipeBuildJob()
- build = queue_record.specific_job.build
+ build = self.factory.makeSourcePackageRecipeBuild(
+ duration=datetime.timedelta(minutes=5))
+ queue_record = self.factory.makeSourcePackageRecipeBuildJob(
+ recipe_build=build)
removeSecurityProxy(build).buildstate = BuildStatus.FULLYBUILT
queue_record.builder = self.factory.makeBuilder()
slave = WaitingSlave('BuildStatus.OK')
@@ -372,7 +378,8 @@
distroseries.nominatedarchindep = distroseries_i386
build = self.factory.makeSourcePackageRecipeBuild(
distroseries=distroseries,
- status=BuildStatus.FULLYBUILT)
+ status=BuildStatus.FULLYBUILT,
+ duration=datetime.timedelta(minutes=5))
build.queueBuild(build)
return build
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-07-30 16:30:12 +0000
+++ lib/lp/testing/factory.py 2010-07-30 19:24:53 +0000
@@ -166,8 +166,6 @@
time_counter,
)
from lp.translations.interfaces.potemplate import IPOTemplateSet
-from lp.translations.interfaces.translationimportqueue import (
- RosettaImportStatus)
from lp.translations.interfaces.translationgroup import (
ITranslationGroupSet)
from lp.translations.interfaces.translationimportqueue import (
@@ -1870,7 +1868,8 @@
requester=None, archive=None,
sourcename=None, distroseries=None,
pocket=None, date_created=None,
- status=BuildStatus.NEEDSBUILD):
+ status=BuildStatus.NEEDSBUILD,
+ duration=None):
"""Make a new SourcePackageRecipeBuild."""
if recipe is None:
recipe = self.makeSourcePackageRecipe(name=sourcename)
@@ -1887,7 +1886,8 @@
archive=archive,
requester=requester,
pocket=pocket,
- date_created=date_created)
+ date_created=date_created,
+ duration=duration)
removeSecurityProxy(spr_build).buildstate = status
return spr_build
@@ -2767,12 +2767,18 @@
# security wrappers for them, as well as for objects created by
# other Python libraries.
unwrapped_types = (
- DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str, unicode)
+ DSCFile, InstanceType, MergeDirective2, Message, datetime, int, str,
+ unicode)
+
def is_security_proxied_or_harmless(obj):
+<<<<<<< TREE
"""Check that the given object is security wrapped or a harmless object.
"""
+=======
+ """Check that the object is security wrapped or a harmless object."""
+>>>>>>> MERGE-SOURCE
if obj is None:
return True
if builtin_isinstance(obj, Proxy):
