launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00363
[Merge] lp:~wgrant/launchpad/test-ddeb-matching into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/test-ddeb-matching into lp:launchpad with lp:~wgrant/launchpad/refactor-nuf-creation as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch factors out and adds unit tests for the DDEB-matching part of NascentUpload.
To make the tests less foul, I've altered NascentUpload to allow passing in an existing ChangesFile-like object, rather than requiring a path to a real file. This should eventually let us shrink existing tests throughout archiveuploader, and even remove test package data.
I've left the existing basic end-to-end test at the end of nascentupload-ddebs.txt, to ensure that detected errors do in fact cause rejection and not terrible explosions.
--
https://code.launchpad.net/~wgrant/launchpad/test-ddeb-matching/+merge/31482
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/test-ddeb-matching into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py 2010-08-01 06:40:41 +0000
+++ lib/lp/archiveuploader/nascentupload.py 2010-08-01 08:28:55 +0000
@@ -85,14 +85,13 @@
# Defined if we successfully do_accept() and storeObjectsInDatabase()
queue_root = None
- def __init__(self, changesfile_path, policy, logger):
- """Setup a ChangesFile based on given changesfile path.
+ def __init__(self, changesfile, policy, logger):
+ """Setup a ChangesFile based on given changesfile or path.
May raise FatalUploadError due to unrecoverable problems building
the ChangesFile object.
Also store given and initialized Upload Policy, as 'policy'
"""
- self.changesfile_path = changesfile_path
self.policy = policy
self.logger = logger
@@ -100,17 +99,24 @@
self.warnings = []
self.librarian = getUtility(ILibraryFileAliasSet)
- try:
- self.changes = ChangesFile(
- changesfile_path, self.policy, self.logger)
- except UploadError, e:
- # We can't run reject() because unfortunately we don't have
- # the address of the uploader to notify -- we broke in that
- # exact step.
- # XXX cprov 2007-03-26: we should really be emailing this
- # rejection to the archive admins. For now, this will end
- # up in the script log.
- raise FatalUploadError(str(e))
+
+ # If a path is given, a new ChangesFile object will be
+ # constructed. Otherwise we use the given object.
+ if isinstance(changesfile, str):
+ self.changesfile_path = changesfile
+ try:
+ self.changes = ChangesFile(
+ changesfile, self.policy, self.logger)
+ except UploadError, e:
+ # We can't run reject() because unfortunately we don't have
+ # the address of the uploader to notify -- we broke in that
+ # exact step.
+ # XXX cprov 2007-03-26: we should really be emailing this
+ # rejection to the archive admins. For now, this will end
+ # up in the script log.
+ raise FatalUploadError(str(e))
+ else:
+ self.changes = changesfile
def process(self):
"""Process this upload, checking it against policy, loading it into
@@ -147,6 +153,7 @@
self._check_sourceful_consistency()
if self.binaryful:
self._check_binaryful_consistency()
+ self.run_and_collect_errors(self._matchDDEBs)
self.run_and_collect_errors(self.changes.verify)
@@ -154,35 +161,6 @@
for uploaded_file in self.changes.files:
self.run_and_collect_errors(uploaded_file.verify)
- unmatched_ddebs = {}
- for uploaded_file in self.changes.files:
- if isinstance(uploaded_file, DdebBinaryUploadFile):
- ddeb_key = (uploaded_file.package, uploaded_file.version,
- uploaded_file.architecture)
- if ddeb_key in unmatched_ddebs:
- self.reject("Duplicated debug packages: %s %s (%s)" %
- ddeb_key)
- else:
- unmatched_ddebs[ddeb_key] = uploaded_file
-
- for uploaded_file in self.changes.files:
- # We need exactly a DEB, not a DDEB.
- if (isinstance(uploaded_file, DebBinaryUploadFile) and
- not isinstance(uploaded_file, DdebBinaryUploadFile)):
- try:
- matching_ddeb = unmatched_ddebs.pop(
- (uploaded_file.package + '-dbgsym',
- uploaded_file.version,
- uploaded_file.architecture))
- except KeyError:
- continue
- uploaded_file.ddeb_file = matching_ddeb
- matching_ddeb.deb_file = uploaded_file
-
- if len(unmatched_ddebs) > 0:
- self.reject("Orphaned debug packages: %s" % ', '.join('%s %s (%s)' % d
- for d in unmatched_ddebs))
-
if (len(self.changes.files) == 1 and
isinstance(self.changes.files[0], CustomUploadFile)):
self.logger.debug("Single Custom Upload detected.")
@@ -328,7 +306,8 @@
"""Heuristic checks on a sourceful upload.
Raises AssertionError when called for a non-sourceful upload.
- Ensures a sourceful upload has exactly one DSC.
+ Ensures a sourceful upload has exactly one DSC. All further source
+ checks are performed later by the DSC.
"""
assert self.sourceful, (
"Source consistency check called for a non-source upload")
@@ -368,6 +347,37 @@
if len(considered_archs) > max:
self.reject("Upload has more architetures than it is supported.")
+ def _matchDDEBs(self):
+ unmatched_ddebs = {}
+ for uploaded_file in self.changes.files:
+ if isinstance(uploaded_file, DdebBinaryUploadFile):
+ ddeb_key = (uploaded_file.package, uploaded_file.version,
+ uploaded_file.architecture)
+ if ddeb_key in unmatched_ddebs:
+ yield UploadError(
+ "Duplicated debug packages: %s %s (%s)" % ddeb_key)
+ else:
+ unmatched_ddebs[ddeb_key] = uploaded_file
+
+ for uploaded_file in self.changes.files:
+ # We need exactly a DEB, not a DDEB.
+ if (isinstance(uploaded_file, DebBinaryUploadFile) and
+ not isinstance(uploaded_file, DdebBinaryUploadFile)):
+ try:
+ matching_ddeb = unmatched_ddebs.pop(
+ (uploaded_file.package + '-dbgsym',
+ uploaded_file.version,
+ uploaded_file.architecture))
+ except KeyError:
+ continue
+ uploaded_file.ddeb_file = matching_ddeb
+ matching_ddeb.deb_file = uploaded_file
+
+ if len(unmatched_ddebs) > 0:
+ yield UploadError(
+ "Orphaned debug packages: %s" % ', '.join(
+ '%s %s (%s)' % d for d in unmatched_ddebs))
+
#
# Helpers for warnings and rejections
#
=== added file 'lib/lp/archiveuploader/tests/test_nascentupload.py'
--- lib/lp/archiveuploader/tests/test_nascentupload.py 1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/test_nascentupload.py 2010-08-01 08:28:55 +0000
@@ -0,0 +1,87 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test NascentUpload functionality."""
+
+__metaclass__ = type
+
+from testtools import TestCase
+
+from canonical.testing import LaunchpadZopelessLayer
+from lp.archiveuploader.changesfile import determine_file_class_and_name
+from lp.archiveuploader.nascentupload import NascentUpload
+from lp.archiveuploader.tests import mock_logger
+
+
+def get_error_text(error_list):
+ return [str(e) for e in error_list]
+
+
+class FakeChangesFile:
+
+ def __init__(self):
+ self.files = []
+
+
+class TestMatchDDEBs(TestCase):
+ """Tests that NascentUpload correctly links DEBs to their DDEBs.
+
+ Also verifies detection of DDEB-related error cases.
+ """
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestMatchDDEBs, self).setUp()
+ self.changes = FakeChangesFile()
+ self.upload = NascentUpload(self.changes, None, mock_logger)
+
+ def addFile(self, filename):
+ """Add a file of the right type to the upload."""
+ package, cls = determine_file_class_and_name(filename)
+ file = cls(
+ filename, None, 100, 'devel', 'extra', package, '666',
+ self.changes, None, self.upload.logger)
+ self.changes.files.append(file)
+
+ def testNoBinaries(self):
+ # No links will be made if there are no binaries whatsoever.
+ self.addFile('something_1.0.diff.gz')
+ self.assertEquals([], list(self.upload._matchDDEBs()))
+
+ def testJustDEBs(self):
+ # No links will be made if there are no DDEBs.
+ self.addFile('blah_1.0_all.deb')
+ self.addFile('libblah_1.0_i386.deb')
+ self.assertEquals([], list(self.upload._matchDDEBs()))
+ for file in self.changes.files:
+ self.assertIs(None, file.ddeb_file)
+
+ def testMatchingDDEB(self):
+ # DDEBs will be linked to their matching DEBs.
+ self.addFile('blah_1.0_all.deb')
+ self.addFile('libblah_1.0_i386.deb')
+ self.addFile('libblah-dbgsym_1.0_i386.ddeb')
+ self.assertEquals([], list(self.upload._matchDDEBs()))
+ self.assertIs(None, self.changes.files[0].ddeb_file)
+ self.assertIs(self.changes.files[2], self.changes.files[1].ddeb_file)
+ self.assertIs(self.changes.files[1], self.changes.files[2].deb_file)
+ self.assertIs(None, self.changes.files[2].ddeb_file)
+
+ def testDuplicateDDEB(self):
+ # An error will be raised if a DEB has more than one matching
+ # DDEB.
+ self.addFile('libblah_1.0_i386.deb')
+ self.addFile('libblah-dbgsym_1.0_i386.ddeb')
+ self.addFile('libblah-dbgsym_1.0_i386.ddeb')
+ self.assertEquals(
+ ['Duplicated debug packages: libblah-dbgsym 666 (i386)'],
+ get_error_text(self.upload._matchDDEBs()))
+
+ def testMismatchedDDEB(self):
+ # An error will be raised if a DDEB has no matching DEB.
+ self.addFile('libblah_1.0_i386.deb')
+ self.addFile('libblah-dbgsym_1.0_amd64.ddeb')
+ self.assertEquals(
+ ['Orphaned debug packages: libblah-dbgsym 666 (amd64)'],
+ get_error_text(self.upload._matchDDEBs()))
