Merge lp:~bigkevmcd/offspring/fix-properties-on-builder-and-project-group into lp:offspring

[Kevin McDermott]

This removes the previously untested code for calculating the metrics for ProjectGroup and Project, and delegates them to the metrics Manager provided on the BuildResult, also adds a formatting filter to output the time, and updates the uses of average_build_time to filter through the time filter.

[lp:~bigkevmcd/offspring/fix-properties-on-builder-and-project-group]

162. By Kevin McDermott on 2011-12-15

Failing test for Lexbuilder.completion_time.

163. By Kevin McDermott on 2011-12-15

Tests for Lexbuilder.completion_time.

164. By Kevin McDermott on 2011-12-15

Failing tests for metrics returning None if there are no matching BuildResults to count.

165. By Kevin McDermott on 2011-12-15

Update the Manager to return a None if there are no results.

166. By Kevin McDermott on 2011-12-15

Failing test for template filter outputting None.

167. By Kevin McDermott on 2011-12-15

Fix the template filter.

168. By Kevin McDermott on 2011-12-15

Failing test for passing through strings in seconds conversion filter.

169. By Kevin McDermott on 2011-12-15

And implement.

170. By Kevin McDermott on 2011-12-15

All tests pass.

171. By Kevin McDermott on 2011-12-15

Update HTML output.

172. By Kevin McDermott on 2011-12-15

Some lint tidyups.

173. By Kevin McDermott on 2011-12-15

Failing test for returning ASAP when completion is over the expected time.

174. By Kevin McDermott on 2011-12-15

Return ASAP if the current elapsed build time is greater than the average.

175. By Kevin McDermott on 2011-12-16

Merge forward from trunk.

176. By Kevin McDermott on 2011-12-16

Ensure we're merged up to date.

177. By Kevin McDermott on 2012-01-12

Merge forward.

178. By Kevin McDermott on 2012-01-16

Merge forward, fix conflicts.

[Cody A.W. Somerville (cody-somerville)]

>>> === modified file 'Makefile'
>>> --- Makefile 2012-01-11 18:22:50 +0000
>>> +++ Makefile 2012-01-16 18:45:54 +0000
>>> @@ -79,7 +79,7 @@
>>> ./bin/offspring-web test offspring.web.queuemanager -s --settings=offspring.web.settings_test
>>>
>>> test-web-postgres: web web-test install-test-runner
>>> - ./bin/offspring-web test offspring.web.queuemanager -s --settings=offspring.web.settings_devel
>>> + ./bin/offspring-web test offspring.web.queuemanager --settings=offspring.web.settings_devel
>>>
>>> test: master slave web install-test-runner test-web master-test
>>> ./.virtualenv/bin/nosetests -e 'queuemanager.*'
>>>

Why is this change required?

<snip>
>>> === modified file 'lib/offspring/web/queuemanager/models.py'
>>> --- lib/offspring/web/queuemanager/models.py 2012-01-16 17:28:38 +0000
>>> +++ lib/offspring/web/queuemanager/models.py 2012-01-16 18:45:54 +0000
<snip>
>>> @@ -357,24 +322,16 @@
>>> @property
>>> def estimated_completion(self):
>>> if self.current_job is not None:
>>> - #XXX: Duplication of code in Project to get average build time
>>> - cursor = connection.cursor()
>>> - cursor.execute("SELECT SUM(finished_at - started_at) / count(*) from buildresults WHERE project_name = %s AND result = %s", [self.current_job.project.name, "COMPLETED"])
>>> - row = cursor.fetchone()
>>> - if row[0] is not None:
>>> - averageBuildTime = row[0]
>>> - currentElapsedTime = datetime.now() - self.current_job.started_at
>>> - estimatedTimeRemaining = averageBuildTime - currentElapsedTime
>>> - #XXX: Duplication of code in Project to format output
>>> - result = str(estimatedTimeRemaining)
>>> - if result.startswith("-"):
>>> + average_build_time = BuildResult.metrics.get_average_build_time(
>>> + project=self.current_job.project)

You should get the average_build_time via the average_build_time property on self.current_job.project.

>>> + if average_build_time is not None:
>>> + elapsed_time = (datetime.now() - self.current_job.started_at)
>>> + estimated_completion =(average_build_time -
>>> + elapsed_time.seconds)
>>> + if estimated_completion < 0.0:
>>> return "ASAP"
>>> else:
>>> - hours, minutes, seconds = result.split(":")
>>> - if int(hours) < 1:
>>> - return "%s mins, %s secs" % (minutes, math.floor(float(seconds)))
>>> - else:
>>> - return "%s hrs, %s mins, %s secs" % (hours, minutes, math.floor(float(seconds)))
>>> + return estimated_completion
>>> else:
>>> return "Unknown"
>>> else:
>>>

API wise, I think we should return a datetime object of when we expect the build to complete - 'self.current_job.started_at + datetime.timedelta(seconds=self.current_job.project.average_build_time)'. Then we can just use the Django template filter 'timeuntil' inst...

review: Needs Fixing

[Kevin McDermott (bigkevmcd)]

>
> Why is this change required?

I think this was a mismerge, I've been merging forward as branches have landed.

> You should get the average_build_time via the average_build_time property on
> self.current_job.project.

Good idea, fixed.

>
> API wise, I think we should return a datetime object of when we expect the
> build to complete - 'self.current_job.started_at +
> datetime.timedelta(seconds=self.current_job.project.average_build_time)'. Then
> we can just use the Django template filter 'timeuntil' instead of writing and
> maintaining our own filter.

This would substantially change the current behaviour, also, timeuntil would trigger an error for the Unknown and ASAP cases...which isn't very good...

> This docstring needs to be reworded.
Done

>
> I think a better name might be
> 'test_get_average_build_time_of_automated_builds_only'.

Changed...

> Instead of having magic numbers, you should calculate the average of the list
> you pass to create_build_results_with_durations and make 60.0 a constant.
Good idea, create_build_results/create_build_durations both now return the average, am not sure that SECONDS_IN_MINUTE is any clearer than 60?

>
> On a small tangent, I question the value of some of these tests. For example,
> you have a similar battery of tests for average build time as you do for
> average queue time. You had to make the same changes in both sets of tests
> because of a change you made to a function get_average_for_timeperiods that
> both code pathways use. Would it not be better to have tests for the behaviour
> of get_average_for_timeperiods /
> BuildResultMetricsManager._get_average_time_for_query and tests for the
> specific behaviours of BuildResultMetricsManager.get_average_build_time and
> BuildResultMetricsManager.get_average_queue_time? - that way you don't end up
> basically doing the same tests twice.>

So, there isn't actually as much repetition of the tests as it appears, also, the tests for BuldResultsMetricsManager._get_average_time_for_query would need to operate on some model, they might as well operate on a working one...

Also, given that both calculations use different query parameters, they do need to be tested.

> This appears to be a syntax error.

Oops...fixed.

thanks for the review

> Thank you for your work on this!!

thanks for the attention to detail.

Unmerged revisions

=== modified file 'Makefile'
--- Makefile	2012-01-11 18:22:50 +0000
+++ Makefile	2012-01-16 18:45:54 +0000
@@ -79,7 +79,7 @@
 	./bin/offspring-web test offspring.web.queuemanager -s --settings=offspring.web.settings_test
 
 test-web-postgres: web web-test install-test-runner
-	./bin/offspring-web test offspring.web.queuemanager -s --settings=offspring.web.settings_devel
+	./bin/offspring-web test offspring.web.queuemanager --settings=offspring.web.settings_devel
 
 test: master slave web install-test-runner test-web master-test
 	./.virtualenv/bin/nosetests -e 'queuemanager.*'

=== modified file 'lib/offspring/web/queuemanager/managers.py'
--- lib/offspring/web/queuemanager/managers.py	2011-12-15 08:50:53 +0000
+++ lib/offspring/web/queuemanager/managers.py	2012-01-16 18:45:54 +0000
@@ -16,7 +16,8 @@
 
 def get_average_for_timeperiods(queryset, columns):
     """
-    Return the average time between two columns in a QuerySet.A
+    Return the average time between two columns in a QuerySet, or None if there
+    are no matching results.
 
     queryset: A QuerySet of BuildResults to be averaged.
     columns: A sequence of two column names to be extracted, this must be
@@ -24,7 +25,7 @@
     """
     time_stamps = queryset.values_list(*columns)
     if not time_stamps:
-        return 0.0
+        return None
 
     times = [delta_to_seconds(end_time - start_time)
         for (start_time, end_time) in time_stamps]

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2012-01-16 17:28:38 +0000
+++ lib/offspring/web/queuemanager/models.py	2012-01-16 18:45:54 +0000
@@ -1,7 +1,6 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-import math
 from datetime import date, datetime, timedelta
 
 from django.contrib.auth.models import AnonymousUser, User
@@ -93,24 +92,7 @@
 
     @property
     def average_build_time(self):
-        cursor = connection.cursor()
-        cursor.execute("SELECT SUM(finished_at - started_at) / count(*) from buildresults br, projects WHERE br.project_name = projects.name AND projects.project_group_id = %s AND br.result = %s", [self.name, "COMPLETED"])
-        row = cursor.fetchone()
-        if row[0] is not None:
-            result = str(row[0])
-            hours, minutes, seconds = result.split(":")
-            #XXX: The following can't handle builds that have an average 
-            #XXX: duration of greater then 23 hours and 59 minutes and 59 
-            #XXX: seconds (e.g. if a build gets stuck and lasts over a day).
-            try:
-                if int(hours) < 1:
-                    return "%s mins, %s secs" % (minutes, math.floor(float(seconds)))
-                else:
-                    return "%s hrs, %s mins, %s secs" % (hours, minutes, math.floor(float(seconds)))
-            except ValueError:
-                #XXX: We'll potentially incorrectly report >1 day until we fix above.
-                return ">1 Day"
-        return "UNKNOWN"
+        return BuildResult.metrics.get_average_build_time(project_group=self)
 
 
 class PublicOnlyObjectsManager(models.Manager):
@@ -265,7 +247,7 @@
     def display_name(self):
         return "%s (%s)" % (self.title, self.name)
 
-    def builder(self): 
+    def builder(self):
         return Lexbuilder.objects.filter(current_job__project = self)[0:1].get()
 
     @property
@@ -274,24 +256,7 @@
 
     @property
     def average_build_time(self):
-        cursor = connection.cursor()
-        cursor.execute("SELECT SUM(finished_at - started_at) / count(*) from buildresults WHERE project_name = %s AND result = %s", [self.name, "COMPLETED"])
-        row = cursor.fetchone()
-        if row[0] is not None:
-            result = str(row[0])
-            hours, minutes, seconds = result.split(":")
-            #XXX: The following can't handle builds that have an average 
-            #XXX: duration of greater then 23 hours and 59 minutes and 59 
-            #XXX: seconds (e.g. if a build gets stuck and lasts over a day).
-            try:
-                if int(hours) < 1:
-                    return "%s mins, %s secs" % (minutes, math.floor(float(seconds)))
-                else:
-                    return "%s hrs, %s mins, %s secs" % (hours, minutes, math.floor(float(seconds)))
-            except ValueError:
-                #XXX: We'll potentially incorrectly report >1 day until we fix above.
-                return ">1 Day"
-        return "UNKNOWN"
+        return BuildResult.metrics.get_average_build_time(project=self)
 
     def latest_build(self):
         try:
@@ -357,24 +322,16 @@
     @property
     def estimated_completion(self):
         if self.current_job is not None:
-            #XXX: Duplication of code in Project to get average build time
-            cursor = connection.cursor()
-            cursor.execute("SELECT SUM(finished_at - started_at) / count(*) from buildresults WHERE project_name = %s AND result = %s", [self.current_job.project.name, "COMPLETED"])
-            row = cursor.fetchone()
-            if row[0] is not None:
-                averageBuildTime = row[0]
-                currentElapsedTime = datetime.now() - self.current_job.started_at
-                estimatedTimeRemaining = averageBuildTime - currentElapsedTime
-                #XXX: Duplication of code in Project to format output
-                result = str(estimatedTimeRemaining)
-                if result.startswith("-"):
+            average_build_time = BuildResult.metrics.get_average_build_time(
+                project=self.current_job.project)
+            if average_build_time is not None:
+                elapsed_time = (datetime.now() - self.current_job.started_at)
+                estimated_completion =(average_build_time -
+                                       elapsed_time.seconds)
+                if estimated_completion < 0.0:
                     return "ASAP"
                 else:
-                    hours, minutes, seconds = result.split(":")
-                    if int(hours) < 1:
-                        return "%s mins, %s secs" % (minutes, math.floor(float(seconds)))
-                    else:
-                        return "%s hrs, %s mins, %s secs" % (hours, minutes, math.floor(float(seconds)))
+                    return estimated_completion
             else:
                 return "Unknown"
         else:

=== added file 'lib/offspring/web/queuemanager/templatetags/format_seconds.py'
--- lib/offspring/web/queuemanager/templatetags/format_seconds.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/templatetags/format_seconds.py	2012-01-16 18:45:54 +0000
@@ -0,0 +1,40 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Time in seconds formatting."""
+
+from django.template import Library
+
+register = Library()
+
+ONE_MINUTE = 60
+ONE_HOUR = 60 * ONE_MINUTE
+ONE_DAY = 24 * ONE_HOUR
+
+
[e-mail]
+def seconds_to_time(value):
+    """
+    Converts seconds to a time representation.
+
+    If value is None, return "Unknown"
+    """
+    if value is None:
+        return "Unknown"
+
+    try:
+        value = float(value)
+        days, remainder = divmod(value, ONE_DAY)
+        hours, remainder = divmod(remainder, ONE_HOUR)
+        minutes, seconds = divmod(remainder, ONE_MINUTE)
+
+        times = [(days, "days"), (hours, "hours"), (minutes, "minutes"),
+                 (seconds, "seconds")]
+
+        periods = ["%d %s" % (time, period) for time, period in times
+                   if time > 0]
+
+        return ", ".join(periods)
+    except ValueError:
+        return value
+
+seconds_to_time.is_safe = True

=== added directory 'lib/offspring/web/queuemanager/templatetags/tests'
=== added file 'lib/offspring/web/queuemanager/templatetags/tests/__init__.py'
=== added file 'lib/offspring/web/queuemanager/templatetags/tests/test_format_seconds.py'
--- lib/offspring/web/queuemanager/templatetags/tests/test_format_seconds.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/templatetags/tests/test_format_seconds.py	2012-01-16 18:45:54 +0000
@@ -0,0 +1,66 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from django.template import Template, Context
+from django.test import TestCase
+
+from offspring.web.queuemanager.templatetags.format_seconds import (
+    seconds_to_time)
+
+
+class TestSecondsToTimeFilter(TestCase):
+
+    def test_seconds_to_time_seconds(self):
+        """
+        seconds_to_time should render the number of seconds accurately.
+        """
+        self.assertEqual("35 seconds", seconds_to_time(35))
+
+    def test_seconds_to_time_with_none(self):
+        """
+        seconds_to_time should return Unknown if the time period is None.
+        """
+        self.assertEqual("Unknown", seconds_to_time(None))
+
+    def test_seconds_to_time_with_string(self):
+        """
+        seconds_to_time should pass strings that can't be converted to floating
+        point numbers unchanged.
+        """
+        self.assertEqual("Unknown", seconds_to_time("Unknown"))
+
+    def test_seconds_to_time_minutes(self):
+        """
+        seconds_to_time should render the number of minutes accurately.
+        """
+        self.assertEqual("10 minutes", seconds_to_time(600))
+
+    def test_seconds_to_time_hours(self):
+        """
+        seconds_to_time should render the number of hours accurately.
+        """
+        self.assertEqual("2 hours", seconds_to_time(7200))
+
+    def test_seconds_to_time_days(self):
+        """
+        seconds_to_time should render the number of hours accurately.
+        """
+        self.assertEqual("3 days", seconds_to_time(72 * 60 * 60))
+
+    def test_seconds_to_time_period(self):
+        """
+        seconds_to_time should render the time period accurately.
+        """
+        # 2 days, 2 hours, 10 minutes, 35 seconds
+        seconds = (2 * 24 * 60 * 60) + 7200 + 600 + 35
+        self.assertEqual("2 days, 2 hours, 10 minutes, 35 seconds",
+                         seconds_to_time(seconds))
+
+    def test_seconds_to_time_filter_in_template(self):
+        """
+        The seconds_to_time filter converts an integer value for a number of
+        seconds to a human readable duration.
+        """
+        rendered = Template("{% load format_seconds %}"
+                            "{{ 600|seconds_to_time }}").render(Context())
+        self.assertTrue("10 minutes" in rendered, "Time incorrectly rendered.")

=== modified file 'lib/offspring/web/queuemanager/tests/test_managers.py'
--- lib/offspring/web/queuemanager/tests/test_managers.py	2012-01-12 08:29:41 +0000
+++ lib/offspring/web/queuemanager/tests/test_managers.py	2012-01-16 18:45:54 +0000
@@ -43,27 +43,27 @@
         create_build_results_with_durations(
             self.project, [10, 20], result=ProjectBuildStates.PENDING)
 
-        self.assertEqual(0.0,
+        self.assertEqual(None,
                          BuildResult.metrics.get_average_build_time(
                              self.project))
 
     def test_get_average_build_time_no_builds(self):
         """
-        If there are no builds, we should get an average of 0.0.
+        If there are no builds, we should get an average of None.
         """
-        self.assertEqual(0.0,
+        self.assertEqual(None,
                          BuildResult.metrics.get_average_build_time(
                              self.project))
 
     def test_get_average_build_time_unfinished_build(self):
         """
-        If a build has started, but not yet finished, this we should get 0.0
+        If a build has started, but not yet finished, this we should get None,
         for the average for this entry.
         """
         started_at = datetime.utcnow()
         build_result = factory.make_build_result(project=self.project,
                                                  started_at=started_at)
-        self.assertEqual(0.0,
+        self.assertEqual(None,
                          BuildResult.metrics.get_average_build_time(
                              self.project))
 
@@ -114,22 +114,29 @@
         self.assertEqual(45.0 * 60,
                          BuildResult.metrics.get_average_build_time())
 
-    def test_get_average_build_time_with_automated_flag(self):
+    def test_get_average_build_time_with_automated_builds(self):
         """
         Calling get_average_build_time with automated=True should filter the
         results to only include automated builds, which are defined as builds
+        without a requestor.
+        """
+        create_build_results_with_durations(self.project, [10, 20])
+        self.assertEqual(
+            15 * 60.0,
+            BuildResult.metrics.get_average_build_time(automated=True))
+
+    def test_get_average_build_time_with_non_automated_builds(self):
+        """
+        Calling get_average_build_time with automated=False should filter the
+        results to only include manual builds, which are defined as builds
         with a requestor.
         """
         user = factory.make_user()
-        create_build_results_with_durations(self.project, [10, 20])
         create_build_results_with_durations(
             factory.make_project(), [50, 100], requestor=user)
-        self.assertEqual(15 * 60.0,
-                         BuildResult.metrics.get_average_build_time(
-                             automated=True))
-        self.assertEqual(75 * 60.0,
-                         BuildResult.metrics.get_average_build_time(
-                             automated=False))
+        self.assertEqual(
+            75 * 60.0,
+            BuildResult.metrics.get_average_build_time(automated=False))
 
     def test_get_average_build_time_filters_by_date(self):
         """
@@ -146,7 +153,7 @@
         # From the day after we create our builds 'til today.
         period = (build_date + timedelta(days=1), datetime.utcnow())
         self.assertEqual(
-            0.0,
+            None,
             BuildResult.metrics.get_average_build_time(period=period))
 
     def test_get_average_build_time_includes_by_date(self):
@@ -207,10 +214,10 @@
 
     def test_average_queue_time_no_builds(self):
         """
-        If there are no builds, we should get an average of 0.0.
+        If there are no builds, we should get an average of None.
         """
         self.assertEqual(
-            0.0,
+            None,
             BuildResult.metrics.get_average_queue_time(self.project))
 
     def test_average_queue_time_includes_by_date(self):
@@ -266,22 +273,27 @@
         self.assertEqual(45.0 * 60,
                          BuildResult.metrics.get_average_queue_time())
 
-    def test_get_average_queue_time_with_automated_flag(self):
+    def test_get_average_queue_time_with_automated_builds(self):
         """
         Calling get_average_queue_time with automated=True should filter the
         results to only include automated builds, which are defined as builds
+        without a requestor.
+        """
+        create_build_results_with_queue_times(self.project, [10, 20])
+        self.assertEqual(
+            15 * 60.0,
+            BuildResult.metrics.get_average_queue_time(automated=True))
+
+    def test_get_average_queue_time_with_non_automated_builds(self):
+        """
+        Calling get_average_queue_time with automated=False should filter the
+        results to only include manual builds, which are defined as builds
         with a requestor.
         """
-        user = factory.make_user()
         create_build_results_with_queue_times(self.project, [10, 20])
-        create_build_results_with_queue_times(
-            factory.make_project(), [50, 100], requestor=user)
         self.assertEqual(
             15 * 60.0,
             BuildResult.metrics.get_average_queue_time(automated=True))
-        self.assertEqual(
-            75 * 60.0,
-            BuildResult.metrics.get_average_queue_time(automated=False))
 
     def test_get_average_queue_time_filters_by_date(self):
         """
@@ -298,7 +310,7 @@
         # From the day after we create our builds 'til today.
         period = (build_date + timedelta(days=1), datetime.utcnow())
         self.assertEqual(
-            0.0,
+            None,
             BuildResult.metrics.get_average_queue_time(period=period))
 
     def test_get_average_queue_time_includes_by_date(self):
@@ -318,7 +330,7 @@
         # From the day after we create our builds 'til today.
         period = (build_date + timedelta(days=1), datetime.utcnow())
         self.assertEqual(
-            75 * 60,
+            75.0 * 60,
             BuildResult.metrics.get_average_queue_time(period=period))
 
     def test_get_average_queue_time_filters_by_date_and_automated(self):

=== modified file 'lib/offspring/web/queuemanager/tests/test_models.py'
--- lib/offspring/web/queuemanager/tests/test_models.py	2012-01-16 17:28:38 +0000
+++ lib/offspring/web/queuemanager/tests/test_models.py	2012-01-16 18:45:54 +0000
@@ -1,4 +1,7 @@
-from datetime import datetime
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+from datetime import datetime, timedelta
+
 from operator import attrgetter
 
 from django.contrib.auth.models import AnonymousUser
@@ -7,7 +10,11 @@
 from offspring.enums import ProjectBuildStates
 from offspring.web.queuemanager.models import (
     BuildRequest, BuildResult, Lexbuilder, Project, Release)
+
 from offspring.web.queuemanager.tests.factory import factory
+from offspring.web.queuemanager.tests.helpers import (
+    create_build_results_with_durations)
+from mocker import MockerTestCase
 
 
 class ProjectTests(TestCase):
@@ -72,7 +79,7 @@
         Check that the .objects model manager return only public projects.
         """
         project = factory.make_project(is_private=False)
-        project2 = factory.make_project(is_private=True)
+        factory.make_project(is_private=True)
         self.assertEqual([project], list(Project.objects.all()))
 
     def test_all_objects_model_manager(self):
@@ -127,6 +134,15 @@
         objects = Project.all_objects.accessible_by_user(user)
         self.assertEqual([project], list(objects))
 
+    def test_average_build_time(self):
+        """
+        ProjectGroup.average_build_time should use the BuildResult metric.
+        """
+        project = factory.make_project()
+        create_build_results_with_durations(
+            project, [50, 50], result=ProjectBuildStates.SUCCESS)
+        self.assertEqual(50 * 60.0, project.average_build_time)
+
 
 class ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin(object):
     """A mixin with tests that work for the following models:
@@ -213,8 +229,7 @@
         """
         Check that the .objects model manager return only public objects.
         """
-        private_object = self.factoryMethod(
-            factory.make_project(is_private=True))
+        self.factoryMethod(factory.make_project(is_private=True))
         public_object = self.factoryMethod(
             factory.make_project(is_private=False))
         self.assertEqual([public_object], list(self.model.objects.all()))
@@ -246,8 +261,7 @@
             factory.make_project(is_private=True, access_groups=[group]))
         # This object is linked to a private project which has no access
         # groups set up, so the user cannot see it.
-        invisible_object = self.factoryMethod(
-            project=factory.make_project(is_private=True))
+        self.factoryMethod(project=factory.make_project(is_private=True))
         self.assertEqual(
             [visible_object],
             list(self.model.all_objects.accessible_by_user(user)))
@@ -263,13 +277,97 @@
 
 
 class LexbuilderTests(
-        TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+        TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin,
+        MockerTestCase):
     model = Lexbuilder
 
     def factoryMethod(self, project):
         return factory.make_lexbuilder(
             current_job=factory.make_build_result(project=project))
 
+    def test_estimated_completion(self):
+        """
+        Lexbuilder.estimated_completion_time should be calculated as:
+            average build time - (current time - start time)
+        """
+        project = factory.make_project()
+
+        create_build_results_with_durations(
+            project=project, durations=[25, 25],
+            result=ProjectBuildStates.SUCCESS)
+
+        build_result = factory.make_build_result(
+            project=project, result=ProjectBuildStates.PENDING)
+        started_at = datetime.now() - timedelta(minutes=10)
+        build_result.started_at = started_at
+        build_result.save()
+
+        builder = Lexbuilder.objects.create(current_job=build_result)
+
+        datetime_mock = self.mocker.replace(datetime)
+        datetime_mock.now()
+        self.mocker.result(started_at + timedelta(minutes=5))
+        self.mocker.replay()
+
+        # Average time = 25 minutes
+        # Started 5 minutes ago (started_at + timedelta(minutes=5))
+        # 20 minutes remaining
+
+        self.assertEqual(20 * 60, builder.estimated_completion)
+
+    def test_estimated_completion_with_no_average_time(self):
+        """
+        If the average cannot be determined because there are no builds, we
+        should get "Unknown" as the estimated completion time.
+        """
+        project = factory.make_project()
+
+        build_result = factory.make_build_result(
+            project=project, result=ProjectBuildStates.PENDING)
+        started_at = datetime.now() - timedelta(minutes=10)
+        build_result.started_at = started_at
+        build_result.save()
+
+        builder = Lexbuilder.objects.create(current_job=build_result)
+
+        self.assertEqual("Unknown", builder.estimated_completion)
+
+    def test_estimated_completion_with_no_current_build(self):
+        """
+        If the average cannot be determined because there are no builds, we
+        should get "Unknown" as the estimated completion time.
+        """
+        builder = Lexbuilder.objects.create()
+        self.assertEqual("No build in progress", builder.estimated_completion)
+
+    def test_estimated_completion_negative_time(self):
+        """
+        If the current elapsed time is higher than the average, we should get
+        "ASAP" as the estimated completion.
+        """
+        project = factory.make_project()
+
+        create_build_results_with_durations(
+            project=project, durations=[5, 5],
+            result=ProjectBuildStates.SUCCESS)
+
+        build_result = factory.make_build_result(
+            project=project, result=ProjectBuildStates.PENDING)
+        started_at = datetime.now() - timedelta(minutes=10)
+        build_result.started_at = started_at
+        build_result.save()
+
+        builder = Lexbuilder.objects.create(current_job=build_result)
+
+        datetime_mock = self.mocker.replace(datetime)
+        datetime_mock.now()
+        self.mocker.result(started_at + timedelta(minutes=10))
+        self.mocker.replay()
+
+        # Average time = 5 minutes
+        # Started 10 minutes ago (started_at + timedelta(minutes=10))
+        self.assertEqual("ASAP", builder.estimated_completion)
+
     def test_lexbuilder_without_current_job(self):
         """
         A Lexbuilder with no current_job is not considered private and thus
@@ -370,9 +468,18 @@
             is_private=False, project_group=group)
         private_project_visible_to_user = factory.make_project(
             is_private=True, project_group=group, owner=user)
-        private_project = factory.make_project(
-            is_private=True, project_group=group)
+        factory.make_project(is_private=True, project_group=group)
         self.assertEqual(
             sorted([public_project, private_project_visible_to_user],
                    key=attrgetter("name")),
             sorted(group.get_projects(user), key=attrgetter("name")))
+
+    def test_average_build_time(self):
+        """
+        ProjectGroup.average_build_time should use the BuildResult metric.
+        """
+        project_group = factory.make_project_group()
+        project = factory.make_project(project_group=project_group)
+        create_build_results_with_durations(
+            project, [10, 20], result=ProjectBuildStates.SUCCESS)
+        self.assertEqual(15 * 60.0, project_group.average_build_time)

=== modified file 'lib/offspring/web/templates/queuemanager/builder_details.html'
--- lib/offspring/web/templates/queuemanager/builder_details.html	2011-12-02 17:04:03 +0000
+++ lib/offspring/web/templates/queuemanager/builder_details.html	2012-01-16 18:45:54 +0000
@@ -1,4 +1,5 @@
 {% extends "base.html" %}
+{% load format_seconds %}
 
 {% block header_css %}
 <link rel="stylesheet" type="text/css" href="/assets/css/multi-column-lists.css"/>
@@ -65,7 +66,7 @@
 
 <dl>
     <dt>Estimated Completion:</dt>
-    <dd>{{builder.estimated_completion}}</dd>
+    <dd>{{builder.estimated_completion|seconds_to_time}}</dd>
 </dl>
 
 <dl style="clear: both;">

=== modified file 'lib/offspring/web/templates/queuemanager/builders.html'
--- lib/offspring/web/templates/queuemanager/builders.html	2011-12-02 15:44:50 +0000
+++ lib/offspring/web/templates/queuemanager/builders.html	2012-01-16 18:45:54 +0000
@@ -1,5 +1,7 @@
 {% extends "base.html" %}
 {% load lexbuilder_helpers %}
+{% load format_seconds %}
+
 
 {% block title %}
 Build Farm
@@ -25,7 +27,8 @@
                 {{builder.machine_type}}
             </td>
             <td align="center">
-                <span title="{{builder.name}} is currently {{builder.current_state}}{% ifequal builder.current_state "BUILDING" %}, estimated completion {{builder.estimated_completion}}{% endifequal %}">{{builder.current_state}}</span>
+                <span title="{{builder.name}} is currently
+{{builder.current_state}}{% ifequal builder.current_state "BUILDING" %}, estimated completion {{builder.estimated_completion|seconds_to_time}}{% endifequal %}">{{builder.current_state}}</span>
             </td>
             <td align="center"> 
             {% if not build_visible %}

=== modified file 'lib/offspring/web/templates/queuemanager/project_details.html'
--- lib/offspring/web/templates/queuemanager/project_details.html	2011-12-02 14:23:30 +0000
+++ lib/offspring/web/templates/queuemanager/project_details.html	2012-01-16 18:45:54 +0000
@@ -1,5 +1,6 @@
 {% extends "base.html" %}
 {% load lexbuilder_helpers %}
+{% load format_seconds %}
 
 {% block header_css %}
 <link rel="stylesheet" type="text/css" href="/assets/css/project_details.css"/>
@@ -832,7 +833,7 @@
 
 isc.Label.create({
     ID: "projectBuildHistorySummary",
-    contents: "<i>{{ project_build_results|length }} builds; {{ build_stats.success_count|default:"0" }} successful and {{ build_stats.fail_count|default:"0" }} failed. Average build time: {{ project.average_build_time }}</i>",
+    contents: "<i>{{ project_build_results|length }} builds; {{ build_stats.success_count|default:"0" }} successful and {{ build_stats.fail_count|default:"0" }} failed. Average build time:{{ project.average_build_time|seconds_to_time }}</i>",
     position: null,
     autoDraw: false,
     height: 20,

=== modified file 'lib/offspring/web/templates/queuemanager/projectgroup_details.html'
--- lib/offspring/web/templates/queuemanager/projectgroup_details.html	2011-11-18 15:45:06 +0000
+++ lib/offspring/web/templates/queuemanager/projectgroup_details.html	2012-01-16 18:45:54 +0000
@@ -1,5 +1,6 @@
 {% extends "base.html" %}
 {% load lexbuilder_helpers %}
+{% load format_seconds %}
 
 {% block header %}
 <link rel="stylesheet" type="text/css" href="/assets/css/project_details.css"/>
@@ -15,7 +16,7 @@
     <tr><th><h2>General Information</h2></th><th><h2>Build Information</h2></th></tr>
 </table>
 <table width="60%" style="table-layout: fixed; float: right;text-align: right">
-    <tr><td width="40%"><b>Average Build:</b></td><td>{{ projectGroup.average_build_time }}</td></tr>
+    <tr><td width="40%"><b>Average Build:</b></td><td>{{projectGroup.average_build_time|seconds_to_time }}</td></tr>
     <tr><td><b>Build Count:</b></td><td>{{ projectgroup_build_results|length }}</td></tr>
     <tr><td><b>Build Results:</b></td><td>{{ build_stats.success_count|default:"0" }} successful build{{ build_stats.success_count|pluralize }} and {{ build_stats.fail_count|default:"0" }} failed. </td></tr>
 </table>

=== modified file 'setup_test_database.sh'
--- setup_test_database.sh	2011-11-30 08:19:58 +0000
+++ setup_test_database.sh	2012-01-16 18:45:54 +0000
@@ -4,6 +4,7 @@
   CREATE ROLE offspring_test
   CREATEROLE INHERIT LOGIN
   PASSWORD 'temp1234'
+  WITH CREATEDB
 END
 
 sudo su postgres -c psql <<END