launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/builders-timeout-bug-887078-use-getSpecificJobs into lp:launchpad with lp:~rvb/launchpad/builders-timeout-bug-887078 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #887078 in Launchpad itself: "Builder:+history timeout"
  https://bugs.launchpad.net/launchpad/+bug/887078

For more details, see:
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078-use-getSpecificJobs/+merge/81736

This branches simply uses the newly created method getSpecificJobs when creating the builds list for page builder:+history.  The number of queries issued by BuildRecordsView.setupBuildList now only depends on the number of build types (and not on the number of builds).

= Tests =

./bin/test -vvc test_builder_views test_build_history_queries_count

= Q/A =

None
-- 
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078-use-getSpecificJobs/+merge/81736
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/builders-timeout-bug-887078-use-getSpecificJobs into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py	2011-11-09 14:25:42 +0000
+++ lib/lp/soyuz/browser/build.py	2011-11-09 14:25:46 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for builds."""
@@ -461,7 +461,8 @@
     Return a list of built CompleteBuild instances, or empty
     list if no builds were contained in the received batch.
     """
-    builds = [build.getSpecificJob() for build in batch]
+    build_farm_job_set = getUtility(IBuildFarmJobSet)
+    builds = build_farm_job_set.getSpecificJobs(batch)
     if not builds:
         return []
 

=== modified file 'lib/lp/soyuz/browser/tests/test_builder_views.py'
--- lib/lp/soyuz/browser/tests/test_builder_views.py	2011-01-05 07:36:35 +0000
+++ lib/lp/soyuz/browser/tests/test_builder_views.py	2011-11-09 14:25:46 +0000
@@ -1,17 +1,24 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 
+from storm.locals import Store
+from testtools.matchers import Equals
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.ftests import login
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import LaunchpadFunctionalLayer
 from lp.soyuz.browser.builder import BuilderEditView
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    StormStatementRecorder,
+    TestCaseWithFactory,
+    )
 from lp.testing.fakemethod import FakeMethod
+from lp.testing.matchers import HasQueryCount
 from lp.testing.sampledata import ADMIN_EMAIL
+from lp.testing.views import create_initialized_view
 
 
 class TestBuilderEditView(TestCaseWithFactory):
@@ -27,8 +34,8 @@
 
     def initialize_view(self):
         form = {
-            "field.manual" : "on",
-            "field.actions.update" : "Change",
+            "field.manual": "on",
+            "field.actions.update": "Change",
             }
         request = LaunchpadTestRequest(method="POST", form=form)
         view = BuilderEditView(self.builder, request)
@@ -48,4 +55,38 @@
 
         # If the dummy slaveStatusSentence() was called the call count
         # would not be zero.
-        self.assertTrue(view.context.slaveStatusSentence.call_count == 0 )
+        self.assertTrue(view.context.slaveStatusSentence.call_count == 0)
+
+
+class TestBuilderHistoryView(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def createRecipeBuildWithBuilder(self, builder=None):
+        build = self.factory.makeSourcePackageRecipeBuild()
+        Store.of(build).flush()
+        if builder is None:
+            builder = self.factory.makeBuilder()
+        removeSecurityProxy(build).builder = builder
+        return build
+
+    def test_build_history_queries_count(self):
+        # The number of queries issued by setupBuildList is not dependent
+        # on the number of builds.
+        builder = self.factory.makeBuilder()
+        [self.createRecipeBuildWithBuilder(builder) for i in xrange(2)]
+        # Record how many queries are issued when setupBuildList is
+        # called with 2 builds.
+        with StormStatementRecorder() as recorder1:
+            view = create_initialized_view(builder, '+history')
+            view.setupBuildList()
+            self.assertEqual(2, len(view.complete_builds))
+        # Create two more builds.
+        [self.createRecipeBuildWithBuilder(builder) for i in xrange(2)]
+        # Record again the number of queries issued.
+        with StormStatementRecorder() as recorder2:
+            view = create_initialized_view(builder, '+history')
+            view.setupBuildList()
+            self.assertEqual(4, len(view.complete_builds))
+
+        self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))