Launchpad itself

Merge lp:~gmb/launchpad/another-checkwatches-keyerror-bug-497414 into lp:launchpad/db-devel

  1. another-checkwatches-keyerror-bug-497414
  2. Merge into db-devel
Proposed by Graham Binns
Status: Merged
Approved by: Gavin Panella
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~gmb/launchpad/another-checkwatches-keyerror-bug-497414
Merge into: lp:launchpad/db-devel
Diff against target: 128 lines (+93/-1)
2 files modified
lib/lp/bugs/scripts/checkwatches.py (+10/-1)
lib/lp/bugs/scripts/tests/test_checkwatches.py (+83/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/another-checkwatches-keyerror-bug-497414
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Canonical Launchpad Engineering code Pending
Review via email: mp+16243@code.launchpad.net

Commit message

Catch another KeyError in checkwatches.

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

This branch fixes bug 497414 by stopping a KeyError from being raised in checkwatches.

I've added a regression test for the bug and also a test to make sure that the new code logs an error appropriately.

Revision history for this message
Gavin Panella (allenap) wrote :

Lovely.

review: Approve

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-12-15 16:28:20 +0000
3+++ lib/lp/bugs/scripts/checkwatches.py 2010-01-04 11:41:16 +0000
4@@ -789,7 +789,16 @@
5 )
6
7 for bug_id in all_remote_ids:
8- bug_watches = bug_watches_by_remote_bug[bug_id]
9+ try:
10+ bug_watches = bug_watches_by_remote_bug[bug_id]
11+ except KeyError:
12+ # If there aren't any bug watches for this remote bug,
13+ # just log a warning and carry on.
14+ self.warning(
15+ "Spurious remote bug ID: No watches found for "
16+ "remote bug %s on %s" % (bug_id, remotesystem.baseurl))
17+ continue
18+
19 for bug_watch in bug_watches:
20 bug_watch.lastchecked = UTC_NOW
21 if bug_id in unmodified_remote_ids:
22
23=== modified file 'lib/lp/bugs/scripts/tests/test_checkwatches.py'
24--- lib/lp/bugs/scripts/tests/test_checkwatches.py 2009-12-21 16:08:35 +0000
25+++ lib/lp/bugs/scripts/tests/test_checkwatches.py 2010-01-04 11:41:16 +0000
26@@ -15,6 +15,8 @@
27
28 from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI
29 from lp.bugs.scripts import checkwatches
30+from lp.bugs.scripts.checkwatches import CheckWatchesErrorUtility
31+from lp.bugs.tests.externalbugtracker import TestBugzillaAPIXMLRPCTransport
32 from lp.testing import TestCaseWithFactory
33
34
35@@ -23,6 +25,52 @@
36 return BugzillaAPI(bugtracker.baseurl)
37
38
39+class NonConnectingBugzillaAPI(BugzillaAPI):
40+ """A non-connected version of the BugzillaAPI ExternalBugTracker."""
41+
42+ bugs = {
43+ 1: {'product': 'test-product'},
44+ }
45+
46+ def getCurrentDBTime(self):
47+ return None
48+
49+ def getExternalBugTrackerToUse(self):
50+ return self
51+
52+ def getProductsForRemoteBugs(self, remote_bugs):
53+ """Return the products for some remote bugs.
54+
55+ This method is basically the same as that of the superclass but
56+ without the call to initializeRemoteBugDB().
57+ """
58+ bug_products = {}
59+ for bug_id in bug_ids:
60+ # If one of the bugs we're trying to get the product for
61+ # doesn't exist, just skip it.
62+ try:
63+ actual_bug_id = self._getActualBugId(bug_id)
64+ except BugNotFound:
65+ continue
66+
67+ bug_dict = self._bugs[actual_bug_id]
68+ bug_products[bug_id] = bug_dict['product']
69+
70+ return bug_products
71+
72+
73+class NoBugWatchesByRemoteBugUpdater(checkwatches.BugWatchUpdater):
74+ """A subclass of BugWatchUpdater with methods overridden for testing."""
75+
76+ def _getBugWatchesByRemoteBug(self, bug_watch_ids):
77+ """Return an empty dict.
78+
79+ This method overrides _getBugWatchesByRemoteBug() so that bug
80+ 497141 can be regression-tested.
81+ """
82+ return {}
83+
84+
85 class TestCheckwatchesWithSyncableGnomeProducts(TestCaseWithFactory):
86
87 layer = LaunchpadZopelessLayer
88@@ -63,5 +111,40 @@
89 gnome_bugzilla, [bug_watch_1, bug_watch_2])
90
91
92+class TestBugWatchUpdater(TestCaseWithFactory):
93+
94+ layer = LaunchpadZopelessLayer
95+
96+ def test_bug_497141(self):
97+ # Regression test for bug 497141. KeyErrors raised in
98+ # BugWatchUpdater.updateBugWatches() shouldn't cause
99+ # checkwatches to abort.
100+ updater = NoBugWatchesByRemoteBugUpdater(
101+ transaction, QuietFakeLogger())
102+
103+ # Create a couple of bug watches for testing purposes.
104+ bug_tracker = self.factory.makeBugTracker()
105+ bug_watches = [
106+ self.factory.makeBugWatch(bugtracker=bug_tracker)
107+ for i in range(2)]
108+
109+ # Use a test XML-RPC transport to ensure no connections happen.
110+ test_transport = TestBugzillaAPIXMLRPCTransport(bug_tracker.baseurl)
111+ remote_system = NonConnectingBugzillaAPI(
112+ bug_tracker.baseurl, xmlrpc_transport=test_transport)
113+
114+ # Calling updateBugWatches() shouldn't raise a KeyError, even
115+ # though with our broken updater _getExternalBugTrackersAndWatches()
116+ # will return an empty dict.
117+ updater.updateBugWatches(remote_system, bug_watches)
118+
119+ # An error will have been logged instead of the KeyError being
120+ # raised.
121+ error_utility = CheckWatchesErrorUtility()
122+ last_oops = error_utility.getLastOopsReport()
123+ self.assertTrue(
124+ last_oops.value.startswith('Spurious remote bug ID'))
125+
126+
127 def test_suite():
128 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: