← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~lifeless/launchpad/bug-717394 into lp:launchpad

 

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-717394 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #717394 Distribution:+bugs timeouts
  https://bugs.launchpad.net/bugs/717394

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-717394/+merge/50110

This branch changes from in-python late evaluation to up front aggregation to determine the series statistics for bug searches. Per https://bugs.launchpad.net/launchpad/+bug/717394 this will reduce bug searches in Ubuntu by 1.4 seconds : the new query runs in 300ms on qastaging, vs an aggregate time of 1.7 seconds.

I've laid the ground work for also doing the same thing on the milestones portlet on the same page, but haven't yet tested the resulting query or times. Some of the code is a little raw, but I couldn't see any flexible prior examples, so hopefully this is a reasonable compromise.

I haven't tested some of the noddy refactorings I did to make various things reusable (like the open bugs search); I'm open to direct tests, but I don't see any particular value in them at this point: they would be redundant with the higher layer tests and we have no layer interception facilities in use at the moment to make the higher layer tests massively cheaper, so it was a tradeoff decision not to add such tests.

I did TDD for the entire branch and I'm happy with my test coverage overall.

I think key things for a reviewer to look for are logic or structural errors surrounding the intent of the patch - the exposure of the new aggregating API - is it in a good place (I couldn't see a better, or existing general solution), is it understandable, etc.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-717394/+merge/50110
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-717394 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-02-11 15:14:37 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-02-17 09:21:08 +0000
@@ -125,6 +125,7 @@
 from lp.bugs.interfaces.bugtracker import IBugTracker
 from lp.bugs.interfaces.malone import IMaloneApplication
 from lp.bugs.interfaces.securitycontact import IHasSecurityContact
+from lp.bugs.model.bugtask import BugTask
 from lp.bugs.utilities.filebugdataparser import FileBugData
 from lp.hardwaredb.interfaces.hwdb import IHWSubmissionSet
 from lp.registry.browser.product import ProductConfigureBase
@@ -1174,7 +1175,7 @@
             series = self.context.product.series
         else:
             raise AssertionError("series_list called with illegal context")
-        return series
+        return list(series)
 
     @property
     def series_buglistings(self):
@@ -1186,8 +1187,24 @@
         able to see in a listing.
         """
         series_buglistings = []
+        bug_task_set = getUtility(IBugTaskSet)
+        series = any(*self.series_list)
+        open_bugs = bug_task_set.open_bugtask_search
+        open_bugs.setTarget(series)
+        # This would be better as delegation not a case statement.
+        if IDistribution(self.context, None):
+            backlink = BugTask.distroseriesID
+        elif IProduct(self.context, None):
+            backlink = BugTask.productseriesID
+        elif IDistroSeries(self.context, None):
+            backlink = BugTask.distroseriesID
+        elif IProductSeries(self.context, None):
+            backlink = BugTask.productseriesID
+        else:
+            raise AssertionError("illegal context %r" % self.context)
+        counts = bug_task_set.countBugs(open_bugs, (backlink,))
         for series in self.series_list:
-            series_bug_count = series.open_bugtasks.count()
+            series_bug_count = counts.get((series.id,), 0)
             if series_bug_count > 0:
                 series_buglistings.append(
                     dict(
@@ -1195,7 +1212,6 @@
                         url=canonical_url(series) + "/+bugs",
                         count=series_bug_count,
                         ))
-
         return series_buglistings
 
     @property

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-02-17 01:03:34 +0000
+++ lib/lp/bugs/configure.zcml	2011-02-17 09:21:08 +0000
@@ -316,6 +316,15 @@
                 set_schema="lp.bugs.interfaces.bugtask.IBugTask"/>
         </class>
 
+        <!-- BugTaskSearchParams -->
+        <class
+            class="lp.bugs.interfaces.bugtask.BugTaskSearchParams">
+            <allow
+                attributes="
+                    setTarget
+                    "/>
+        </class>
+
         <!-- BugTaskSet -->
 
         <class

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-02-14 11:54:25 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-02-17 09:21:08 +0000
@@ -1236,6 +1236,29 @@
         # Import this here to avoid circular dependencies
         from lp.registry.interfaces.sourcepackage import (
             ISourcePackage)
+        if isinstance(sourcepackage, any):
+            # Unwrap the source package.
+            self.sourcepackagename = any(*[
+                pkg.sourcepackagename for pkg in sourcepackage.query_values])
+            distroseries = any(*[pkg.distroseries for pkg in
+                sourcepackage.query_values if ISourcePackage.providedBy(pkg)])
+            distributions = any(*[pkg.distribution for pkg in
+                sourcepackage.query_values
+                if not ISourcePackage.providedBy(pkg)])
+            if distroseries.query_values and not distributions.query_values:
+                self.distroseries = distroseries
+            elif not distroseries.query_values and distributions.query_values:
+                self.distributions = distributions
+            else:
+                # Either we have both distroseries and distributions or neither
+                # of them: set both, which will give us the cross product
+                # due to the search of source packages being sourcepackagename
+                # specific rather than actually context specific. This is not
+                # ideal but is tolerable given no actual use of mixed type
+                # any() exists today.
+                self.distroseries = distroseries
+                self.distributions = distributions
+            return
         if ISourcePackage.providedBy(sourcepackage):
             # This is a sourcepackage in a distro series.
             self.distroseries = sourcepackage.distroseries
@@ -1244,6 +1267,33 @@
             self.distribution = sourcepackage.distribution
         self.sourcepackagename = sourcepackage.sourcepackagename
 
+    def setTarget(self, target):
+        """Constrain the search to only return items in target.
+
+        This is equivalent to calling setProduct etc but the type of target
+        does not need to be known to the caller.
+
+        :param target: A `IHasBug`, or some search term like all/any/none on
+            `IHasBug`. If using all/any all the targets must be of the same
+            type due to implementation limitations. Currently only distroseries
+            and productseries `IHasBug` implementations are supported.
+        """
+        # Yay circular deps.
+        from lp.registry.interfaces.distroseries import IDistroSeries
+        from lp.registry.interfaces.productseries import IProductSeries
+        if isinstance(target, (any, all)):
+            assert len(target.query_values), \
+                'cannot determine target with no targets'
+            instance = target.query_values[0]
+        else:
+            instance = target
+        if IDistroSeries.providedBy(instance):
+            self.setDistroSeries(target)
+        elif IProductSeries.providedBy(instance):
+            self.setProductSeries(target)
+        else:
+            raise AssertionError("unknown target type %r" % target)
+
     @classmethod
     def _anyfy(cls, value):
         """If value is a sequence, wrap its items with the `any` combinator.
@@ -1434,6 +1484,16 @@
         :param params: the BugTaskSearchParams to search on.
         """
 
+    def countBugs(params, group_on):
+        """Count bugs that match params, grouping by group_on.
+
+        :param param: A BugTaskSearchParams object.
+        :param group_on: The column(s) group on - .e.g (
+            Bugtask.distroseriesID, BugTask.milestoneID) will cause grouping by
+            distro series and then milestone.
+        :return: A dict {group_instance: count, ...}
+        """
+
     def getStatusCountsForProductSeries(user, product_series):
         """Returns status counts for a product series' bugs.
 
@@ -1543,6 +1603,8 @@
         The assignee and the assignee's validity are precached.
         """
 
+    open_bugtask_search = Attribute("A search returning open bugTasks.")
+
 
 def valid_remote_bug_url(value):
     """Verify that the URL is to a bug to a known bug tracker."""

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2011-01-28 21:02:14 +0000
+++ lib/lp/bugs/model/bugtarget.py	2011-02-17 09:21:08 +0000
@@ -141,12 +141,7 @@
     @property
     def open_bugtasks(self):
         """See `IHasBugs`."""
-        open_tasks_query = BugTaskSearchParams(
-            user=getUtility(ILaunchBag).user,
-            status=any(*UNRESOLVED_BUGTASK_STATUSES),
-            omit_dupes=True)
-
-        return self.searchTasks(open_tasks_query)
+        return self.searchTasks(BugTaskSet().open_bugtask_search)
 
     @property
     def new_bugtasks(self):

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-02-15 03:27:27 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-02-17 09:21:08 +0000
@@ -37,6 +37,7 @@
 from storm.expr import (
     Alias,
     And,
+    Count,
     Desc,
     In,
     Join,
@@ -1511,6 +1512,14 @@
 
     title = "A set of bug tasks"
 
+    @property
+    def open_bugtask_search(self):
+        """See `IBugTaskSet`."""
+        return BugTaskSearchParams(
+            user=getUtility(ILaunchBag).user,
+            status=any(*UNRESOLVED_BUGTASK_STATUSES),
+            omit_dupes=True)
+
     def get(self, task_id):
         """See `IBugTaskSet`."""
         # XXX: JSK: 2007-12-19: This method should probably return
@@ -1737,7 +1746,11 @@
         :return: A query, the tables to query, ordering expression and a
             decorator to call on each returned row.
         """
-        assert isinstance(params, BugTaskSearchParams)
+        assert zope_isinstance(params, BugTaskSearchParams)
+        if not isinstance(params, BugTaskSearchParams):
+            # Browser code let this get wrapped, unwrap it here as its just a
+            # dumb data store that has no security implications.
+            params = removeSecurityProxy(params)
         from lp.bugs.model.bug import Bug
         extra_clauses = ['Bug.id = BugTask.bug']
         clauseTables = [BugTask, Bug]
@@ -2486,6 +2499,18 @@
         """See `IBugTaskSet`."""
         return self._search(BugTask.bugID, [], None, params).result_set
 
+    def countBugs(self, params, group_on):
+        """See `IBugTaskSet`."""
+        resultset = self._search(
+            group_on + (Count(BugTask.bugID),), [], None, params).result_set
+        # We group on the related field:
+        resultset.group_by(*group_on)
+        resultset.order_by()
+        result = {}
+        for row in resultset:
+            result[row[:-1]] = row[-1]
+        return result
+
     def getPrecachedNonConjoinedBugTasks(self, user, milestone):
         """See `IBugTaskSet`."""
         params = BugTaskSearchParams(

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2011-02-14 04:41:43 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2011-02-17 09:21:08 +0000
@@ -69,6 +69,19 @@
         expected = self.resultValuesForBugtasks(expected_bugtasks)
         self.assertEqual(expected, search_result)
 
+    def test_aggregate_by_target(self):
+        # BugTaskSet.search supports returning the counts for each target (as
+        # long only one type of target was selected).
+        if self.group_on is None:
+            # Not a useful/valid permutation.
+            return
+        params = self.getBugTaskSearchParams(user=None, multitarget=True)
+        # The test data has 3 bugs for searchtarget and 6 for searchtarget2.
+        expected = {(self.targetToGroup(self.searchtarget),): 3,
+            (self.targetToGroup(self.searchtarget2),): 6}
+        self.assertEqual(expected, self.bugtask_set.countBugs(params,
+            group_on=self.group_on))
+
     def test_search_all_bugtasks_for_target(self):
         # BugTaskSet.search() returns all bug tasks for a given bug
         # target, if only the bug target is passed as a search parameter.
@@ -557,28 +570,68 @@
 
 
 class BugTargetTestBase:
-    """A base class for the bug target mixin classes."""
-
-    def makeBugTasks(self, bugtarget):
-        self.bugtasks = []
-        with person_logged_in(self.owner):
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=bugtarget))
-            self.bugtasks[-1].importance = BugTaskImportance.HIGH
-            self.bugtasks[-1].transitionToStatus(
-                BugTaskStatus.TRIAGED, self.owner)
-
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=bugtarget))
-            self.bugtasks[-1].importance = BugTaskImportance.LOW
-            self.bugtasks[-1].transitionToStatus(
-                BugTaskStatus.NEW, self.owner)
-
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=bugtarget))
-            self.bugtasks[-1].importance = BugTaskImportance.CRITICAL
-            self.bugtasks[-1].transitionToStatus(
-                BugTaskStatus.FIXCOMMITTED, self.owner)
+    """A base class for the bug target mixin classes.
+    
+    :ivar searchtarget: A bug context to search within.
+    :ivar searchtarget2: A sibling bug context for testing cross-context
+        searches. Created on demand when
+        getBugTaskSearchParams(multitarget=True) is called.
+    :ivar bugtasks2: Bugtasks created for searchtarget2. Twice as many are made
+        as for searchtarget.
+    :ivar group_on: The columns to group on when calling countBugs. None
+        if the target being testing is not sensible/implemented for counting
+        bugs. For instance, grouping by project group may be interesting but
+        at the time of writing is not implemented.
+    """
+
+    def makeBugTasks(self, bugtarget=None, bugtasks=None, owner=None):
+        if bugtasks is None:
+            self.bugtasks = []
+            bugtasks = self.bugtasks
+        if bugtarget is None:
+            bugtarget = self.searchtarget
+        if owner is None:
+            owner = self.owner
+        with person_logged_in(owner):
+            bugtasks.append(
+                self.factory.makeBugTask(target=bugtarget))
+            bugtasks[-1].importance = BugTaskImportance.HIGH
+            bugtasks[-1].transitionToStatus(
+                BugTaskStatus.TRIAGED, owner)
+
+            bugtasks.append(
+                self.factory.makeBugTask(target=bugtarget))
+            bugtasks[-1].importance = BugTaskImportance.LOW
+            bugtasks[-1].transitionToStatus(
+                BugTaskStatus.NEW, owner)
+
+            bugtasks.append(
+                self.factory.makeBugTask(target=bugtarget))
+            bugtasks[-1].importance = BugTaskImportance.CRITICAL
+            bugtasks[-1].transitionToStatus(
+                BugTaskStatus.FIXCOMMITTED, owner)
+
+    def getBugTaskSearchParams(self, multitarget=False, *args, **kw):
+        """Return a BugTaskSearchParams object for the given parameters.
+
+        Also, set the bug target.
+
+        :param multitarget: If True multiple targets are used using any(
+            self.searchtarget, self.searchtarget2).
+        """
+        params = BugTaskSearchParams(*args, **kw)
+        if multitarget and getattr(self, 'searchtarget2', None) is None:
+            self.setUpTarget2()
+        if not multitarget:
+            target = self.searchtarget
+        else:
+            target = any(self.searchtarget, self.searchtarget2)
+        self.setBugParamsTarget(params, target)
+        return params
+
+    def targetToGroup(self, target):
+        """Convert a search target to a group_on result."""
+        return target.id
 
 
 class BugTargetWithBugSuperVisor:
@@ -610,18 +663,21 @@
 
     def setUp(self):
         super(ProductTarget, self).setUp()
+        self.group_on = (BugTask.productID,)
         self.searchtarget = self.factory.makeProduct()
         self.owner = self.searchtarget.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setProduct(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeProduct()
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setProduct(target)
 
     def makeSeries(self):
         """See `ProductAndDistributionTests`."""
@@ -638,18 +694,22 @@
 
     def setUp(self):
         super(ProductSeriesTarget, self).setUp()
+        self.group_on = (BugTask.productseriesID,)
         self.searchtarget = self.factory.makeProductSeries()
         self.owner = self.searchtarget.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setProductSeries(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeProductSeries(
+            product=self.searchtarget.product)
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setProductSeries(target)
 
     def changeStatusOfBugTaskForOtherProduct(self, bugtask, new_status):
         # Change the status of another bugtask of the same bug to the
@@ -682,50 +742,59 @@
 
     def setUp(self):
         super(ProjectGroupTarget, self).setUp()
+        self.group_on = None
         self.searchtarget = self.factory.makeProject()
         self.owner = self.searchtarget.owner
         self.makeBugTasks()
 
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setProject(self.searchtarget)
-        return params
-
-    def makeBugTasks(self):
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeProject()
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setProject(target)
+
+    def makeBugTasks(self, bugtarget=None, bugtasks=None, owner=None):
         """Create bug tasks for the search target."""
-        self.bugtasks = []
+        if bugtasks is None:
+            self.bugtasks = []
+            bugtasks = self.bugtasks
+        if bugtarget is None:
+            bugtarget = self.searchtarget
+        if owner is None:
+            owner = self.owner
         self.products = []
-        with person_logged_in(self.owner):
-            product = self.factory.makeProduct(owner=self.owner)
-            self.products.append(product)
-            product.project = self.searchtarget
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=product))
-            self.bugtasks[-1].importance = BugTaskImportance.HIGH
-            self.bugtasks[-1].transitionToStatus(
-                BugTaskStatus.TRIAGED, self.owner)
-
-            product = self.factory.makeProduct(owner=self.owner)
-            self.products.append(product)
-            product.project = self.searchtarget
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=product))
-            self.bugtasks[-1].importance = BugTaskImportance.LOW
-            self.bugtasks[-1].transitionToStatus(
-            BugTaskStatus.NEW, self.owner)
-
-            product = self.factory.makeProduct(owner=self.owner)
-            self.products.append(product)
-            product.project = self.searchtarget
-            self.bugtasks.append(
-                self.factory.makeBugTask(target=product))
-            self.bugtasks[-1].importance = BugTaskImportance.CRITICAL
-            self.bugtasks[-1].transitionToStatus(
-                BugTaskStatus.FIXCOMMITTED, self.owner)
+        with person_logged_in(owner):
+            product = self.factory.makeProduct(owner=owner)
+            self.products.append(product)
+            product.project = self.searchtarget
+            bugtasks.append(
+                self.factory.makeBugTask(target=product))
+            bugtasks[-1].importance = BugTaskImportance.HIGH
+            bugtasks[-1].transitionToStatus(
+                BugTaskStatus.TRIAGED, owner)
+
+            product = self.factory.makeProduct(owner=owner)
+            self.products.append(product)
+            product.project = self.searchtarget
+            bugtasks.append(
+                self.factory.makeBugTask(target=product))
+            bugtasks[-1].importance = BugTaskImportance.LOW
+            bugtasks[-1].transitionToStatus(
+            BugTaskStatus.NEW, owner)
+
+            product = self.factory.makeProduct(owner=owner)
+            self.products.append(product)
+            product.project = self.searchtarget
+            bugtasks.append(
+                self.factory.makeBugTask(target=product))
+            bugtasks[-1].importance = BugTaskImportance.CRITICAL
+            bugtasks[-1].transitionToStatus(
+                BugTaskStatus.FIXCOMMITTED, owner)
 
     def setSupervisor(self, supervisor):
         """Set the bug supervisor for the bug task targets."""
@@ -782,24 +851,38 @@
     def setUp(self):
         super(MilestoneTarget, self).setUp()
         self.product = self.factory.makeProduct()
+        self.group_on = (BugTask.milestoneID,)
         self.searchtarget = self.factory.makeMilestone(product=self.product)
         self.owner = self.product.owner
-        self.makeBugTasks(self.product)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(milestone=self.searchtarget, *args, **kw)
-        return params
-
-    def makeBugTasks(self, bugtarget):
+        self.makeBugTasks(bugtarget=self.product)
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeMilestone(product=self.product)
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.product,
+            bugtasks=self.bugtasks2, owner=self.product.owner,
+            searchtarget=self.searchtarget2)
+        self.makeBugTasks(bugtarget=self.product,
+            bugtasks=self.bugtasks2, owner=self.product.owner,
+            searchtarget=self.searchtarget2)
+
+    def setBugParamsTarget(self, params, target):
+        params.milestone = target
+
+    def makeBugTasks(self, bugtarget=None, bugtasks=None, owner=None,
+        searchtarget=None):
         """Create bug tasks for a product and assign them to a milestone."""
-        super(MilestoneTarget, self).makeBugTasks(bugtarget)
-        with person_logged_in(self.owner):
-            for bugtask in self.bugtasks:
-                bugtask.transitionToMilestone(self.searchtarget, self.owner)
+        super(MilestoneTarget, self).makeBugTasks(bugtarget=bugtarget,
+            bugtasks=bugtasks, owner=owner)
+        if bugtasks is None:
+            bugtasks = self.bugtasks
+        if owner is None:
+            owner = self.owner
+        if searchtarget is None:
+            searchtarget = self.searchtarget
+        with person_logged_in(owner):
+            for bugtask in bugtasks:
+                bugtask.transitionToMilestone(searchtarget, owner)
 
     def findBugtaskForOtherProduct(self, bugtask):
         # Return the bugtask for the product that not related to the
@@ -814,18 +897,21 @@
 
     def setUp(self):
         super(DistributionTarget, self).setUp()
+        self.group_on = (BugTask.distributionID,)
         self.searchtarget = self.factory.makeDistribution()
         self.owner = self.searchtarget.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setDistribution(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeDistribution()
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setDistribution(target)
 
     def makeSeries(self):
         """See `ProductAndDistributionTests`."""
@@ -849,18 +935,22 @@
 
     def setUp(self):
         super(DistroseriesTarget, self).setUp()
+        self.group_on = (BugTask.distroseriesID,)
         self.searchtarget = self.factory.makeDistroSeries()
         self.owner = self.searchtarget.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setDistroSeries(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeDistroSeries(
+            distribution=self.searchtarget.distribution)
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2, owner=self.searchtarget2.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setDistroSeries(target)
 
 
 class SourcePackageTarget(BugTargetTestBase):
@@ -868,18 +958,24 @@
 
     def setUp(self):
         super(SourcePackageTarget, self).setUp()
+        self.group_on = (BugTask.sourcepackagenameID,)
         self.searchtarget = self.factory.makeSourcePackage()
         self.owner = self.searchtarget.distroseries.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setSourcePackage(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeSourcePackage(
+            distroseries=self.searchtarget.distroseries)
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2,
+            owner=self.searchtarget2.distroseries.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2,
+            owner=self.searchtarget2.distroseries.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setSourcePackage(target)
 
     def subscribeToTarget(self, subscriber):
         # Subscribe the given person to the search target.
@@ -889,6 +985,9 @@
             self.searchtarget.distroseries.addSubscription(
                 subscriber, subscribed_by=subscriber)
 
+    def targetToGroup(self, target):
+        return target.sourcepackagename.id
+
 
 class DistributionSourcePackageTarget(BugTargetTestBase,
                                       BugTargetWithBugSuperVisor):
@@ -896,18 +995,24 @@
 
     def setUp(self):
         super(DistributionSourcePackageTarget, self).setUp()
+        self.group_on = (BugTask.sourcepackagenameID,)
         self.searchtarget = self.factory.makeDistributionSourcePackage()
         self.owner = self.searchtarget.distribution.owner
-        self.makeBugTasks(self.searchtarget)
-
-    def getBugTaskSearchParams(self, *args, **kw):
-        """Return a BugTaskSearchParams object for the given parameters.
-
-        Also, set the bug target.
-        """
-        params = BugTaskSearchParams(*args, **kw)
-        params.setSourcePackage(self.searchtarget)
-        return params
+        self.makeBugTasks()
+
+    def setUpTarget2(self):
+        self.searchtarget2 = self.factory.makeDistributionSourcePackage(
+            distribution=self.searchtarget.distribution)
+        self.bugtasks2 = []
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2,
+            owner=self.searchtarget2.distribution.owner)
+        self.makeBugTasks(bugtarget=self.searchtarget2,
+            bugtasks=self.bugtasks2,
+            owner=self.searchtarget2.distribution.owner)
+
+    def setBugParamsTarget(self, params, target):
+        params.setSourcePackage(target)
 
     def setSupervisor(self, supervisor):
         """Set the bug supervisor for the bug task target."""
@@ -915,6 +1020,9 @@
             self.searchtarget.distribution.setBugSupervisor(
                 supervisor, self.owner)
 
+    def targetToGroup(self, target):
+        return target.sourcepackagename.id
+
 
 bug_targets_mixins = (
     DistributionTarget,