Merge lp:~thumper/launchpad/recipe-binary-builds into lp:launchpad

[Tim Penhey]

[r=abentley][ui=rockstar][bug=602508] Show related binary builds on the recipe index page.

Adds binary build information to the latest builds section on the recipe index page.

http://people.canonical.com/~tim/recipe-latest-builds.png shows an example of how it looks.

I made some of the properties of the view that were accessed multiple times
cached properties. This required a few tweaks in the tests too.

The binary builds for the successful recipe builds are indented slightly and
show underneath the recipe build that they relate to.

During interactive hacking, I found that I wanted some nicer repr methods
for the processors.

A drive-by fix on the soyuz build-index.pt to just specify the content as
buildlog instead of using tal:content="string: buildlog"

[Paul Hummer (rockstar)]

This looks really excellent!

review: Approve (ui)

[Aaron Bentley (abentley)]

You are showing the start time for binary builds, while the source package recipe builds show the end time. The time values in this column should have a consistent meaning.

review: Needs Fixing

[Tim Penhey (thumper)]

Aaron, I've gone with the expected completed time, and added the same properties to the BuildView. I feel that we should use a common mixin, but I can't think where to actually put the code.

My first thought was actually something like lp.shared.browser or lp.app.browser.shared...

Not sure.

[Aaron Bentley (abentley)]

Looks good. At some point, we might want to collapse eta and date into a single method call that returns a tuple.

review: Approve

=== modified file 'lib/canonical/launchpad/doc/hierarchical-menu.txt'
--- lib/canonical/launchpad/doc/hierarchical-menu.txt	2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/doc/hierarchical-menu.txt	2010-11-30 01:38:20 +0000
@@ -90,6 +90,7 @@
     # have to create fake views and other stuff to test breadcrumbs here. The
     # functionality provided by that method is tested in
     # webapp/tests/test_breadcrumbs.py.
+    >>> make_breadcrumb_func = Hierarchy.makeBreadcrumbForRequestedPage
     >>> Hierarchy.makeBreadcrumbForRequestedPage = lambda self: None
 
     # Note that the Hierarchy assigns the breadcrumb's URL, but we need to
@@ -269,3 +270,8 @@
 
     >>> homepage_hierarchy.render().strip()
     u''
+
+
+Put the monkey patched method back.
+
+    >>> Hierarchy.makeBreadcrumbForRequestedPage = make_breadcrumb_func

=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
--- lib/lp/code/browser/sourcepackagerecipebuild.py	2010-11-23 23:22:27 +0000
+++ lib/lp/code/browser/sourcepackagerecipebuild.py	2010-11-30 01:38:20 +0000
@@ -34,6 +34,7 @@
     ISourcePackageRecipeBuild,
     )
 from lp.services.job.interfaces.job import JobStatus
+from lp.services.propertycache import cachedproperty
 
 
 UNEDITABLE_BUILD_STATES = (
@@ -95,7 +96,7 @@
             BuildStatus.FAILEDTOUPLOAD: 'Could not be uploaded correctly',
             }.get(self.context.status, self.context.status.title)
 
-    @property
+    @cachedproperty
     def eta(self):
         """The datetime when the build job is estimated to complete.
 
@@ -114,14 +115,14 @@
         duration = queue_record.estimated_duration
         return start_time + duration
 
-    @property
+    @cachedproperty
     def date(self):
         """The date when the build completed or is estimated to complete."""
         if self.estimate:
             return self.eta
         return self.context.date_finished
 
-    @property
+    @cachedproperty
     def estimate(self):
         """If true, the date value is an estimate."""
         if self.context.date_finished is not None:

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-11-24 18:40:48 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-11-30 01:38:20 +0000
@@ -42,6 +42,7 @@
 from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT
 from lp.code.tests.helpers import recipe_parser_newest_version
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.propertycache import clear_property_cache
 from lp.soyuz.model.processor import ProcessorFamily
 from lp.testing import (
     ANONYMOUS,
@@ -691,7 +692,7 @@
 
 class TestSourcePackageRecipeView(TestCaseForRecipe):
 
-    layer = DatabaseFunctionalLayer
+    layer = LaunchpadFunctionalLayer
 
     def test_index(self):
         recipe = self.makeRecipe()
@@ -716,7 +717,7 @@
             Distribution series: Secret Squirrel
 
             Latest builds
-            Status Time Distribution series Archive
+            Status When complete Distribution series Archive
             Successful build on 2010-03-16 Secret Squirrel Secret PPA
             Request build\(s\)
 
@@ -724,12 +725,86 @@
             # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
             lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))
 
+    def test_index_success_with_buildlog(self):
+        # The buildlog is shown if it is there.
+        recipe = self.makeRecipe()
+        build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
+            recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
+        build.status = BuildStatus.FULLYBUILT
+        build.date_started = datetime(2010, 03, 16, tzinfo=utc)
+        build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+        build.log = self.factory.makeLibraryFileAlias()
+
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            Latest builds
+            Status .* Archive
+            Successful build on 2010-03-16 buildlog \(.*\) Secret Squirrel Secret PPA
+            Request build\(s\)""", self.getMainText(recipe))
+
+    def test_index_success_with_binary_builds(self):
+        # Binary builds are shown after the recipe builds if there are any.
+        recipe = self.makeRecipe()
+        build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
+            recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
+        build.status = BuildStatus.FULLYBUILT
+        build.date_started = datetime(2010, 03, 16, tzinfo=utc)
+        build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+        build.log = self.factory.makeLibraryFileAlias()
+        package_name = self.factory.getOrMakeSourcePackageName('chocolate')
+        source_package_release = self.factory.makeSourcePackageRelease(
+            archive=self.ppa, sourcepackagename=package_name, distroseries=self.squirrel,
+            source_package_recipe_build=build, version='0+r42')
+        builder = self.factory.makeBuilder()
+        binary_build = self.factory.makeBinaryPackageBuild(
+            source_package_release=source_package_release,
+            distroarchseries=self.squirrel.nominatedarchindep,
+            processor=builder.processor)
+        binary_build.queueBuild()
+
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            Latest builds
+            Status .* Archive
+            Successful build on 2010-03-16 buildlog \(.*\) Secret Squirrel Secret PPA
+              chocolate - 0\+r42 in .* \(estimated\) i386
+            Request build\(s\)""", self.getMainText(recipe))
+
+    def test_index_success_with_completed_binary_build(self):
+        # Binary builds show their buildlog too.
+        recipe = self.makeRecipe()
+        build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
+            recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
+        build.status = BuildStatus.FULLYBUILT
+        build.date_started = datetime(2010, 03, 16, tzinfo=utc)
+        build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+        build.log = self.factory.makeLibraryFileAlias()
+        package_name = self.factory.getOrMakeSourcePackageName('chocolate')
+        source_package_release = self.factory.makeSourcePackageRelease(
+            archive=self.ppa, sourcepackagename=package_name, distroseries=self.squirrel,
+            source_package_recipe_build=build, version='0+r42')
+        builder = self.factory.makeBuilder()
+        binary_build = removeSecurityProxy(self.factory.makeBinaryPackageBuild(
+            source_package_release=source_package_release,
+            distroarchseries=self.squirrel.nominatedarchindep,
+            processor=builder.processor))
+        binary_build.queueBuild()
+        binary_build.status = BuildStatus.FULLYBUILT
+        binary_build.date_started = datetime(2010, 04, 16, tzinfo=utc)
+        binary_build.date_finished = datetime(2010, 04, 16, tzinfo=utc)
+        binary_build.log = self.factory.makeLibraryFileAlias()
+
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            Latest builds
+            Status .* Archive
+            Successful build on 2010-03-16 buildlog \(.*\) Secret Squirrel Secret PPA
+              chocolate - 0\+r42 on 2010-04-16 buildlog \(.*\) i386
+            Request build\(s\)""", self.getMainText(recipe))
+
     def test_index_no_builds(self):
         """A message should be shown when there are no builds."""
         recipe = self.makeRecipe()
         self.assertTextMatchesExpressionIgnoreWhitespace("""\
             Latest builds
-            Status Time Distribution series Archive
+            Status .* Archive
             This recipe has not been built yet.""", self.getMainText(recipe))
 
     def test_index_no_suitable_builders(self):
@@ -738,7 +813,7 @@
             recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
         self.assertTextMatchesExpressionIgnoreWhitespace("""
             Latest builds
-            Status Time Distribution series Archive
+            Status .* Archive
             No suitable builders Secret Squirrel Secret PPA
             Request build\(s\)""", self.getMainText(recipe))
 
@@ -756,7 +831,7 @@
         self.factory.makeBuilder()
         pattern = """\
             Latest builds
-            Status Time Distribution series Archive
+            Status .* Archive
             Pending build in .* \(estimated\) Secret Squirrel Secret PPA
             Request build\(s\)
 
@@ -946,8 +1021,10 @@
         view = self.makeBuildView()
         self.assertTrue(view.estimate)
         view.context.buildqueue_record.job.start()
+        clear_property_cache(view)
         self.assertTrue(view.estimate)
         removeSecurityProxy(view.context).date_finished = datetime.now(utc)
+        clear_property_cache(view)
         self.assertFalse(view.estimate)
 
     def test_eta(self):
@@ -966,11 +1043,13 @@
             recipe_build=build)
         queue_entry._now = lambda: datetime(1970, 1, 1, 0, 0, 0, 0, utc)
         self.factory.makeBuilder()
+        clear_property_cache(view)
         self.assertIsNot(None, view.eta)
         self.assertEqual(
             queue_entry.getEstimatedJobStartTime() +
             queue_entry.estimated_duration, view.eta)
         queue_entry.job.start()
+        clear_property_cache(view)
         self.assertEqual(
             queue_entry.job.date_started + queue_entry.estimated_duration,
             view.eta)

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt	2010-11-09 04:24:09 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt	2010-11-30 01:38:20 +0000
@@ -7,6 +7,13 @@
   i18n:domain="launchpad"
 >
 
+<metal:block fill-slot="head_epilogue">
+  <style type="text/css">
+    .binary-build .indent {
+      padding-left: 2em;
+    }
+  </style>
+</metal:block>
 
 <body>
 
@@ -86,14 +93,15 @@
           <thead>
             <tr>
               <th>Status</th>
-              <th>Time</th>
+              <th>When complete</th>
               <th>Distribution series</th>
               <th>Archive</th>
             </tr>
           </thead>
           <tbody>
-            <tr tal:repeat="build view/builds">
+            <tal:recipe-builds repeat="build view/builds">
               <tal:build-view define="buildview nocall:build/@@+index">
+            <tr>
               <td>
                 <span tal:replace="structure build/image:icon" />
                 <a tal:content="buildview/status"
@@ -102,8 +110,15 @@
               <td>
                 <tal:date replace="buildview/date/fmt:displaydate" />
                 <tal:estimate condition="buildview/estimate">
-                (estimated)
+                  (estimated)
                 </tal:estimate>
+
+                <tal:build-log define="file build/log"
+                               tal:condition="file">
+                  <a class="sprite download"
+                     tal:attributes="href build/log_url">buildlog</a>
+                  (<span tal:replace="file/content/filesize/fmt:bytes" />)
+                </tal:build-log>
               </td>
               <td>
                 <tal:distro
@@ -112,8 +127,38 @@
               <td>
                 <tal:archive replace="structure build/archive/fmt:link"/>
               </td>
-              </tal:build-view>
             </tr>
+            <tal:binary-builds repeat="binary buildview/binary_builds">
+              <tr tal:define="binaryview nocall:binary/@@+index"
+                  class="binary-build">
+                <td class="indent">
+                  <span tal:replace="structure binary/image:icon"/>
+                  <a tal:content="binary/source_package_release/title"
+                     tal:attributes="href binary/fmt:url">package - version</a>
+                </td>
+                <td>
+                  <tal:date replace="binaryview/date/fmt:displaydate" />
+                  <tal:estimate condition="binaryview/estimate">
+                    (estimated)
+                  </tal:estimate>
+
+                  <tal:build-log define="file binary/log"
+                                 tal:condition="file">
+                    <a class="sprite download"
+                       tal:attributes="href binary/log_url">buildlog</a>
+                    (<span tal:replace="file/content/filesize/fmt:bytes" />)
+                  </tal:build-log>
+                </td>
+                <td class="indent">
+                  <a class="sprite distribution"
+                     tal:define="archseries binary/distro_arch_series"
+                     tal:attributes="href archseries/fmt:url"
+                     tal:content="archseries/architecturetag">i386</a>
+                </td>
+              </tr>
+            </tal:binary-builds>
+            </tal:build-view>
+            </tal:recipe-builds>
           </tbody>
         </table>
         <p tal:condition="not: view/builds">

=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py	2010-11-23 23:22:27 +0000
+++ lib/lp/soyuz/browser/build.py	2010-11-30 01:38:20 +0000
@@ -242,6 +242,39 @@
             self.context.status == BuildStatus.NEEDSBUILD and
             self.context.buildqueue_record.job.status == JobStatus.WAITING)
 
+    @cachedproperty
+    def eta(self):
+        """The datetime when the build job is estimated to complete.
+
+        This is the BuildQueue.estimated_duration plus the
+        Job.date_started or BuildQueue.getEstimatedJobStartTime.
+        """
+        if self.context.buildqueue_record is None:
+            return None
+        queue_record = self.context.buildqueue_record
+        if queue_record.job.status == JobStatus.WAITING:
+            start_time = queue_record.getEstimatedJobStartTime()
+            if start_time is None:
+                return None
+        else:
+            start_time = queue_record.job.date_started
+        duration = queue_record.estimated_duration
+        return start_time + duration
+
+    @cachedproperty
+    def date(self):
+        """The date when the build completed or is estimated to complete."""
+        if self.estimate:
+            return self.eta
+        return self.context.date_finished
+
+    @cachedproperty
+    def estimate(self):
+        """If true, the date value is an estimate."""
+        if self.context.date_finished is not None:
+            return False
+        return self.eta is not None
+
 
 class BuildRetryView(BuildView):
     """View class for retrying `IBinaryPackageBuild`s"""

=== modified file 'lib/lp/soyuz/model/processor.py'
--- lib/lp/soyuz/model/processor.py	2010-08-20 20:31:18 +0000
+++ lib/lp/soyuz/model/processor.py	2010-11-30 01:38:20 +0000
@@ -38,6 +38,9 @@
     title = StringCol(dbName='title', notNull=True)
     description = StringCol(dbName='description', notNull=True)
 
+    def __repr__(self):
+        return "<Processor %r>" % self.title
+
 
 class ProcessorFamily(SQLBase):
     implements(IProcessorFamily)
@@ -55,6 +58,9 @@
         return Processor(family=self, name=name, title=title,
             description=description)
 
+    def __repr__(self):
+        return "<ProcessorFamily %r>" % self.title
+
 
 class ProcessorFamilySet:
     implements(IProcessorFamilySet)

=== modified file 'lib/lp/soyuz/templates/build-index.pt'
--- lib/lp/soyuz/templates/build-index.pt	2010-08-06 16:01:38 +0000
+++ lib/lp/soyuz/templates/build-index.pt	2010-11-30 01:38:20 +0000
@@ -190,8 +190,7 @@
       <li tal:define="file context/log"
           tal:condition="file">
         <a class="sprite download"
-           tal:attributes="href context/log_url"
-           tal:content="string: buildlog">BUILDLOG</a>
+           tal:attributes="href context/log_url">buildlog</a>
         (<span tal:replace="file/content/filesize/fmt:bytes" />)
       </li>
       <li tal:define="file context/upload_log"

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-11-25 17:24:59 +0000
+++ lib/lp/testing/factory.py	2010-11-30 01:38:20 +0000
@@ -2771,7 +2771,7 @@
 
     def makeBinaryPackageBuild(self, source_package_release=None,
             distroarchseries=None, archive=None, builder=None,
-            status=None, pocket=None):
+            status=None, pocket=None, date_created=None, processor=None):
         """Create a BinaryPackageBuild.
 
         If archive is not supplied, the source_package_release is used
@@ -2795,7 +2795,8 @@
             self.makeSourcePackagePublishingHistory(
                 distroseries=source_package_release.upload_distroseries,
                 archive=archive, sourcepackagerelease=source_package_release)
-        processor = self.makeProcessor()
+        if processor is None:
+            processor = self.makeProcessor()
         if distroarchseries is None:
             distroarchseries = self.makeDistroArchSeries(
                 distroseries=source_package_release.upload_distroseries,
@@ -2804,6 +2805,8 @@
             status = BuildStatus.NEEDSBUILD
         if pocket is None:
             pocket = PackagePublishingPocket.RELEASE
+        if date_created is None:
+            date_created = self.getUniqueDate()
         binary_package_build = getUtility(IBinaryPackageBuildSet).new(
             source_package_release=source_package_release,
             processor=processor,
@@ -2811,7 +2814,7 @@
             status=status,
             archive=archive,
             pocket=pocket,
-            date_created=self.getUniqueDate())
+            date_created=date_created)
         naked_build = removeSecurityProxy(binary_package_build)
         naked_build.builder = builder
         return binary_package_build