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
=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py 2010-08-27 12:21:25 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py 2010-09-10 13:29:46 +0000
@@ -8,9 +8,11 @@
8 'close_bugs',8 'close_bugs',
9 'close_bugs_for_queue_item',9 'close_bugs_for_queue_item',
10 'close_bugs_for_sourcepublication',10 'close_bugs_for_sourcepublication',
11 'get_bugs_from_changes_file',
11 'ProcessAccepted',12 'ProcessAccepted',
12 ]13 ]
1314
15from debian.deb822 import Deb822Dict
14import sys16import sys
1517
16from zope.component import getUtility18from zope.component import getUtility
@@ -47,7 +49,7 @@
47 """49 """
48 contents = changes_file.read()50 contents = changes_file.read()
49 changes_lines = contents.splitlines(True)51 changes_lines = contents.splitlines(True)
50 tags = parse_tagfile_lines(changes_lines, allow_unsigned=True)52 tags = Deb822Dict(parse_tagfile_lines(changes_lines, allow_unsigned=True))
51 bugs_fixed_line = tags.get('Launchpad-bugs-fixed', '')53 bugs_fixed_line = tags.get('Launchpad-bugs-fixed', '')
52 bugs = []54 bugs = []
53 for bug_id in bugs_fixed_line.split():55 for bug_id in bugs_fixed_line.split():
@@ -105,7 +107,8 @@
105 the upload is processed and committed.107 the upload is processed and committed.
106108
107 In practice, 'changesfile_object' is only set when we are closing bugs109 In practice, 'changesfile_object' is only set when we are closing bugs
108 in upload-time (see archiveuploader/ftests/nascentupload-closing-bugs.txt).110 in upload-time (see
111 archiveuploader/ftests/nascentupload-closing-bugs.txt).
109112
110 Skip bug-closing if the upload is target to pocket PROPOSED or if113 Skip bug-closing if the upload is target to pocket PROPOSED or if
111 the upload is for a PPA.114 the upload is for a PPA.
@@ -172,7 +175,7 @@
172 content = (175 content = (
173 "This bug was fixed in the package %s"176 "This bug was fixed in the package %s"
174 "\n\n---------------\n%s" % (177 "\n\n---------------\n%s" % (
175 source_release.title, source_release.changelog_entry,))178 source_release.title, source_release.changelog_entry))
176 bug.newMessage(179 bug.newMessage(
177 owner=janitor,180 owner=janitor,
178 subject=bug.followup_subject(),181 subject=bug.followup_subject(),
@@ -286,4 +289,3 @@
286 self.txn.abort()289 self.txn.abort()
287290
288 return 0291 return 0
289
290292
=== modified file 'lib/lp/soyuz/tests/test_processaccepted.py'
--- lib/lp/soyuz/tests/test_processaccepted.py 2010-08-24 15:29:01 +0000
+++ lib/lp/soyuz/tests/test_processaccepted.py 2010-09-10 13:29:46 +0000
@@ -5,6 +5,8 @@
55
6from cStringIO import StringIO6from cStringIO import StringIO
77
8from debian.deb822 import Changes
9
8from canonical.config import config10from canonical.config import config
9from canonical.launchpad.scripts import QuietFakeLogger11from canonical.launchpad.scripts import QuietFakeLogger
10from canonical.launchpad.webapp.errorlog import ErrorReportingUtility12from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
@@ -14,7 +16,10 @@
14 ArchivePurpose,16 ArchivePurpose,
15 PackagePublishingStatus,17 PackagePublishingStatus,
16 )18 )
17from lp.soyuz.scripts.processaccepted import ProcessAccepted19from lp.soyuz.scripts.processaccepted import (
20 get_bugs_from_changes_file,
21 ProcessAccepted,
22 )
18from lp.soyuz.tests.test_publishing import SoyuzTestPublisher23from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
19from lp.testing import TestCaseWithFactory24from lp.testing import TestCaseWithFactory
2025
@@ -42,8 +47,8 @@
42 script.txn = self.layer.txn47 script.txn = self.layer.txn
43 return script48 return script
4449
45 def createWaitingAcceptancePackage(self, distroseries, archive=None, 50 def createWaitingAcceptancePackage(self, distroseries, archive=None,
46 sourcename=None):51 sourcename=None):
47 """Create some pending publications."""52 """Create some pending publications."""
48 if archive is None:53 if archive is None:
49 archive = self.distro.main_archive54 archive = self.distro.main_archive
@@ -54,11 +59,11 @@
54 spr_only=True)59 spr_only=True)
5560
56 def test_robustness(self):61 def test_robustness(self):
57 """Test that a broken package doesn't block the publication of other 62 """Test that a broken package doesn't block the publication of other
58 packages."""63 packages."""
59 # Attempt to upload one source to a supported series64 # Attempt to upload one source to a supported series
60 # The record is created first and then the status of the series65 # The record is created first and then the status of the series
61 # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible 66 # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible
62 # to create the record67 # to create the record
63 distroseries = self.factory.makeDistroSeries(distribution=self.distro)68 distroseries = self.factory.makeDistroSeries(distribution=self.distro)
64 broken_source = self.createWaitingAcceptancePackage(69 broken_source = self.createWaitingAcceptancePackage(
@@ -85,7 +90,7 @@
85 fp = StringIO()90 fp = StringIO()
86 error_report.write(fp)91 error_report.write(fp)
87 error_text = fp.getvalue()92 error_text = fp.getvalue()
88 expected_error = "error-explanation=Failure processing queue_item" 93 expected_error = "error-explanation=Failure processing queue_item"
89 self.assertTrue(expected_error in error_text)94 self.assertTrue(expected_error in error_text)
9095
91 def test_accept_copy_archives(self):96 def test_accept_copy_archives(self):
@@ -126,3 +131,60 @@
126 published_copy.status, PackagePublishingStatus.PENDING)131 published_copy.status, PackagePublishingStatus.PENDING)
127 self.assertEqual(copy_source, published_copy.sourcepackagerelease)132 self.assertEqual(copy_source, published_copy.sourcepackagerelease)
128133
134
135class TestBugsFromChangesFile(TestCaseWithFactory):
136 """Test get_bugs_from_changes_file."""
137
138 layer = LaunchpadZopelessLayer
139 dbuser = config.uploadqueue.dbuser
140
141 def setUp(self):
142 super(TestBugsFromChangesFile, self).setUp()
143 self.changes = Changes({
144 'Format': '1.8',
145 'Source': 'swat',
146 })
147
148 def getBugs(self):
149 """Serialize self.changes and use get_bugs_from_changes_file to
150 extract bugs from it.
151 """
152 stream = StringIO()
153 self.changes.dump(stream)
154 stream.seek(0)
155 return get_bugs_from_changes_file(stream)
156
157 def test_no_bugs(self):
158 # An empty list is returned if there are no bugs
159 # mentioned.
160 self.assertEquals([], self.getBugs())
161
162 def test_invalid_bug_id(self):
163 # Invalid bug ids (i.e. containing non-digit characters) are ignored.
164 self.changes["Launchpad-Bugs-Fixed"] = "bla"
165 self.assertEquals([], self.getBugs())
166
167 def test_unknown_bug_id(self):
168 # Unknown bug ids are ignored.
169 self.changes["Launchpad-Bugs-Fixed"] = "45120"
170 self.assertEquals([], self.getBugs())
171
172 def test_valid_bug(self):
173 # For valid bug ids the bug object is returned.
174 bug = self.factory.makeBug()
175 self.changes["Launchpad-Bugs-Fixed"] = "%d" % bug.id
176 self.assertEquals([bug], self.getBugs())
177
178 def test_case_sensitivity(self):
179 # The spelling of Launchpad-Bugs-Fixed is case-insensitive.
180 bug = self.factory.makeBug()
181 self.changes["LaUnchpad-Bugs-fixed"] = "%d" % bug.id
182 self.assertEquals([bug], self.getBugs())
183
184 def test_multiple_bugs(self):
185 # Multiple bug ids can be specified, separated by spaces.
186 bug1 = self.factory.makeBug()
187 bug2 = self.factory.makeBug()
188 self.changes["Launchpad-Bugs-Fixed"] = "%d invalid %d" % (
189 bug1.id, bug2.id)
190 self.assertEquals([bug1, bug2], self.getBugs())