launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/bug-832652 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #832652 in Launchpad itself: "In InitializeDistroSeries.check, we should always check for pending builds before we initialize."
  https://bugs.launchpad.net/launchpad/+bug/832652

For more details, see:
https://code.launchpad.net/~rvb/launchpad/bug-832652/+merge/72702

This branches fixes the check that we do before a series is initialized. We should always check for pending builds.

This branch also features two drive-by fixes: 
- "len(self.arches) != 0" is more solid than "self.arches is not ()". 
- It also fixes test_do_not_copy_superseded_sources and test_failure_with_binary_queue_items_in_packagesets (having twice the child parameter must have been the result of a bad merge)

= Tests =

(modified tests)
./bin/test -vvc test_initialize_distroseries

= Q/A =

Create pending builds for a series "s1" and initialize a new series with "s1" as a parent. The initialization should be prevented.
-- 
https://code.launchpad.net/~rvb/launchpad/bug-832652/+merge/72702
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/bug-832652 into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2011-08-22 11:44:04 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2011-08-24 11:44:20 +0000
@@ -156,8 +156,7 @@
                     child=self.distroseries))
         self._checkParents()
         for parent in self.derivation_parents:
-            if self.distroseries.distribution.id == parent.distribution.id:
-                self._checkBuilds(parent)
+            self._checkBuilds(parent)
             self._checkQueue(parent)
         self._checkSeries()
 
@@ -189,7 +188,7 @@
         # spns=None means no packagesets selected so we need to consider
         # all sources.
 
-        arch_tags = self.arches if self.arches is not () else None
+        arch_tags = self.arches if len(self.arches) != 0 else None
         pending_builds = parent.getBuildRecords(
             BuildStatus.NEEDSBUILD, pocket=INIT_POCKETS,
             arch_tag=arch_tags, name=spns)

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-08-22 11:44:04 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2011-08-24 11:44:20 +0000
@@ -218,8 +218,7 @@
                 distroseries=self.parent,
                 pocket=pocket)
             source.createMissingBuilds()
-            child = self.factory.makeDistroSeries(
-                distribution=self.parent.parent, previous_series=self.parent)
+            child = self.factory.makeDistroSeries()
             ids = InitializeDistroSeries(child, [self.parent.id])
             self.assertRaisesWithContent(
                 InitializationError,
@@ -240,8 +239,7 @@
                 distroseries=self.parent,
                 pocket=pocket)
             source.createMissingBuilds()
-            child = self.factory.makeDistroSeries(
-                distribution=self.parent.parent, previous_series=self.parent)
+            child = self.factory.makeDistroSeries()
             ids = InitializeDistroSeries(child, [self.parent.id])
             # No exception should be raised.
             ids.check()
@@ -255,10 +253,9 @@
         # Create builds for the architecture of parent_das2.
         source.createMissingBuilds(architectures_available=[parent_das2])
         # Initialize only with parent_das2's architecture.
-        child = self.factory.makeDistroSeries(
-            distribution=parent.distribution, previous_series=parent)
+        child = self.factory.makeDistroSeries()
         ids = InitializeDistroSeries(
-            child, arches=[parent_das2.architecturetag])
+            child, [parent.id], arches=[parent_das2.architecturetag])
 
         self.assertRaisesWithContent(
             InitializationError,
@@ -354,18 +351,6 @@
         self.assertDistroSeriesInitializedCorrectly(
             child, self.parent, self.parent_das)
 
-    def test_success_with_pending_builds(self):
-        # If the parent series has pending builds, and the child's
-        # distribution is different, we can initialize.
-        self.parent, self.parent_das = self.setupParent()
-        source = self.factory.makeSourcePackagePublishingHistory(
-            distroseries=self.parent,
-            pocket=PackagePublishingPocket.RELEASE)
-        source.createMissingBuilds()
-        child = self._fullInitialize([self.parent])
-        self.assertDistroSeriesInitializedCorrectly(
-            child, self.parent, self.parent_das)
-
     def test_do_not_copy_superseded_sources(self):
         # Make sure we don't copy superseded sources from the parent,
         # we only want (pending, published).
@@ -523,8 +508,8 @@
             source_package_release=spr2))
         child = self.factory.makeDistroSeries()
         # Initialize with packageset1 only.
-        ids = InitializeDistroSeries(child, [parent.id],
-            child, packagesets=(str(packageset1.id),))
+        ids = InitializeDistroSeries(
+            child, [parent.id], packagesets=(str(packageset1.id),))
 
         # No exception should be raised.
         ids.check()
@@ -547,8 +532,8 @@
             source_package_release=spr1))
         child = self.factory.makeDistroSeries()
         # Initialize with packageset1 only.
-        ids = InitializeDistroSeries(child, [parent.id],
-            child, packagesets=(str(packageset1.id),))
+        ids = InitializeDistroSeries(
+            child, [parent.id], packagesets=(str(packageset1.id),))
 
         self.assertRaisesWithContent(
             InitializationError,