On Mon, Aug 9, 2010 at 11:39 PM, James Westby <[email protected]> wrote: > James Westby has proposed merging > lp:~james-w/launchpad/move-soyuz-test-publisher into lp:launchpad/devel with > lp:~james-w/launchpad/more-matchers as a prerequisite. > > Requested reviews: > Launchpad code reviewers (launchpad-reviewers) > > > Hi, > > This branch moves SoyuzTestPublisher and the associated > TestNativePublishingBase to lp.soyuz.testing.publisher, > and also changes the API somewhat. >
Thanks! FWIW, code reviews are much simpler if simple code moves are separated from API changes. However, I do not wish to look a gift horse in the mouth, so... > The main thrust of this change is to get tests away from > sampledata, but there are far to many using these classes > to do it in one go. Therefore I created a new class that > can be transitioned to gradually. > Seems sensible. > In addition to this I changed the API somewhat to be what > I consider to be cleaner. This way mainly a taste thing, > but some of the changes will also stop tests being too > tightly bound to the implementation of these classes, giving > us more freedom to change things in future. That's ok, I have excellent taste. > Some of the changes > were a little more than that though, such as removing lots > of commits from the tests using TestNativePublishingBase. > If there are lots of tests that need that many commits then > we can add it back as an opt-in thing, we should punish > every test with it. > +1 > In addition I also documented the new interface with docstrings, > and a module docstring to help with porting. > Sweet! Only typos noticed below, along with a couple of small nitpicks. Feel free to land without getting another review. ... > === added file 'lib/lp/soyuz/testing/__init__.py' > === renamed file 'lib/lp/soyuz/testing/__init__.py' => > 'lib/lp/soyuz/testing/publisher.py' > --- lib/lp/soyuz/testing/__init__.py 2010-08-09 22:38:48 +0000 > +++ lib/lp/soyuz/testing/publisher.py 2010-08-09 22:38:53 +0000 > @@ -0,0 +1,1059 @@ > +# Copyright 2010 Canonical Ltd. This software is licensed under the > +# GNU Affero General Public License version 3 (see the file LICENSE). > + > +"""Testing helpers for publishing. > + > +Porting from TestNativePublishingBase to PublishingTestCase: > + ... > + > +Porting from `lp.soyuz.tests.test_publishing.SoyuzTestPublisher` to > +`lp.soyuz.testing.publisher.SoyuzTestPublisher`: > + ... > +""" > + > +__metaclass__ = type > + > +__all__ = [ > + 'makeDistributionLucilleconfig', > + 'makeDistroSeriesLucilleconfig', > + 'PublishingTestCase', > + 'SoyuzTestPublisher', > +] > + ... > + > +class SoyuzTestPublisher: > + """Helper class able to publish source and binary packages. > + ... > + > + Each method has a number of paramters that can be used to control "parameters" ... > + def addFile(self, filename, filecontent=None, restricted=False, > + expires=None): > + """Add a file to the Librarian. > + > + :param filename: the filename to add. > + :param filecontent: the content of the file, or None to generate > + some content. > + :param restricted: whether the file should be on the restricted > + librarian. > + :param exipres: a `datetime.datetime` at which the file should "expires" ... > + def makeSourcePackageRelease(self, distroseries=None, sourcename=None, > + archive=None, version=None, urgency=None, > + component_name=None, section_name=None, > + maintainer=None, creator=None, > + builddepends=None, builddependsindep=None, > + build_conflicts=None, > + build_conflicts_indep=None, > + architecturehintlist=None, > + dsc_signing_key=None, > + dsc_maintainer_rfc822=None, > + dsc_standards_version=None, > + dsc_format=None, dsc_binaries=None, > + date_uploaded=None, changelog_entry=None): > + """Make a `SourcePackageRelease`. > + > + :param distroseries: the distroseries that the package should > + be targered to, or None to use the default. "targeted" ... > + def createSource(self, sourcename, version, archive=None, > + distroseries=None, new_version=None, > + component_name=None): > + """Create source with meaningful '.changes' file. > + ... > + :rtype: `SourcePackagePublishingHistory` or None > + """ > + top = 'lib/lp/archiveuploader/tests/data/suite' Yikes. A nicer way to do this is to import lp.archiveuploader.tests and use its __file__ attribute to get the directory location. > +class PublishingTestCase(TestCaseWithFactory): > + """A test case that has access to a `SoyuzTestPublisher`. > + > + In addition: > + > + * the default distribution and distroseries of > + the publisher will have lucille configurations, so that they > + can be used with the publisher. I once spent many hours trying to find out what "lucille" was, only to find it's actually the old name for Launchpad, which was then only what we now call Soyuz. What do you mean by lucille here? Is there some way we could put your answer into the documentation? Other than that, all good. jml -- https://code.launchpad.net/~james-w/launchpad/move-soyuz-test-publisher/+merge/32157 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~james-w/launchpad/move-soyuz-test-publisher into lp:launchpad/devel. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
