Merge lp:~james-w/launchpad/no-more-sampledata-2 into lp:launchpad

[James Westby]

The SoyuzTestPublisher now uses the factory. The main difference for now is that you are more likely to get a proxied object from it.

Hi,

Apologies for the bumper branch. Very small changes snowballed
very quickly.

This branch moves SoyuzTestPublisher to back on to the
factory, as a precursor to moving it away from sampledata.

It now runs pretty much the same code inside the factory as it
was running before, and now SoyuzTestPublisher is just a layer
on top of the factory with some conveniences for complex
tests.

The major change is that there are now lots more proxied objects
in soyuz tests, so most of the changes are fallout from that.
There's a lot of removeSecurityProxy involved there, as most of
the problems we in test setup code that is reaching around the
interface to do test set up. I'm not entirely happy that we have
to do that, they should be able to come in by the front door, but
fixing all of that would be too much for one branch.

There's also a bit of having the tests specify the parameters
for the creation of objects where they will assert something
based on that parameter. I didn't do that across the board though.

Plus there are a few drive-by fixes and cleanups as always.

Breakdown of the changes, as not all are immediately obvious. It's perhaps
best to review in reverse order of the changes listed here (odd that bzr
diff gave me the files in roughly reverse order).

lib/lp/archivepublisher/tests/test_dominator.py - just making the values explicit.

lib/lp/archivepublisher/tests/test_publisher.py, lib/lp/soyuz/scripts/tests/test_publishdistro.py, lib/lp/soyuz/tests/test_publishing_top_level_api.py - using sync() from SQLObject, which isn't
on the interface.

lib/lp/archiveuploader/tests/nascentupload-ddebs.txt - getPubSource now creates a user, which
requires the extra setup that only used to be done later.

lib/lp/soyuz/doc/archive-files.txt - setting a value that could be set at creation time
if there were an appropriate way to do that. It's just set up though, so this has the
same effect.

lib/lp/soyuz/doc/packageupload-lookups.txt - Using rSP to ensure that the repr is
what is expected.

lib/lp/soyuz/doc/publishing.txt - changing purpose of an archive or parent series aren't normally allowed. New archives/series could be created instead. Changing the published
component isn't possible, but you could republish in the new component.

lib/lp/soyuz/doc/sourcepackagerelease.txt - deletion is protected, but this is just test
cleanup, so we don't care.

lib/lp/soyuz/scripts/tests/test_changeoverride.py - specifying values and changing the
version of a publication, which isn't normally allowed.

lib/lp/soyuz/scripts/tests/test_copypackage.py - various instances of bypassing the
interface for test setup.

lib/lp/soyuz/tests/test_binarypackagebuild.py - the proxy stops you concatenating lists, so
listify each of them.

lib/lp/soyuz/tests/test_publish_archive_indexes.py - specify all the tested values. Also
a case of forcing invalid data in to a field. I also deleted some copy paste comments
that made no sense in the context.

lib/lp/soyuz/tests/test_publishing.py - the big one, using the factory and lp.testing.sampledata rather than creating objects itself, then a whole bunch of fixes
from the fallout of this due to the proxy.

Thanks,

James

[Jonathan Lange (jml)]

On Thu, Aug 5, 2010 at 10:00 PM, James Westby <email address hidden> wrote:
...
> Hi,
>
> Apologies for the bumper branch. Very small changes snowballed
> very quickly.

It happens.

> This branch moves SoyuzTestPublisher to back on to the
> factory, as a precursor to moving it away from sampledata.
>
> It now runs pretty much the same code inside the factory as it
> was running before, and now SoyuzTestPublisher is just a layer
> on top of the factory with some conveniences for complex
> tests.
>

This seems like a sensible approach.

> The major change is that there are now lots more proxied objects
> in soyuz tests, so most of the changes are fallout from that.
> There's a lot of removeSecurityProxy involved there, as most of
> the problems we in test setup code that is reaching around the
> interface to do test set up. I'm not entirely happy that we have
> to do that, they should be able to come in by the front door, but
> fixing all of that would be too much for one branch.
>

Agreed.

...
> Breakdown of the changes, as not all are immediately obvious. It's perhaps
> best to review in reverse order of the changes listed here (odd that bzr
> diff gave me the files in roughly reverse order).
>
> lib/lp/archivepublisher/tests/test_dominator.py - just making the values explicit.
>
> lib/lp/archivepublisher/tests/test_publisher.py, lib/lp/soyuz/scripts/tests/test_publishdistro.py, lib/lp/soyuz/tests/test_publishing_top_level_api.py - using sync() from SQLObject, which isn't
> on the interface.
>
> lib/lp/archiveuploader/tests/nascentupload-ddebs.txt - getPubSource now creates a user, which
> requires the extra setup that only used to be done later.
>
> lib/lp/soyuz/doc/archive-files.txt - setting a value that could be set at creation time
> if there were an appropriate way to do that. It's just set up though, so this has the
> same effect.
>
> lib/lp/soyuz/doc/packageupload-lookups.txt - Using rSP to ensure that the repr is
> what is expected.
>
> lib/lp/soyuz/doc/publishing.txt - changing purpose of an archive or parent series aren't normally allowed. New archives/series could be created instead. Changing the published
> component isn't possible, but you could republish in the new component.
>
> lib/lp/soyuz/doc/sourcepackagerelease.txt - deletion is protected, but this is just test
> cleanup, so we don't care.
>
> lib/lp/soyuz/scripts/tests/test_changeoverride.py - specifying values and changing the
> version of a publication, which isn't normally allowed.
>
> lib/lp/soyuz/scripts/tests/test_copypackage.py - various instances of bypassing the
> interface for test setup.
>
> lib/lp/soyuz/tests/test_binarypackagebuild.py - the proxy stops you concatenating lists, so
> listify each of them.
>
> lib/lp/soyuz/tests/test_publish_archive_indexes.py - specify all the tested values. Also
> a case of forcing invalid data in to a field. I also deleted some copy paste comments
> that made no sense in the context.
>
> lib/lp/soyuz/tests/test_publishing.py - the big one, using the factory and lp.testing.sampledata rather than creating objects itself, then a whole bunch of fixes
> from the fallout of this due to the proxy.
>
> Thanks,
>

My pleasure....

[James Westby (james-w)]

Thanks for the review.

> Most of the review comments are about dropping calls to sync()
> entirely and adding comments to certain calls to removeSecurityProxy –
> it's good to know why a test feels the need to fiddle with internals.

wgrant said pretty much the same things too, I'm going to take a bit
more time to try and do things more cleanly.

> Also, I feel obliged to include this snippet of a conversation I had
> with Julian, in case they are relevant to this patch or trigger new
> thinking about the no-more-sampledata work you've been doing.
>
> #launchpad-code, 2010-08-03, 16:13-16:19:
> <bigjools> jml: I'm a bit uncomfortable with the factory changes to skip STP
> <jml> bigjools, why so?
> <bigjools> jml: a few reasons. One, because it's not differentiating
> between binary and source publications. Two, someone will want to make
> it do binaries at some point and there will be pain because they're an
> order of magnitiude more complicated to set up, and three, the STP
> relies on sample data because there's a load of publisher config in it
> which is not being set up in the factory methods so you can end up
> with data
> <bigjools> that can't exist in the real world.
> <bigjools> and probably more problems if I look harder
> <bigjools> not insurmountable of course, but nonetheless the problems exist

I saw this conversation. I'm keen to solve these problems such that the
factory is sufficient, but I don't know of many concrete cases where the
sampledata is different, one of the problems of sampledata.

One thing I know is that lucilleconfig isn't set on factory created
distributions and distroseries, and I am working on a way to deal
with that. That's an issue where your tests won't run, rather than
testing the wrong thing though.

Thanks,

James

[Abel Deuring (adeuring)]

Hi James,

again, a nice branch!

Just one minor remark:

> @@ -540,7 +518,8 @@
> for pub in pubs:
> self.checkPastDate(pub.datesuperseded)
> if supersededby is not None:
> - if isinstance(pub, BinaryPackagePublishingHistory):
> + if isinstance(
> + removeSecurityProxy(pub), BinaryPackagePublishingHistory):
> dominant = supersededby.binarypackagerelease.build
> else:
> dominant = supersededby.sourcepackagerelease

I think this is one occasion where you don't need to call removeSecurityProxy(): The Zope security machinery monkeypatches isinstance() so that a possibly existing security proxy is removed before the "real" isinstance function is called.

review: Approve (code)

[Francis J. Lacoste (flacoste)]

On August 6, 2010, Abel Deuring wrote:
> Review: Approve code
> Hi James,
>
> again, a nice branch!
>
> Just one minor remark:
> > @@ -540,7 +518,8 @@
> >
> > for pub in pubs:
> > self.checkPastDate(pub.datesuperseded)
> >
> > if supersededby is not None:
> > - if isinstance(pub, BinaryPackagePublishingHistory):
> > + if isinstance(
> >
> > + removeSecurityProxy(pub),
BinaryPackagePublishingHistory):
> > dominant = supersededby.binarypackagerelease.build
> >
> > else:
> > dominant = supersededby.sourcepackagerelease
>
> I think this is one occasion where you don't need to call
> removeSecurityProxy(): The Zope security machinery monkeypatches
> isinstance() so that a possibly existing security proxy is removed before
> the "real" isinstance function is called.

It doesn't monkeypatch by default, but it is common to shadow it by importing

from zope.security.proxy import isinstance

And that one will do the right thing.

--
Francis J. Lacoste
<email address hidden>

[lp:~james-w/launchpad/no-more-sampledata-2]

11319. By James Westby on 2010-08-06

Merged soyuz-factory-improvements into no-more-sampledata-2.

11320. By James Westby on 2010-08-06

Drop all the calls to sync, as they are now unnecessary.

11321. By James Westby on 2010-08-06

Document why we rSP the package diff.

11322. By James Westby on 2010-08-06

Avoid rSP by logging in as the owner.

11323. By James Westby on 2010-08-06

Avoid an rSP by passing dsc_binaries to getPubSource.

11324. By James Westby on 2010-08-06

Remove more uses of rSP by changing the setup to allow specifying the needed values.

Also use zope's isinstance rather than the builtin one to avoid the proxy.

11325. By James Westby on 2010-08-06

Comment why we are calling removeSecurityProxy when printing.

11326. By James Westby on 2010-08-06

Avoid changing the purpose of an existing archive.

11327. By James Westby on 2010-08-06

Avoid another rSP by improving the test publisher.

Also add a couple of comments where rSP is unavoidable.

11328. By James Westby on 2010-08-06

Remove the last of the rSP with some reorganisation.

11329. By James Westby on 2010-08-06

Damn, missed one that required a comment.

[James Westby (james-w)]

Hi,

Please review again.

All should be much rosier now.

Some of the changes are now more complex, as I opted to
restructure some things rather than using
removeSecurityProxy.

Thanks,

James

[lp:~james-w/launchpad/no-more-sampledata-2]

11330. By James Westby on 2010-08-06

Merge devel.

[Jonathan Lange (jml)]

Looks good. There are a few comments that aren't correctly punctuated sentences, but since I don't care enough to enumerate them, you needn't bother to fix them. :)

review: Approve

[lp:~james-w/launchpad/no-more-sampledata-2]

11331. By James Westby on 2010-08-12

Merge devel.

11332. By James Westby on 2010-08-16

Merge improve-soyuz-factory in to no-more-sampledata-2.

[Brad Crittenden (bac)]

James is this branch ready to land? Is there any way I can assist you in landing the several approved branches?

Unmerged revisions

11332. By James Westby on 2010-08-16

Merge improve-soyuz-factory in to no-more-sampledata-2.

11331. By James Westby on 2010-08-12

Merge devel.

11330. By James Westby on 2010-08-06

Merge devel.

11329. By James Westby on 2010-08-06

Damn, missed one that required a comment.

11328. By James Westby on 2010-08-06

Remove the last of the rSP with some reorganisation.

11327. By James Westby on 2010-08-06

Avoid another rSP by improving the test publisher.

Also add a couple of comments where rSP is unavoidable.

11326. By James Westby on 2010-08-06

Avoid changing the purpose of an existing archive.

11325. By James Westby on 2010-08-06

Comment why we are calling removeSecurityProxy when printing.

11324. By James Westby on 2010-08-06

Remove more uses of rSP by changing the setup to allow specifying the needed values.

Also use zope's isinstance rather than the builtin one to avoid the proxy.

11323. By James Westby on 2010-08-06

Avoid an rSP by passing dsc_binaries to getPubSource.

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2010-07-22 08:19:27 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2010-08-16 22:07:55 +0000
@@ -83,7 +83,6 @@
         publisher.A_publish(False)
         self.layer.txn.commit()
 
-        pub_source.sync()
         self.assertDirtyPocketsContents(
             [('breezy-autotest', 'RELEASE')], publisher.dirty_pockets)
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
@@ -226,8 +225,6 @@
         publisher.A_publish(force_publishing=False)
         self.layer.txn.commit()
 
-        pub_source.sync()
-        pub_source2.sync()
         self.assertDirtyPocketsContents(
             [('hoary-test', 'RELEASE')], publisher.dirty_pockets)
         self.assertEqual(pub_source2.status,
@@ -259,8 +256,6 @@
         publisher.A_publish(force_publishing=False)
         self.layer.txn.commit()
 
-        pub_source.sync()
-        pub_source2.sync()
         self.assertDirtyPocketsContents(
             [('breezy-autotest', 'UPDATES')], publisher.dirty_pockets)
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
@@ -360,7 +355,6 @@
         publisher.A_publish(False)
         self.layer.txn.commit()
 
-        pub_source.sync()
         self.assertDirtyPocketsContents(
             [('breezy-autotest', 'RELEASE')], publisher.dirty_pockets)
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)

=== modified file 'lib/lp/archiveuploader/tests/nascentupload-ddebs.txt'
--- lib/lp/archiveuploader/tests/nascentupload-ddebs.txt	2010-07-21 08:07:08 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-ddebs.txt	2010-08-16 22:07:55 +0000
@@ -15,6 +15,8 @@
 
     >>> from lp.soyuz.tests.test_publishing import (
     ...     SoyuzTestPublisher)
+    >>> from canonical.config import config
+    >>> from canonical.testing import LaunchpadZopelessLayer
 
     >>> test_publisher = SoyuzTestPublisher()
 

=== modified file 'lib/lp/soyuz/doc/archive-files.txt'
--- lib/lp/soyuz/doc/archive-files.txt	2010-07-21 11:06:37 +0000
+++ lib/lp/soyuz/doc/archive-files.txt	2010-08-16 22:07:55 +0000
@@ -143,9 +143,13 @@
     >>> print another_test_source.sourcepackagerelease.package_diffs.count()
     1
 
+We don't care about having a realistic diff for the two packages, for this
+test, and getting one through the normal means would be expensive, so
+we'll take a shortcut and set the content to be a dummy file.
+
     >>> diff_name = 'test-pkg_1.0_1.1.diff.gz'
     >>> diff = test_publisher.addMockFile(diff_name)
-    >>> package_diff.diff_content = diff
+    >>> removeSecurityProxy(package_diff).diff_content = diff
 
     >>> diff == cprov.archive.getFileByName(diff_name)
     True

=== modified file 'lib/lp/soyuz/doc/packageupload-lookups.txt'
--- lib/lp/soyuz/doc/packageupload-lookups.txt	2010-03-06 04:57:40 +0000
+++ lib/lp/soyuz/doc/packageupload-lookups.txt	2010-08-16 22:07:55 +0000
@@ -98,6 +98,7 @@
     # Create a testing source and its binaries in
     # ubuntutest/breezy-autotest/i386.
     >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+    >>> from zope.security.proxy import removeSecurityProxy
     >>> login('[e-mail]')
     >>> test_publisher = SoyuzTestPublisher()
     >>> test_publisher.prepareBreezyAutotest()
@@ -112,7 +113,8 @@
 The `SourcePackageRelease` 'package_upload' and 'upload_changesfile'
 
     >>> original_source_upload = source.sourcepackagerelease.package_upload
-    >>> print original_source_upload
+    >>> # avoid the __repr__ of the security proxy
+    >>> print removeSecurityProxy(original_source_upload)
     <PackageUpload ...>
 
     >>> source_changesfile = source.sourcepackagerelease.upload_changesfile

=== modified file 'lib/lp/soyuz/doc/publishing.txt'
--- lib/lp/soyuz/doc/publishing.txt	2010-07-12 03:14:42 +0000
+++ lib/lp/soyuz/doc/publishing.txt	2010-08-16 22:07:55 +0000
@@ -2,10 +2,11 @@
 SourcePackagePublishingHistory
 ==============================
 
-This class provides public access to publishing records via a SQL view.
+This class provides public access to publishing records.
 
-    >>> from canonical.launchpad.database import SourcePackagePublishingHistory
-    >>> from canonical.launchpad.database import BinaryPackagePublishingHistory
+    >>> from lp.soyuz.model.publishing import (
+    ...     BinaryPackagePublishingHistory, SourcePackagePublishingHistory)
+    >>> from zope.security.proxy import removeSecurityProxy
 
 Select a publishing record from the sampledata (pmount is a
 interesting one):
@@ -172,17 +173,36 @@
 a rebuild archive then the status summary will always display
 FULLY_BUILT.
 
+Create a COPY archive that will build for hppa and i386.
+
     >>> from lp.soyuz.interfaces.archive import ArchivePurpose
-    >>> spph.archive.purpose = ArchivePurpose.COPY
-    >>> build_status_summary = spph.getStatusSummaryForBuilds()
+    >>> from lp.soyuz.interfaces.archivearch import IArchiveArchSet
+    >>> from lp.soyuz.interfaces.processor import IProcessorFamilySet
+    >>> archive = factory.makeArchive(
+    ...     purpose=ArchivePurpose.COPY,
+    ...     distribution=test_publisher.ubuntutest,
+    ...     virtualized=False)
+    >>> processor_family = getUtility(IProcessorFamilySet).getByName('hppa')
+    >>> archive_arch_set = getUtility(IArchiveArchSet).new(
+    ...     archive, processor_family)
+
+Publish a source to it, create the builds and mark them fully built.
+
+    >>> copy_spph = test_publisher.getPubSource(
+    ...     sourcename="abc", version="666", archive=archive,
+    ...     architecturehintlist='any')
+    >>> copy_builds = copy_spph.createMissingBuilds()
+    >>> for build in copy_builds:
+    ...     build.status = BuildStatus.FULLYBUILT
+
+The check that the status summary marks all builds as FULLYBUILT.
+
+    >>> build_status_summary = copy_spph.getStatusSummaryForBuilds()
     >>> print_build_status_summary(build_status_summary)
     FULLYBUILT
     hppa build of abc 666 in ubuntutest breezy-autotest RELEASE
     i386 build of abc 666 in ubuntutest breezy-autotest RELEASE
 
-    # Just set the purpose back before continuing on.
-    >>> spph.archive.purpose = ArchivePurpose.PRIMARY
-
 If one of the builds becomes published, it will not appear in the summary:
 
     >>> from lp.soyuz.interfaces.publishing import (
@@ -779,8 +799,10 @@
 This simulates a rebuild in of the same source in a more recent
 distroseries, like rebuilding SRUs for constant sources.
 
-    >>> breezy_autotest.parent_series = None
-    >>> hoary_test.parent_series = breezy_autotest
+    >>> # removeSecurityProxy is required as no-one normally has the
+    >>> # permission to do this
+    >>> removeSecurityProxy(breezy_autotest).parent_series = None
+    >>> removeSecurityProxy(hoary_test).parent_series = breezy_autotest
 
     >>> ppa_source.createMissingBuilds()
     []
@@ -791,8 +813,10 @@
 Now, let's check the opposite, as if the copy was from a more recent
 distroseries to a older one, like a backport rebuild.
 
-    >>> breezy_autotest.parent_series = hoary_test
-    >>> hoary_test.parent_series = None
+    >>> # removeSecurityProxy is required as no-one normally has the
+    >>> # permission to do this
+    >>> removeSecurityProxy(breezy_autotest).parent_series = hoary_test
+    >>> removeSecurityProxy(hoary_test).parent_series = None
 
     >>> ppa_source.createMissingBuilds()
     []
@@ -1064,7 +1088,7 @@
     main
 
     >>> test_bin_pubs = test_publisher.getPubBinaries(
-    ...     pub_source=test_source_pub)
+    ...     pub_source=test_source_pub, component='universe')
     >>> test_bin_pub = test_bin_pubs[0]
     >>> ppa_pub = publishing_set.newBinaryPublication(
     ...     archive=mark.archive,
@@ -1330,7 +1354,6 @@
 
     >>> diff_file = getUtility(ILibraryFileAliasSet)[1]
 
-    >>> from zope.security.proxy import removeSecurityProxy
     >>> naked_diff = removeSecurityProxy(pmount_diff)
     >>> naked_diff.diff_content = diff_file
     >>> flush_database_updates()
@@ -1810,7 +1833,10 @@
     ...     sourcename='testing', version='2.0', component='universe',
     ...     architecturehintlist='i386')
     >>> test_binary = test_publisher.getPubBinaries(
-    ...     binaryname='testing-bin', pub_source=test_source)[0]
+    ...     binaryname='testing-bin', pub_source=test_source,
+    ...     component='multiverse')[0]
+    >>> test_binary = test_binary.changeOverride(
+    ...     new_component=factory.makeComponent(name='universe'))
 
 We will create a helper function to inspect ancestries. It simply pass
 any given keyword argument to 'test_source' and 'test_binary'
@@ -1876,9 +1902,6 @@
 
 It works in the same way for binaries.
 
-    >>> multiverse = getUtility(IComponentSet)['multiverse']
-    >>> test_binary.binarypackagerelease.component = multiverse
-
     >>> print test_binary.component.name
     universe
 

=== modified file 'lib/lp/soyuz/doc/sourcepackagerelease.txt'
--- lib/lp/soyuz/doc/sourcepackagerelease.txt	2010-05-19 15:39:52 +0000
+++ lib/lp/soyuz/doc/sourcepackagerelease.txt	2010-08-16 22:07:55 +0000
@@ -14,6 +14,7 @@
 
 Let's get one from the database:
 
+   >>> from __future__ import with_statement
    >>> from canonical.launchpad.database import SourcePackageRelease
    >>> spr = SourcePackageRelease.get(14)
    >>> spr.name
@@ -196,10 +197,10 @@
 
   >>> new_spr = hoary.createUploadedSourcePackageRelease(
   ...     arg_name, version, arg_maintainer,
-  ...     builddepends, builddependsindep, archhintlist, arg_comp, arg_creator,
-  ...     arg_urgency, changelog, changelog_entry, dsc, arg_key, arg_sect,
-  ...     dsc_maintainer_rfc822, dsc_standards_version, dsc_format,
-  ...     dsc_binaries, archive, copyright=copyright,
+  ...     builddepends, builddependsindep, archhintlist, arg_comp,
+  ...     arg_creator, arg_urgency, changelog, changelog_entry, dsc,
+  ...     arg_key, arg_sect, dsc_maintainer_rfc822, dsc_standards_version,
+  ...     dsc_format, dsc_binaries, archive, copyright=copyright,
   ...     build_conflicts=None, build_conflicts_indep=None,
   ...     source_package_recipe_build=arg_recipebuild)
 
@@ -349,7 +350,9 @@
 private PPA, it gets deleted from that private PPA.  At this point the
 package is still public:
 
-    >>> private_publication.requestDeletion(cprov)
+    >>> from lp.testing import person_logged_in
+    >>> with person_logged_in(cprov):
+    ...     private_publication.requestDeletion(cprov)
     >>> transaction.commit()
     >>> login('[e-mail]')
     >>> check_permission('launchpad.View', test_sourcepackagerelease)

=== modified file 'lib/lp/soyuz/scripts/tests/test_changeoverride.py'
--- lib/lp/soyuz/scripts/tests/test_changeoverride.py	2010-07-20 12:06:36 +0000
+++ lib/lp/soyuz/scripts/tests/test_changeoverride.py	2010-08-16 22:07:55 +0000
@@ -14,7 +14,8 @@
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.interfaces.publishing import PackagePublishingPriority
+from lp.soyuz.interfaces.publishing import (
+    PackagePublishingPriority, PackagePublishingStatus)
 from lp.soyuz.interfaces.section import ISectionSet
 from canonical.launchpad.scripts import FakeLogger
 from lp.soyuz.scripts.changeoverride import (
@@ -252,18 +253,24 @@
          * 'boingo-data' binaries PENDING in warty i386 & hppa.
         """
         source = self.test_publisher.getPubSource(
-            sourcename="boingo", version='1.0', distroseries=self.warty)
+            sourcename="boingo", version='1.0', distroseries=self.warty,
+            component="main", section="base",
+            status=PackagePublishingStatus.PENDING)
 
         binaries = self.test_publisher.getPubBinaries(
-            'boingo-bin', pub_source=source, distroseries=self.warty)
+            'boingo-bin', pub_source=source, distroseries=self.warty,
+            version="1.0", component="main", section="base",
+            priority=PackagePublishingPriority.STANDARD,
+            status=PackagePublishingStatus.PENDING)
 
         build = binaries[0].binarypackagerelease.build
         other_binary = self.test_publisher.uploadBinaryForBuild(
-            build, 'boingo-data')
-        other_binary.version = '0.9'
+            build, 'boingo-data', version='0.9')
         binaries.extend(
             self.test_publisher.publishBinaryInArchive(
-                other_binary, source.archive))
+                other_binary, source.archive, section="base",
+                priority=PackagePublishingPriority.STANDARD,
+                status=PackagePublishingStatus.PENDING))
 
     def test_changeoverride_operations(self):
         """Check if `IArchivePublisher.changeOverride` is wrapped correctly.

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2010-07-20 12:06:36 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2010-08-16 22:07:55 +0000
@@ -654,6 +654,24 @@
         self.test_publisher = SoyuzTestPublisher()
         self.test_publisher.prepareBreezyAutotest()
 
+    def makeTargetArchive(self):
+        return self.factory.makeArchive(
+            distribution=self.test_publisher.ubuntutest,
+            purpose=ArchivePurpose.PPA)
+
+    def test_checkCopy_can_copy_unexpired_binaries(self):
+        source = self.test_publisher.getPubSource()
+        binaries = self.test_publisher.getPubBinaries(pub_source=source)
+        archive = self.makeTargetArchive()
+        series = source.distroseries
+        pocket = source.pocket
+        copy_checker = CopyChecker(archive, include_binaries=False)
+        self.assertIs(
+            None, copy_checker.checkCopy(source, series, pocket))
+        copy_checker = CopyChecker(archive, include_binaries=True)
+        self.assertIs(
+            None, copy_checker.checkCopy(source, series, pocket))
+
     def test_checkCopy_cannot_copy_expired_binaries(self):
         # checkCopy() raises CannotCopy if the copy includes binaries
         # and the binaries contain expired files. Publications of
@@ -661,30 +679,14 @@
         # the file is unreachable.
 
         # Create a testing source and binaries.
+        old_date = datetime.datetime(1970, 1, 1, tzinfo=pytz.UTC)
         source = self.test_publisher.getPubSource()
-        binaries = self.test_publisher.getPubBinaries(pub_source=source)
-
-        # Create a fresh PPA which will be the destination copy.
-        archive = self.factory.makeArchive(
-            distribution=self.test_publisher.ubuntutest,
-            purpose=ArchivePurpose.PPA)
+        binaries = self.test_publisher.getPubBinaries(
+            pub_source=source, files_expire=old_date)
+        archive = self.makeTargetArchive()
         series = source.distroseries
         pocket = source.pocket
 
-        # At this point copy is allowed with or without binaries.
-        copy_checker = CopyChecker(archive, include_binaries=False)
-        self.assertIs(
-            None, copy_checker.checkCopy(source, series, pocket))
-        copy_checker = CopyChecker(archive, include_binaries=True)
-        self.assertIs(
-            None, copy_checker.checkCopy(source, series, pocket))
-
-        # Set the expiration date of one of the testing binary files.
-        utc = pytz.timezone('UTC')
-        old_date = datetime.datetime(1970, 1, 1, tzinfo=utc)
-        a_binary_file = binaries[0].binarypackagerelease.files[0]
-        a_binary_file.libraryfile.expires = old_date
-
         # Now source-only copies are allowed.
         copy_checker = CopyChecker(archive, include_binaries=False)
         self.assertIs(
@@ -702,20 +704,13 @@
         # source that contain expired files. Publications of expired
         # files can't be processed by the publisher since the file is
         # unreachable.
-        source = self.test_publisher.getPubSource()
-
-        archive = self.factory.makeArchive(
-            distribution=self.test_publisher.ubuntutest,
-            purpose=ArchivePurpose.PPA)
+        expire = datetime.datetime.now(
+            pytz.UTC) + datetime.timedelta(days=365)
+        source = self.test_publisher.getPubSource(
+            files_expire=expire)
+        archive = self.makeTargetArchive()
         series = source.distroseries
         pocket = source.pocket
-
-        utc = pytz.timezone('UTC')
-        expire = datetime.datetime.now(utc) + datetime.timedelta(days=365)
-
-        a_source_file = source.sourcepackagerelease.files[0]
-        a_source_file.libraryfile.expires = expire
-
         copy_checker = CopyChecker(archive, include_binaries=False)
         self.assertRaisesWithContent(
             CannotCopy,
@@ -974,7 +969,7 @@
         self.test_publisher.breezy_autotest.status = (
             SeriesStatus.CURRENT)
 
-    def createDelayedCopyContext(self):
+    def createDelayedCopyContext(self, component='main'):
         """Create a context to allow delayed-copies test.
 
         The returned source publication in a private archive with
@@ -986,7 +981,8 @@
         ppa.buildd_secret = 'x'
         ppa.private = True
 
-        source = self.test_publisher.createSource(ppa, 'foocomm', '1.0-2')
+        source = self.test_publisher.createSource(
+            ppa, 'foocomm', '1.0-2', component=component)
         self.test_publisher.getPubBinaries(pub_source=source)
 
         [build] = source.getBuilds()
@@ -1060,14 +1056,13 @@
         overridden when uploaded to the PPA, but when copying it to another
         archive, only the ancestry in the destination archive can be used.
         If that ancestry doesn't exist, an exception is raised."""
-        # We'll simulate an upload that was overridden to main in the
-        # ppa, by explicitly setting the spr's and bpr's component to
-        # something else.
-        source = self.createDelayedCopyContext()
-        contrib = getUtility(IComponentSet).new('contrib')
-        source.sourcepackagerelease.component = contrib
+        # Upload to contrib, with automatic override to main due
+        # to it being a PPA.
+        source = self.createDelayedCopyContext(component='contrib')
+        # But then override in to main, like a PPA would
         [build] = source.getBuilds()
         [binary] = build.binarypackages
+        contrib = self.factory.makeComponent(name="contrib")
         binary.override(component=contrib)
         self.layer.txn.commit()
 
@@ -1084,14 +1079,12 @@
         archive, only the ancestry in the destination archive can be used.
         If an ancestor is found in the destination archive, its component
         is assumed for this package upload."""
-        # We'll simulate an upload that was overridden to main in the
-        # ppa, by explicitly setting the spr's and bpr's component to
-        # something else.
-        source = self.createDelayedCopyContext()
-        contrib = getUtility(IComponentSet).new('contrib')
-        source.sourcepackagerelease.component = contrib
+        # Upload to contrib, with automatic override to main due
+        # to it being a PPA.
+        source = self.createDelayedCopyContext(component='contrib')
         [build] = source.getBuilds()
         [binary] = build.binarypackages
+        contrib = self.factory.makeComponent(name='contrib')
         binary.override(component=contrib)
 
         # This time, we'll ensure that there is already an ancestor for
@@ -2234,7 +2227,16 @@
         test_publisher.addFakeChroots(warty)
         ppa_binaries = test_publisher.getPubBinaries(
             pub_source=ppa_source, distroseries=warty,
-            status=PackagePublishingStatus.PUBLISHED)
+            status=PackagePublishingStatus.PUBLISHED,
+            component='universe')
+
+        # Override the ppa binaries to main, like a PPA would
+        new_ppa_binaries = []
+        main = self.factory.makeComponent(name="main")
+        for binary in ppa_binaries:
+            new_ppa_binaries.append(
+                binary.changeOverride(new_component=main))
+        ppa_binaries = new_ppa_binaries
 
         # Give the new source a private package diff.
         sourcepackagerelease = other_source.sourcepackagerelease
@@ -2263,10 +2265,6 @@
         universe = getUtility(IComponentSet)['universe']
         ancestry_source.component = universe
 
-        # Override the copied binarypackagerelease to 'universe'.
-        for binary in ppa_binaries:
-            binary.binarypackagerelease.component = universe
-
         self.layer.txn.commit()
 
         # Now we can invoke the unembargo script and check its results.
@@ -2364,9 +2362,9 @@
                 sourcename='buggy-source', version=version,
                 distroseries=warty, archive=archive, pocket=pocket,
                 changes_file_content=changes_file_content,
-                status=PackagePublishingStatus.PUBLISHED)
-            source.sourcepackagerelease.changelog_entry = (
-                "Required for close_bugs_for_sourcepublication")
+                status=PackagePublishingStatus.PUBLISHED,
+                changelog_entry="Required for "
+                "close_bugs_for_sourcepublication")
             binaries = test_publisher.getPubBinaries(
                 pub_source=source, distroseries=warty, archive=archive,
                 pocket=pocket, status=PackagePublishingStatus.PUBLISHED)

=== modified file 'lib/lp/soyuz/scripts/tests/test_publishdistro.py'
--- lib/lp/soyuz/scripts/tests/test_publishdistro.py	2010-03-09 07:29:18 +0000
+++ lib/lp/soyuz/scripts/tests/test_publishdistro.py	2010-08-16 22:07:55 +0000
@@ -72,7 +72,6 @@
 
         rc, out, err = self.runPublishDistroScript()
 
-        pub_source.sync()
         self.assertEqual(0, rc, "Publisher failed with:\n%s\n%s" % (out, err))
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
 
@@ -88,7 +87,6 @@
         pub_source = self.getPubSource(filecontent='foo')
         self.layer.txn.commit()
         self.runPublishDistro()
-        pub_source.sync()
 
         random_person = getUtility(IPersonSet).getByName('name16')
         pub_source.requestDeletion(random_person)
@@ -96,7 +94,6 @@
         self.assertTrue(pub_source.scheduleddeletiondate is None,
             "pub_source.scheduleddeletiondate should not be set, and it is.")
         self.runPublishDistro()
-        pub_source.sync()
         self.assertTrue(pub_source.scheduleddeletiondate is not None,
             "pub_source.scheduleddeletiondate should be set, and it's not.")
 
@@ -123,8 +120,6 @@
 
         self.runPublishDistro(['-s', 'hoary-test'])
 
-        pub_source.sync()
-        pub_source2.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PENDING)
         self.assertEqual(
             pub_source2.status, PackagePublishingStatus.PUBLISHED)
@@ -212,9 +207,6 @@
 
         self.runPublishDistro(['--ppa'])
 
-        pub_source.sync()
-        pub_source2.sync()
-        pub_source3.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PENDING)
         self.assertEqual(
             pub_source2.status, PackagePublishingStatus.PUBLISHED)
@@ -246,21 +238,19 @@
             private=True, distribution=ubuntutest)
 
         # Publish something to the private PPA:
-        pub_source =  self.getPubSource(
+        pub_source = self.getPubSource(
             sourcename='baz', filecontent='baz', archive=private_ppa)
         self.layer.txn.commit()
 
         # Try a plain PPA run, to ensure the private one is NOT published.
         self.runPublishDistro(['--ppa'])
 
-        pub_source.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PENDING)
 
         # Now publish the private PPAs and make sure they are really
         # published.
         self.runPublishDistro(['--private-ppa'])
 
-        pub_source.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
 
     def testPublishPrimaryDebug(self):
@@ -288,8 +278,10 @@
         # operation, see nascentupload-ddebs.txt.
         self.prepareBreezyAutotest()
         pub_binaries = self.getPubBinaries(format=BinaryPackageFormat.DDEB)
+        # changing the archive of a publication is not normally
+        # possible, so we have to use removeSecurityProxy.
         for binary in pub_binaries:
-            binary.archive = debug_archive
+            removeSecurityProxy(binary).archive = debug_archive
 
         # Commit setup changes, so the script can operate on them.
         self.layer.txn.commit()
@@ -330,7 +322,7 @@
         removeSecurityProxy(copy_archive).publish = True
 
         # Publish something.
-        pub_source =  self.getPubSource(
+        pub_source = self.getPubSource(
             sourcename='baz', filecontent='baz', archive=copy_archive)
 
         # Try a plain PPA run, to ensure the copy archive is not published.

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-08-04 13:30:49 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-08-16 22:07:55 +0000
@@ -314,18 +314,18 @@
         self.sources = []
         gedit_src_hist = self.publisher.getPubSource(
             sourcename="gedit", status=PackagePublishingStatus.PUBLISHED)
-        self.builds += gedit_src_hist.createMissingBuilds()
+        self.builds += list(gedit_src_hist.createMissingBuilds())
         self.sources.append(gedit_src_hist)
 
         firefox_src_hist = self.publisher.getPubSource(
             sourcename="firefox", status=PackagePublishingStatus.PUBLISHED)
-        self.builds += firefox_src_hist.createMissingBuilds()
+        self.builds += list(firefox_src_hist.createMissingBuilds())
         self.sources.append(firefox_src_hist)
 
         gtg_src_hist = self.publisher.getPubSource(
             sourcename="getting-things-gnome",
             status=PackagePublishingStatus.PUBLISHED)
-        self.builds += gtg_src_hist.createMissingBuilds()
+        self.builds += list(gtg_src_hist.createMissingBuilds())
         self.sources.append(gtg_src_hist)
 
 

=== modified file 'lib/lp/soyuz/tests/test_publish_archive_indexes.py'
--- lib/lp/soyuz/tests/test_publish_archive_indexes.py	2010-07-20 12:06:36 +0000
+++ lib/lp/soyuz/tests/test_publish_archive_indexes.py	2010-08-16 22:07:55 +0000
@@ -8,6 +8,10 @@
 import tempfile
 import unittest
 
+from zope.security.proxy import removeSecurityProxy
+
+from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
+from lp.soyuz.interfaces.publishing import PackagePublishingPriority
 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
 
 
@@ -31,8 +35,14 @@
         the package in question.
         """
         pub_source = self.getPubSource(
+            sourcename='foo', dsc_binaries='foo-bin',
+            version='666', section='base',
             builddepends='fooish', builddependsindep='pyfoo',
-            build_conflicts='bar', build_conflicts_indep='pybar')
+            build_conflicts='bar', build_conflicts_indep='pybar',
+            dsc_maintainer_rfc822='Foo Bar <[e-mail]>',
+            architecturehintlist='all', dsc_standards_version='3.6.2',
+            dsc_format='1.0', filename='foo_666.dsc',
+            filecontent='I do not care about sources.')
 
         self.assertEqual(
             [u'Package: foo',
@@ -52,16 +62,29 @@
              u' 5913c3ad52c14a62e6ae7eef51f9ef42 28 foo_666.dsc'],
             pub_source.getIndexStanza().splitlines())
 
+    def getPubSourceForBinary(self):
+        return self.getPubSource(
+            sourcename='foo', dsc_maintainer_rfc822='Foo Bar <[e-mail]>')
+
     def testBinaryStanza(self):
         """Check just-created binary publication Index stanza.
 
         See also testSourceStanza, it must present something similar for
         binary packages.
         """
+        pub_source = self.getPubSourceForBinary()
         pub_binaries = self.getPubBinaries(
+            binaryname='foo-bin',
+            priority=PackagePublishingPriority.STANDARD, section="base",
+            installed_size=100, architecturespecific=True,
+            version='666',
             depends='biscuit', recommends='foo-dev', suggests='pyfoo',
             conflicts='old-foo', replaces='old-foo', provides='foo-master',
-            pre_depends='master-foo', enhances='foo-super', breaks='old-foo')
+            pre_depends='master-foo', enhances='foo-super', breaks='old-foo',
+            filecontent='bbbiiinnnaaarrryyy', summary='Foo app is great',
+            format=BinaryPackageFormat.DEB,
+            description='Well ...\nit does nothing, though',
+            pub_source=pub_source)
         pub_binary = pub_binaries[0]
         self.assertEqual(
             [u'Package: foo-bin',
@@ -114,8 +137,15 @@
             "Normal\nNormal"
             "\n.\n.\n."
             "\n %s" % ('x' * 100))
+        pub_source = self.getPubSourceForBinary()
         pub_binary = self.getPubBinaries(
-            description=description)[0]
+            description=description, pub_source=pub_source,
+            binaryname='foo-bin',
+            priority=PackagePublishingPriority.STANDARD,
+            section="base", installed_size=100, architecturespecific=False,
+            format=BinaryPackageFormat.DEB,
+            version="666", filecontent="bbbiiinnnaaarrryyy",
+            summary='Foo app is great')[0]
 
         self.assertEqual(
             [u'Package: foo-bin',
@@ -150,8 +180,14 @@
         'utf-8' for disk writing.
         """
         description = u'Using non-ascii as: \xe7\xe3\xe9\xf3'
+        pub_source = self.getPubSourceForBinary()
         pub_binary = self.getPubBinaries(
-            description=description)[0]
+            description=description, pub_source=pub_source,
+            binaryname='foo-bin',
+            priority=PackagePublishingPriority.STANDARD, section="base",
+            installed_size=100, architecturespecific=False,
+            version="666", format=BinaryPackageFormat.DEB,
+            filecontent='bbbiiinnnaaarrryyy', summary='Foo app is great')[0]
 
         self.assertEqual(
             [u'Package: foo-bin',
@@ -200,7 +236,9 @@
 
     def test_getIndexStanza_binary_stanza(self):
         """Check a binary stanza with APT parser."""
-        pub_binary = self.getPubBinaries()[0]
+        pub_binary = self.getPubBinaries(
+            binaryname='foo-bin', summary='Foo app is great',
+            description='Well ...\n it does nothing, though')[0]
 
         parser = self.write_stanza_and_reparse(pub_binary.getIndexStanza())
 
@@ -211,7 +249,8 @@
 
     def test_getIndexStanza_source_stanza(self):
         """Check a source stanza with APT parser."""
-        pub_source = self.getPubSource()
+        pub_source = self.getPubSource(
+            sourcename="foo", dsc_maintainer_rfc822='Foo Bar <[e-mail]>')
 
         parser = self.write_stanza_and_reparse(pub_source.getIndexStanza())
 
@@ -243,12 +282,11 @@
         This test can be removed if the parser is fixed and the corrupt
         data has been cleaned.
         """
-        pub_source = self.getPubSource()
-
         # An example of a corrupt dsc_binaries field. We need to ensure
         # that the corruption is not carried over into the index stanza.
-        pub_source.sourcepackagerelease.dsc_binaries = (
-            'foo_bin,\nbar_bin,\nzed_bin')
+        pub_source = self.getPubSource(
+            sourcename="foo", version="666",
+            dsc_binaries="foo_bin,\nbar_bin,\nzed_bin")
 
         parser = self.write_stanza_and_reparse(pub_source.getIndexStanza())
 
@@ -278,24 +316,16 @@
         the binary field in the same way that apt_pkg.ParseTagFiles would,
         that it will continue to be written correctly to index files.
         """
-        pub_source = self.getPubSource()
-
-        # An example of a corrupt dsc_binaries field. We need to ensure
-        # that the corruption is not carried over into the index stanza.
-        pub_source.sourcepackagerelease.dsc_binaries = (
-            'foo_bin,\n bar_bin,\n zed_bin')
+        pub_source = self.getPubSource(
+            sourcename='foo', version="666",
+            dsc_binaries='foo_bin,\n bar_bin,\n zed_bin')
 
         parser = self.write_stanza_and_reparse(pub_source.getIndexStanza())
 
         self.assertEqual('foo', parser.Section['Package'])
-
-        # Without the fix, this raises a key-error due to apt-pkg not
-        # being able to parse the file.
         self.assertEqual(
             '666', parser.Section['Version'],
             'The Version field should be parsed correctly.')
-
-        # Without the fix, the second binary would not be parsed at all.
         self.assertEqual(
             'foo_bin,\n bar_bin,\n zed_bin', parser.Section['Binary'])
 

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2010-08-12 14:03:47 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-08-16 22:07:55 +0000
@@ -13,11 +13,10 @@
 import transaction
 import pytz
 from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
+from zope.security.proxy import isinstance, removeSecurityProxy
 
 from canonical.config import config
 from canonical.database.constants import UTC_NOW
-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
 from canonical.testing.layers import reconnect_stores
 from canonical.testing import (
@@ -34,21 +33,18 @@
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.soyuz.model.processor import ProcessorFamily
-from lp.soyuz.model.publishing import (
-    SourcePackagePublishingHistory, BinaryPackagePublishingHistory)
+from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
 from lp.soyuz.interfaces.archive import ArchivePurpose, IArchiveSet
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
-from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
 from lp.soyuz.interfaces.component import IComponentSet
-from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.interfaces.publishing import (
     IPublishingSet, PackagePublishingPriority, PackagePublishingStatus)
 from lp.soyuz.interfaces.queue import PackageUploadStatus
 from canonical.launchpad.scripts import FakeLogger
 from lp.testing import TestCaseWithFactory
-from lp.testing.factory import (
-    LaunchpadObjectFactory, remove_security_proxy_and_shout_at_engineer)
+from lp.testing.factory import LaunchpadObjectFactory
+from lp.testing.sampledata import UBUNTU_DEVELOPER_ADMIN_NAME
 from lp.testing.fakemethod import FakeMethod
 
 
@@ -77,7 +73,8 @@
             distroseries = self.factory.makeDistroRelease()
         self.distroseries = distroseries
         # Set up a person that has a GPG key.
-        self.person = getUtility(IPersonSet).getByName('name16')
+        self.person = getUtility(IPersonSet).getByName(
+            UBUNTU_DEVELOPER_ADMIN_NAME)
         # Make sure the name exists in the database, to make it easier
         # to get packages from distributions and distro series.
         name_set = getUtility(ISourcePackageNameSet)
@@ -126,35 +123,28 @@
         self.breezy_autotest_i386 = self.breezy_autotest['i386']
         self.breezy_autotest_hppa = self.breezy_autotest['hppa']
 
-    def addMockFile(self, filename, filecontent='nothing', restricted=False):
+    def addMockFile(self, filename, filecontent='nothing', restricted=False,
+                    expires=None):
         """Add a mock file in Librarian.
 
         Returns a ILibraryFileAlias corresponding to the file uploaded.
         """
-        library_file = getUtility(ILibraryFileAliasSet).create(
-            filename, len(filecontent), StringIO(filecontent),
-            'application/text', restricted=restricted)
-        return library_file
+        return self.factory.makeLibraryFileAlias(
+            filename=filename, content=filecontent, restricted=restricted,
+            content_type='application/text', expires=expires)
 
     def addPackageUpload(self, archive, distroseries,
                          pocket=PackagePublishingPocket.RELEASE,
                          changes_file_name="foo_666_source.changes",
                          changes_file_content="fake changes file content",
                          upload_status=PackageUploadStatus.DONE):
-        signing_key = self.person.gpg_keys[0]
-        package_upload = distroseries.createQueueEntry(
-            pocket, changes_file_name, changes_file_content, archive,
-            signing_key)
-
-        status_to_method = {
-            PackageUploadStatus.DONE: 'setDone',
-            PackageUploadStatus.ACCEPTED: 'setAccepted',
-            }
-        naked_package_upload = removeSecurityProxy(package_upload)
-        method = getattr(
-            naked_package_upload, status_to_method[upload_status])
-        method()
-
+        person = self.factory.makePerson()
+        signing_key = self.factory.makeGPGKey(person)
+        package_upload = self.factory.makePackageUpload(
+            archive=archive, distroseries=distroseries, pocket=pocket,
+            changes_filename=changes_file_name,
+            changes_file_content=changes_file_content,
+            signing_key=signing_key, status=upload_status)
         return package_upload
 
     def getPubSource(self, sourcename=None, version='666', component='main',
@@ -172,7 +162,8 @@
                      build_conflicts_indep=None,
                      dsc_maintainer_rfc822='Foo Bar <[e-mail]>',
                      maintainer=None, creator=None, date_uploaded=UTC_NOW,
-                     spr_only=False):
+                     spr_only=False, files_expire=None,
+                     changelog_entry=None):
         """Return a mock source publishing record.
 
         if spr_only is specified, the source is not published and the
@@ -180,26 +171,35 @@
         """
         if sourcename is None:
             sourcename = self.default_package_name
-        spn = getUtility(ISourcePackageNameSet).getOrCreateByName(sourcename)
-
-        component = getUtility(IComponentSet)[component]
-        section = getUtility(ISectionSet)[section]
+        spn = self.factory.getOrMakeSourcePackageName(name=sourcename)
+        component = self.factory.makeComponent(name=component)
 
         if distroseries is None:
             distroseries = self.distroseries
         if archive is None:
             archive = distroseries.main_archive
-        if maintainer is None:
-            maintainer = self.person
         if creator is None:
-            creator = self.person
-
-        spr = distroseries.createUploadedSourcePackageRelease(
+            creator = self.factory.makePerson()
+            self.factory.makeGPGKey(creator)
+
+        changes_file_name = "%s_%s_source.changes" % (sourcename, version)
+        if spr_only:
+            upload_status = PackageUploadStatus.ACCEPTED
+        else:
+            upload_status = PackageUploadStatus.DONE
+        package_upload = self.addPackageUpload(
+            archive, distroseries, pocket,
+            changes_file_name=changes_file_name,
+            changes_file_content=changes_file_content,
+            upload_status=upload_status)
+
+        spr = self.factory.makeSourcePackageRelease(
+            distroseries=distroseries,
             sourcepackagename=spn,
             maintainer=maintainer,
             creator=creator,
             component=component,
-            section=section,
+            section_name=section,
             urgency=urgency,
             version=version,
             builddepends=builddepends,
@@ -207,58 +207,39 @@
             build_conflicts=build_conflicts,
             build_conflicts_indep=build_conflicts_indep,
             architecturehintlist=architecturehintlist,
-            changelog=None,
-            changelog_entry=None,
-            dsc=None,
-            copyright='placeholder ...',
-            dscsigningkey=self.person.gpg_keys[0],
+            dscsigningkey=creator.gpg_keys[0],
             dsc_maintainer_rfc822=dsc_maintainer_rfc822,
             dsc_standards_version=dsc_standards_version,
             dsc_format=dsc_format,
             dsc_binaries=dsc_binaries,
-            archive=archive, dateuploaded=date_uploaded)
-
-        changes_file_name = "%s_%s_source.changes" % (sourcename, version)
-        if spr_only:
-            upload_status = PackageUploadStatus.ACCEPTED
-        else:
-            upload_status = PackageUploadStatus.DONE
-        package_upload = self.addPackageUpload(
-            archive, distroseries, pocket,
-            changes_file_name=changes_file_name,
-            changes_file_content=changes_file_content,
-            upload_status=upload_status)
-        naked_package_upload = removeSecurityProxy(
-            package_upload)
-        naked_package_upload.addSource(spr)
+            archive=archive,
+            date_uploaded=date_uploaded,
+            changelog_entry=changelog_entry,
+            )
+        removeSecurityProxy(package_upload).addSource(spr)
 
         if filename is None:
             filename = "%s_%s.dsc" % (sourcename, version)
         alias = self.addMockFile(
-            filename, filecontent, restricted=archive.private)
+            filename, filecontent, restricted=archive.private,
+            expires=files_expire)
         spr.addFile(alias)
 
         if spr_only:
             return spr
 
-        if status == PackagePublishingStatus.PUBLISHED:
-            datepublished = UTC_NOW
-        else:
-            datepublished = None
-
-        spph = SourcePackagePublishingHistory(
+        spph = self.factory.makeSourcePackagePublishingHistory(
             distroseries=distroseries,
             sourcepackagerelease=spr,
             component=spr.component,
-            section=spr.section,
+            section_name=spr.section.name,
             status=status,
-            datecreated=date_uploaded,
+            date_uploaded=date_uploaded,
             dateremoved=dateremoved,
-            datepublished=datepublished,
             scheduleddeletiondate=scheduleddeletiondate,
             pocket=pocket,
-            archive=archive)
-
+            archive=archive,
+            )
         return spph
 
     def getPubBinaries(self, binaryname='foo-bin', summary='Foo app is great',
@@ -279,7 +260,11 @@
                        architecturespecific=False,
                        builder=None,
                        component='main',
-                       with_debug=False):
+                       section='base',
+                       priority=PackagePublishingPriority.STANDARD,
+                       installed_size=100, files_expire=None,
+                       with_debug=False,
+                       ):
         """Return a list of binary publishing records."""
         if distroseries is None:
             distroseries = self.distroseries
@@ -298,7 +283,7 @@
                 sourcename=sourcename, status=status, pocket=pocket,
                 archive=archive, distroseries=distroseries,
                 version=version, architecturehintlist=architecturehintlist,
-                component=component)
+                component=component, files_expire=files_expire)
         else:
             archive = pub_source.archive
 
@@ -323,10 +308,12 @@
                 build, binaryname, filecontent, summary, description,
                 shlibdep, depends, recommends, suggests, conflicts, replaces,
                 provides, pre_depends, enhances, breaks, format,
-                binarypackagerelease_ddeb)
+                installed_size=installed_size, component=component,
+                files_expire=files_expire,
+                debug_package=binarypackagerelease_ddeb)
             pub_binaries += self.publishBinaryInArchive(
                 binarypackagerelease, archive, status, pocket,
-                scheduleddeletiondate, dateremoved)
+                scheduleddeletiondate, dateremoved, section, priority)
             published_binaries.extend(pub_binaries)
             package_upload = self.addPackageUpload(
                 archive, distroseries, pocket,
@@ -344,20 +331,21 @@
         summary="summary", description="description", shlibdep=None,
         depends=None, recommends=None, suggests=None, conflicts=None,
         replaces=None, provides=None, pre_depends=None, enhances=None,
-        breaks=None, format=BinaryPackageFormat.DEB, debug_package=None):
+        breaks=None, format=BinaryPackageFormat.DEB, installed_size=None,
+        component=None, version=None, files_expire=None, debug_package=None):
         """Return the corresponding `BinaryPackageRelease`."""
         sourcepackagerelease = build.source_package_release
         distroarchseries = build.distro_arch_series
         architecturespecific = (
             not sourcepackagerelease.architecturehintlist == 'all')
-
-        binarypackagename = getUtility(
-            IBinaryPackageNameSet).getOrCreateByName(binaryname)
-
-        binarypackagerelease = build.createBinaryPackageRelease(
-            version=sourcepackagerelease.version,
-            component=sourcepackagerelease.component,
-            section=sourcepackagerelease.section,
+        if component is not None:
+            component = self.factory.makeComponent(name=component)
+
+        binarypackagename = self.factory.getOrMakeBinaryPackageName(
+            name=binaryname)
+
+        binarypackagerelease = self.factory.makeBinaryPackageRelease(
+            build=build,
             binarypackagename=binarypackagename,
             summary=summary,
             description=description,
@@ -372,11 +360,14 @@
             enhances=enhances,
             breaks=breaks,
             essential=False,
-            installedsize=100,
             architecturespecific=architecturespecific,
             binpackageformat=format,
             priority=PackagePublishingPriority.STANDARD,
-            debug_package=debug_package)
+            installed_size=installed_size,
+            component=component,
+            version=version,
+            debug_package=debug_package,
+            )
 
         # Create the corresponding binary file.
         if architecturespecific:
@@ -387,7 +378,7 @@
                                     filearchtag, format.name.lower())
         alias = self.addMockFile(
             filename, filecontent=filecontent,
-            restricted=build.archive.private)
+            restricted=build.archive.private, expires=files_expire)
         binarypackagerelease.addFile(alias)
 
         # Adjust the build record in way it looks complete.
@@ -414,7 +405,8 @@
         self, binarypackagerelease, archive,
         status=PackagePublishingStatus.PENDING,
         pocket=PackagePublishingPocket.RELEASE,
-        scheduleddeletiondate=None, dateremoved=None):
+        scheduleddeletiondate=None, dateremoved=None, section=None,
+        priority=None, component=None):
         """Return the corresponding BinaryPackagePublishingHistory."""
         distroarchseries = binarypackagerelease.build.distro_arch_series
 
@@ -426,20 +418,17 @@
 
         pub_binaries = []
         for arch in archs:
-            pub = BinaryPackagePublishingHistory(
+            pub = self.factory.makeBinaryPackagePublishingHistory(
                 distroarchseries=arch,
                 binarypackagerelease=binarypackagerelease,
-                component=binarypackagerelease.component,
-                section=binarypackagerelease.section,
-                priority=binarypackagerelease.priority,
                 status=status,
                 scheduleddeletiondate=scheduleddeletiondate,
                 dateremoved=dateremoved,
-                datecreated=UTC_NOW,
                 pocket=pocket,
-                archive=archive)
-            if status == PackagePublishingStatus.PUBLISHED:
-                pub.datepublished = UTC_NOW
+                archive=archive,
+                section_name=section,
+                priority=priority,
+                )
             pub_binaries.append(pub)
 
         return pub_binaries
@@ -455,7 +444,7 @@
 
     def createSource(
         self, archive, sourcename, version, distroseries=None,
-        new_version=None):
+        new_version=None, component='main'):
         """Create source with meaningful '.changes' file."""
         top = 'lib/lp/archiveuploader/tests/data/suite'
         name_fragment = '%s_%s' % (sourcename, version)
@@ -476,7 +465,7 @@
             source = self.getPubSource(
                 sourcename=sourcename, archive=archive, version=new_version,
                 changes_file_content=changesfile_content,
-                distroseries=distroseries)
+                distroseries=distroseries, component=component)
 
         return source
 
@@ -691,7 +680,6 @@
         self.layer.commit()
 
         foo_name = "%s/main/f/foo/foo_666.dsc" % self.pool_dir
-        pub_source.sync()
         self.assertEqual(
             pub_source.status, PackagePublishingStatus.PUBLISHED)
         self.assertEqual(open(foo_name).read().strip(), 'foo is happy')
@@ -703,7 +691,6 @@
         pub_source2.publish(self.disk_pool, self.logger)
         self.layer.commit()
 
-        pub_source2.sync()
         self.assertEqual(
             pub_source2.status, PackagePublishingStatus.PENDING)
         self.assertEqual(open(foo_name).read().strip(), 'foo is happy')
@@ -720,7 +707,6 @@
         self.layer.commit()
         bar_name = "%s/main/b/bar/bar_666.dsc" % self.pool_dir
         self.assertEqual(open(bar_name).read().strip(), 'bar is good')
-        pub_source.sync()
         self.assertEqual(
             pub_source.status, PackagePublishingStatus.PUBLISHED)
 
@@ -728,7 +714,6 @@
             sourcename='bar', filecontent='bar is good')
         pub_source2.publish(self.disk_pool, self.logger)
         self.layer.commit()
-        pub_source2.sync()
         self.assertEqual(
             pub_source2.status, PackagePublishingStatus.PUBLISHED)
 
@@ -748,8 +733,6 @@
         pub_source2.publish(self.disk_pool, self.logger)
         self.layer.commit()
 
-        pub_source.sync()
-        pub_source2.sync()
         self.assertEqual(
             pub_source.status, PackagePublishingStatus.PUBLISHED)
         self.assertEqual(
@@ -768,7 +751,6 @@
         pub_source3.publish(self.disk_pool, self.logger)
         self.layer.commit()
 
-        pub_source3.sync()
         self.assertEqual(
             pub_source3.status, PackagePublishingStatus.PENDING)
 
@@ -789,7 +771,6 @@
         pub_source.publish(test_disk_pool, self.logger)
         self.layer.commit()
 
-        pub_source.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
         self.assertEqual(pub_source.sourcepackagerelease.upload_archive,
                          cprov.archive)
@@ -833,17 +814,21 @@
                 'Cannot override published records.',
                 source.overrideFromAncestry)
 
-    def makeSource(self):
+    def makeSource(self, source_component='main', binary_component='main'):
         """Return a 'source' publication.
 
         It's pending publication with binaries in a brand new PPA
-        and in 'main' component.
+        and in 'main' component, unless otherwise specified. The binaries
+        can be placed in a different component to the source by
+        passing binary_component different to source_component.
         """
         test_archive = self.factory.makeArchive(
             distribution=self.test_publisher.ubuntutest,
             purpose = ArchivePurpose.PPA)
-        source = self.test_publisher.getPubSource(archive=test_archive)
-        self.test_publisher.getPubBinaries(pub_source=source)
+        source = self.test_publisher.getPubSource(
+            archive=test_archive, component=source_component)
+        self.test_publisher.getPubBinaries(
+            pub_source=source, component=binary_component)
         return source
 
     def copyAndCheck(self, pub_record, series, component_name):
@@ -871,23 +856,14 @@
     def test_overrideFromAncestry_fallback_to_source_component(self):
         # overrideFromancestry on the lack of ancestry, falls back to the
         # component the source was originally uploaded to.
-        source = self.makeSource()
-
-        # Adjust the source package release original component.
-        universe = getUtility(IComponentSet)['universe']
-        source.sourcepackagerelease.component = universe
-
+        source = self.makeSource(source_component='universe')
         self.copyAndCheck(source, source.distroseries, 'universe')
 
     def test_overrideFromAncestry_fallback_to_binary_component(self):
         # overrideFromAncestry on the lack of ancestry, falls back to the
         # component the binary was originally uploaded to.
-        binary = self.makeSource().getPublishedBinaries()[0]
-
-        # Adjust the binary package release original component.
-        universe = getUtility(IComponentSet)['universe']
-        removeSecurityProxy(binary.binarypackagerelease).component = universe
-
+        binary = self.makeSource(
+            binary_component='universe').getPublishedBinaries()[0]
         self.copyAndCheck(
             binary, binary.distroarchseries.distroseries, 'universe')
 
@@ -985,8 +961,9 @@
         """
         available_archs = [self.sparc_distroarch, self.avr_distroarch]
         pubrec = self.getPubSource(architecturehintlist='any')
+        naked_pubrec = removeSecurityProxy(pubrec)
         self.assertEquals([self.sparc_distroarch],
-            pubrec._getAllowedArchitectures(available_archs))
+            naked_pubrec._getAllowedArchitectures(available_archs))
 
     def test__getAllowedArchitectures_restricted_override(self):
         """Test _getAllowedArchitectures honors overrides of restricted archs.
@@ -997,8 +974,9 @@
         available_archs = [self.sparc_distroarch, self.avr_distroarch]
         getUtility(IArchiveArchSet).new(self.archive, self.avr_family)
         pubrec = self.getPubSource(architecturehintlist='any')
+        naked_pubrec = removeSecurityProxy(pubrec)
         self.assertEquals([self.sparc_distroarch, self.avr_distroarch],
-            pubrec._getAllowedArchitectures(available_archs))
+            naked_pubrec._getAllowedArchitectures(available_archs))
 
     def test_createMissingBuilds_restricts_any(self):
         """createMissingBuilds() should limit builds targeted at 'any'

=== modified file 'lib/lp/soyuz/tests/test_publishing_top_level_api.py'
--- lib/lp/soyuz/tests/test_publishing_top_level_api.py	2010-07-20 12:06:36 +0000
+++ lib/lp/soyuz/tests/test_publishing_top_level_api.py	2010-08-16 22:07:55 +0000
@@ -109,8 +109,6 @@
         self._publish(pocket=pocket)
 
         # source and binary PUBLISHED in database.
-        pub_source.sync()
-        pub_bin.sync()
         self.assertEqual(pub_source.status, PackagePublishingStatus.PUBLISHED)
         self.assertEqual(pub_bin.status, PackagePublishingStatus.PUBLISHED)
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-08-16 22:07:51 +0000
+++ lib/lp/testing/factory.py	2010-08-16 22:07:55 +0000
@@ -2319,7 +2319,8 @@
                                  dsc_format='1.0', dsc_binaries='foo-bin',
                                  date_uploaded=UTC_NOW,
                                  source_package_recipe_build=None,
-                                 dscsigningkey=None):
+                                 dscsigningkey=None,
+                                 changelog_entry=None):
         """Make a `SourcePackageRelease`."""
         if distroseries is None:
             if source_package_recipe_build is not None:
@@ -2376,7 +2377,7 @@
             build_conflicts_indep=build_conflicts_indep,
             architecturehintlist=architecturehintlist,
             changelog=None,
-            changelog_entry=None,
+            changelog_entry=changelog_entry,
             dsc=None,
             copyright=self.getUniqueString(),
             dscsigningkey=dscsigningkey,
@@ -2636,16 +2637,12 @@
         if installed_size is None:
             installed_size = self.getUniqueInteger()
         bpr = build.createBinaryPackageRelease(
-                binarypackagename=binarypackagename, version=version,
-                binpackageformat=binpackageformat,
-                component=component, section=section, priority=priority,
-                summary=summary, description=description,
-                architecturespecific=architecturespecific,
-                shlibdeps=shlibdeps, depends=depends, recommends=recommends,
-                suggests=suggests, conflicts=conflicts, replaces=replaces,
-                provides=provides, pre_depends=pre_depends,
-                enhances=enhances, breaks=breaks, essential=essential,
-                installedsize=installed_size, debug_package=debug_package)
+                binarypackagename, version, summary, description,
+                binpackageformat, component, section, priority,
+                shlibdeps, depends, recommends, suggests, conflicts,
+                replaces, provides, pre_depends, enhances, breaks,
+                essential, installed_size, architecturespecific,
+                debug_package)
         if date_created is not None:
             removeSecurityProxy(bpr).datecreated = date_created
         return bpr

=== modified file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py	2010-08-16 22:07:51 +0000
+++ lib/lp/testing/tests/test_factory.py	2010-08-16 22:07:55 +0000
@@ -458,6 +458,17 @@
             dsc_maintainer_rfc822=maintainer)
         self.assertEqual(maintainer, spr.dsc_maintainer_rfc822)
 
+    def test_makeSourcePackageRelease_allows_None_changelog_entry(self):
+        spr = self.factory.makeSourcePackageRelease(
+            changelog_entry=None)
+        self.assertEqual(None, spr.changelog_entry)
+
+    def test_makeSourcePackageRelease_uses_changelog_entry(self):
+        changelog_entry = "Best release evar!!1!1!"
+        spr = self.factory.makeSourcePackageRelease(
+            changelog_entry=changelog_entry)
+        self.assertEqual(changelog_entry, spr.changelog_entry)
+
     # makeSuiteSourcePackage
     def test_makeSuiteSourcePackage_returns_ISuiteSourcePackage(self):
         ssp = self.factory.makeSuiteSourcePackage()