← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/ids-no-more-sampledata into lp:launchpad/devel

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/ids-no-more-sampledata into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


First, a little bit of history: The tests for (what is now) InitialiseDistroSeries used to be a doctest, and I split them out in a unit test, replicating every part of the doctest. This branch now removes the tests dependence on sample data, as well as removing two unit tests that no longer make sense since they were dependent on sample data to function correctly.

The tests now creates a fresh distroseries in the setup, and populates it with four source packages, 3 binary packages and 1 failed to build. Mostly this is just moving code around, along with dropping the word 'foobuntu' everywhere I saw it.

It also fixes a bug, which was previously being masked by using the sampledata -- it now creates builds unilaterally if one doesn't exist when the rebuild=True option is passed in, and tests that correctly, rather than using hard-coded numerical values.

Finally, I cleaned up some lint that I noticed.
-- 
https://code.launchpad.net/~stevenk/launchpad/ids-no-more-sampledata/+merge/37102
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/ids-no-more-sampledata into lp:launchpad/devel.
=== modified file 'lib/lp/soyuz/model/packagecloner.py'
--- lib/lp/soyuz/model/packagecloner.py	2010-08-24 15:29:01 +0000
+++ lib/lp/soyuz/model/packagecloner.py	2010-09-30 06:57:46 +0000
@@ -26,6 +26,7 @@
     IStoreSelector,
     MAIN_STORE,
     )
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.packagecloner import IPackageCloner
@@ -90,9 +91,12 @@
             proc_families = []
 
         self._create_missing_builds(
-            destination.distroseries, destination.archive, proc_families)
+            destination.distroseries, destination.archive,
+            distroarchseries_list, proc_families)
 
-    def _create_missing_builds(self, distroseries, archive, proc_families):
+    def _create_missing_builds(
+        self, distroseries, archive, distroarchseries_list,
+        proc_families):
         """Create builds for all cloned source packages.
 
         :param distroseries: the distro series for which to create builds.
@@ -126,7 +130,17 @@
             return pub.sourcepackagerelease.sourcepackagename.name
 
         for pubrec in sources_published:
-            pubrec.createMissingBuilds(architectures_available=architectures)
+            builds = pubrec.createMissingBuilds(
+                architectures_available=architectures)
+            # If a distroarchseries_list wasn't passed in, we don't copy any
+            # binary packages at all -- if the last build was sucessful, we
+            # should create a new build, since createMissingBuilds() won't.
+            if not distroarchseries_list and not builds:
+                for arch in architectures:
+                    build = pubrec.sourcepackagerelease.createBuild(
+                        distro_arch_series=arch, archive=archive,
+                        pocket=PackagePublishingPocket.RELEASE)
+                    build.queueBuild(suspended=not archive.enabled)
             # Commit to avoid MemoryError: bug 304459
             transaction.commit()
 
@@ -245,7 +259,8 @@
                 secsrc.sourcepackagerelease = spr.id AND
                 spr.sourcepackagename = spn.id AND
                 spn.name = mcd.sourcepackagename AND
-                debversion_sort_key(spr.version) > debversion_sort_key(mcd.t_version)
+                debversion_sort_key(spr.version) >
+                debversion_sort_key(mcd.t_version)
         """ % sqlvalues(
                 origin.archive,
                 PackagePublishingStatus.PENDING,

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2010-09-22 06:42:40 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2010-09-30 06:57:46 +0000
@@ -13,7 +13,6 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from canonical.launchpad.interfaces import IDistributionSet
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.buildmaster.enums import BuildStatus
@@ -21,15 +20,16 @@
 from lp.soyuz.enums import SourcePackageFormat
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.packageset import IPackagesetSet
+from lp.soyuz.interfaces.publishing import PackagePublishingStatus
+from lp.soyuz.interfaces.sourcepackageformat import (
+    ISourcePackageFormatSelectionSet,
+    )
 from lp.soyuz.model.distroarchseries import DistroArchSeries
 from lp.soyuz.scripts.initialise_distroseries import (
     InitialisationError,
     InitialiseDistroSeries,
     )
-from lp.testing import (
-    login,
-    TestCaseWithFactory,
-    )
+from lp.testing import TestCaseWithFactory
 
 
 class TestInitialiseDistroSeries(TestCaseWithFactory):
@@ -38,33 +38,59 @@
 
     def setUp(self):
         super(TestInitialiseDistroSeries, self).setUp()
-        login("foo.bar@xxxxxxxxxxxxx")
-        distribution_set = getUtility(IDistributionSet)
-        self.ubuntutest = distribution_set['ubuntutest']
-        self.ubuntu = distribution_set['ubuntu']
-        self.hoary = self.ubuntu['hoary']
-
-    def _create_distroseries(self, parent_series):
-        return self.ubuntutest.newSeries(
-            'foobuntu', 'FooBuntu', 'The Foobuntu', 'yeck', 'doom',
-            '888', parent_series, self.hoary.owner)
-
-    def _set_pending_to_failed(self, distroseries):
-        pending_builds = distroseries.getBuildRecords(
-            BuildStatus.NEEDSBUILD, pocket=PackagePublishingPocket.RELEASE)
-        for build in pending_builds:
-            build.status = BuildStatus.FAILEDTOBUILD
+        self.parent = self.factory.makeDistroSeries()
+        pf = self.factory.makeProcessorFamily()
+        pf.addProcessor('x86', '', '')
+        self.parent_das = self.factory.makeDistroArchSeries(
+            distroseries=self.parent, processorfamily=pf)
+        lf = self.factory.makeLibraryFileAlias()
+        transaction.commit()
+        self.parent_das.addOrUpdateChroot(lf)
+        self.parent_das.supports_virtualized = True
+        self.parent.nominatedarchindep = self.parent_das
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            self.parent, SourcePackageFormat.FORMAT_1_0)
+        self._populate_parent()
+
+    def _populate_parent(self):
+        packages = {'udev': '0.1-1', 'libc6': '2.8-1',
+            'postgresql': '9.0-1', 'chromium': '3.6'}
+        for package in packages.keys():
+            spn = self.factory.makeSourcePackageName(package)
+            spph = self.factory.makeSourcePackagePublishingHistory(
+                sourcepackagename=spn, version=packages[package],
+                distroseries=self.parent,
+                pocket=PackagePublishingPocket.RELEASE,
+                status=PackagePublishingStatus.PUBLISHED)
+            status = BuildStatus.FULLYBUILT
+            if package is 'chromium':
+                status = BuildStatus.FAILEDTOBUILD
+            bpn = self.factory.makeBinaryPackageName(package)
+            build = self.factory.makeBinaryPackageBuild(
+                source_package_release=spph.sourcepackagerelease,
+                distroarchseries=self.parent_das,
+                status=status)
+            bpr = self.factory.makeBinaryPackageRelease(
+                binarypackagename=bpn, build=build,
+                version=packages[package])
+            if package is not 'chromium':
+                self.factory.makeBinaryPackagePublishingHistory(
+                    binarypackagerelease=bpr,
+                    distroarchseries=self.parent_das,
+                    pocket=PackagePublishingPocket.RELEASE,
+                    status=PackagePublishingStatus.PUBLISHED)
 
     def test_failure_with_no_parent_series(self):
         # Initialising a new distro series requires a parent series to be set
-        foobuntu = self._create_distroseries(None)
-        ids = InitialiseDistroSeries(foobuntu)
+        ids = InitialiseDistroSeries(self.factory.makeDistroSeries())
         self.assertRaisesWithContent(
             InitialisationError, "Parent series required.", ids.check)
 
     def test_failure_for_already_released_distroseries(self):
         # Initialising a distro series that has already been used will error
-        ids = InitialiseDistroSeries(self.ubuntutest['breezy-autotest'])
+        child = self.factory.makeDistroSeries(parent_series=self.parent)
+        das = self.factory.makeDistroArchSeries(distroseries=child)
+        ids = InitialiseDistroSeries(child)
         self.assertRaisesWithContent(
             InitialisationError,
             "Can not copy distroarchseries from parent, there are already "
@@ -72,9 +98,13 @@
 
     def test_failure_with_pending_builds(self):
         # If the parent series has pending builds, we can't initialise
-        foobuntu = self._create_distroseries(self.hoary)
-        transaction.commit()
-        ids = InitialiseDistroSeries(foobuntu)
+        source = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=self.parent,
+            pocket=PackagePublishingPocket.RELEASE)
+        source.createMissingBuilds()
+        child = self.factory.makeDistroSeries(
+            parent_series=self.parent)
+        ids = InitialiseDistroSeries(child)
         self.assertRaisesWithContent(
             InitialisationError, "Parent series has pending builds.",
             ids.check)
@@ -82,190 +112,156 @@
     def test_failure_with_queue_items(self):
         # If the parent series has items in its queues, such as NEW and
         # UNAPPROVED, we can't initialise
-        foobuntu = self._create_distroseries(
-            self.ubuntu['breezy-autotest'])
-        ids = InitialiseDistroSeries(foobuntu)
+        self.parent.createQueueEntry(
+            PackagePublishingPocket.RELEASE,
+            'foo.changes', 'bar', self.parent.main_archive)
+        child = self.factory.makeDistroSeries(parent_series=self.parent)
+        ids = InitialiseDistroSeries(child)
         self.assertRaisesWithContent(
             InitialisationError, "Parent series queues are not empty.",
             ids.check)
 
-    def assertDistroSeriesInitialisedCorrectly(self, foobuntu):
-        # Check that 'pmount' has been copied correctly
-        hoary_pmount_pubs = self.hoary.getPublishedSources('pmount')
-        foobuntu_pmount_pubs = foobuntu.getPublishedSources('pmount')
-        self.assertEqual(
-            hoary_pmount_pubs.count(),
-            foobuntu_pmount_pubs.count())
-        hoary_i386_pmount_pubs = self.hoary['i386'].getReleasedPackages(
-            'pmount')
-        foobuntu_i386_pmount_pubs = foobuntu['i386'].getReleasedPackages(
-            'pmount')
-        self.assertEqual(
-            len(hoary_i386_pmount_pubs), len(foobuntu_i386_pmount_pubs))
+    def assertDistroSeriesInitialisedCorrectly(self, child):
+        # Check that 'udev' has been copied correctly
+        parent_udev_pubs = self.parent.getPublishedSources('udev')
+        child_udev_pubs = child.getPublishedSources('udev')
+        self.assertEqual(
+            parent_udev_pubs.count(), child_udev_pubs.count())
+        parent_arch_udev_pubs = self.parent[
+            self.parent_das.architecturetag].getReleasedPackages('udev')
+        child_arch_udev_pubs = child[
+            self.parent_das.architecturetag].getReleasedPackages('udev')
+        self.assertEqual(
+            len(parent_arch_udev_pubs), len(child_arch_udev_pubs))
         # And the binary package, and linked source package look fine too
-        pmount_binrel = (
-            foobuntu['i386'].getReleasedPackages(
-            'pmount')[0].binarypackagerelease)
-        self.assertEqual(pmount_binrel.title, u'pmount-0.1-1')
-        self.assertEqual(pmount_binrel.build.id, 7)
+        udev_bin = child_arch_udev_pubs[0].binarypackagerelease
+        self.assertEqual(udev_bin.title, u'udev-0.1-1')
         self.assertEqual(
-            pmount_binrel.build.title,
-            u'i386 build of pmount 0.1-1 in ubuntu hoary RELEASE')
-        pmount_srcrel = pmount_binrel.build.source_package_release
-        self.assertEqual(pmount_srcrel.title, u'pmount - 0.1-1')
-        # The build of pmount 0.1-1 has been copied across.
-        foobuntu_pmount = pmount_srcrel.getBuildByArch(
-            foobuntu['i386'], foobuntu.main_archive)
-        hoary_pmount = pmount_srcrel.getBuildByArch(
-            self.hoary['i386'], self.hoary.main_archive)
-        self.assertEqual(foobuntu_pmount.id, hoary_pmount.id)
+            udev_bin.build.title,
+            u'%s build of udev 0.1-1 in %s %s RELEASE' % (
+                self.parent_das.architecturetag, self.parent.parent.name,
+                self.parent.name))
+        udev_src = udev_bin.build.source_package_release
+        self.assertEqual(udev_src.title, u'udev - 0.1-1')
+        # The build of udev 0.1-1 has been copied across.
+        child_udev = udev_src.getBuildByArch(
+            child[self.parent_das.architecturetag], child.main_archive)
+        parent_udev = udev_src.getBuildByArch(
+            self.parent[self.parent_das.architecturetag],
+            self.parent.main_archive)
+        self.assertEqual(parent_udev.id, child_udev.id)
         # We also inherient the permitted source formats from our parent
         self.assertTrue(
-            foobuntu.isSourcePackageFormatPermitted(
+            child.isSourcePackageFormatPermitted(
             SourcePackageFormat.FORMAT_1_0))
 
     def _full_initialise(self, arches=(), rebuild=False):
-        foobuntu = self._create_distroseries(self.hoary)
-        self._set_pending_to_failed(self.hoary)
-        transaction.commit()
-        ids = InitialiseDistroSeries(foobuntu, arches, rebuild)
+        child = self.factory.makeDistroSeries(parent_series=self.parent)
+        ids = InitialiseDistroSeries(child, arches, rebuild)
         ids.check()
         ids.initialise()
-        return foobuntu
+        return child
 
     def test_initialise(self):
         # Test a full initialise with no errors
-        foobuntu = self._full_initialise()
-        self.assertDistroSeriesInitialisedCorrectly(foobuntu)
+        child = self._full_initialise()
+        self.assertDistroSeriesInitialisedCorrectly(child)
 
-    def test_initialise_only_i386(self):
+    def test_initialise_only_one_das(self):
         # Test a full initialise with no errors, but only copy i386 to
         # the child
-        foobuntu = self._full_initialise(arches=('i386',))
-        self.assertDistroSeriesInitialisedCorrectly(foobuntu)
+        parent_das = self.factory.makeDistroArchSeries(
+            distroseries=self.parent)
+        child = self._full_initialise(
+            arches=[self.parent_das.architecturetag])
+        self.assertDistroSeriesInitialisedCorrectly(child)
         das = list(IStore(DistroArchSeries).find(
-            DistroArchSeries, distroseries = foobuntu))
+            DistroArchSeries, distroseries = child))
         self.assertEqual(len(das), 1)
-        self.assertEqual(das[0].architecturetag, 'i386')
-
-    def test_check_no_builds(self):
-        # Test that there is no build for pmount 0.1-2 in the
-        # newly-initialised series.
-        foobuntu = self._full_initialise()
-        pmount_source = self.hoary.getSourcePackage(
-            'pmount').currentrelease
-        self.assertEqual(
-            pmount_source.title,
-            '"pmount" 0.1-2 source package in The Hoary Hedgehog Release')
-        pmount_source = foobuntu.getSourcePackage('pmount').currentrelease
-        self.assertEqual(
-            pmount_source.title,
-            '"pmount" 0.1-2 source package in The Foobuntu')
-        self.assertEqual(
-            pmount_source.sourcepackagerelease.getBuildByArch(
-            foobuntu['i386'], foobuntu.main_archive), None)
-        self.assertEqual(
-            pmount_source.sourcepackagerelease.getBuildByArch(
-            foobuntu['hppa'], foobuntu.main_archive), None)
-
-    def test_create_builds(self):
-        # It turns out the sampledata of hoary includes pmount 0.1-1 as well
-        # as pmount 0.1-2 source, and if foobuntu and hoary don't share a
-        # pool, 0.1-1 will be marked as NBS and removed. So let's check that
-        # builds can be created for foobuntu.
-        foobuntu = self._full_initialise()
-        pmount_source = foobuntu.getSourcePackage('pmount').currentrelease
-        created_build = pmount_source.sourcepackagerelease.createBuild(
-            foobuntu['i386'], PackagePublishingPocket.RELEASE,
-            foobuntu.main_archive)
-        retrieved_build = pmount_source.sourcepackagerelease.getBuildByArch(
-            foobuntu['i386'], foobuntu.main_archive)
-        self.assertEqual(retrieved_build.id, created_build.id)
-        self.assertEqual(
-            'i386 build of pmount 0.1-2 in ubuntutest foobuntu RELEASE',
-            created_build.title)
+        self.assertEqual(
+            das[0].architecturetag, self.parent_das.architecturetag)
 
     def test_copying_packagesets(self):
         # If a parent series has packagesets, we should copy them
         uploader = self.factory.makePerson()
         test1 = getUtility(IPackagesetSet).new(
-            u'test1', u'test 1 packageset', self.hoary.owner,
-            distroseries=self.hoary)
+            u'test1', u'test 1 packageset', self.parent.owner,
+            distroseries=self.parent)
         test2 = getUtility(IPackagesetSet).new(
-            u'test2', u'test 2 packageset', self.hoary.owner,
-            distroseries=self.hoary)
+            u'test2', u'test 2 packageset', self.parent.owner,
+            distroseries=self.parent)
         test3 = getUtility(IPackagesetSet).new(
-            u'test3', u'test 3 packageset', self.hoary.owner,
-            distroseries=self.hoary, related_set=test2)
-        test1.addSources('pmount')
+            u'test3', u'test 3 packageset', self.parent.owner,
+            distroseries=self.parent, related_set=test2)
+        test1.addSources('udev')
         getUtility(IArchivePermissionSet).newPackagesetUploader(
-            self.hoary.main_archive, uploader, test1)
-        foobuntu = self._full_initialise()
-        # We can fetch the copied sets from foobuntu
-        foobuntu_test1 = getUtility(IPackagesetSet).getByName(
-            u'test1', distroseries=foobuntu)
-        foobuntu_test2 = getUtility(IPackagesetSet).getByName(
-            u'test2', distroseries=foobuntu)
-        foobuntu_test3 = getUtility(IPackagesetSet).getByName(
-            u'test3', distroseries=foobuntu)
+            self.parent.main_archive, uploader, test1)
+        child = self._full_initialise()
+        # We can fetch the copied sets from the child
+        child_test1 = getUtility(IPackagesetSet).getByName(
+            u'test1', distroseries=child)
+        child_test2 = getUtility(IPackagesetSet).getByName(
+            u'test2', distroseries=child)
+        child_test3 = getUtility(IPackagesetSet).getByName(
+            u'test3', distroseries=child)
         # And we can see they are exact copies, with the related_set for the
         # copies pointing to the packageset in the parent
-        self.assertEqual(test1.description, foobuntu_test1.description)
-        self.assertEqual(test2.description, foobuntu_test2.description)
-        self.assertEqual(test3.description, foobuntu_test3.description)
-        self.assertEqual(foobuntu_test1.relatedSets().one(), test1)
-        self.assertEqual(
-            list(foobuntu_test2.relatedSets()),
-            [test2, test3, foobuntu_test3])
-        self.assertEqual(
-            list(foobuntu_test3.relatedSets()),
-            [test2, foobuntu_test2, test3])
+        self.assertEqual(test1.description, child_test1.description)
+        self.assertEqual(test2.description, child_test2.description)
+        self.assertEqual(test3.description, child_test3.description)
+        self.assertEqual(child_test1.relatedSets().one(), test1)
+        self.assertEqual(
+            list(child_test2.relatedSets()),
+            [test2, test3, child_test3])
+        self.assertEqual(
+            list(child_test3.relatedSets()),
+            [test2, child_test2, test3])
         # The contents of the packagesets will have been copied.
-        foobuntu_srcs = foobuntu_test1.getSourcesIncluded(
+        child_srcs = child_test1.getSourcesIncluded(
             direct_inclusion=True)
-        hoary_srcs = test1.getSourcesIncluded(direct_inclusion=True)
-        self.assertEqual(foobuntu_srcs, hoary_srcs)
+        parent_srcs = test1.getSourcesIncluded(direct_inclusion=True)
+        self.assertEqual(parent_srcs, child_srcs)
         # The uploader can also upload to the new distroseries.
         self.assertTrue(
             getUtility(IArchivePermissionSet).isSourceUploadAllowed(
-                self.hoary.main_archive, 'pmount', uploader,
-                distroseries=self.hoary))
+                self.parent.main_archive, 'udev', uploader,
+                distroseries=self.parent))
         self.assertTrue(
             getUtility(IArchivePermissionSet).isSourceUploadAllowed(
-                foobuntu.main_archive, 'pmount', uploader,
-                distroseries=foobuntu))
+                child.main_archive, 'udev', uploader,
+                distroseries=child))
 
     def test_rebuild_flag(self):
         # No binaries will get copied if we specify rebuild=True
-        foobuntu = self._full_initialise(rebuild=True)
-        foobuntu.updatePackageCount()
-        builds = foobuntu.getBuildRecords(
+        self.parent.updatePackageCount()
+        child = self._full_initialise(rebuild=True)
+        child.updatePackageCount()
+        builds = child.getBuildRecords(
             build_state=BuildStatus.NEEDSBUILD,
-            pocket=PackagePublishingPocket.RELEASE, arch_tag='i386')
-        self.assertEqual(foobuntu.sourcecount, 7)
-        self.assertEqual(foobuntu.binarycount, 0)
-        self.assertEqual(builds.count(), 5)
+            pocket=PackagePublishingPocket.RELEASE)
+        self.assertEqual(self.parent.sourcecount, child.sourcecount)
+        self.assertEqual(child.binarycount, 0)
+        self.assertEqual(builds.count(), self.parent.sourcecount)
 
     def test_script(self):
         # Do an end-to-end test using the command-line tool
         uploader = self.factory.makePerson()
         test1 = getUtility(IPackagesetSet).new(
-            u'test1', u'test 1 packageset', self.hoary.owner,
-            distroseries=self.hoary)
-        test1.addSources('pmount')
+            u'test1', u'test 1 packageset', self.parent.owner,
+            distroseries=self.parent)
+        test1.addSources('udev')
         getUtility(IArchivePermissionSet).newPackagesetUploader(
-            self.hoary.main_archive, uploader, test1)
-        foobuntu = self._create_distroseries(self.hoary)
-        self._set_pending_to_failed(self.hoary)
+            self.parent.main_archive, uploader, test1)
+        child = self.factory.makeDistroSeries(parent_series=self.parent)
         transaction.commit()
         ifp = os.path.join(
             config.root, 'scripts', 'ftpmaster-tools',
             'initialise-from-parent.py')
         process = subprocess.Popen(
-            [sys.executable, ifp, "-vv", "-d", "ubuntutest", "foobuntu"],
-            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            [sys.executable, ifp, "-vv", "-d", child.parent.name,
+            child.name], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         stdout, stderr = process.communicate()
         self.assertEqual(process.returncode, 0)
         self.assertTrue(
             "DEBUG   Committing transaction." in stderr.split('\n'))
-        self.assertDistroSeriesInitialisedCorrectly(foobuntu)
+        self.assertDistroSeriesInitialisedCorrectly(child)


Follow ups