launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/fix-builder-history into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #615562 sourcepackagerecipebuilds break branch history page
  https://bugs.launchpad.net/bugs/615562


= Summary =
Fix bug #615562: sourcepackagerecipebuilds break branch history page

== Proposed fix ==
Provide an adaptor from lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob to
lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJob that converts it
to returns a SourcePackageRecipeBuild.

== Pre-implementation notes ==
Discussed with bigjools

== Implementation details ==
Implemented get_recipe_build_for_build_farm_job as the adaptor.

Stopped returning CompleteBuild for non-BinaryPackageBuilds, because this
causes the wrong URL to be used.

Switched from In(BinaryPackageBuild.id, build_ids) to
BinaryPackageBuild.id.is_in(build_ids) because "In" was generating invalid SQL
when build_ids was empty.

== Tests ==
bin/test -t test_getSpecificJob -t test_builder_history

== Demo and Q/A ==
Create a SourcePackageRecipeBuild.  After it has been built, look at its
builder's history page.  The build should be listed, and should have the
correct URL.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/tests/test_sourcepackagerecipebuild.py
  lib/lp/code/configure.zcml
  lib/lp/soyuz/browser/build.py
  lib/lp/code/model/sourcepackagerecipebuild.py
  lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py
  lib/lp/soyuz/model/binarypackagebuild.py

./lib/lp/soyuz/browser/build.py
      40: 'IHasBuildRecords' imported but unused
     297: E301 expected 1 blank line, found 0
     499: E202 whitespace before ')'
./lib/lp/code/model/sourcepackagerecipebuild.py
      50: E231 missing whitespace after ','
./lib/lp/soyuz/model/binarypackagebuild.py
     376: E202 whitespace before '}'
     447: W602 deprecated form of raising exception
     451: E231 missing whitespace after ','
     481: E501 line too long (83 characters)
     557: E203 whitespace before ':'
     671: E203 whitespace before ':'
     749: E202 whitespace before ')'
     907: E202 whitespace before ']'
     481: Line exceeds 78 characters.
     813: Line exceeds 78 characters.
-- 
https://code.launchpad.net/~abentley/launchpad/fix-builder-history/+merge/32150
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/fix-builder-history into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py	2010-08-01 22:31:42 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py	2010-08-09 20:52:48 +0000
@@ -7,13 +7,14 @@
 __metaclass__ = type
 
 from mechanize import LinkNotFoundError
+from storm.locals import Store
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.testing.pages import (
-    extract_text, find_tags_by_class)
+    extract_text, find_main_content, find_tags_by_class)
 from canonical.launchpad.webapp import canonical_url
 from canonical.testing import DatabaseFunctionalLayer
 from lp.buildmaster.interfaces.buildbase import BuildStatus
@@ -189,3 +190,15 @@
         self.assertRaises(
             LinkNotFoundError,
             browser.getLink, 'Rescore build')
+
+    def test_builder_history(self):
+        build = self.makeRecipeBuild()
+        Store.of(build).flush()
+        build_url = canonical_url(build)
+        removeSecurityProxy(build).builder = self.factory.makeBuilder()
+        browser = self.getViewBrowser(build.builder, '+history')
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+             'Build history.*~chef/chocolate/cake recipe build',
+             extract_text(find_main_content(browser.contents)))
+        self.assertEqual(build_url,
+                browser.getLink('~chef/chocolate/cake recipe build').url)

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-07-26 20:08:06 +0000
+++ lib/lp/code/configure.zcml	2010-08-09 20:52:48 +0000
@@ -990,5 +990,10 @@
         name="RECIPEBRANCHBUILD"
         provides="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob"/>
   <webservice:register module="lp.code.interfaces.webservice" />
-
+    <adapter
+        provides="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJob"
+        for="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob"
+        factory="lp.code.model.sourcepackagerecipebuild.get_recipe_build_for_build_farm_job"
+        name="RECIPEBRANCHBUILD"
+        permission="zope.Public"/>
 </configure>

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-03 08:49:19 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-09 20:52:48 +0000
@@ -26,6 +26,7 @@
 
 from zope.component import getUtility
 from zope.interface import classProvides, implements
+from zope.security.proxy import ProxyFactory
 
 from canonical.launchpad.webapp import errorlog
 from lp.app.errors import NotFoundError
@@ -345,3 +346,12 @@
 
     def score(self):
         return 2405 + self.build.archive.relative_build_score
+
+
+def get_recipe_build_for_build_farm_job(build_farm_job):
+    """Return the SourcePackageRecipeBuild associated with a BuildFarmJob."""
+    store = Store.of(build_farm_job)
+    result = store.find(SourcePackageRecipeBuild,
+        SourcePackageRecipeBuild.package_build_id == PackageBuild.id,
+        PackageBuild.build_farm_job_id == build_farm_job.id)
+    return ProxyFactory(result.one())

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-08-04 13:48:18 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-08-09 20:52:48 +0000
@@ -289,6 +289,14 @@
             BuildStatus.SUPERSEDED,
             build.status)
 
+    def test_getSpecificJob(self):
+        # getSpecificJob returns the SourcePackageRecipeBuild
+        sprb = self.makeSourcePackageRecipeBuild()
+        Store.of(sprb).flush()
+        build = sprb.build_farm_job
+        job = sprb.build_farm_job.getSpecificJob()
+        self.assertEqual(sprb, job)
+
 
 class TestAsBuildmaster(TestCaseWithFactory):
 

=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py	2010-08-04 04:07:21 +0000
+++ lib/lp/soyuz/browser/build.py	2010-08-09 20:52:48 +0000
@@ -331,9 +331,11 @@
 
     complete_builds = []
     for build in builds:
-        buildqueue = prefetched_data.get(build.id)
-        complete_builds.append(CompleteBuild(build, buildqueue))
-
+        if IBinaryPackageBuild.providedBy(build):
+            buildqueue = prefetched_data.get(build.id)
+            complete_builds.append(CompleteBuild(build, buildqueue))
+        else:
+            complete_builds.append(build)
     return complete_builds
 
 

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2010-08-03 08:49:19 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2010-08-09 20:52:48 +0000
@@ -1159,7 +1159,7 @@
             )
         result_set = store.using(*origin).find(
             (BuildQueue, Builder, BuildPackageJob),
-            In(BinaryPackageBuild.id, build_ids))
+            BinaryPackageBuild.id.is_in(build_ids))
 
         return result_set