Merge lp:~bigkevmcd/offspring/build-stats-for-project-group into lp:offspring

[Kevin McDermott]

This adds functionality allowing the metrics to be gathered per project group.

[lp:~bigkevmcd/offspring/build-stats-for-project-group]

132. By Kevin McDermott on 2011-12-14

Add failing test for get_average_queue_time with a ProjectGroup.

133. By Kevin McDermott on 2011-12-14

And add it to the get_average_build_time code.

134. By Kevin McDermott on 2011-12-14

Remove extra character from docstring.

135. By Kevin McDermott on 2011-12-14

Merge forward.

136. By Kevin McDermott on 2011-12-14

Merge forward.

137. By Kevin McDermott on 2011-12-15

Merge forward.

138. By Kevin McDermott on 2011-12-16

Merge forward.

[Guilherme Salgado (salgado)]

You don't seem to use seconds_into_time() anywhere, so I'd just remove it and re-add in whichever branch needs it, if there is one.

I'd have made get_average_time() raise an error if the client passes both a project and a project_group, but given that the docstring states what will happen in that case it's no big deal, so go with whichever you prefer

Here you also use the factory to create a build result and right after it you set its started_at/finished_at attributes, which can now be removed as the factory method should take those attributes as arguments (from the fix you did in your previous branch ;)

The docstring of test_get_average_build_time_with_excludes_projects_not_in_group(self) is identical to that of the previous test method:

140 + """
141 + All Projects in a ProjectGroup should be included in the average
142 + buildtime.
143 + """

review: Needs Fixing

[lp:~bigkevmcd/offspring/build-stats-for-project-group]

139. By Kevin McDermott on 2012-01-11

Address Salgado's review points.

140. By Kevin McDermott on 2012-01-12

Merge forward.

[Kevin McDermott (bigkevmcd)]

> You don't seem to use seconds_into_time() anywhere, so I'd just remove it and
> re-add in whichever branch needs it, if there is one.

Yeah, I was going to use it for the output, but I decided against it :-)

>
> I'd have made get_average_time() raise an error if the client passes both a
> project and a project_group, but given that the docstring states what will
> happen in that case it's no big deal, so go with whichever you prefer

This seems reasonable, lemme do this in a tiny separate branch, merging forward is proving troublesome...

> Here you also use the factory to create a build result and right after it you
> set its started_at/finished_at attributes, which can now be removed as the
> factory method should take those attributes as arguments (from the fix you did
> in your previous branch ;)

Fixed!

> The docstring of
> test_get_average_build_time_with_excludes_projects_not_in_group(self) is
> identical to that of the previous test method:

Fixed!

Thanks for the review.

=== modified file 'lib/offspring/web/queuemanager/metrics.py'
--- lib/offspring/web/queuemanager/metrics.py	2012-01-12 08:35:32 +0000
+++ lib/offspring/web/queuemanager/metrics.py	2012-01-12 08:35:32 +0000
@@ -4,7 +4,8 @@
 from offspring.enums import ProjectBuildStates
 
 
-def get_average_time(query_params, columns, project=None, automated=None):
+def get_average_time(query_params, columns, project=None, automated=None,
+                     project_group=None):
 
     """
     Return the average time between the two named columns.
@@ -19,10 +20,14 @@
 
     project: A Project to filter BuildResults with, if none supplied, will use
         all Projects
+    project_group: A ProjectGroup to filter BuildResults with, if supplied,
+        this will average all Projects within the ProjectGroup.
     automated: If not None, filter BuildResults using the requestor, if True,
         only BuildResults with no requestor will be used, otherwise
         BuildResults with the requestor field populated will be used.
     period: A tuple of datetime objects (start_time, end_time)
+
+    NOTE: If project and project_group are supplied, only project will be used.
     """
     # FIXME: This should really be done using PostgreSQL queries, but the tests
     # don't run using PostgreSQL...
@@ -37,6 +42,9 @@
                 * 10 ** 6) / 10 ** 6)
     if project is not None:
         query_params["project"] = project
+    else:
+        if project_group is not None:
+            query_params["project__project_group"] = project_group
 
     if automated is not None:
         query_params["requestor__isnull"] = automated
@@ -49,7 +57,8 @@
     return float(sum(times)) / len(time_stamps)
 
 
-def get_average_build_time(project=None, automated=None, period=None):
+def get_average_build_time(project=None, automated=None, period=None,
+                           project_group=None):
     """
     Return the average time taken between BuildResult.started_at and
     BuildResult.finished_at in seconds for all BuildResults associated with the
@@ -64,10 +73,12 @@
     if period is not None:
         query_params["finished_at__range"] = period
     return get_average_time(query_params, ("started_at", "finished_at"),
-                            project=project, automated=automated)
-
-
-def get_average_queue_time(project=None, automated=None, period=None):
+                            project=project, automated=automated,
+                            project_group=project_group)
+
+
+def get_average_queue_time(project=None, automated=None, period=None,
+                           project_group=None):
     """
     Return the average time taken between BuildResult.requested_at and
     BuildResult.dispatched_at in seconds for all BuildResults associated with the
@@ -81,7 +92,8 @@
     if period is not None:
         query_params["dispatched_at__range"] = period
     return get_average_time(query_params, ("requested_at", "dispatched_at"),
-                            project=project, automated=automated)
+                            project=project, automated=automated,
+                            project_group=project_group)
 
 
 def get_build_result_metrics():

=== modified file 'lib/offspring/web/queuemanager/tests/test_metrics.py'
--- lib/offspring/web/queuemanager/tests/test_metrics.py	2012-01-12 08:35:32 +0000
+++ lib/offspring/web/queuemanager/tests/test_metrics.py	2012-01-12 08:35:32 +0000
@@ -315,3 +315,61 @@
              ("Average requested queue time (30 days)", 23.25*60),
              ("Average requested queue time (90 days)", 25.5*60)],
             get_build_result_metrics())
+
+
+class ProjectGroupBuildMetricsTests(TestCase):
+
+    def setUp(self):
+        self.project_group = factory.make_project_group()
+        self.project = factory.make_project(project_group=self.project_group)
+
+    def test_get_average_build_time_with_project_group(self):
+        """
+        get_average_build_time should take a project group, and calculate the
+        average build time of all projects within that group.
+        """
+        build_result = factory.make_build_result(
+            project=self.project, result=ProjectBuildStates.SUCCESS)
+        finished_at = datetime.now()
+        build_result.finished_at = finished_at
+        build_result.started_at = finished_at - timedelta(minutes=10)
+        build_result.save()
+        self.assertEqual(
+            10*60,
+            get_average_build_time(project_group=self.project_group))
+
+    def test_get_average_build_time_with_multiple_projects_in_group(self):
+        """
+        All Projects in a ProjectGroup should be included in the average
+        buildtime.
+        """
+        create_build_results_with_durations(self.project, [10, 20])
+        project2 = factory.make_project(project_group=self.project_group)
+        create_build_results_with_durations(project2, [20, 30])
+        # 10 + 20 + 20 + 30 / 4 = 20 minutes
+        self.assertEqual(
+            20*60,
+            get_average_build_time(project_group=self.project_group))
+
+    def test_get_average_build_time_with_excludes_projects_not_in_group(self):
+        """
+        Only Projects in a ProjectGroup should be included in the average
+        buildtime.
+        """
+        create_build_results_with_durations(self.project, [10, 20])
+        project2 = factory.make_project()
+        create_build_results_with_durations(project2, [20, 30])
+        # 10 + 20  / 2 = 15 minutes
+        self.assertEqual(
+            15*60,
+            get_average_build_time(project_group=self.project_group))
+
+    def test_get_average_queue_time_with_project_group(self):
+        """
+        get_average_queue_time should take a project group, and calculate the
+        average queue time of all projects within that group.
+        """
+        create_build_results_with_queue_times(self.project, [10, 20])
+        self.assertEqual(
+            15*60,
+            get_average_queue_time(project_group=self.project_group))