Merge lp:~brian-murray/launchpad/bug-666496-findSimilar into lp:launchpad/db-devel

Proposed by Brian Murray on 2010-11-05
Status: Merged
Approved by: Brian Murray on 2010-11-05
Approved revision: 11824
Merged at revision: 9948
Proposed branch: lp:~brian-murray/launchpad/bug-666496-findSimilar
Merge into: lp:launchpad/db-devel
Diff against target: 72 lines (+24/-10)
3 files modified
lib/lp/bugs/doc/bugtask-find-similar.txt (+20/-1)
lib/lp/bugs/model/bugtask.py (+3/-8)
lib/lp/testing/factory.py (+1/-1)
To merge this branch: bzr merge lp:~brian-murray/launchpad/bug-666496-findSimilar
Reviewer Review Type Date Requested Status
Abel Deuring (community) code 2010-11-05 Approve on 2010-11-05
Review via email: mp+40196@code.launchpad.net

Commit message

Prevent returning the bug one started a findSimilarBugs() search with.

Description of the change

Bug 666496 is about findSimilarBugs() returning the same bug that a search started from, this happens for bug reports containing duplicate bugs. This branch fixes that and adds a test for it to lib/lp/bugs/doc/bugtask-find-similar.txt.

It can be tested via:

bin/test -cvvt bugtask-find-similar.txt

To post a comment you must log in.
Abel Deuring (adeuring) wrote :

Looks good. Just one nitpick:

+XXX bdmurray 2010-11-05: This would be more appropriate as a unit test.

I appreciate that you mention unit tests, but our XXX policy says that an XXX should always have a related bug. But writing a unit test with just the test "no duplicate results" is IMHO pointless: other parts of lib/lp/bugs/doc/bugtask-find-similar.txt would also better live in a unit test, but that's beyond your bug fix. So I think you can simply remove the XXX, instead of explicitly adding a bug report. We already know that we want more unit tests ;)

review: Approve (code)
11825. By Brian Murray on 2010-11-05

modifications based on reviewer feedback

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/doc/bugtask-find-similar.txt'
2--- lib/lp/bugs/doc/bugtask-find-similar.txt 2010-10-18 22:24:59 +0000
3+++ lib/lp/bugs/doc/bugtask-find-similar.txt 2010-11-05 18:02:55 +0000
4@@ -15,7 +15,8 @@
5 >>> from lp.registry.interfaces.person import IPersonSet
6 >>> from lp.registry.interfaces.product import IProductSet
7 >>> firefox = getUtility(IProductSet).getByName('firefox')
8- >>> sample_person = getUtility(IPersonSet).getByEmail('test@canonical.com')
9+ >>> sample_person = getUtility(IPersonSet).getByEmail(
10+ ... 'test@canonical.com')
11 >>> similar_bugs = getUtility(IBugTaskSet).findSimilar(
12 ... sample_person, '', product=firefox)
13 >>> similar_bugs.count()
14@@ -137,3 +138,21 @@
15 ... print bugtask.bug.title
16 Thunderbird crashes
17 >>> #another test bug
18+
19+== Not returning the same bug ==
20+
21+findSimilarBugs() does not include the bug of the bugtask upon which
22+it is invoked.
23+
24+ >>> orig_bug = factory.makeBug(
25+ ... title="So you walk into this restaurant",
26+ ... owner=firefox.owner, product=firefox)
27+
28+ >>> dupe_bug = factory.makeBug(
29+ ... title="So you walk into this restaurant",
30+ ... owner=firefox.owner, product=firefox)
31+ >>> dupe_bug.markAsDuplicate(orig_bug)
32+
33+ >>> similar_bugs = orig_bug.default_bugtask.findSimilarBugs(firefox.owner)
34+ >>> orig_bug in similar_bugs
35+ False
36
37=== modified file 'lib/lp/bugs/model/bugtask.py'
38--- lib/lp/bugs/model/bugtask.py 2010-11-03 18:43:07 +0000
39+++ lib/lp/bugs/model/bugtask.py 2010-11-05 18:02:55 +0000
40@@ -667,16 +667,11 @@
41 matching_bugtasks = getUtility(IBugTaskSet).findSimilar(
42 user, self.bug.title, **context_params)
43
44- # Make sure to exclude the current BugTask from the list of
45- # matching tasks. We use 4*limit as an arbitrary value here to
46- # make sure we select more than :limit: bugtasks.
47- matching_bugtasks = [
48- bug_task for bug_task in matching_bugtasks[:4*limit]
49- if bug_task != self]
50-
51 matching_bugs = getUtility(IBugSet).getDistinctBugsForBugTasks(
52 matching_bugtasks, user, limit)
53- return matching_bugs
54+
55+ # Make sure to exclude the bug of the current bugtask.
56+ return [bug for bug in matching_bugs if bug.id != self.bugID]
57
58 def subscribe(self, person, subscribed_by):
59 """See `IBugTask`."""
60
61=== modified file 'lib/lp/testing/factory.py'
62--- lib/lp/testing/factory.py 2010-11-04 19:59:02 +0000
63+++ lib/lp/testing/factory.py 2010-11-05 18:02:55 +0000
64@@ -1407,7 +1407,7 @@
65 owner = self.makePerson()
66
67 if IProductSeries.providedBy(target):
68- # We can't have a series task without a distribution task.
69+ # We can't have a series task without a product task.
70 self.makeBugTask(bug, target.product)
71 if IDistroSeries.providedBy(target):
72 # We can't have a series task without a distribution task.

Subscribers

People subscribed via source and target branches

to status/vote changes: