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

Proposed by Colin Watson
Status: Merged
Approved by: Richard Harding
Approved revision: no longer in the source branch.
Merged at revision: 15068
Proposed branch: lp:~cjwatson/launchpad/publish-proposed
Merge into: lp:launchpad
Diff against target: 86 lines (+45/-3)
2 files modified
lib/lp/registry/model/distroseries.py (+5/-2)
lib/lp/registry/tests/test_distroseries.py (+40/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/publish-proposed
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Richard Harding (community) code* Approve
Review via email: mp+100985@code.launchpad.net

Commit message

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

Description of the change

== 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.

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

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*)
Revision history for this message
Colin Watson (cjwatson) wrote :

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.

Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1646 # we're not publishing packages into the wrong pocket.1646 # we're not publishing packages into the wrong pocket.
1647 # Unfortunately for careful mode that can't hold true1647 # Unfortunately for careful mode that can't hold true
1648 # because we indeed need to republish everything.1648 # because we indeed need to republish everything.
1649 if (self.isUnstable() and1649 pre_release_pockets = (
1650 publication.pocket != PackagePublishingPocket.RELEASE):1650 PackagePublishingPocket.RELEASE,
1651 PackagePublishingPocket.PROPOSED,
1652 )
1653 if self.isUnstable() and publication.pocket not in pre_release_pockets:
1651 log.error("Tried to publish %s (%s) into a non-release "1654 log.error("Tried to publish %s (%s) into a non-release "
1652 "pocket on unstable series %s, skipping"1655 "pocket on unstable series %s, skipping"
1653 % (publication.displayname, publication.id,1656 % (publication.displayname, publication.id,
16541657
=== 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 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests for distroseries."""4"""Tests for distroseries."""
@@ -6,6 +6,7 @@
6__metaclass__ = type6__metaclass__ = type
77
8from datetime import timedelta8from datetime import timedelta
9from logging import getLogger
910
10import transaction11import transaction
11from zope.component import getUtility12from zope.component import getUtility
@@ -14,6 +15,7 @@
14from lp.registry.errors import NoSuchDistroSeries15from lp.registry.errors import NoSuchDistroSeries
15from lp.registry.interfaces.distroseries import IDistroSeriesSet16from lp.registry.interfaces.distroseries import IDistroSeriesSet
16from lp.registry.interfaces.pocket import PackagePublishingPocket17from lp.registry.interfaces.pocket import PackagePublishingPocket
18from lp.registry.interfaces.series import SeriesStatus
17from lp.services.utils import utc_now19from lp.services.utils import utc_now
18from lp.soyuz.enums import (20from lp.soyuz.enums import (
19 ArchivePurpose,21 ArchivePurpose,
@@ -305,6 +307,43 @@
305 self.assertContentEqual(307 self.assertContentEqual(
306 [comment], distroseries.getDifferenceComments())308 [comment], distroseries.getDifferenceComments())
307309
310 def checkLegalPocket(self, status, pocket):
311 distroseries = self.factory.makeDistroSeries(status=status)
312 spph = self.factory.makeSourcePackagePublishingHistory(
313 distroseries=distroseries, pocket=pocket)
314 return removeSecurityProxy(distroseries).checkLegalPocket(
315 spph, False, getLogger())
316
317 def test_checkLegalPocket_allows_unstable_release(self):
318 """Publishing to RELEASE in a DEVELOPMENT series is allowed."""
319 self.assertTrue(self.checkLegalPocket(
320 SeriesStatus.DEVELOPMENT, PackagePublishingPocket.RELEASE))
321
322 def test_checkLegalPocket_allows_unstable_proposed(self):
323 """Publishing to PROPOSED in a DEVELOPMENT series is allowed."""
324 self.assertTrue(self.checkLegalPocket(
325 SeriesStatus.DEVELOPMENT, PackagePublishingPocket.PROPOSED))
326
327 def test_checkLegalPocket_forbids_unstable_updates(self):
328 """Publishing to UPDATES in a DEVELOPMENT series is forbidden."""
329 self.assertFalse(self.checkLegalPocket(
330 SeriesStatus.DEVELOPMENT, PackagePublishingPocket.UPDATES))
331
332 def test_checkLegalPocket_forbids_stable_release(self):
333 """Publishing to RELEASE in a DEVELOPMENT series is forbidden."""
334 self.assertFalse(self.checkLegalPocket(
335 SeriesStatus.CURRENT, PackagePublishingPocket.RELEASE))
336
337 def test_checkLegalPocket_allows_stable_proposed(self):
338 """Publishing to PROPOSED in a DEVELOPMENT series is allowed."""
339 self.assertTrue(self.checkLegalPocket(
340 SeriesStatus.CURRENT, PackagePublishingPocket.PROPOSED))
341
342 def test_checkLegalPocket_allows_stable_updates(self):
343 """Publishing to UPDATES in a DEVELOPMENT series is allowed."""
344 self.assertTrue(self.checkLegalPocket(
345 SeriesStatus.CURRENT, PackagePublishingPocket.UPDATES))
346
308347
309class TestDistroSeriesPackaging(TestCaseWithFactory):348class TestDistroSeriesPackaging(TestCaseWithFactory):
310349