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

[Jelmer Vernooij]

Parse Launchpad-Bugs-Fixed regardless of casing.

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

[Abel Deuring (adeuring)]

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)

=== 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 @@
     'close_bugs',
     'close_bugs_for_queue_item',
     'close_bugs_for_sourcepublication',
+    'get_bugs_from_changes_file',
     'ProcessAccepted',
     ]
 
+from debian.deb822 import Deb822Dict
 import sys
 
 from zope.component import getUtility
@@ -47,7 +49,7 @@
     """
     contents = changes_file.read()
     changes_lines = contents.splitlines(True)
-    tags = parse_tagfile_lines(changes_lines, allow_unsigned=True)
+    tags = Deb822Dict(parse_tagfile_lines(changes_lines, allow_unsigned=True))
     bugs_fixed_line = tags.get('Launchpad-bugs-fixed', '')
     bugs = []
     for bug_id in bugs_fixed_line.split():
@@ -105,7 +107,8 @@
     the upload is processed and committed.
 
     In practice, 'changesfile_object' is only set when we are closing bugs
-    in upload-time (see archiveuploader/ftests/nascentupload-closing-bugs.txt).
+    in upload-time (see
+    archiveuploader/ftests/nascentupload-closing-bugs.txt).
 
     Skip bug-closing if the upload is target to pocket PROPOSED or if
     the upload is for a PPA.
@@ -172,7 +175,7 @@
             content = (
                 "This bug was fixed in the package %s"
                 "\n\n---------------\n%s" % (
-                source_release.title, source_release.changelog_entry,))
+                source_release.title, source_release.changelog_entry))
             bug.newMessage(
                 owner=janitor,
                 subject=bug.followup_subject(),
@@ -286,4 +289,3 @@
             self.txn.abort()
 
         return 0
-

=== 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 @@
 
 from cStringIO import StringIO
 
+from debian.deb822 import Changes
+
 from canonical.config import config
 from canonical.launchpad.scripts import QuietFakeLogger
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
@@ -14,7 +16,10 @@
     ArchivePurpose,
     PackagePublishingStatus,
     )
-from lp.soyuz.scripts.processaccepted import ProcessAccepted
+from lp.soyuz.scripts.processaccepted import (
+    get_bugs_from_changes_file,
+    ProcessAccepted,
+    )
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import TestCaseWithFactory
 
@@ -42,8 +47,8 @@
         script.txn = self.layer.txn
         return script
 
-    def createWaitingAcceptancePackage(self, distroseries, archive=None, 
-            sourcename=None):
+    def createWaitingAcceptancePackage(self, distroseries, archive=None,
+                                       sourcename=None):
         """Create some pending publications."""
         if archive is None:
             archive = self.distro.main_archive
@@ -54,11 +59,11 @@
             spr_only=True)
 
     def test_robustness(self):
-        """Test that a broken package doesn't block the publication of other 
+        """Test that a broken package doesn't block the publication of other
         packages."""
         # Attempt to upload one source to a supported series
         # The record is created first and then the status of the series
-        # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible 
+        # is changed from DEVELOPMENT to SUPPORTED, otherwise it's impossible
         # to create the record
         distroseries = self.factory.makeDistroSeries(distribution=self.distro)
         broken_source = self.createWaitingAcceptancePackage(
@@ -85,7 +90,7 @@
         fp = StringIO()
         error_report.write(fp)
         error_text = fp.getvalue()
-        expected_error = "error-explanation=Failure processing queue_item" 
+        expected_error = "error-explanation=Failure processing queue_item"
         self.assertTrue(expected_error in error_text)
 
     def test_accept_copy_archives(self):
@@ -126,3 +131,60 @@
             published_copy.status, PackagePublishingStatus.PENDING)
         self.assertEqual(copy_source, published_copy.sourcepackagerelease)
 
+
+class TestBugsFromChangesFile(TestCaseWithFactory):
+    """Test get_bugs_from_changes_file."""
+
+    layer = LaunchpadZopelessLayer
+    dbuser = config.uploadqueue.dbuser
+
+    def setUp(self):
+        super(TestBugsFromChangesFile, self).setUp()
+        self.changes = Changes({
+            'Format': '1.8',
+            'Source': 'swat',
+            })
+
+    def getBugs(self):
+        """Serialize self.changes and use get_bugs_from_changes_file to
+        extract bugs from it.
+        """
+        stream = StringIO()
+        self.changes.dump(stream)
+        stream.seek(0)
+        return get_bugs_from_changes_file(stream)
+
+    def test_no_bugs(self):
+        # An empty list is returned if there are no bugs
+        # mentioned.
+        self.assertEquals([], self.getBugs())
+
+    def test_invalid_bug_id(self):
+        # Invalid bug ids (i.e. containing non-digit characters) are ignored.
+        self.changes["Launchpad-Bugs-Fixed"] = "bla"
+        self.assertEquals([], self.getBugs())
+
+    def test_unknown_bug_id(self):
+        # Unknown bug ids are ignored.
+        self.changes["Launchpad-Bugs-Fixed"] = "45120"
+        self.assertEquals([], self.getBugs())
+
+    def test_valid_bug(self):
+        # For valid bug ids the bug object is returned.
+        bug = self.factory.makeBug()
+        self.changes["Launchpad-Bugs-Fixed"] = "%d" % bug.id
+        self.assertEquals([bug], self.getBugs())
+
+    def test_case_sensitivity(self):
+        # The spelling of Launchpad-Bugs-Fixed is case-insensitive.
+        bug = self.factory.makeBug()
+        self.changes["LaUnchpad-Bugs-fixed"] = "%d" % bug.id
+        self.assertEquals([bug], self.getBugs())
+
+    def test_multiple_bugs(self):
+        # Multiple bug ids can be specified, separated by spaces.
+        bug1 = self.factory.makeBug()
+        bug2 = self.factory.makeBug()
+        self.changes["Launchpad-Bugs-Fixed"] = "%d invalid %d" % (
+            bug1.id, bug2.id)
+        self.assertEquals([bug1, bug2], self.getBugs())