Merge lp:~michael.nelson/launchpad/589058-upload-rejected-date-started-not-set into lp:launchpad

[Michael Nelson]

This fixes bug 589068 (despite the branch name) by ensuring that, when estimating the duration of a build, the previous build is only used if it has both date_started and date_finished.

See the bug for why there are completed builds without date_started (rollout related). This branch just makes sure we don't fall over when this occurs.

to test:
bin/test -vvm test_binarypackagebuild -t test_estimateDuration

No lint (although strange exceptions while running `make lint`.

[Edwin Grubbs (edwin-grubbs)]

Looks good.

review: Approve (code)

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2010-06-08 11:30:29 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2010-06-09 12:18:28 +0000
@@ -484,13 +484,14 @@
             clauseTables=['PackageBuild', 'BuildFarmJob', 'SourcePackageName',
                           'SourcePackageRelease'])
 
+        estimated_duration = None
         if completed_builds.count() > 0:
-            # Historic build data exists, use the most recent value.
+            # Historic build data exists, use the most recent value -
+            # assuming it has valid data.
             most_recent_build = completed_builds[0]
-            date_finished = most_recent_build.date_finished
-            date_started = most_recent_build.date_started
-            estimated_duration = date_finished - date_started
-        else:
+            estimated_duration = most_recent_build.duration
+
+        if estimated_duration is None:
             # Estimate the build duration based on package size if no
             # historic build data exists.
 

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-06-08 17:48:08 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-06-09 12:18:28 +0000
@@ -3,6 +3,8 @@
 
 """Test Build features."""
 
+from datetime import datetime, timedelta
+import pytz
 import unittest
 
 from storm.store import Store
@@ -62,6 +64,34 @@
         self.failIfEqual(None, bq.processor)
         self.failUnless(bq, self.build.buildqueue_record)
 
+    def test_estimateDuration(self):
+        # Without previous builds, a negligable package size estimate is 60s
+        self.assertEqual(60, self.build.estimateDuration().seconds)
+
+    def create_previous_build(self, duration):
+        spr = self.build.source_package_release
+        build = spr.createBuild(
+            distro_arch_series=self.build.distro_arch_series,
+            archive=spr.upload_archive, pocket=spr.package_upload.pocket)
+        build.status = BuildStatus.FULLYBUILT
+        now = datetime.now(pytz.UTC)
+        build.date_finished = now
+        build.date_started = now - timedelta(seconds=duration)
+        return build
+
+    def test_estimateDuration_with_history(self):
+        # Previous builds of the same source are used for estimates.
+        self.create_previous_build(335)
+        self.assertEqual(335, self.build.estimateDuration().seconds)
+
+    def test_estimateDuration_with_bad_history(self):
+        # If the latest matching build has bad data, ignore it.
+        # See bug 589068.
+        previous_build = self.create_previous_build(335)
+        previous_build.date_started = None
+        self.assertEqual(60, self.build.estimateDuration().seconds)
+
+
     def addFakeBuildLog(self):
         lfa = self.factory.makeLibraryFileAlias('mybuildlog.txt')
         removeSecurityProxy(self.build).log = lfa