Merge lp:~cjwatson/launchpad/publish-proposed into lp:launchpad

[Colin Watson]

Fix publishing of pre-release uploads to the PROPOSED pocket.

== Summary ==

As reported in bug 974328, pre-release uploads to Ubuntu precise-proposed are accepted and built but cannot be published.

== Proposed fix ==

I basically just missed a case in my previous fix to allow using -proposed as a staging pocket, and need to permit it in checkLegalPocket as well for publishing.

== Implementation details ==

I'm +37 on LoC for this branch. I need to get this fixed fairly promptly if possible, so perhaps I could use some of the -531 credit from r15032 against this?

== Tests ==

bin/test -vvct checkLegalPocket

(I didn't see any more sensible place to test this, so it seemed best to just add unit tests at the finest possible granularity.)

== Demo and Q/A ==

I think we need an end-to-end test now to make sure there's nothing else, so:

 * Upload a test package (say, hello) to oneiric-proposed on dogfood. (oneiric is DEVELOPMENT there.)
 * Process the upload and wait for it to build everywhere.
 * Do a full publisher run. Ensure that it is actually published.
 * Copy the package to oneiric.
 * Do another full publisher run. Ensure that the copy is actually published.

(Unfortunately this will take a while, dogfood being what it is.)

== lint ==

None.

[Richard Harding (rharding)]

Thanks for the update. Per the test style guide, please add docstrings to the tests.
https://dev.launchpad.net/TestsStyleGuide#line-416

I'm also tempted to ask that the test conditions match the name. It seems backwards that the test "forbids" and yet the assertion is assertTrue as in the tests that "allows".

The change itself looks fine though. Approving the MP with these suggestions.

review: Approve (code*)

[Colin Watson (cjwatson)]

Thanks, will do. And for the conditions: oh, err, the assertions ought to be assertFalse, which raises the question of why it's passing ... I'll look into that.

[j.c.sackett (jcsackett)]

This looks good to me.

review: Approve

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-03-28 13:14:22 +0000
+++ lib/lp/registry/model/distroseries.py	2012-04-05 15:54:19 +0000
@@ -1646,8 +1646,11 @@
         # we're not publishing packages into the wrong pocket.
         # Unfortunately for careful mode that can't hold true
         # because we indeed need to republish everything.
-        if (self.isUnstable() and
-            publication.pocket != PackagePublishingPocket.RELEASE):
+        pre_release_pockets = (
+            PackagePublishingPocket.RELEASE,
+            PackagePublishingPocket.PROPOSED,
+            )
+        if self.isUnstable() and publication.pocket not in pre_release_pockets:
             log.error("Tried to publish %s (%s) into a non-release "
                       "pocket on unstable series %s, skipping"
                       % (publication.displayname, publication.id,

=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py	2012-02-07 08:09:36 +0000
+++ lib/lp/registry/tests/test_distroseries.py	2012-04-05 15:54:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for distroseries."""
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 from datetime import timedelta
+from logging import getLogger
 
 import transaction
 from zope.component import getUtility
@@ -14,6 +15,7 @@
 from lp.registry.errors import NoSuchDistroSeries
 from lp.registry.interfaces.distroseries import IDistroSeriesSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.utils import utc_now
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -305,6 +307,43 @@
         self.assertContentEqual(
             [comment], distroseries.getDifferenceComments())
 
+    def checkLegalPocket(self, status, pocket):
+        distroseries = self.factory.makeDistroSeries(status=status)
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, pocket=pocket)
+        return removeSecurityProxy(distroseries).checkLegalPocket(
+            spph, False, getLogger())
+
+    def test_checkLegalPocket_allows_unstable_release(self):
+        """Publishing to RELEASE in a DEVELOPMENT series is allowed."""
+        self.assertTrue(self.checkLegalPocket(
+            SeriesStatus.DEVELOPMENT, PackagePublishingPocket.RELEASE))
+
+    def test_checkLegalPocket_allows_unstable_proposed(self):
+        """Publishing to PROPOSED in a DEVELOPMENT series is allowed."""
+        self.assertTrue(self.checkLegalPocket(
+            SeriesStatus.DEVELOPMENT, PackagePublishingPocket.PROPOSED))
+
+    def test_checkLegalPocket_forbids_unstable_updates(self):
+        """Publishing to UPDATES in a DEVELOPMENT series is forbidden."""
+        self.assertFalse(self.checkLegalPocket(
+            SeriesStatus.DEVELOPMENT, PackagePublishingPocket.UPDATES))
+
+    def test_checkLegalPocket_forbids_stable_release(self):
+        """Publishing to RELEASE in a DEVELOPMENT series is forbidden."""
+        self.assertFalse(self.checkLegalPocket(
+            SeriesStatus.CURRENT, PackagePublishingPocket.RELEASE))
+
+    def test_checkLegalPocket_allows_stable_proposed(self):
+        """Publishing to PROPOSED in a DEVELOPMENT series is allowed."""
+        self.assertTrue(self.checkLegalPocket(
+            SeriesStatus.CURRENT, PackagePublishingPocket.PROPOSED))
+
+    def test_checkLegalPocket_allows_stable_updates(self):
+        """Publishing to UPDATES in a DEVELOPMENT series is allowed."""
+        self.assertTrue(self.checkLegalPocket(
+            SeriesStatus.CURRENT, PackagePublishingPocket.UPDATES))
+
 
 class TestDistroSeriesPackaging(TestCaseWithFactory):