Merge lp:~abentley/launchpad/estimate-duration into lp:launchpad

[Aaron Bentley]

Provide a data-based estimateDuration.

= Summary =
Fix bug #507764: Need proper SourcePackageRecipeBuild.estimateDuration()

== Proposed fix ==
For sourcepackagerecipes with one or more builds which has a duration, use the
median of those durations. The status of the these builds is not considered,
so if builds of this recipe always fail, this will estimate how long it will
take to fail.

Using the median means that extreme values will not unduly influence the
estimate.

If there are no builds with durations, the estimate is 10 minutes, which is a
reasonable value for small packages.

== Pre-implementation notes ==
bigjools impressed the importance of this bug upon me. Mid-implementation with
rockstar re: status

== Implementation details ==
This implements SourcePackageRecipe.getMedianBuildDuration as the underlying
calculation, and then implements estimateDuration on top of it.

Where the number of values is even, this is not a true median, because it takes
the lower-middle value instead of combining the lower-middle and higher-middle
values.

There are some small drive-by fixes, as well.

== Tests ==
bin/test -t estimateDuration -t getMedian test_sourcepackagerecipe

== Demo and Q/A ==
Request a build. The estimated duration should start at 10 minutes.
Request a second build. The estimated duration should be the actual previous
duration.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/model/tests/test_sourcepackagerecipebuild.py
  lib/lp/code/model/tests/test_sourcepackagerecipe.py
  lib/lp/code/model/sourcepackagerecipebuild.py
  lib/lp/code/model/sourcepackagerecipe.py

[Paul Hummer (rockstar)]

review: Approve (code)

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2010-05-10 15:50:23 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2010-05-18 19:27:28 +0000
@@ -20,7 +20,7 @@
 from zope.interface import classProvides, implements
 
 from canonical.database.datetimecol import UtcDateTimeCol
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
 
 from lp.code.interfaces.sourcepackagerecipe import (
     ISourcePackageRecipe, ISourcePackageRecipeSource,
@@ -176,3 +176,16 @@
             *clauses)
         result.order_by(Desc(SourcePackageRecipeBuild.datebuilt))
         return result
+
+    def getMedianBuildDuration(self):
+        """Return the median duration of builds of this recipe."""
+        store = IStore(self)
+        result = store.find(
+            SourcePackageRecipeBuild.buildduration,
+            SourcePackageRecipeBuild.recipe==self.id,
+            SourcePackageRecipeBuild.buildduration != None)
+        result.order_by(Desc(SourcePackageRecipeBuild.buildduration))
+        count = result.count()
+        if count == 0:
+            return None
+        return result[count/2]

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-05-14 21:24:48 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-05-18 19:27:28 +0000
@@ -214,8 +214,10 @@
 
     def estimateDuration(self):
         """See `IBuildBase`."""
-        # XXX: wgrant 2010-01-19 bug=507764: Need proper implementation.
-        return datetime.timedelta(minutes=2)
+        median = self.recipe.getMedianBuildDuration()
+        if median is not None:
+            return median
+        return datetime.timedelta(minutes=10)
 
     def verifySuccessfulUpload(self):
         return self.source_package_release is not None

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-05-17 19:02:37 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-05-18 19:27:28 +0000
@@ -7,7 +7,7 @@
 
 __metaclass__ = type
 
-from datetime import datetime
+from datetime import datetime, timedelta
 import textwrap
 import unittest
 
@@ -25,7 +25,8 @@
 
 from canonical.launchpad.webapp.authorization import check_permission
 from lp.soyuz.interfaces.archive import (
-    ArchiveDisabled, ArchivePurpose, CannotUploadToArchive, InvalidPocketForPPA)
+    ArchiveDisabled, ArchivePurpose, CannotUploadToArchive,
+    InvalidPocketForPPA)
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.interfaces.sourcepackagerecipe import (
@@ -319,6 +320,25 @@
         # Show no database constraints were violated
         Store.of(recipe).flush()
 
+    def test_getMedianBuildDuration(self):
+        recipe = removeSecurityProxy(self.factory.makeSourcePackageRecipe())
+        self.assertIs(None, recipe.getMedianBuildDuration())
+        build = removeSecurityProxy(
+            self.factory.makeSourcePackageRecipeBuild(recipe=recipe))
+        build.buildduration = timedelta(minutes=10)
+        self.assertEqual(
+            timedelta(minutes=10), recipe.getMedianBuildDuration())
+        def addBuild(minutes):
+            build = removeSecurityProxy(
+                self.factory.makeSourcePackageRecipeBuild(recipe=recipe))
+            build.buildduration = timedelta(minutes=minutes)
+        addBuild(20)
+        self.assertEqual(
+            timedelta(minutes=10), recipe.getMedianBuildDuration())
+        addBuild(11)
+        self.assertEqual(
+            timedelta(minutes=11), recipe.getMedianBuildDuration())
+
 
 class TestRecipeBranchRoundTripping(TestCaseWithFactory):
 

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-05-17 17:40:32 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-05-18 19:27:28 +0000
@@ -78,8 +78,8 @@
         self.assertProvides(bq.specific_job, ISourcePackageRecipeBuildJob)
         self.assertEqual(True, bq.virtualized)
 
-        # The processor for SourcePackageRecipeBuilds should not be None.  They
-        # do require specific environments.
+        # The processor for SourcePackageRecipeBuilds should not be None.
+        # They do require specific environments.
         self.assertNotEqual(None, bq.processor)
         self.assertEqual(
             spb.distroseries.nominatedarchindep.default_processor,
@@ -136,10 +136,17 @@
         self.assertFalse(check_permission('launchpad.View', build))
 
     def test_estimateDuration(self):
-        # The duration estimate is currently hard-coded as two minutes.
+        # If there are no successful builds, estimate 10 minutes.
         spb = self.makeSourcePackageRecipeBuild()
         self.assertEqual(
-            datetime.timedelta(minutes=2), spb.estimateDuration())
+            datetime.timedelta(minutes=10), spb.estimateDuration())
+        for minutes in [20, 5, 1]:
+            build = removeSecurityProxy(
+                self.factory.makeSourcePackageRecipeBuild(recipe=spb.recipe))
+            build.buildduration = datetime.timedelta(minutes=minutes)
+        self.assertEqual(
+            datetime.timedelta(minutes=5), spb.estimateDuration())
+
 
     def test_datestarted(self):
         """Datestarted is taken from job if not specified in the build.