Launchpad itself

Merge lp:~jelmer/launchpad/634045-lp-bugs-fixed into lp:launchpad

  1. 634045-lp-bugs-fixed
  2. Merge into devel
Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 11529
Proposed branch: lp:~jelmer/launchpad/634045-lp-bugs-fixed
Merge into: lp:launchpad
Diff against target: 168 lines (+74/-10)
2 files modified
lib/lp/soyuz/scripts/processaccepted.py (+6/-4)
lib/lp/soyuz/tests/test_processaccepted.py (+68/-6)
To merge this branch: bzr merge lp:~jelmer/launchpad/634045-lp-bugs-fixed
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+35096@code.launchpad.net

Commit message

Parse Launchpad-Bugs-Fixed regardless of casing.

Description of the change

This fixes lookups of launchpad-bugs-fixed to be case-insensitive once again. This was broken after parse_tagfile_lines was changed to no longer lowercase all field names anymore.

I've also taken the liberty of adding some more tests for get_bugs_from_changes_file.

test: ./bin/test lp.soyuz.tests.test_processaccepted

lint: all cleaned up

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Jelmer,

a nice branch, just one nitpick:

> - def createWaitingAcceptancePackage(self, distroseries, archive=None,
> + def createWaitingAcceptancePackage(self, distroseries, archive=None,
> sourcename=None):

Our style guide says that this should either be

    def createWaitingAcceptancePackage(self, distroseries, archive=None,
                                       sourcename=None):
or

    def createWaitingAcceptancePackage(
        self, distroseries, archive=None, sourcename=None):

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
2--- lib/lp/soyuz/scripts/processaccepted.py 2010-08-27 12:21:25 +0000
3+++ lib/lp/soyuz/scripts/processaccepted.py 2010-09-10 13:29:46 +0000
4@@ -8,9 +8,11 @@
5 'close_bugs',
6 'close_bugs_for_queue_item',
7 'close_bugs_for_sourcepublication',
8+ 'get_bugs_from_changes_file',
9 'ProcessAccepted',
10 ]
11
12+from debian.deb822 import Deb822Dict
13 import sys
14
15 from zope.component import getUtility
16@@ -47,7 +49,7 @@
17 """
18 contents = changes_file.read()
19 changes_lines = contents.splitlines(True)
20- tags = parse_tagfile_lines(changes_lines, allow_unsigned=True)
21+ tags = Deb822Dict(parse_tagfile_lines(changes_lines, allow_unsigned=True))
22 bugs_fixed_line = tags.get('Launchpad-bugs-fixed', '')
23 bugs = []
24 for bug_id in bugs_fixed_line.split():
25@@ -105,7 +107,8 @@
26 the upload is processed and committed.
27
28 In practice, 'changesfile_object' is only set when we are closing bugs
29- in upload-time (see archiveuploader/ftests/nascentupload-closing-bugs.txt).
30+ in upload-time (see
31+ archiveuploader/ftests/nascentupload-closing-bugs.txt).
32
33 Skip bug-closing if the upload is target to pocket PROPOSED or if
34 the upload is for a PPA.
35@@ -172,7 +175,7 @@
36 content = (
37 "This bug was fixed in the package %s"
38 "\n\n---------------\n%s" % (
39- source_release.title, source_release.changelog_entry,))
40+ source_release.title, source_release.changelog_entry))
41 bug.newMessage(
42 owner=janitor,
43 subject=bug.followup_subject(),
44@@ -286,4 +289,3 @@
45 self.txn.abort()
46
47 return 0
48-
49
50=== modified file 'lib/lp/soyuz/tests/test_processaccepted.py'
51--- lib/lp/soyuz/tests/test_processaccepted.py 2010-08-24 15:29:01 +0000
52+++ lib/lp/soyuz/tests/test_processaccepted.py 2010-09-10 13:29:46 +0000
53@@ -5,6 +5,8 @@
54
55 from cStringIO import StringIO
56
57+from debian.deb822 import Changes
58+
59 from canonical.config import config
60 from canonical.launchpad.scripts import QuietFakeLogger
61 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
62@@ -14,7 +16,10 @@
63 ArchivePurpose,
64 PackagePublishingStatus,
65 )
66-from lp.soyuz.scripts.processaccepted import ProcessAccepted
67+from lp.soyuz.scripts.processaccepted import (
68+ get_bugs_from_changes_file,
69+ ProcessAccepted,
70+ )
71 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
72 from lp.testing import TestCaseWithFactory
73
74@@ -42,8 +47,8 @@
75 script.txn = self.layer.txn
76 return script
77
78- def createWaitingAcceptancePackage(self, distroseries, archive=None,
79- sourcename=None):
80+ def createWaitingAcceptancePackage(self, distroseries, archive=None,
81+ sourcename=None):
82 """Create some pending publications."""
83 if archive is None:
84 archive = self.distro.main_archive
85@@ -54,11 +59,11 @@
86 spr_only=True)
87
88 def test_robustness(self):
89- """Test that a broken package doesn't block the publication of other
90+ """Test that a broken package doesn't block the publication of other
91 packages."""
92 # Attempt to upload one source to a supported series
93 # The record is created first and then the status of the series
94- # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible
95+ # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible
96 # to create the record
97 distroseries = self.factory.makeDistroSeries(distribution=self.distro)
98 broken_source = self.createWaitingAcceptancePackage(
99@@ -85,7 +90,7 @@
100 fp = StringIO()
101 error_report.write(fp)
102 error_text = fp.getvalue()
103- expected_error = "error-explanation=Failure processing queue_item"
104+ expected_error = "error-explanation=Failure processing queue_item"
105 self.assertTrue(expected_error in error_text)
106
107 def test_accept_copy_archives(self):
108@@ -126,3 +131,60 @@
109 published_copy.status, PackagePublishingStatus.PENDING)
110 self.assertEqual(copy_source, published_copy.sourcepackagerelease)
111
112+
113+class TestBugsFromChangesFile(TestCaseWithFactory):
114+ """Test get_bugs_from_changes_file."""
115+
116+ layer = LaunchpadZopelessLayer
117+ dbuser = config.uploadqueue.dbuser
118+
119+ def setUp(self):
120+ super(TestBugsFromChangesFile, self).setUp()
121+ self.changes = Changes({
122+ 'Format': '1.8',
123+ 'Source': 'swat',
124+ })
125+
126+ def getBugs(self):
127+ """Serialize self.changes and use get_bugs_from_changes_file to
128+ extract bugs from it.
129+ """
130+ stream = StringIO()
131+ self.changes.dump(stream)
132+ stream.seek(0)
133+ return get_bugs_from_changes_file(stream)
134+
135+ def test_no_bugs(self):
136+ # An empty list is returned if there are no bugs
137+ # mentioned.
138+ self.assertEquals([], self.getBugs())
139+
140+ def test_invalid_bug_id(self):
141+ # Invalid bug ids (i.e. containing non-digit characters) are ignored.
142+ self.changes["Launchpad-Bugs-Fixed"] = "bla"
143+ self.assertEquals([], self.getBugs())
144+
145+ def test_unknown_bug_id(self):
146+ # Unknown bug ids are ignored.
147+ self.changes["Launchpad-Bugs-Fixed"] = "45120"
148+ self.assertEquals([], self.getBugs())
149+
150+ def test_valid_bug(self):
151+ # For valid bug ids the bug object is returned.
152+ bug = self.factory.makeBug()
153+ self.changes["Launchpad-Bugs-Fixed"] = "%d" % bug.id
154+ self.assertEquals([bug], self.getBugs())
155+
156+ def test_case_sensitivity(self):
157+ # The spelling of Launchpad-Bugs-Fixed is case-insensitive.
158+ bug = self.factory.makeBug()
159+ self.changes["LaUnchpad-Bugs-fixed"] = "%d" % bug.id
160+ self.assertEquals([bug], self.getBugs())
161+
162+ def test_multiple_bugs(self):
163+ # Multiple bug ids can be specified, separated by spaces.
164+ bug1 = self.factory.makeBug()
165+ bug2 = self.factory.makeBug()
166+ self.changes["Launchpad-Bugs-Fixed"] = "%d invalid %d" % (
167+ bug1.id, bug2.id)
168+ self.assertEquals([bug1, bug2], self.getBugs())