Merge lp://staging/~al-maisan/launchpad/expired-lfas-516922 into lp://staging/launchpad
- expired-lfas-516922
- Merge into devel
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | not available | ||||
| Proposed branch: | lp://staging/~al-maisan/launchpad/expired-lfas-516922 | ||||
| Merge into: | lp://staging/launchpad | ||||
| Diff against target: |
356 lines (+202/-76) 4 files modified
lib/lp/soyuz/model/packagediff.py (+26/-1) lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py (+4/-71) lib/lp/soyuz/tests/soyuz.py (+80/-4) lib/lp/soyuz/tests/test_packagediff.py (+92/-0) |
||||
| To merge this branch: | bzr merge lp://staging/~al-maisan/launchpad/expired-lfas-516922 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Francis J. Lacoste (community) | release-critical | Approve | |
| Michael Nelson (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
Description of the change
| Muharem Hrnjadovic (al-maisan) wrote : | # |
| Michael Nelson (michael.nelson) wrote : | # |
> === modified file 'lib/lp/
Hi Muharem,
My only real qualm with this branch is the chunk of test setup code that looks identical to that in test_processpen
> --- a/lib/lp/
> +++ b/lib/lp/
> @@ -22,7 +22,7 @@
> from canonical.
> from canonical.
> from canonical.
> -from canonical.
> +from canonical.
> from canonical.
> from lp.soyuz.
> IPackageDiff, IPackageDiffSet, PackageDiffStatus)
> @@ -130,6 +130,25 @@
> """See `IPackageDiff`."""
> return self.to_
>
> + def _countExpiredLF
> + """How many files associated with either source package were
> + already expired by the librarian?"""
> + store = getUtility(
> + query = """
> + SELECT COUNT(lfa.id)
> + FROM
> + SourcePackageRe
> + LibraryFileAlias lfa
> + WHERE
> + spr.id IN %s
> + AND sprf.SourcePack
> + AND sprf.libraryfile = lfa.id
> + AND lfa.expires IS NOT NULL
> + AND lfa.content IS NULL
> + """ % sqlvalues(
> + result = store.execute(
> + return (0 if result is None else result[0])
> +
I'm assuming this wasn't easy with storm ;)
> def performDiff(self):
> """See `IPackageDiff`.
>
> @@ -137,6 +156,12 @@
> from both SPRs involved from the librarian, running debdiff, storing
> the output in the librarian and updating the PackageDiff record.
> """
> + # Make sure the files associated with the two source packages are
> + # still available in the librarian.
> + if self._countExpi
> + self.status = PackageDiffStat
> + return
> +
It's a shame that there isn't a way to communicate the failure. I wonder
whether it's worth a new status, FAILED_
that up to you.
> # Create the temporary directory where the files will be
> # downloaded to and where the debdiff will be performed.
> tmp_dir = tempfile.mkdtemp()
>
> === added file 'lib/lp/
> --- a/lib/lp/
> +++ b/lib/lp/
> @@ -0,0 +1,155 @@
> +# Copyright 2010 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Test source package diffs."""
> +
> +__metaclass__ = type
> +
> +from datetime ...
| Muharem Hrnjadovic (al-maisan) wrote : | # |
On 02/04/2010 05:02 PM, Michael Nelson wrote:
> Review: Needs Fixing code
>> === modified file 'lib/lp/
> Hi Muharem,
>
> My only real qualm with this branch is the chunk of test setup code
> that looks identical to that in test_processpen
> Please create a base test case and re-use it.
Hello Michael,
thanks for the review, I did as you advised, please see the enclosed
incremental diff and my replies below.
>> --- a/lib/lp/
>> +++ b/lib/lp/
>> @@ -22,7 +22,7 @@
>> from canonical.
>> from canonical.
>> from canonical.
>> -from canonical.
>> +from canonical.
>> from canonical.
>> from lp.soyuz.
>> IPackageDiff, IPackageDiffSet, PackageDiffStatus)
>> @@ -130,6 +130,25 @@
>> """See `IPackageDiff`."""
>> return self.to_
>>
>> + def _countExpiredLF
>> + """How many files associated with either source package were
>> + already expired by the librarian?"""
>> + store = getUtility(
>> + query = """
>> + SELECT COUNT(lfa.id)
>> + FROM
>> + SourcePackageRe
>> + LibraryFileAlias lfa
>> + WHERE
>> + spr.id IN %s
>> + AND sprf.SourcePack
>> + AND sprf.libraryfile = lfa.id
>> + AND lfa.expires IS NOT NULL
>> + AND lfa.content IS NULL
>> + """ % sqlvalues(
>> + result = store.execute(
>> + return (0 if result is None else result[0])
>> +
>
> I'm assuming this wasn't easy with storm ;)
Not really.
>> def performDiff(self):
>> """See `IPackageDiff`.
>>
>> @@ -137,6 +156,12 @@
>> from both SPRs involved from the librarian, running debdiff, storing
>> the output in the librarian and updating the PackageDiff record.
>> """
>> + # Make sure the files associated with the two source packages are
>> + # still available in the librarian.
>> + if self._countExpi
>> + self.status = PackageDiffStat
>> + return
>> +
>
> It's a shame that there isn't a way to communicate the failure. I wonder
> whether it's worth a new status, FAILED_
> that up to you.
This sounds like an enhancement we can introduce if and when users ask
for it.
>> # Create the temporary directory where the files will be
>> # downloaded to and where the debdiff will be performed.
>> tmp_dir = tempfile.mkdtemp()
>>
>> === added file 'lib/lp/
>> --- a/lib/lp/
| 1 | === modified file 'lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py' |
| 2 | --- lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py 2009-06-25 04:06:00 +0000 |
| 3 | +++ lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py 2010-02-04 16:34:35 +0000 |
| 4 | @@ -11,83 +11,19 @@ |
| 5 | from zope.component import getUtility |
| 6 | |
| 7 | from canonical.config import config |
| 8 | -from canonical.launchpad.ftests import import_public_test_keys |
| 9 | -from lp.registry.interfaces.distribution import IDistributionSet |
| 10 | -from lp.soyuz.interfaces.packagediff import IPackageDiffSet |
| 11 | -from canonical.launchpad.testing.fakepackager import FakePackager |
| 12 | from canonical.launchpad.scripts import QuietFakeLogger |
| 13 | -from lp.soyuz.scripts.packagediff import ( |
| 14 | - ProcessPendingPackageDiffs) |
| 15 | -from canonical.launchpad.database import LibraryFileAlias |
| 16 | +from lp.soyuz.scripts.packagediff import ProcessPendingPackageDiffs |
| 17 | +from lp.soyuz.tests.soyuz import TestPackageDiffsBase |
| 18 | from canonical.testing import LaunchpadZopelessLayer |
| 19 | |
| 20 | |
| 21 | -class TestProcessPendingPackageDiffsScript(unittest.TestCase): |
| 22 | +class TestProcessPendingPackageDiffsScript(TestPackageDiffsBase): |
| 23 | """Test the process-pending-packagediffs.py script.""" |
| 24 | layer = LaunchpadZopelessLayer |
| 25 | dbuser = config.uploader.dbuser |
| 26 | |
| 27 | def setUp(self): |
| 28 | - """Setup proper DB connection and contents for tests |
| 29 | - |
| 30 | - Connect to the DB as the 'uploader' user (same user used in the |
| 31 | - script), upload the test packages (see `uploadTestPackages`) and |
| 32 | - commit the transaction. |
| 33 | - |
| 34 | - Store the `FakePackager` object used in the test uploads as `packager` |
| 35 | - so the tests can reuse it if necessary. |
| 36 | - """ |
| 37 | - self.layer.alterConnection(dbuser='launchpad') |
| 38 | - |
| 39 | - fake_chroot = LibraryFileAlias.get(1) |
| 40 | - ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
| 41 | - warty = ubuntu.getSeries('warty') |
| 42 | - warty['i386'].addOrUpdateChroot(fake_chroot) |
| 43 | - |
| 44 | - self.layer.txn.commit() |
| 45 | - |
| 46 | - self.layer.alterConnection(dbuser=self.dbuser) |
| 47 | - self.packager = self.uploadTestPackages() |
| 48 | - self.layer.txn.commit() |
| 49 | - |
| 50 | - def uploadTestPackages(self): |
| 51 | - """Upload packages for testing `PackageDiff` generation script. |
| 52 | - |
| 53 | - Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between |
| 54 | - them is created. |
| 55 | - |
| 56 | - Assert there is not pending `PackageDiff` in the DB before uploading |
| 57 | - the package and also assert that there is one after the uploads. |
| 58 | - |
| 59 | - :return: the FakePackager object used to generate and upload the test, |
| 60 | - packages, so the tests can upload subsequent version if necessary. |
| 61 | - """ |
| 62 | - # No pending PackageDiff available in sampledata. |
| 63 | - self.assertEqual(self.getPendingDiffs().count(), 0) |
| 64 | - |
| 65 | - import_public_test_keys() |
| 66 | - # Use FakePackager to upload a base package to ubuntu. |
| 67 | - packager = FakePackager( |
| 68 | - 'zeca', '1.0', 'foo.bar@canonical.com-passwordless.sec') |
| 69 | - packager.buildUpstream() |
| 70 | - packager.buildSource() |
| 71 | - packager.uploadSourceVersion('1.0-1', suite="warty-updates") |
| 72 | - |
| 73 | - # Upload a new version of the source, so a PackageDiff can |
| 74 | - # be created. |
| 75 | - packager.buildVersion('1.0-2', changelog_text="cookies") |
| 76 | - packager.buildSource(include_orig=False) |
| 77 | - packager.uploadSourceVersion('1.0-2', suite="warty-updates") |
| 78 | - |
| 79 | - # Check if there is exactly one pending PackageDiff record and |
| 80 | - # It's the one we have just created. |
| 81 | - self.assertEqual(self.getPendingDiffs().count(), 1) |
| 82 | - |
| 83 | - return packager |
| 84 | - |
| 85 | - def getPendingDiffs(self): |
| 86 | - """Pending `PackageDiff` available.""" |
| 87 | - return getUtility(IPackageDiffSet).getPendingDiffs() |
| 88 | + super(TestProcessPendingPackageDiffsScript, self).setUp() |
| 89 | |
| 90 | def runProcessPendingPackageDiffs(self, extra_args=None): |
| 91 | """Run process-pending-packagediffs.py. |
| 92 | @@ -147,7 +83,7 @@ |
| 93 | |
| 94 | # The pending PackageDiff request was processed. |
| 95 | # See doc/package-diff.txt for more information. |
| 96 | - pending_diffs = getUtility(IPackageDiffSet).getPendingDiffs() |
| 97 | + pending_diffs = self.getPendingDiffs() |
| 98 | self.assertEqual(pending_diffs.count(), 0) |
| 99 | |
| 100 | def testLimitedRun(self): |
| 101 | |
| 102 | === modified file 'lib/lp/soyuz/tests/soyuz.py' |
| 103 | --- lib/lp/soyuz/tests/soyuz.py 2009-08-28 07:34:44 +0000 |
| 104 | +++ lib/lp/soyuz/tests/soyuz.py 2010-02-04 16:42:17 +0000 |
| 105 | @@ -7,18 +7,26 @@ |
| 106 | |
| 107 | __all__ = [ |
| 108 | 'SoyuzTestHelper', |
| 109 | + 'TestPackageDiffsBase', |
| 110 | ] |
| 111 | |
| 112 | +import unittest |
| 113 | + |
| 114 | from zope.component import getUtility |
| 115 | |
| 116 | -from lp.soyuz.model.publishing import ( |
| 117 | - SecureSourcePackagePublishingHistory, |
| 118 | - SecureBinaryPackagePublishingHistory) |
| 119 | -from canonical.launchpad.ftests import syncUpdate |
| 120 | +from canonical.config import config |
| 121 | +from canonical.launchpad.database import LibraryFileAlias |
| 122 | +from canonical.launchpad.ftests import import_public_test_keys, syncUpdate |
| 123 | +from canonical.launchpad.testing.fakepackager import FakePackager |
| 124 | +from canonical.testing import LaunchpadZopelessLayer |
| 125 | from lp.registry.interfaces.distribution import IDistributionSet |
| 126 | from lp.registry.interfaces.person import IPersonSet |
| 127 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
| 128 | +from lp.soyuz.interfaces.packagediff import IPackageDiffSet |
| 129 | from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
| 130 | +from lp.soyuz.model.publishing import ( |
| 131 | + SecureSourcePackagePublishingHistory, |
| 132 | + SecureBinaryPackagePublishingHistory) |
| 133 | |
| 134 | |
| 135 | class SoyuzTestHelper: |
| 136 | @@ -123,3 +131,71 @@ |
| 137 | """ |
| 138 | return [p.id for p in expected] == [r.id for r in given] |
| 139 | |
| 140 | + |
| 141 | +class TestPackageDiffsBase(unittest.TestCase): |
| 142 | + """Base class facilitating tests related to package diffs.""" |
| 143 | + layer = LaunchpadZopelessLayer |
| 144 | + dbuser = config.uploader.dbuser |
| 145 | + |
| 146 | + def setUp(self): |
| 147 | + """Setup proper DB connection and contents for tests |
| 148 | + |
| 149 | + Connect to the DB as the 'uploader' user (same user used in the |
| 150 | + script), upload the test packages (see `uploadTestPackages`) and |
| 151 | + commit the transaction. |
| 152 | + |
| 153 | + Store the `FakePackager` object used in the test uploads as `packager` |
| 154 | + so the tests can reuse it if necessary. |
| 155 | + """ |
| 156 | + self.layer.alterConnection(dbuser='launchpad') |
| 157 | + |
| 158 | + fake_chroot = LibraryFileAlias.get(1) |
| 159 | + ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
| 160 | + warty = ubuntu.getSeries('warty') |
| 161 | + warty['i386'].addOrUpdateChroot(fake_chroot) |
| 162 | + |
| 163 | + self.layer.txn.commit() |
| 164 | + |
| 165 | + self.layer.alterConnection(dbuser=self.dbuser) |
| 166 | + self.packager = self.uploadTestPackages() |
| 167 | + self.layer.txn.commit() |
| 168 | + |
| 169 | + def uploadTestPackages(self): |
| 170 | + """Upload packages for testing `PackageDiff` generation script. |
| 171 | + |
| 172 | + Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between |
| 173 | + them is created. |
| 174 | + |
| 175 | + Assert there is not pending `PackageDiff` in the DB before uploading |
| 176 | + the package and also assert that there is one after the uploads. |
| 177 | + |
| 178 | + :return: the FakePackager object used to generate and upload the test, |
| 179 | + packages, so the tests can upload subsequent version if necessary. |
| 180 | + """ |
| 181 | + # No pending PackageDiff available in sampledata. |
| 182 | + self.assertEqual(self.getPendingDiffs().count(), 0) |
| 183 | + |
| 184 | + import_public_test_keys() |
| 185 | + # Use FakePackager to upload a base package to ubuntu. |
| 186 | + packager = FakePackager( |
| 187 | + 'zeca', '1.0', 'foo.bar@canonical.com-passwordless.sec') |
| 188 | + packager.buildUpstream() |
| 189 | + packager.buildSource() |
| 190 | + packager.uploadSourceVersion('1.0-1', suite="warty-updates") |
| 191 | + |
| 192 | + # Upload a new version of the source, so a PackageDiff can |
| 193 | + # be created. |
| 194 | + packager.buildVersion('1.0-2', changelog_text="cookies") |
| 195 | + packager.buildSource(include_orig=False) |
| 196 | + packager.uploadSourceVersion('1.0-2', suite="warty-updates") |
| 197 | + |
| 198 | + # Check if there is exactly one pending PackageDiff record and |
| 199 | + # It's the one we have just created. |
| 200 | + self.assertEqual(self.getPendingDiffs().count(), 1) |
| 201 | + |
| 202 | + return packager |
| 203 | + |
| 204 | + def getPendingDiffs(self): |
| 205 | + """Pending `PackageDiff` available.""" |
| 206 | + return getUtility(IPackageDiffSet).getPendingDiffs() |
| 207 | + |
| 208 | |
| 209 | === modified file 'lib/lp/soyuz/tests/test_packagediff.py' |
| 210 | --- lib/lp/soyuz/tests/test_packagediff.py 2010-02-04 11:20:47 +0000 |
| 211 | +++ lib/lp/soyuz/tests/test_packagediff.py 2010-02-04 16:40:06 +0000 |
| 212 | @@ -13,80 +13,20 @@ |
| 213 | |
| 214 | from canonical.config import config |
| 215 | from canonical.database.sqlbase import sqlvalues |
| 216 | -from canonical.launchpad.ftests import import_public_test_keys |
| 217 | -from lp.registry.interfaces.distribution import IDistributionSet |
| 218 | -from lp.soyuz.interfaces.packagediff import IPackageDiffSet, PackageDiffStatus |
| 219 | -from canonical.launchpad.testing.fakepackager import FakePackager |
| 220 | -from canonical.launchpad.database import LibraryFileAlias |
| 221 | from canonical.launchpad.webapp.interfaces import ( |
| 222 | IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR) |
| 223 | from canonical.testing import LaunchpadZopelessLayer |
| 224 | - |
| 225 | - |
| 226 | -class TestPackageDiffs(unittest.TestCase): |
| 227 | +from lp.soyuz.interfaces.packagediff import PackageDiffStatus |
| 228 | +from lp.soyuz.tests.soyuz import TestPackageDiffsBase |
| 229 | + |
| 230 | + |
| 231 | +class TestPackageDiffs(TestPackageDiffsBase): |
| 232 | """Test package diffs.""" |
| 233 | layer = LaunchpadZopelessLayer |
| 234 | dbuser = config.uploader.dbuser |
| 235 | |
| 236 | def setUp(self): |
| 237 | - """Setup proper DB connection and contents for tests |
| 238 | - |
| 239 | - Connect to the DB as the 'uploader' user. |
| 240 | - |
| 241 | - Store the `FakePackager` object used in the test uploads as `packager` |
| 242 | - so the tests can reuse it if necessary. |
| 243 | - """ |
| 244 | - self.layer.alterConnection(dbuser='launchpad') |
| 245 | - |
| 246 | - fake_chroot = LibraryFileAlias.get(1) |
| 247 | - ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
| 248 | - warty = ubuntu.getSeries('warty') |
| 249 | - warty['i386'].addOrUpdateChroot(fake_chroot) |
| 250 | - |
| 251 | - self.layer.txn.commit() |
| 252 | - |
| 253 | - self.layer.alterConnection(dbuser=self.dbuser) |
| 254 | - self.packager = self.uploadTestPackages() |
| 255 | - self.layer.txn.commit() |
| 256 | - |
| 257 | - def uploadTestPackages(self): |
| 258 | - """Upload packages for testing `PackageDiff` generation. |
| 259 | - |
| 260 | - Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between |
| 261 | - them is created. |
| 262 | - |
| 263 | - Assert there is not pending `PackageDiff` in the DB before uploading |
| 264 | - the package and also assert that there is one after the uploads. |
| 265 | - |
| 266 | - :return: the FakePackager object used to generate and upload the test, |
| 267 | - packages, so the tests can upload subsequent version if necessary. |
| 268 | - """ |
| 269 | - # No pending PackageDiff available in sampledata. |
| 270 | - self.assertEqual(self.getPendingDiffs().count(), 0) |
| 271 | - |
| 272 | - import_public_test_keys() |
| 273 | - # Use FakePackager to upload a base package to ubuntu. |
| 274 | - packager = FakePackager( |
| 275 | - 'zeca', '1.0', 'foo.bar@canonical.com-passwordless.sec') |
| 276 | - packager.buildUpstream() |
| 277 | - packager.buildSource() |
| 278 | - packager.uploadSourceVersion('1.0-1', suite="warty-updates") |
| 279 | - |
| 280 | - # Upload a new version of the source, so a PackageDiff can |
| 281 | - # be created. |
| 282 | - packager.buildVersion('1.0-2', changelog_text="cookies") |
| 283 | - packager.buildSource(include_orig=False) |
| 284 | - packager.uploadSourceVersion('1.0-2', suite="warty-updates") |
| 285 | - |
| 286 | - # Check if there is exactly one pending PackageDiff record and |
| 287 | - # It's the one we have just created. |
| 288 | - self.assertEqual(self.getPendingDiffs().count(), 1) |
| 289 | - |
| 290 | - return packager |
| 291 | - |
| 292 | - def getPendingDiffs(self): |
| 293 | - """Pending `PackageDiff` available.""" |
| 294 | - return getUtility(IPackageDiffSet).getPendingDiffs() |
| 295 | + super(TestPackageDiffs, self).setUp() |
| 296 | |
| 297 | def test_packagediff_working(self): |
| 298 | # Test the case where none of the files required for the diff are |
| Michael Nelson (michael.nelson) wrote : | # |
<al-maisan> noodles775: you have mail :)
<noodles775> al-maisan: checking
<al-maisan> noodles775: thank you!
<noodles775> al-maisan: You don't need a TestProcessPend
<al-maisan> noodles775: ah, I see.
<al-maisan> let me remove it then
<al-maisan> the same holds for the other test class
<noodles775> Yep, other than that, r=me.
<noodles775> al-maisan: ^^
<al-maisan> noodles775: thank you!
| Francis J. Lacoste (flacoste) : | # |
Preview Diff
| 1 | === modified file 'lib/lp/soyuz/model/packagediff.py' |
| 2 | --- lib/lp/soyuz/model/packagediff.py 2009-08-13 01:19:34 +0000 |
| 3 | +++ lib/lp/soyuz/model/packagediff.py 2010-02-04 17:10:37 +0000 |
| 4 | @@ -22,7 +22,7 @@ |
| 5 | from canonical.database.constants import UTC_NOW |
| 6 | from canonical.database.datetimecol import UtcDateTimeCol |
| 7 | from canonical.database.enumcol import EnumCol |
| 8 | -from canonical.database.sqlbase import SQLBase |
| 9 | +from canonical.database.sqlbase import SQLBase, sqlvalues |
| 10 | from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet |
| 11 | from lp.soyuz.interfaces.packagediff import ( |
| 12 | IPackageDiff, IPackageDiffSet, PackageDiffStatus) |
| 13 | @@ -130,6 +130,25 @@ |
| 14 | """See `IPackageDiff`.""" |
| 15 | return self.to_source.upload_archive.private |
| 16 | |
| 17 | + def _countExpiredLFAs(self): |
| 18 | + """How many files associated with either source package were |
| 19 | + already expired by the librarian?""" |
| 20 | + store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR) |
| 21 | + query = """ |
| 22 | + SELECT COUNT(lfa.id) |
| 23 | + FROM |
| 24 | + SourcePackageRelease spr, SourcePackageReleaseFile sprf, |
| 25 | + LibraryFileAlias lfa |
| 26 | + WHERE |
| 27 | + spr.id IN %s |
| 28 | + AND sprf.SourcePackageRelease = spr.id |
| 29 | + AND sprf.libraryfile = lfa.id |
| 30 | + AND lfa.expires IS NOT NULL |
| 31 | + AND lfa.content IS NULL |
| 32 | + """ % sqlvalues((self.from_source.id, self.to_source.id)) |
| 33 | + result = store.execute(query).get_one() |
| 34 | + return (0 if result is None else result[0]) |
| 35 | + |
| 36 | def performDiff(self): |
| 37 | """See `IPackageDiff`. |
| 38 | |
| 39 | @@ -137,6 +156,12 @@ |
| 40 | from both SPRs involved from the librarian, running debdiff, storing |
| 41 | the output in the librarian and updating the PackageDiff record. |
| 42 | """ |
| 43 | + # Make sure the files associated with the two source packages are |
| 44 | + # still available in the librarian. |
| 45 | + if self._countExpiredLFAs() > 0: |
| 46 | + self.status = PackageDiffStatus.FAILED |
| 47 | + return |
| 48 | + |
| 49 | # Create the temporary directory where the files will be |
| 50 | # downloaded to and where the debdiff will be performed. |
| 51 | tmp_dir = tempfile.mkdtemp() |
| 52 | |
| 53 | === modified file 'lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py' |
| 54 | --- lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py 2009-06-25 04:06:00 +0000 |
| 55 | +++ lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py 2010-02-04 17:10:37 +0000 |
| 56 | @@ -11,84 +11,17 @@ |
| 57 | from zope.component import getUtility |
| 58 | |
| 59 | from canonical.config import config |
| 60 | -from canonical.launchpad.ftests import import_public_test_keys |
| 61 | -from lp.registry.interfaces.distribution import IDistributionSet |
| 62 | -from lp.soyuz.interfaces.packagediff import IPackageDiffSet |
| 63 | -from canonical.launchpad.testing.fakepackager import FakePackager |
| 64 | from canonical.launchpad.scripts import QuietFakeLogger |
| 65 | -from lp.soyuz.scripts.packagediff import ( |
| 66 | - ProcessPendingPackageDiffs) |
| 67 | -from canonical.launchpad.database import LibraryFileAlias |
| 68 | +from lp.soyuz.scripts.packagediff import ProcessPendingPackageDiffs |
| 69 | +from lp.soyuz.tests.soyuz import TestPackageDiffsBase |
| 70 | from canonical.testing import LaunchpadZopelessLayer |
| 71 | |
| 72 | |
| 73 | -class TestProcessPendingPackageDiffsScript(unittest.TestCase): |
| 74 | +class TestProcessPendingPackageDiffsScript(TestPackageDiffsBase): |
| 75 | """Test the process-pending-packagediffs.py script.""" |
| 76 | layer = LaunchpadZopelessLayer |
| 77 | dbuser = config.uploader.dbuser |
| 78 | |
| 79 | - def setUp(self): |
| 80 | - """Setup proper DB connection and contents for tests |
| 81 | - |
| 82 | - Connect to the DB as the 'uploader' user (same user used in the |
| 83 | - script), upload the test packages (see `uploadTestPackages`) and |
| 84 | - commit the transaction. |
| 85 | - |
| 86 | - Store the `FakePackager` object used in the test uploads as `packager` |
| 87 | - so the tests can reuse it if necessary. |
| 88 | - """ |
| 89 | - self.layer.alterConnection(dbuser='launchpad') |
| 90 | - |
| 91 | - fake_chroot = LibraryFileAlias.get(1) |
| 92 | - ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
| 93 | - warty = ubuntu.getSeries('warty') |
| 94 | - warty['i386'].addOrUpdateChroot(fake_chroot) |
| 95 | - |
| 96 | - self.layer.txn.commit() |
| 97 | - |
| 98 | - self.layer.alterConnection(dbuser=self.dbuser) |
| 99 | - self.packager = self.uploadTestPackages() |
| 100 | - self.layer.txn.commit() |
| 101 | - |
| 102 | - def uploadTestPackages(self): |
| 103 | - """Upload packages for testing `PackageDiff` generation script. |
| 104 | - |
| 105 | - Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between |
| 106 | - them is created. |
| 107 | - |
| 108 | - Assert there is not pending `PackageDiff` in the DB before uploading |
| 109 | - the package and also assert that there is one after the uploads. |
| 110 | - |
| 111 | - :return: the FakePackager object used to generate and upload the test, |
| 112 | - packages, so the tests can upload subsequent version if necessary. |
| 113 | - """ |
| 114 | - # No pending PackageDiff available in sampledata. |
| 115 | - self.assertEqual(self.getPendingDiffs().count(), 0) |
| 116 | - |
| 117 | - import_public_test_keys() |
| 118 | - # Use FakePackager to upload a base package to ubuntu. |
| 119 | - packager = FakePackager( |
| 120 | - 'zeca', '1.0', 'foo.bar@canonical.com-passwordless.sec') |
| 121 | - packager.buildUpstream() |
| 122 | - packager.buildSource() |
| 123 | - packager.uploadSourceVersion('1.0-1', suite="warty-updates") |
| 124 | - |
| 125 | - # Upload a new version of the source, so a PackageDiff can |
| 126 | - # be created. |
| 127 | - packager.buildVersion('1.0-2', changelog_text="cookies") |
| 128 | - packager.buildSource(include_orig=False) |
| 129 | - packager.uploadSourceVersion('1.0-2', suite="warty-updates") |
| 130 | - |
| 131 | - # Check if there is exactly one pending PackageDiff record and |
| 132 | - # It's the one we have just created. |
| 133 | - self.assertEqual(self.getPendingDiffs().count(), 1) |
| 134 | - |
| 135 | - return packager |
| 136 | - |
| 137 | - def getPendingDiffs(self): |
| 138 | - """Pending `PackageDiff` available.""" |
| 139 | - return getUtility(IPackageDiffSet).getPendingDiffs() |
| 140 | - |
| 141 | def runProcessPendingPackageDiffs(self, extra_args=None): |
| 142 | """Run process-pending-packagediffs.py. |
| 143 | |
| 144 | @@ -147,7 +80,7 @@ |
| 145 | |
| 146 | # The pending PackageDiff request was processed. |
| 147 | # See doc/package-diff.txt for more information. |
| 148 | - pending_diffs = getUtility(IPackageDiffSet).getPendingDiffs() |
| 149 | + pending_diffs = self.getPendingDiffs() |
| 150 | self.assertEqual(pending_diffs.count(), 0) |
| 151 | |
| 152 | def testLimitedRun(self): |
| 153 | |
| 154 | === modified file 'lib/lp/soyuz/tests/soyuz.py' |
| 155 | --- lib/lp/soyuz/tests/soyuz.py 2009-08-28 07:34:44 +0000 |
| 156 | +++ lib/lp/soyuz/tests/soyuz.py 2010-02-04 17:10:37 +0000 |
| 157 | @@ -7,18 +7,26 @@ |
| 158 | |
| 159 | __all__ = [ |
| 160 | 'SoyuzTestHelper', |
| 161 | + 'TestPackageDiffsBase', |
| 162 | ] |
| 163 | |
| 164 | +import unittest |
| 165 | + |
| 166 | from zope.component import getUtility |
| 167 | |
| 168 | -from lp.soyuz.model.publishing import ( |
| 169 | - SecureSourcePackagePublishingHistory, |
| 170 | - SecureBinaryPackagePublishingHistory) |
| 171 | -from canonical.launchpad.ftests import syncUpdate |
| 172 | +from canonical.config import config |
| 173 | +from canonical.launchpad.database import LibraryFileAlias |
| 174 | +from canonical.launchpad.ftests import import_public_test_keys, syncUpdate |
| 175 | +from canonical.launchpad.testing.fakepackager import FakePackager |
| 176 | +from canonical.testing import LaunchpadZopelessLayer |
| 177 | from lp.registry.interfaces.distribution import IDistributionSet |
| 178 | from lp.registry.interfaces.person import IPersonSet |
| 179 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
| 180 | +from lp.soyuz.interfaces.packagediff import IPackageDiffSet |
| 181 | from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
| 182 | +from lp.soyuz.model.publishing import ( |
| 183 | + SecureSourcePackagePublishingHistory, |
| 184 | + SecureBinaryPackagePublishingHistory) |
| 185 | |
| 186 | |
| 187 | class SoyuzTestHelper: |
| 188 | @@ -123,3 +131,71 @@ |
| 189 | """ |
| 190 | return [p.id for p in expected] == [r.id for r in given] |
| 191 | |
| 192 | + |
| 193 | +class TestPackageDiffsBase(unittest.TestCase): |
| 194 | + """Base class facilitating tests related to package diffs.""" |
| 195 | + layer = LaunchpadZopelessLayer |
| 196 | + dbuser = config.uploader.dbuser |
| 197 | + |
| 198 | + def setUp(self): |
| 199 | + """Setup proper DB connection and contents for tests |
| 200 | + |
| 201 | + Connect to the DB as the 'uploader' user (same user used in the |
| 202 | + script), upload the test packages (see `uploadTestPackages`) and |
| 203 | + commit the transaction. |
| 204 | + |
| 205 | + Store the `FakePackager` object used in the test uploads as `packager` |
| 206 | + so the tests can reuse it if necessary. |
| 207 | + """ |
| 208 | + self.layer.alterConnection(dbuser='launchpad') |
| 209 | + |
| 210 | + fake_chroot = LibraryFileAlias.get(1) |
| 211 | + ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
| 212 | + warty = ubuntu.getSeries('warty') |
| 213 | + warty['i386'].addOrUpdateChroot(fake_chroot) |
| 214 | + |
| 215 | + self.layer.txn.commit() |
| 216 | + |
| 217 | + self.layer.alterConnection(dbuser=self.dbuser) |
| 218 | + self.packager = self.uploadTestPackages() |
| 219 | + self.layer.txn.commit() |
| 220 | + |
| 221 | + def uploadTestPackages(self): |
| 222 | + """Upload packages for testing `PackageDiff` generation script. |
| 223 | + |
| 224 | + Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between |
| 225 | + them is created. |
| 226 | + |
| 227 | + Assert there is not pending `PackageDiff` in the DB before uploading |
| 228 | + the package and also assert that there is one after the uploads. |
| 229 | + |
| 230 | + :return: the FakePackager object used to generate and upload the test, |
| 231 | + packages, so the tests can upload subsequent version if necessary. |
| 232 | + """ |
| 233 | + # No pending PackageDiff available in sampledata. |
| 234 | + self.assertEqual(self.getPendingDiffs().count(), 0) |
| 235 | + |
| 236 | + import_public_test_keys() |
| 237 | + # Use FakePackager to upload a base package to ubuntu. |
| 238 | + packager = FakePackager( |
| 239 | + 'zeca', '1.0', 'foo.bar@canonical.com-passwordless.sec') |
| 240 | + packager.buildUpstream() |
| 241 | + packager.buildSource() |
| 242 | + packager.uploadSourceVersion('1.0-1', suite="warty-updates") |
| 243 | + |
| 244 | + # Upload a new version of the source, so a PackageDiff can |
| 245 | + # be created. |
| 246 | + packager.buildVersion('1.0-2', changelog_text="cookies") |
| 247 | + packager.buildSource(include_orig=False) |
| 248 | + packager.uploadSourceVersion('1.0-2', suite="warty-updates") |
| 249 | + |
| 250 | + # Check if there is exactly one pending PackageDiff record and |
| 251 | + # It's the one we have just created. |
| 252 | + self.assertEqual(self.getPendingDiffs().count(), 1) |
| 253 | + |
| 254 | + return packager |
| 255 | + |
| 256 | + def getPendingDiffs(self): |
| 257 | + """Pending `PackageDiff` available.""" |
| 258 | + return getUtility(IPackageDiffSet).getPendingDiffs() |
| 259 | + |
| 260 | |
| 261 | === added file 'lib/lp/soyuz/tests/test_packagediff.py' |
| 262 | --- lib/lp/soyuz/tests/test_packagediff.py 1970-01-01 00:00:00 +0000 |
| 263 | +++ lib/lp/soyuz/tests/test_packagediff.py 2010-02-04 17:10:37 +0000 |
| 264 | @@ -0,0 +1,92 @@ |
| 265 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 266 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 267 | + |
| 268 | +"""Test source package diffs.""" |
| 269 | + |
| 270 | +__metaclass__ = type |
| 271 | + |
| 272 | +from datetime import datetime |
| 273 | +import unittest |
| 274 | + |
| 275 | +from zope.component import getUtility |
| 276 | +from zope.security.proxy import removeSecurityProxy |
| 277 | + |
| 278 | +from canonical.config import config |
| 279 | +from canonical.database.sqlbase import sqlvalues |
| 280 | +from canonical.launchpad.webapp.interfaces import ( |
| 281 | + IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR) |
| 282 | +from canonical.testing import LaunchpadZopelessLayer |
| 283 | +from lp.soyuz.interfaces.packagediff import PackageDiffStatus |
| 284 | +from lp.soyuz.tests.soyuz import TestPackageDiffsBase |
| 285 | + |
| 286 | + |
| 287 | +class TestPackageDiffs(TestPackageDiffsBase): |
| 288 | + """Test package diffs.""" |
| 289 | + layer = LaunchpadZopelessLayer |
| 290 | + dbuser = config.uploader.dbuser |
| 291 | + |
| 292 | + def test_packagediff_working(self): |
| 293 | + # Test the case where none of the files required for the diff are |
| 294 | + # expired in the librarian and where everything works as expected. |
| 295 | + [diff] = self.getPendingDiffs() |
| 296 | + self.assertEqual(0, removeSecurityProxy(diff)._countExpiredLFAs()) |
| 297 | + diff.performDiff() |
| 298 | + self.assertEqual(PackageDiffStatus.COMPLETED, diff.status) |
| 299 | + |
| 300 | + def expireLFAsForSource(self, source, delete_as_well=True): |
| 301 | + """Expire the files associated with the given source package in the |
| 302 | + librarian.""" |
| 303 | + store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR) |
| 304 | + query = """ |
| 305 | + UPDATE LibraryFileAlias lfa |
| 306 | + SET |
| 307 | + expires = %s |
| 308 | + """ % sqlvalues(datetime.utcnow()) |
| 309 | + if delete_as_well: |
| 310 | + # Expire *and* delete files from librarian. |
| 311 | + query += """ |
| 312 | + , content = NULL |
| 313 | + """ |
| 314 | + query += """ |
| 315 | + FROM |
| 316 | + SourcePackageRelease spr, SourcePackageReleaseFile sprf |
| 317 | + WHERE |
| 318 | + spr.id = %s |
| 319 | + AND sprf.SourcePackageRelease = spr.id |
| 320 | + AND sprf.libraryfile = lfa.id |
| 321 | + """ % sqlvalues(source.id) |
| 322 | + self.layer.alterConnection(dbuser='launchpad') |
| 323 | + result = store.execute(query) |
| 324 | + self.layer.txn.commit() |
| 325 | + self.layer.alterConnection(dbuser=self.dbuser) |
| 326 | + |
| 327 | + def test_packagediff_with_expired_and_deleted_lfas(self): |
| 328 | + # Test the case where files required for the diff are expired *and* |
| 329 | + # deleted in the librarian causing a package diff failure. |
| 330 | + [diff] = self.getPendingDiffs() |
| 331 | + # Expire and delete the files associated with the 'from_source' |
| 332 | + # package. |
| 333 | + self.expireLFAsForSource(diff.from_source) |
| 334 | + # The helper method now finds 3 expired files. |
| 335 | + self.assertEqual(3, removeSecurityProxy(diff)._countExpiredLFAs()) |
| 336 | + diff.performDiff() |
| 337 | + # The diff fails due to the presence of expired files. |
| 338 | + self.assertEqual(PackageDiffStatus.FAILED, diff.status) |
| 339 | + |
| 340 | + def test_packagediff_with_expired_but_not_deleted_lfas(self): |
| 341 | + # Test the case where files required for the diff are expired but |
| 342 | + # not deleted in the librarian still allowing the package diff to be |
| 343 | + # performed. |
| 344 | + [diff] = self.getPendingDiffs() |
| 345 | + # Expire but don't delete the files associated with the 'from_source' |
| 346 | + # package. |
| 347 | + self.expireLFAsForSource(diff.from_source, delete_as_well=False) |
| 348 | + # The helper method now finds no expired files. |
| 349 | + self.assertEqual(0, removeSecurityProxy(diff)._countExpiredLFAs()) |
| 350 | + diff.performDiff() |
| 351 | + # The diff succeeds as expected. |
| 352 | + self.assertEqual(PackageDiffStatus.COMPLETED, diff.status) |
| 353 | + |
| 354 | + |
| 355 | +def test_suite(): |
| 356 | + return unittest.TestLoader().loadTestsFromName(__name__) |
Hello there!
Now that source package files are expired and deleted in the librarian much
more aggressively than before we may end up in a situation where a source
package diff fails due to the fact that a file required is not available any
more.
The branch at hand checks that none of the files required for the package diff
is expired *and* deleted before actually performing the diff.
If any of the files is expired *and* deleted in the librarian the package diff
operation aborts straightaway.
Tests to run:
bin/test -vv -t test_packagediff -t package-diff
No "make lint" errors or warnings.