Launchpad itself

Merge lp:~gmb/launchpad/checkwatches-keyerror-oops-bug-496988 into lp:launchpad

  1. checkwatches-keyerror-oops-bug-496988
  2. Merge into devel
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/checkwatches-keyerror-oops-bug-496988
Merge into: lp:launchpad
Diff against target: 116 lines (+100/-1)
2 files modified
lib/lp/bugs/scripts/checkwatches.py (+1/-1)
lib/lp/bugs/scripts/tests/test_checkwatches.py (+99/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/checkwatches-keyerror-oops-bug-496988
Reviewer Review Type Date Requested Status
Данило Шеган (community) release-critical Approve
Brad Crittenden (community) code Approve
Review via email: mp+16204@code.launchpad.net

Commit message

KeyErrors in BugWatchUpdater._getExternalBugTrackersAndWatches() will no longer cause checkwatches to abort prematurely.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes bug 406988 by preventing KeyErrors from being raised with the BugWatchUpdater.

I've added a regression test for the bug and changed the offending line of code.

Revision history for this message
Brad Crittenden (bac) wrote :

Actually bug 496988, as shown below.

Revision history for this message
Brad Crittenden (bac) wrote :

With the addition of the comment to your final test, as discussed on IRC, this branch looks fine. Thanks for the fix.

review: Approve (code)
Revision history for this message
Данило Шеган (danilo) wrote :

Let's get those gnome bugwatches going again.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/scripts/checkwatches.py'
2--- lib/lp/bugs/scripts/checkwatches.py 2009-11-28 08:31:15 +0000
3+++ lib/lp/bugs/scripts/checkwatches.py 2009-12-15 16:31:16 +0000
4@@ -462,7 +462,7 @@
5 remotesystem_for_others.sync_comments = False
6
7 for bug_watch in bug_watches:
8- if (remote_products[bug_watch.remotebug] in
9+ if (remote_products.get(bug_watch.remotebug, None) in
10 self._syncable_gnome_products):
11 syncable_watches.append(bug_watch)
12 else:
13
14=== added file 'lib/lp/bugs/scripts/tests/test_checkwatches.py'
15--- lib/lp/bugs/scripts/tests/test_checkwatches.py 1970-01-01 00:00:00 +0000
16+++ lib/lp/bugs/scripts/tests/test_checkwatches.py 2009-12-15 16:31:16 +0000
17@@ -0,0 +1,99 @@
18+# Copyright 2009 Canonical Ltd. This software is licensed under the
19+# GNU Affero General Public License version 3 (see the file LICENSE).
20+"""Checkwatches unit tests."""
21+
22+__metaclass__ = type
23+
24+import unittest
25+import transaction
26+
27+from zope.component import getUtility
28+
29+from canonical.launchpad.scripts.logger import QuietFakeLogger
30+from canonical.launchpad.interfaces import ILaunchpadCelebrities
31+from canonical.testing import LaunchpadZopelessLayer
32+
33+from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI
34+from lp.bugs.scripts import checkwatches
35+from lp.testing import TestCaseWithFactory
36+
37+
38+def fudged_get_external_bugtracker(bugtracker):
39+ """A version of get_external_bugtracker that returns BugzillaAPI."""
40+ return BugzillaAPI(bugtracker.baseurl)
41+
42+
43+class NonConnectingBugzillaAPI(BugzillaAPI):
44+ """A non-connected version of the BugzillaAPI ExternalBugTracker."""
45+
46+ bugs = {
47+ 1: {'product': 'test-product'},
48+ }
49+
50+ def getExternalBugTrackerToUse(self):
51+ return self
52+
53+ def getProductsForRemoteBugs(self, remote_bugs):
54+ """Return the products for some remote bugs.
55+
56+ This method is basically the same as that of the superclass but
57+ without the call to initializeRemoteBugDB().
58+ """
59+ bug_products = {}
60+ for bug_id in bug_ids:
61+ # If one of the bugs we're trying to get the product for
62+ # doesn't exist, just skip it.
63+ try:
64+ actual_bug_id = self._getActualBugId(bug_id)
65+ except BugNotFound:
66+ continue
67+
68+ bug_dict = self._bugs[actual_bug_id]
69+ bug_products[bug_id] = bug_dict['product']
70+
71+ return bug_products
72+
73+
74+class TestCheckwatchesWithSyncableGnomeProducts(TestCaseWithFactory):
75+
76+ layer = LaunchpadZopelessLayer
77+
78+ def setUp(self):
79+ super(TestCheckwatchesWithSyncableGnomeProducts, self).setUp()
80+
81+ # We monkey-patch externalbugtracker.get_external_bugtracker()
82+ # so that it always returns what we want.
83+ self.original_get_external_bug_tracker = (
84+ checkwatches.externalbugtracker.get_external_bugtracker)
85+ checkwatches.externalbugtracker.get_external_bugtracker = (
86+ fudged_get_external_bugtracker)
87+
88+ # Create an updater with a limited set of syncable gnome
89+ # products.
90+ self.updater = checkwatches.BugWatchUpdater(
91+ transaction, QuietFakeLogger(), ['test-product'])
92+
93+ def tearDown(self):
94+ checkwatches.externalbugtracker.get_external_bugtracker = (
95+ self.original_get_external_bug_tracker)
96+ super(TestCheckwatchesWithSyncableGnomeProducts, self).tearDown()
97+
98+ def test_bug_496988(self):
99+ # Regression test for bug 496988. KeyErrors when looking for the
100+ # remote product for a given bug shouldn't travel upwards and
101+ # cause the script to abort.
102+ gnome_bugzilla = getUtility(ILaunchpadCelebrities).gnome_bugzilla
103+ bug_watch_1 = self.factory.makeBugWatch(
104+ remote_bug=1, bugtracker=gnome_bugzilla)
105+ bug_watch_2 = self.factory.makeBugWatch(
106+ remote_bug=2, bugtracker=gnome_bugzilla)
107+
108+ # Calling this method shouldn't raise a KeyError, even though
109+ # there's no bug 2 on the bug tracker that we pass to it.
110+ self.updater._getExternalBugTrackersAndWatches(
111+ gnome_bugzilla, [bug_watch_1, bug_watch_2])
112+
113+
114+def test_suite():
115+ return unittest.TestLoader().loadTestsFromName(__name__)
116+