Launchpad itself

Merge lp:~jelmer/launchpad/nascentuploadfile-tests into lp:launchpad

nascentuploadfile-tests
Merge into devel

Proposed by Jelmer Vernooij on 2010-08-17

Status:

Merged

Approved by:

Graham Binns on 2010-08-18

Approved revision:

no longer in the source branch.

Merged at revision:

11400

Proposed branch:

lp:~jelmer/launchpad/nascentuploadfile-tests

Merge into:

lp:launchpad

Diff against target:

493 lines (+395/-13)

5 files modified

lib/lp/archiveuploader/changesfile.py (+11/-3)
lib/lp/archiveuploader/dscfile.py (+1/-0)
lib/lp/archiveuploader/nascentuploadfile.py (+6/-4)
lib/lp/archiveuploader/tests/test_changesfile.py (+134/-6)
lib/lp/archiveuploader/tests/test_nascentuploadfile.py (+243/-0)

To merge this branch:

bzr merge lp:~jelmer/launchpad/nascentuploadfile-tests

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Graham Binns (community)	code	2010-08-17	Approve on 2010-08-18
Review via email: mp+32874@code.launchpad.net

Commit message

Add more tests for ChangesFile, DebBinaryUploadFile, DSCFile, CustomUploadFile in the archiveuploader. Make Changed-By and Date field mandatory in ChangesFile.

Description of the change

This branch adds a bunch of tests and testinfrastructure for ChangesFile, DSCFile and two of the classes in nascentuploadfile. I'm adding these tests in preparation of adding some more fields (bug 613468).

While writing the tests I noticed that we seem to unconditionally require the "Changed-By" and "Date" files in the Changes file but don't reject uploads with these fields, so I've added them to the list of mandatory fields (bug filed & linked).

Revision history for this message

Graham Binns (gmb) wrote on 2010-08-17:

Download full text (18.3 KiB)

Hi Jelmer,

Thanks for adding tests; always a good thing.

This is a good branch, but it suffers from a lot of issues w.r.t our
coding standards. All of the tests should have a leading comment
explaining the behaviour that they test. Also, the formatting of
callsites is inconsistent and should be changed to match our standards.

> === modified file 'lib/lp/archiveuploader/changesfile.py'
> --- lib/lp/archiveuploader/changesfile.py 2010-08-01 06:59:04 +0000
> +++ lib/lp/archiveuploader/changesfile.py 2010-08-17 13:21:05 +0000
> @@ -148,7 +152,7 @@
> def isCustom(self, component_and_section):
> """Check if given 'component_and_section' matches a custom upload.
>
> - We recognize an upload as custom if it is taget to a section like
> + We recognize an upload as custom if it is targetted at a section like

Targeted has only two 't's.

> 'raw-<something>'.
> Further checks will be performed in CustomUploadFile class.
> """
> @@ -215,6 +219,10 @@
> if len(self.files) == 0:
> yield UploadError("No files found in the changes")
>
> + if not 'urgency' in self._dict:

This should be expressed as:

if 'urgency' not in self._dict:

The current condition confused me.

> + # Urgency is recommended but not mandatory. Default to 'low'
> + self._dict['urgency'] = "low"
> +
> raw_urgency = self._dict['urgency'].lower()
> if raw_urgency not in self.urgency_map:
> yield UploadWarning(
>
> === modified file 'lib/lp/archiveuploader/tests/test_changesfile.py'
> --- lib/lp/archiveuploader/tests/test_changesfile.py 2010-08-01 06:59:04 +0000
> +++ lib/lp/archiveuploader/tests/test_changesfile.py 2010-08-17 13:21:05 +0000
> @@ -5,14 +5,20 @@
>
> __metaclass__ = type
>
> -from testtools import TestCase
> +from debian.deb822 import Changes
> +import os
> +
> +from canonical.launchpad.scripts.logger import BufferLogger
> +from canonical.testing import LaunchpadZopelessLayer
>
> from lp.archiveuploader.changesfile import (CannotDetermineFileTypeError,
> - determine_file_class_and_name)
> + ChangesFile, determine_file_class_and_name)

Since you're here, you can tidy this up into the new one-line-per-import
format.

> from lp.archiveuploader.dscfile import DSCFile
> from lp.archiveuploader.nascentuploadfile import (
> DebBinaryUploadFile, DdebBinaryUploadFile, SourceUploadFile,
> - UdebBinaryUploadFile)
> + UdebBinaryUploadFile, UploadError)

Same here.

Hi Jelmer,

Thanks for adding tests; always a good thing.

> === modified file 'lib/lp/archiveuploader/changesfile.py'
> --- lib/lp/archiveuploader/changesfile.py	2010-08-01 06:59:04 +0000
> +++ lib/lp/archiveuploader/changesfile.py	2010-08-17 13:21:05 +0000
> @@ -148,7 +152,7 @@
>      def isCustom(self, component_and_section):
>          """Check if given 'component_and_section' matches a custom upload.
>  
> -        We recognize an upload as custom if it is taget to a section like
> +        We recognize an upload as custom if it is targetted at a section like

Targeted has only two 't's.

>          'raw-<something>'.
>          Further checks will be performed in CustomUploadFile class.
>          """
> @@ -215,6 +219,10 @@
>          if len(self.files) == 0:
>              yield UploadError("No files found in the changes")
>  
> +        if not 'urgency' in self._dict:

This should be expressed as:

if 'urgency' not in self._dict:

The current condition confused me.

> +            # Urgency is recommended but not mandatory. Default to 'low'
> +            self._dict['urgency'] = "low"
> +
>          raw_urgency = self._dict['urgency'].lower()
>          if raw_urgency not in self.urgency_map:
>              yield UploadWarning(
>
> === modified file 'lib/lp/archiveuploader/tests/test_changesfile.py'
> --- lib/lp/archiveuploader/tests/test_changesfile.py	2010-08-01 06:59:04 +0000
> +++ lib/lp/archiveuploader/tests/test_changesfile.py	2010-08-17 13:21:05 +0000
> @@ -5,14 +5,20 @@
>  
>  __metaclass__ = type
>  
> -from testtools import TestCase
> +from debian.deb822 import Changes
> +import os
> +
> +from canonical.launchpad.scripts.logger import BufferLogger
> +from canonical.testing import LaunchpadZopelessLayer
>  
>  from lp.archiveuploader.changesfile import (CannotDetermineFileTypeError,
> -    determine_file_class_and_name)
> +    ChangesFile, determine_file_class_and_name)

Since you're here, you can tidy this up into the new one-line-per-import
format.

>  from lp.archiveuploader.dscfile import DSCFile
>  from lp.archiveuploader.nascentuploadfile import (
>      DebBinaryUploadFile, DdebBinaryUploadFile, SourceUploadFile,
> -    UdebBinaryUploadFile)
> +    UdebBinaryUploadFile, UploadError)

Same here.

> +from lp.archiveuploader.uploadpolicy import AbsolutelyAnythingGoesUploadPolicy
> +from lp.testing import TestCase
>  
>  
>  class TestDetermineFileClassAndName(TestCase):
> @@ -52,3 +58,104 @@
>              CannotDetermineFileTypeError,
>              determine_file_class_and_name,
>              'foo')
> +
> +
> +class ChangesFileTests(TestCase):
> +    """Tests for ChangesFile."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def setUp(self):
> +        super(ChangesFileTests, self).setUp()
> +        self.logger = BufferLogger()
> +        self.policy = AbsolutelyAnythingGoesUploadPolicy()
> +
> +    def createChangesFile(self, filename, changes):
> +        tempdir = self.makeTemporaryDirectory()
> +        path = os.path.join(tempdir, filename)
> +        changes_fd = open(path, "w")
> +        try:
> +            changes.dump(changes_fd)
> +        finally:
> +            changes_fd.close()
> +        return ChangesFile(path, self.policy, self.logger)
> +
> +    def getBaseChanges(self):
> +        contents = Changes()
> +        contents["Source"] = "mypkg"
> +        contents["Binary"] = "binary"
> +        contents["Date"] = "Fri, 25 Jun 2010 11:20:22 -0600"
> +        contents["Architecture"] = "i386"
> +        contents["Version"] = "0.1"
> +        contents["Distribution"] = "nifty"
> +        contents["Maintainer"] = "Somebody"
> +        contents["Changes"] = "Something changed"
> +        contents["Description"] = "\n An awesome package."
> +        contents["Changed-By"] = "Somebody <somebody@ubuntu.com>"
> +        contents["Files"] = [{
> +            "md5sum": "d2bd347b3fed184fe28e112695be491c",
> +            "size": "1791",
> +            "section": "python",
> +            "priority": "optional",
> +            "name": "dulwich_0.4.1-1.dsc"}]
> +        return contents
> +
> +    def test_checkFileName(self):

This test needs a comment listing the expected behaviour it tests (e.g.
"# If the frobnob is poked it goes 'bloop'" at the start.

> +        contents = self.getBaseChanges()
> +        changes = self.createChangesFile("mypkg_0.1_i386.changes", contents)
> +        self.assertEquals([], list(changes.checkFileName()))
> +        changes = self.createChangesFile("mypkg_0.1.changes", contents)
> +        errors = list(changes.checkFileName())
> +        self.assertIsInstance(errors[0], UploadError)
> +        self.assertEquals(1, len(errors))
> +
> +    def test_filename(self):

Here too.

> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            self.getBaseChanges())
> +        self.assertEquals("mypkg_0.1_i386.changes", changes.filename)
> +
> +    def test_suite_name(self):

And here.

> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            self.getBaseChanges())

This should be formatted as

changes = self.createChangesFile(
            "mypkg_0.1_i386.changes",
            self.getBaseChanges())

> +        self.assertEquals("nifty", changes.suite_name)
> +
> +    def test_version(self):

Comment needed.

> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            self.getBaseChanges())

Formatting as above.

> +        self.assertEquals("0.1", changes.version)
> +
> +    def test_architectures(self):

Comment needed.

> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            self.getBaseChanges())

Formatting.

> +        self.assertEquals("i386", changes.architecture_line)
> +        self.assertEquals(set(["i386"]), changes.architectures)
> +
> +    def test_source(self):

Comment needed.

> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            self.getBaseChanges())

Formatting.

> +        self.assertEquals("mypkg", changes.source)
> +
> +    def test_processAddresses(self):

Comment needed.

> +        contents = self.getBaseChanges()
> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            contents)

Formatting.

> +        self.assertEquals(None, changes.changed_by)
> +        errors = list(changes.processAddresses())
> +        self.assertEquals(0, len(errors), "Errors: %r" % errors)
> +        self.assertEquals("Somebody <somebody@ubuntu.com>",
> +            changes.changed_by['rfc822'])

Formatting.

> +
> +    def test_simulated_changelog(self):

Comment.

> +        contents = self.getBaseChanges()
> +        changes = self.createChangesFile("mypkg_0.1_i386.changes",
> +            contents)

Formatting.

> +        self.assertEquals([], list(changes.processAddresses()))
> +        self.assertEquals("Something changed\n"
> +            " -- Somebody <somebody@ubuntu.com>   Fri, 25 Jun 2010 11:20:22 -0600",
> +            changes.simulated_changelog)

Formatting.

> +
> +    def test_requires_changed_by(self):

Comment.

> +        contents = self.getBaseChanges()
> +        del contents["Changed-By"]
> +        self.assertRaises(UploadError,
> +            self.createChangesFile, "mypkg_0.1_i386.changes", contents)

Formatting.

> 
> === added file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py'
> --- lib/lp/archiveuploader/tests/test_nascentuploadfile.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-08-17 13:21:05 +0000
> @@ -0,0 +1,232 @@
> +# Copyright 2010 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Test NascentUploadFile functionality."""
> +
> +__metaclass__ = type
> +
> +from debian.deb822 import Changes, Dsc
> +import hashlib
> +import os
> +
> +from canonical.launchpad.scripts.logger import BufferLogger
> +from lp.archiveuploader.changesfile import ChangesFile
> +from lp.archiveuploader.dscfile import DSCFile
> +from lp.archiveuploader.nascentuploadfile import (CustomUploadFile,
> +    DebBinaryUploadFile)

One line per import, first import on a new line, thus:

from lp.archiveuploader.nascentuploadfile import (
    CustomUploadFile,
    DebBinaryUploadFile,
    )

> +from lp.archiveuploader.uploadpolicy import AbsolutelyAnythingGoesUploadPolicy
> +from lp.soyuz.interfaces.queue import PackageUploadCustomFormat
> +from lp.testing import TestCaseWithFactory
> +from canonical.testing import LaunchpadZopelessLayer

This should be up with the other from canonical import.

> +
> +
> +class NascentUploadFileTestCase(TestCaseWithFactory):
> +    """Base class for all tests of classes deriving from NascentUploadFile."""
> +
> +    def setUp(self):
> +        super(NascentUploadFileTestCase, self).setUp()
> +        self.logger = BufferLogger()
> +        self.policy = AbsolutelyAnythingGoesUploadPolicy()
> +        self.distro = self.factory.makeDistribution()
> +        self.policy.archive = self.factory.makeArchive(
> +            distribution=self.distro)
> +
> +    def writeUploadFile(self, filename, contents):
> +        """Write a temporary file but with a specific filename.
> +
> +        :param filename: Filename to use
> +        :param contents: Contents of the file
> +        :return: Tuple with path, digest and size
> +        """
> +        path = os.path.join(self.makeTemporaryDirectory(), filename)
> +        f = open(path, 'w')
> +        try:
> +            f.write(contents)
> +        finally:
> +            f.close()
> +        return (path, hashlib.sha1(contents), len(contents))
> +
> +
> +class CustomUploadFileTests(NascentUploadFileTestCase):
> +    """Tests for CustomUploadFile."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def createCustomUploadFile(self, filename, contents,
> +        component_and_section, priority_name):

Arguments on a new line should line up with the first argument in the
definition.

> +        """Simple wrapper to create a CustomUploadFile."""
> +        (path, digest, size) = self.writeUploadFile(filename, contents)
> +        uploadfile = CustomUploadFile(
> +            path, digest, size, component_and_section, priority_name,
> +            self.policy, self.logger)
> +        return uploadfile
> +
> +    def test_custom_type(self):

Comment needed.

> +        uploadfile = self.createCustomUploadFile(
> +            "bla.txt", "data", "main/raw-installer", "extra")
> +        self.assertEquals(
> +            PackageUploadCustomFormat.DEBIAN_INSTALLER,
> +            uploadfile.custom_type)
> +
> +    def test_storeInDatabase(self):

Comment needed.

> +        uploadfile = self.createCustomUploadFile(
> +            "bla.txt", "data", "main/raw-installer", "extra")
> +        self.assertEquals("application/octet-stream", uploadfile.content_type)
> +        libraryfile = uploadfile.storeInDatabase()
> +        self.assertEquals("bla.txt", libraryfile.filename)
> +        self.assertEquals("application/octet-stream", libraryfile.mimetype)
> +
> +
> +class PackageUploadFileTestCase(NascentUploadFileTestCase):
> +    """Base class for all tests of classes deriving from PackageUploadFile."""
> +
> +    def setUp(self):
> +        super(PackageUploadFileTestCase, self).setUp()
> +        self.policy.distroseries = self.factory.makeDistroSeries(
> +            distribution=self.distro)
> +
> +    def getBaseChanges(self):
> +        contents = Changes()
> +        contents["Source"] = "mypkg"
> +        contents["Binary"] = "binary"
> +        contents["Architecture"] = "i386"
> +        contents["Version"] = "0.1"
> +        contents["Distribution"] = "nifty"
> +        contents["Description"] = "\n Foo"
> +        contents["Maintainer"] = "Somebody"
> +        contents["Changes"] = "Something changed"
> +        contents["Date"] = "Fri, 25 Jun 2010 11:20:22 -0600"
> +        contents["Urgency"] = "low"
> +        contents["Changed-By"] = "Seombody Else <somebody@example.com>"
> +        contents["Files"] = [{
> +            "md5sum": "d2bd347b3fed184fe28e112695be491c",
> +            "size": "1791",
> +            "section": "python",
> +            "priority": "optional",
> +            "name": "dulwich_0.4.1-1.dsc"}]
> +        return contents
> +
> +    def createChangesFile(self, filename, changes):
> +        tempdir = self.makeTemporaryDirectory()
> +        path = os.path.join(tempdir, filename)
> +        changes_fd = open(path, "w")
> +        try:
> +            changes.dump(changes_fd)
> +        finally:
> +            changes_fd.close()
> +        return ChangesFile(path, self.policy, self.logger)
> +
> +
> +class DSCFileTests(PackageUploadFileTestCase):
> +    """Tests for DSCFile."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def getBaseDsc(self):
> +        dsc = Dsc()
> +        dsc["Architecture"] = "all"
> +        dsc["Version"] = "0.42"
> +        dsc["Source"] = "dulwich"
> +        dsc["Binary"] = "python-dulwich"
> +        dsc["Standards-Version"] = "0.2.2"
> +        dsc["Maintainer"] = "Jelmer Vernooij <jelmer@ubuntu.com>"
> +        dsc["Files"] = [{
> +            "md5sum": "5e8ba79b4074e2f305ddeaf2543afe83",
> +            "size": "182280",
> +            "name": "dulwich_0.42.tar.gz"}]
> +        return dsc
> +
> +    def createDSCFile(self, filename, dsc, component_and_section,
> +            priority_name, package, version, changes):

Argument formatting as above, please.

> +        (path, digest, size) = self.writeUploadFile(filename, dsc.dump())
> +        if changes:
> +            self.assertEquals([], list(changes.processAddresses()))
> +        return DSCFile(
> +            path, digest, size, component_and_section, priority_name, package,
> +            version, changes, self.policy, self.logger)
> +
> +    def test_filetype(self):

Needs a Comment.

> +        dsc = self.getBaseDsc()
> +        uploadfile = self.createDSCFile("foo.dsc", dsc,
> +            "main/net", "extra", "dulwich", "0.42", None)

Formatting as above.

> +        self.assertEquals("text/x-debian-source-package",
> +            uploadfile.content_type)

Here, too.

> +
> +    def test_storeInDatabase(self):

Comment needed.

> +        dsc = self.getBaseDsc()
> +        dsc["Build-Depends"] = "dpkg, bzr"
> +        changes = self.getBaseChanges()
> +        uploadfile = self.createDSCFile(
> +            "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42",
> +            self.createChangesFile("foo.changes", changes))
> +        (uploadfile.changelog_path, changelog_digest, changelog_size) = \
> +            self.writeUploadFile("changelog", "DUMMY")

We don't use \ to separate lines. Instead, this should use parens:

(uploadfile.changelog_path, changelog_digest, changelog_size) = (
            self.writeUploadFile("changelog", "DUMMY"))

> +        uploadfile.files = []
> +        release = uploadfile.storeInDatabase(None)
> +        self.assertEquals("0.42", release.version)
> +        self.assertEquals("dpkg, bzr", release.builddepends)
> +
> +
> +class DebBinaryUploadFileTests(PackageUploadFileTestCase):
> +    """Tests for DebBinaryUploadFile."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def getBaseControl(self):
> +        return {
> +            "Package": "python-dulwich",
> +            "Source": "dulwich",
> +            "Version": "0.42",
> +            "Architecture": "i386",
> +            "Maintainer": "Jelmer Vernooij <jelmer@debian.org>",
> +            "Installed-Size": "524",
> +            "Depends": "python (<< 2.7), python (>= 2.5)",
> +            "Provides": "python2.5-dulwich, python2.6-dulwich",
> +            "Section": "python",
> +            "Priority": "optional",
> +            "Homepage": "http://samba.org/~jelmer/dulwich",
> +            "Description": "Pure-python Git library\n"
> +                "Dulwich is a Python implementation of the file formats and "
> +                "protocols"}

The closing brace should be on its own line, aligned with the items in
the dict, and the last item should have a trailling comma, thus:

"Description": "Pure-python Git library\n"
                "Dulwich is a Python implementation of the file formats and "
                "protocols",
            }

> +
> +    def createDebBinaryUploadFile(self, filename, component_and_section,
> +        priority_name, package, version, changes):
> +        """Create a DebBinaryUploadFile."""
> +        (path, digest, size) = self.writeUploadFile(filename, "DUMMY DATA")
> +        return DebBinaryUploadFile(
> +            path, digest, size, component_and_section, priority_name, package,
> +            version, changes, self.policy, self.logger)
> +
> +    def test_unknown_priority(self):

Comment needed.

> +        uploadfile = self.createDebBinaryUploadFile(
> +            "foo_0.42_i386.deb", "main/net", "unknown", "mypkg", "0.42", None)
> +        self.assertEquals("extra", uploadfile.priority_name)
> +
> +    def test_parseControl(self):

Comment needed.

> +        uploadfile = self.createDebBinaryUploadFile(
> +            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
> +            None)
> +        control = self.getBaseControl()
> +        uploadfile.parseControl(control)
> +        self.assertEquals("python", uploadfile.section_name)
> +        self.assertEquals("dulwich", uploadfile.source_name)
> +        self.assertEquals("0.42", uploadfile.source_version)
> +        self.assertEquals("0.42", uploadfile.control_version)
> +
> +    def test_storeInDatabase(self):

Comment needed.

> +        uploadfile = self.createDebBinaryUploadFile(
> +            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
> +            None)
> +        control = self.getBaseControl()
> +        uploadfile.parseControl(control)
> +        build = self.factory.makeBinaryPackageBuild()
> +        bpr = uploadfile.storeInDatabase(build)
> +        self.assertEquals(u'python (<< 2.7), python (>= 2.5)', bpr.depends)
> +        self.assertEquals(
> +            u"Dulwich is a Python implementation of the file formats "
> +            u"and protocols", bpr.description)
> +        self.assertEquals(False, bpr.essential)
> +        self.assertEquals(524, bpr.installedsize)
> +        self.assertEquals(True, bpr.architecturespecific)
> +        self.assertEquals(None, bpr.recommends)
> +        self.assertEquals("0.42", bpr.version)
>

review: Needs Fixing (code)

Revision history for this message

Graham Binns (gmb) wrote on 2010-08-18:

Hi Jelmer,

Thanks for those changes. Everything looks good, but you need to change the comments you've added to the start of the tests from the "Test that.." form to a straight statement of the expected behaviour.

So, instead of " # Test that unknown priorities automatically get changed to 'extra'," the comment should read " # Unknown priorities automatically get changed to 'extra'." This is a carry-over from doctest land, but being able to glance at a comment and know that foo should do bar rather than having to parse out the 'test that...' is nice. We know it's testing something because the method name starts with test_.

Other than that, this branch is good to land, so I'll mark it approved. No need to ping me about any subsequent changes unless they're not in-scope for this review.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Barki Mustapha

Celso Providelo

Christian Reis

Christy Awad

Colin Watson

Harpianto,ANDI

James Troup

Jelmer Vernooij

John A Meinel

Kevin bush

Launchpad code reviewers

Launchpad code reviewers from Canonical

Matthew Tanner

Maximiliano Bertacchini

Oguz Ersoz

Simon Brakhane

Ubuntu-BR DevOps

William Grant

alhawiti

api.ng

pedro cavazos

todaioan

wenjingwen

to status/vote changes:

Tzaddi

Tzaddi Belding

 === modified file 'lib/lp/archiveuploader/changesfile.py'
 --- lib/lp/archiveuploader/changesfile.py	2010-08-01 06:59:04 +0000
 +++ lib/lp/archiveuploader/changesfile.py	2010-08-19 15:51:44 +0000
@@ -41,7 +41,11 @@
      mandatory_fields = set([
          "source", "binary", "architecture", "version", "distribution",
--        "maintainer", "files", "changes"])
++        "maintainer", "files", "changes", "date",
++        # Changed-By is not technically mandatory according to
++        # Debian policy but Soyuz relies on it being set in
++        # various places.
++        "changed-by"])
      # Map urgencies to their dbschema values.
      # Debian policy only permits low, medium, high, emergency.
@@ -148,7 +152,7 @@
      def isCustom(self, component_and_section):
          """Check if given 'component_and_section' matches a custom upload.
--        We recognize an upload as custom if it is taget to a section like
++        We recognize an upload as custom if it is targeted at a section like
          'raw-<something>'.
          Further checks will be performed in CustomUploadFile class.
          """
@@ -215,6 +219,10 @@
          if len(self.files) == 0:
              yield UploadError("No files found in the changes")
++        if 'urgency' not in self._dict:
++            # Urgency is recommended but not mandatory. Default to 'low'
++            self._dict['urgency'] = "low"
++
          raw_urgency = self._dict['urgency'].lower()
          if raw_urgency not in self.urgency_map:
              yield UploadWarning(
@@ -321,7 +329,7 @@
      @property
      def source(self):
--        """Return changesfiel claimed source name"""
++        """Return changesfile claimed source name."""
          return self._dict['source']
      @property
 === modified file 'lib/lp/archiveuploader/dscfile.py'
 --- lib/lp/archiveuploader/dscfile.py	2010-08-03 08:49:19 +0000
 +++ lib/lp/archiveuploader/dscfile.py	2010-08-19 15:51:44 +0000
@@ -740,6 +740,7 @@
      logger.debug("Copying copyright contents.")
      dsc_file.copyright = open(copyright_file).read().strip()
++
  def findChangelog(dsc_file, source_dir, logger):
      """Find and move any debian/changelog.
 === modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
 --- lib/lp/archiveuploader/nascentuploadfile.py	2010-07-21 11:13:19 +0000
 +++ lib/lp/archiveuploader/nascentuploadfile.py	2010-08-19 15:51:44 +0000
@@ -321,7 +321,6 @@
                  "%s: Unknown component %r" % (
                  self.filename, self.component_name))
--
      @property
      def component(self):
          """Return an IComponent for self.component.name."""
@@ -492,12 +491,15 @@
                  yield UploadError(
                      "%s: control file lacks mandatory field %r"
                       % (self.filename, mandatory_field))
++        control = {}
++        for key in control_lines.keys():
++            control[key] = control_lines.Find(key)
++        self.parseControl(control)
++    def parseControl(self, control):
          # XXX kiko 2007-02-15: We never use the Maintainer information in
          # the control file for anything. Should we? --
--        self.control = {}
--        for key in control_lines.keys():
--            self.control[key] = control_lines.Find(key)
++        self.control = control
          control_source = self.control.get("Source", None)
          if control_source is not None:
 === modified file 'lib/lp/archiveuploader/tests/test_changesfile.py'
 --- lib/lp/archiveuploader/tests/test_changesfile.py	2010-08-01 06:59:04 +0000
 +++ lib/lp/archiveuploader/tests/test_changesfile.py	2010-08-19 15:51:44 +0000
@@ -5,14 +5,27 @@
  __metaclass__ = type
--from testtools import TestCase
--
--from lp.archiveuploader.changesfile import (CannotDetermineFileTypeError,
--    determine_file_class_and_name)
++from debian.deb822 import Changes
++import os
++
++from canonical.launchpad.scripts.logger import BufferLogger
++from canonical.testing import LaunchpadZopelessLayer
++
++from lp.archiveuploader.changesfile import (
++    CannotDetermineFileTypeError,
++    ChangesFile,
++    determine_file_class_and_name,
++    )
  from lp.archiveuploader.dscfile import DSCFile
  from lp.archiveuploader.nascentuploadfile import (
--    DebBinaryUploadFile, DdebBinaryUploadFile, SourceUploadFile,
--    UdebBinaryUploadFile)
++    DebBinaryUploadFile,
++    DdebBinaryUploadFile,
++    SourceUploadFile,
++    UdebBinaryUploadFile,
++    UploadError,
++    )
++from lp.archiveuploader.tests import AbsolutelyAnythingGoesUploadPolicy
++from lp.testing import TestCase
  class TestDetermineFileClassAndName(TestCase):
@@ -52,3 +65,118 @@
              CannotDetermineFileTypeError,
              determine_file_class_and_name,
              'foo')
++
++
++class ChangesFileTests(TestCase):
++    """Tests for ChangesFile."""
++
++    layer = LaunchpadZopelessLayer
++
++    def setUp(self):
++        super(ChangesFileTests, self).setUp()
++        self.logger = BufferLogger()
++        self.policy = AbsolutelyAnythingGoesUploadPolicy()
++
++    def createChangesFile(self, filename, changes):
++        tempdir = self.makeTemporaryDirectory()
++        path = os.path.join(tempdir, filename)
++        changes_fd = open(path, "w")
++        try:
++            changes.dump(changes_fd)
++        finally:
++            changes_fd.close()
++        return ChangesFile(path, self.policy, self.logger)
++
++    def getBaseChanges(self):
++        contents = Changes()
++        contents["Source"] = "mypkg"
++        contents["Binary"] = "binary"
++        contents["Date"] = "Fri, 25 Jun 2010 11:20:22 -0600"
++        contents["Architecture"] = "i386"
++        contents["Version"] = "0.1"
++        contents["Distribution"] = "nifty"
++        contents["Maintainer"] = "Somebody"
++        contents["Changes"] = "Something changed"
++        contents["Description"] = "\n An awesome package."
++        contents["Changed-By"] = "Somebody <somebody@ubuntu.com>"
++        contents["Files"] = [{
++            "md5sum": "d2bd347b3fed184fe28e112695be491c",
++            "size": "1791",
++            "section": "python",
++            "priority": "optional",
++            "name": "dulwich_0.4.1-1.dsc"}]
++        return contents
++
++    def test_checkFileName(self):
++        # checkFileName() yields an UploadError if the filename is invalid.
++        contents = self.getBaseChanges()
++        changes = self.createChangesFile("mypkg_0.1_i386.changes", contents)
++        self.assertEquals([], list(changes.checkFileName()))
++        changes = self.createChangesFile("mypkg_0.1.changes", contents)
++        errors = list(changes.checkFileName())
++        self.assertIsInstance(errors[0], UploadError)
++        self.assertEquals(1, len(errors))
++
++    def test_filename(self):
++        # Filename gets set to the basename of the changes file on disk.
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", self.getBaseChanges())
++        self.assertEquals("mypkg_0.1_i386.changes", changes.filename)
++
++    def test_suite_name(self):
++        # The suite name gets extracted from the changes file.
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", self.getBaseChanges())
++        self.assertEquals("nifty", changes.suite_name)
++
++    def test_version(self):
++        # The version gets extracted from the changes file.
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", self.getBaseChanges())
++        self.assertEquals("0.1", changes.version)
++
++    def test_architectures(self):
++        # The architectures get extracted from the changes file
++        # and parsed correctly.
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", self.getBaseChanges())
++        self.assertEquals("i386", changes.architecture_line)
++        self.assertEquals(set(["i386"]), changes.architectures)
++
++    def test_source(self):
++        # The source package name gets extracted from the changes file.
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", self.getBaseChanges())
++        self.assertEquals("mypkg", changes.source)
++
++    def test_processAddresses(self):
++        # processAddresses parses the changes file and sets the
++        # changed_by field.
++        contents = self.getBaseChanges()
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", contents)
++        self.assertEquals(None, changes.changed_by)
++        errors = list(changes.processAddresses())
++        self.assertEquals(0, len(errors), "Errors: %r" % errors)
++        self.assertEquals(
++            "Somebody <somebody@ubuntu.com>", changes.changed_by['rfc822'])
++
++    def test_simulated_changelog(self):
++        # The simulated_changelog property returns a changelog entry based on
++        # the control fields.
++        contents = self.getBaseChanges()
++        changes = self.createChangesFile(
++            "mypkg_0.1_i386.changes", contents)
++        self.assertEquals([], list(changes.processAddresses()))
++        self.assertEquals(
++            "Something changed\n"
++            " -- Somebody <somebody@ubuntu.com>   Fri, 25 Jun 2010 11:20:22 -0600",
++            changes.simulated_changelog)
++
++    def test_requires_changed_by(self):
++        # A changes file is rejected if it does not have a Changed-By field.
++        contents = self.getBaseChanges()
++        del contents["Changed-By"]
++        self.assertRaises(
++            UploadError,
++            self.createChangesFile, "mypkg_0.1_i386.changes", contents)
 === added file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py'
 --- lib/lp/archiveuploader/tests/test_nascentuploadfile.py	1970-01-01 00:00:00 +0000
 +++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-08-19 15:51:44 +0000
@@ -0,0 +1,243 @@
++# Copyright 2010 Canonical Ltd.  This software is licensed under the
++# GNU Affero General Public License version 3 (see the file LICENSE).
++
++"""Test NascentUploadFile functionality."""
++
++__metaclass__ = type
++
++from debian.deb822 import Changes, Dsc
++import hashlib
++import os
++
++from canonical.launchpad.scripts.logger import BufferLogger
++from canonical.testing import LaunchpadZopelessLayer
++
++from lp.archiveuploader.changesfile import ChangesFile
++from lp.archiveuploader.dscfile import DSCFile
++from lp.archiveuploader.nascentuploadfile import (
++    CustomUploadFile,
++    DebBinaryUploadFile,
++    )
++from lp.archiveuploader.tests import AbsolutelyAnythingGoesUploadPolicy
++from lp.soyuz.interfaces.queue import PackageUploadCustomFormat
++from lp.testing import TestCaseWithFactory
++
++
++class NascentUploadFileTestCase(TestCaseWithFactory):
++    """Base class for all tests of classes deriving from NascentUploadFile."""
++
++    def setUp(self):
++        super(NascentUploadFileTestCase, self).setUp()
++        self.logger = BufferLogger()
++        self.policy = AbsolutelyAnythingGoesUploadPolicy()
++        self.distro = self.factory.makeDistribution()
++        self.policy.archive = self.factory.makeArchive(
++            distribution=self.distro)
++
++    def writeUploadFile(self, filename, contents):
++        """Write a temporary file but with a specific filename.
++
++        :param filename: Filename to use
++        :param contents: Contents of the file
++        :return: Tuple with path, digest and size
++        """
++        path = os.path.join(self.makeTemporaryDirectory(), filename)
++        f = open(path, 'w')
++        try:
++            f.write(contents)
++        finally:
++            f.close()
++        return (path, hashlib.sha1(contents), len(contents))
++
++
++class CustomUploadFileTests(NascentUploadFileTestCase):
++    """Tests for CustomUploadFile."""
++
++    layer = LaunchpadZopelessLayer
++
++    def createCustomUploadFile(self, filename, contents,
++                               component_and_section, priority_name):
++        """Simple wrapper to create a CustomUploadFile."""
++        (path, digest, size) = self.writeUploadFile(filename, contents)
++        uploadfile = CustomUploadFile(
++            path, digest, size, component_and_section, priority_name,
++            self.policy, self.logger)
++        return uploadfile
++
++    def test_custom_type(self):
++        # The mime type gets set according to PackageUploadCustomFormat.
++        uploadfile = self.createCustomUploadFile(
++            "bla.txt", "data", "main/raw-installer", "extra")
++        self.assertEquals(
++            PackageUploadCustomFormat.DEBIAN_INSTALLER,
++            uploadfile.custom_type)
++
++    def test_storeInDatabase(self):
++        # storeInDatabase creates a library file.
++        uploadfile = self.createCustomUploadFile(
++            "bla.txt", "data", "main/raw-installer", "extra")
++        self.assertEquals("application/octet-stream", uploadfile.content_type)
++        libraryfile = uploadfile.storeInDatabase()
++        self.assertEquals("bla.txt", libraryfile.filename)
++        self.assertEquals("application/octet-stream", libraryfile.mimetype)
++
++
++class PackageUploadFileTestCase(NascentUploadFileTestCase):
++    """Base class for all tests of classes deriving from PackageUploadFile."""
++
++    def setUp(self):
++        super(PackageUploadFileTestCase, self).setUp()
++        self.policy.distroseries = self.factory.makeDistroSeries(
++            distribution=self.distro)
++
++    def getBaseChanges(self):
++        contents = Changes()
++        contents["Source"] = "mypkg"
++        contents["Binary"] = "binary"
++        contents["Architecture"] = "i386"
++        contents["Version"] = "0.1"
++        contents["Distribution"] = "nifty"
++        contents["Description"] = "\n Foo"
++        contents["Maintainer"] = "Somebody"
++        contents["Changes"] = "Something changed"
++        contents["Date"] = "Fri, 25 Jun 2010 11:20:22 -0600"
++        contents["Urgency"] = "low"
++        contents["Changed-By"] = "Seombody Else <somebody@example.com>"
++        contents["Files"] = [{
++            "md5sum": "d2bd347b3fed184fe28e112695be491c",
++            "size": "1791",
++            "section": "python",
++            "priority": "optional",
++            "name": "dulwich_0.4.1-1.dsc"}]
++        return contents
++
++    def createChangesFile(self, filename, changes):
++        tempdir = self.makeTemporaryDirectory()
++        path = os.path.join(tempdir, filename)
++        changes_fd = open(path, "w")
++        try:
++            changes.dump(changes_fd)
++        finally:
++            changes_fd.close()
++        return ChangesFile(path, self.policy, self.logger)
++
++
++class DSCFileTests(PackageUploadFileTestCase):
++    """Tests for DSCFile."""
++
++    layer = LaunchpadZopelessLayer
++
++    def getBaseDsc(self):
++        dsc = Dsc()
++        dsc["Architecture"] = "all"
++        dsc["Version"] = "0.42"
++        dsc["Source"] = "dulwich"
++        dsc["Binary"] = "python-dulwich"
++        dsc["Standards-Version"] = "0.2.2"
++        dsc["Maintainer"] = "Jelmer Vernooij <jelmer@ubuntu.com>"
++        dsc["Files"] = [{
++            "md5sum": "5e8ba79b4074e2f305ddeaf2543afe83",
++            "size": "182280",
++            "name": "dulwich_0.42.tar.gz"}]
++        return dsc
++
++    def createDSCFile(self, filename, dsc, component_and_section,
++                      priority_name, package, version, changes):
++        (path, digest, size) = self.writeUploadFile(filename, dsc.dump())
++        if changes:
++            self.assertEquals([], list(changes.processAddresses()))
++        return DSCFile(
++            path, digest, size, component_and_section, priority_name, package,
++            version, changes, self.policy, self.logger)
++
++    def test_filetype(self):
++        # The filetype attribute is set based on the file extension.
++        dsc = self.getBaseDsc()
++        uploadfile = self.createDSCFile(
++            "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42", None)
++        self.assertEquals(
++            "text/x-debian-source-package", uploadfile.content_type)
++
++    def test_storeInDatabase(self):
++        # storeInDatabase creates a SourcePackageRelease.
++        dsc = self.getBaseDsc()
++        dsc["Build-Depends"] = "dpkg, bzr"
++        changes = self.getBaseChanges()
++        uploadfile = self.createDSCFile(
++            "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42",
++            self.createChangesFile("foo.changes", changes))
++        (uploadfile.changelog_path, changelog_digest, changelog_size) = (
++            self.writeUploadFile("changelog", "DUMMY"))
++        uploadfile.files = []
++        release = uploadfile.storeInDatabase(None)
++        self.assertEquals("0.42", release.version)
++        self.assertEquals("dpkg, bzr", release.builddepends)
++
++
++class DebBinaryUploadFileTests(PackageUploadFileTestCase):
++    """Tests for DebBinaryUploadFile."""
++
++    layer = LaunchpadZopelessLayer
++
++    def getBaseControl(self):
++        return {
++            "Package": "python-dulwich",
++            "Source": "dulwich",
++            "Version": "0.42",
++            "Architecture": "i386",
++            "Maintainer": "Jelmer Vernooij <jelmer@debian.org>",
++            "Installed-Size": "524",
++            "Depends": "python (<< 2.7), python (>= 2.5)",
++            "Provides": "python2.5-dulwich, python2.6-dulwich",
++            "Section": "python",
++            "Priority": "optional",
++            "Homepage": "http://samba.org/~jelmer/dulwich",
++            "Description": "Pure-python Git library\n"
++                "Dulwich is a Python implementation of the file formats and "
++                "protocols"
++            }
++
++    def createDebBinaryUploadFile(self, filename, component_and_section,
++                                  priority_name, package, version, changes):
++        """Create a DebBinaryUploadFile."""
++        (path, digest, size) = self.writeUploadFile(filename, "DUMMY DATA")
++        return DebBinaryUploadFile(
++            path, digest, size, component_and_section, priority_name, package,
++            version, changes, self.policy, self.logger)
++
++    def test_unknown_priority(self):
++        # Unknown priorities automatically get changed to 'extra'.
++        uploadfile = self.createDebBinaryUploadFile(
++            "foo_0.42_i386.deb", "main/net", "unknown", "mypkg", "0.42", None)
++        self.assertEquals("extra", uploadfile.priority_name)
++
++    def test_parseControl(self):
++        # parseControl sets various fields on DebBinaryUploadFile.
++        uploadfile = self.createDebBinaryUploadFile(
++            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
++            None)
++        control = self.getBaseControl()
++        uploadfile.parseControl(control)
++        self.assertEquals("python", uploadfile.section_name)
++        self.assertEquals("dulwich", uploadfile.source_name)
++        self.assertEquals("0.42", uploadfile.source_version)
++        self.assertEquals("0.42", uploadfile.control_version)
++
++    def test_storeInDatabase(self):
++        # storeInDatabase creates a BinaryPackageRelease.
++        uploadfile = self.createDebBinaryUploadFile(
++            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
++            None)
++        control = self.getBaseControl()
++        uploadfile.parseControl(control)
++        build = self.factory.makeBinaryPackageBuild()
++        bpr = uploadfile.storeInDatabase(build)
++        self.assertEquals(u'python (<< 2.7), python (>= 2.5)', bpr.depends)
++        self.assertEquals(
++            u"Dulwich is a Python implementation of the file formats "
++            u"and protocols", bpr.description)
++        self.assertEquals(False, bpr.essential)
++        self.assertEquals(524, bpr.installedsize)
++        self.assertEquals(True, bpr.architecturespecific)
++        self.assertEquals(u"", bpr.recommends)
++        self.assertEquals("0.42", bpr.version)