← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~adeuring/launchpad/bug-594247-unittests-for-searchtasks-2 into lp:launchpad/devel

 

Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-594247-unittests-for-searchtasks-2 into lp:launchpad/devel with lp:~adeuring/launchpad/bug-594247-unittests-for-searchtasks as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


.


This branch adds more unit test to lp.bugs.tests.test_bugtasksearch:

- show that private bug are included in search results for admins
- search by bug reporter
- search by bug commenter
- search by affected person
- search by bugtask assignee
- search by subscriber
- search by bug attachment
- search for bugs nominated for a series

I also refactored the tests for private bugs a bit: Since each test
is executed 24 times, I think it is better to run all tests
for private bugs in one test method, instead of running separate
tests for different types of users: the tests need on my laptop
ca 0.7 seconds, so, merging four tests into one saves roughly
24*3*0.7=50.4 seconds (assuming that most time for each test is soent
in setUp() and tearDown()).

One test involves bug attachments which requires a working Librarian,
so I changed the test layer from DatabaseFunctionalLayer to
LaunchpadFunctionalLayer.

Looking at the source code of the class BugTaskSearchParams in
lp.bugs.interfaces.bugtask and at the "search stuff" in
lp.bugs.model.bugtask in order to figure out a useful test for the
parameter statusexplanation, I noticed that is it nowhere used, so
I removed it from BugTaskSearchParams. (I also checked that it is
not used anywhere.)

Similary, I removed the line

    search_params.has_patch = has_patch

from BugTaskSearchParams.fromSearchForm(): search_params.has_patch
is not needed -- we use search_params.attachmenttype instead.


test: ./bin/test -vvt lp.bugs.tests.test_bugtask_search

no lint.

-- 
https://code.launchpad.net/~adeuring/launchpad/bug-594247-unittests-for-searchtasks-2/+merge/39147
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-594247-unittests-for-searchtasks-2 into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2010-09-21 09:37:06 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2010-10-22 14:02:30 +0000
@@ -1128,16 +1128,16 @@
     def __init__(self, user, bug=None, searchtext=None, fast_searchtext=None,
                  status=None, importance=None, milestone=None,
                  assignee=None, sourcepackagename=None, owner=None,
-                 statusexplanation=None, attachmenttype=None,
-                 orderby=None, omit_dupes=False, subscriber=None,
-                 component=None, pending_bugwatch_elsewhere=False,
-                 resolved_upstream=False, open_upstream=False,
-                 has_no_upstream_bugtask=False, tag=None, has_cve=False,
-                 bug_supervisor=None, bug_reporter=None, nominated_for=None,
-                 bug_commenter=None, omit_targeted=False, date_closed=None,
-                 affected_user=None, affects_me=False, hardware_bus=None,
-                 hardware_vendor_id=None, hardware_product_id=None,
-                 hardware_driver_name=None, hardware_driver_package_name=None,
+                 attachmenttype=None, orderby=None, omit_dupes=False,
+                 subscriber=None, component=None,
+                 pending_bugwatch_elsewhere=False, resolved_upstream=False,
+                 open_upstream=False, has_no_upstream_bugtask=False, tag=None,
+                 has_cve=False, bug_supervisor=None, bug_reporter=None,
+                 nominated_for=None, bug_commenter=None, omit_targeted=False,
+                 date_closed=None, affected_user=None, affects_me=False,
+                 hardware_bus=None, hardware_vendor_id=None,
+                 hardware_product_id=None, hardware_driver_name=None,
+                 hardware_driver_package_name=None,
                  hardware_owner_is_bug_reporter=None,
                  hardware_owner_is_affected_by_bug=False,
                  hardware_owner_is_subscribed_to_bug=False,
@@ -1154,7 +1154,6 @@
         self.assignee = assignee
         self.sourcepackagename = sourcepackagename
         self.owner = owner
-        self.statusexplanation = statusexplanation
         self.attachmenttype = attachmenttype
         self.user = user
         self.orderby = orderby
@@ -1281,7 +1280,6 @@
             from lp.bugs.interfaces.bugattachment import (
                 BugAttachmentType)
             search_params.attachmenttype = BugAttachmentType.PATCH
-            search_params.has_patch = has_patch
         search_params.has_cve = has_cve
         if zope_isinstance(tags, (list, tuple)):
             if len(tags) > 0:

=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py	2010-10-22 14:02:22 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py	2010-10-22 14:02:30 +0000
@@ -9,14 +9,19 @@
 
 from zope.component import getUtility
 
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.launchpad.searchbuilder import any
+from canonical.testing.layers import (
+    LaunchpadFunctionalLayer,
+    )
 
+from lp.bugs.interfaces.bugattachment import BugAttachmentType
 from lp.bugs.interfaces.bugtask import (
     BugTaskImportance,
     BugTaskSearchParams,
     BugTaskStatus,
     IBugTaskSet,
     )
+from lp.registry.interfaces.person import IPersonSet
 from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
@@ -26,7 +31,7 @@
 class SearchTestBase:
     """A mixin class with tests useful for all targets and search variants."""
 
-    layer = DatabaseFunctionalLayer
+    layer = LaunchpadFunctionalLayer
 
     def setUp(self):
         super(SearchTestBase, self).setUp()
@@ -40,7 +45,7 @@
         expected = self.resultValuesForBugtasks(self.bugtasks)
         self.assertEqual(expected, search_result)
 
-    def test_private_bug_not_in_search_result_for_anonymous(self):
+    def test_private_bug_in_search_result(self):
         # Private bugs are not included in search results for anonymous users.
         with person_logged_in(self.owner):
             self.bugtasks[-1].bug.setPrivate(True, self.owner)
@@ -49,28 +54,148 @@
         expected = self.resultValuesForBugtasks(self.bugtasks)[:-1]
         self.assertEqual(expected, search_result)
 
-    def test_private_bug_not_in_search_result_for_regular_user(self):
         # Private bugs are not included in search results for ordinary users.
-        with person_logged_in(self.owner):
-            self.bugtasks[-1].bug.setPrivate(True, self.owner)
         user = self.factory.makePerson()
         params = self.getBugTaskSearchParams(user=user)
         search_result = self.runSearch(params)
         expected = self.resultValuesForBugtasks(self.bugtasks)[:-1]
         self.assertEqual(expected, search_result)
 
-    def test_private_bug_in_search_result_for_subscribed_user(self):
-        # Private bugs are included in search results for ordinary users
-        # which are subscribed to the bug.
+        # If the user is subscribed to the bug, it is included in the
+        # search result.
         user = self.factory.makePerson()
         with person_logged_in(self.owner):
-            self.bugtasks[-1].bug.setPrivate(True, self.owner)
             self.bugtasks[-1].bug.subscribe(user, self.owner)
         params = self.getBugTaskSearchParams(user=user)
         search_result = self.runSearch(params)
         expected = self.resultValuesForBugtasks(self.bugtasks)
         self.assertEqual(expected, search_result)
 
+        # Private bugs are included in search results for admins.
+        admin = getUtility(IPersonSet).getByEmail('foo.bar@xxxxxxxxxxxxx')
+        params = self.getBugTaskSearchParams(user=admin)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks(self.bugtasks)
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_bug_reporter(self):
+        # Search results can be limited to bugs filed by a given person.
+        bugtask = self.bugtasks[0]
+        reporter = bugtask.bug.owner
+        params = self.getBugTaskSearchParams(
+            user=None, bug_reporter=reporter)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks([bugtask])
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_bug_commenter(self):
+        # Search results can be limited to bugs having a comment from a
+        # given person.
+        # Note that this does not include the bug description (which is
+        # stored as the first comment of a bug.) Hence, if we let the
+        # reporter of our first test bug comment on the second test bug,
+        # a search for bugs having comments from this person retruns only
+        # the second bug.
+        commenter = self.bugtasks[0].bug.owner
+        expected = self.bugtasks[1]
+        with person_logged_in(commenter):
+            expected.bug.newMessage(owner=commenter, content='a comment')
+        params = self.getBugTaskSearchParams(
+            user=None, bug_commenter=commenter)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks([expected])
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_person_affected_by_bug(self):
+        # Search results can be limited to bugs which affect a given person.
+        affected_user = self.factory.makePerson()
+        expected = self.bugtasks[0]
+        with person_logged_in(affected_user):
+            expected.bug.markUserAffected(affected_user)
+        params = self.getBugTaskSearchParams(
+            user=None, affected_user=affected_user)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks([expected])
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_bugtask_assignee(self):
+        # Search results can be limited to bugtask assigned to a given
+        # person.
+        assignee = self.factory.makePerson()
+        expected = self.bugtasks[0]
+        with person_logged_in(assignee):
+            expected.transitionToAssignee(assignee)
+        params = self.getBugTaskSearchParams(user=None, assignee=assignee)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks([expected])
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_bug_subscriber(self):
+        # Search results can be limited to bugs to which a given person
+        # is subscribed.
+        subscriber = self.factory.makePerson()
+        expected = self.bugtasks[0]
+        with person_logged_in(subscriber):
+            expected.bug.subscribe(subscriber, subscribed_by=subscriber)
+        params = self.getBugTaskSearchParams(user=None, subscriber=subscriber)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks([expected])
+        self.assertEqual(expected, search_result)
+
+    def test_search_by_bug_attachment(self):
+        # Search results can be limited to bugs having attachments of
+        # a given type.
+        with person_logged_in(self.owner):
+            self.bugtasks[0].bug.addAttachment(
+                owner=self.owner, data='filedata', comment='a comment',
+                filename='file1.txt', is_patch=False)
+            self.bugtasks[1].bug.addAttachment(
+                owner=self.owner, data='filedata', comment='a comment',
+                filename='file1.txt', is_patch=True)
+        # We can search for bugs with non-patch attachments...
+        params = self.getBugTaskSearchParams(
+            user=None, attachmenttype=BugAttachmentType.UNSPECIFIED)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks(self.bugtasks[:1])
+        self.assertEqual(expected, search_result)
+        # ... for bugs with patches...
+        params = self.getBugTaskSearchParams(
+            user=None, attachmenttype=BugAttachmentType.PATCH)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks(self.bugtasks[1:2])
+        self.assertEqual(expected, search_result)
+        # and for bugs with patches or attachments
+        params = self.getBugTaskSearchParams(
+            user=None, attachmenttype=any(
+                BugAttachmentType.PATCH,
+                BugAttachmentType.UNSPECIFIED))
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks(self.bugtasks[:2])
+        self.assertEqual(expected, search_result)
+
+
+class ProductAndDistributionTests:
+    """Tests which are useful for distributions and products."""
+
+    def makeSeries(self):
+        """Return a series for the main bug target of this class."""
+        raise NotImplementedError
+
+    def test_search_by_bug_nomination(self):
+        # Search results can be limited to bugs nominated to a given
+        # series.
+        series1 = self.makeSeries()
+        series2 = self.makeSeries()
+        nominator = self.factory.makePerson()
+        with person_logged_in(self.owner):
+            self.bugtasks[0].bug.addNomination(nominator, series1)
+            self.bugtasks[1].bug.addNomination(nominator, series2)
+        params = self.getBugTaskSearchParams(
+            user=None, nominated_for=series1)
+        search_result = self.runSearch(params)
+        expected = self.resultValuesForBugtasks(self.bugtasks[:1])
+        self.assertEqual(expected, search_result)
+
 
 class BugTargetTestBase:
     """A base class for the bug target mixin classes."""
@@ -123,7 +248,8 @@
             self.searchtarget.setBugSupervisor(supervisor, self.owner)
 
 
-class ProductTarget(BugTargetTestBase, BugTargetWithBugSuperVisor):
+class ProductTarget(BugTargetTestBase, ProductAndDistributionTests,
+                    BugTargetWithBugSuperVisor):
     """Use a product as the bug target."""
 
     def setUp(self):
@@ -141,6 +267,10 @@
         params.setProduct(self.searchtarget)
         return params
 
+    def makeSeries(self):
+        """See `ProductAndDistributionTests`."""
+        return self.factory.makeProductSeries(product=self.searchtarget)
+
 
 class ProductSeriesTarget(BugTargetTestBase):
     """Use a product series as the bug target."""
@@ -239,7 +369,8 @@
                 bugtask.transitionToMilestone(self.searchtarget, self.owner)
 
 
-class DistributionTarget(BugTargetTestBase, BugTargetWithBugSuperVisor):
+class DistributionTarget(BugTargetTestBase, ProductAndDistributionTests,
+                         BugTargetWithBugSuperVisor):
     """Use a distribution as the bug target."""
 
     def setUp(self):
@@ -257,6 +388,10 @@
         params.setDistribution(self.searchtarget)
         return params
 
+    def makeSeries(self):
+        """See `ProductAndDistributionTests`."""
+        return self.factory.makeDistroSeries(distribution=self.searchtarget)
+
 
 class DistroseriesTarget(BugTargetTestBase):
     """Use a distro series as the bug target."""