Merge lp:~rockstar/launchpad/use-suspended-job-status into lp:launchpad

Proposed by Paul Hummer
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/use-suspended-job-status
Merge into: lp:launchpad
Prerequisite: lp:~rockstar/launchpad/job-suspended-status
Diff against target: 258 lines (+175/-3)
4 files modified
lib/lp/soyuz/configure.zcml (+1/-1)
lib/lp/soyuz/interfaces/archive.py (+6/-0)
lib/lp/soyuz/model/archive.py (+35/-1)
lib/lp/soyuz/tests/test_archive.py (+133/-1)
To merge this branch: bzr merge lp:~rockstar/launchpad/use-suspended-job-status
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+17191@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

Hi Jeroen-

  This branch just adds an IArchive.enable and IArchive.disable method to the
IArchive to appropriately tests it.

Paul

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Went over this in IRL.

Some small superficial points—mainly indentation of query text, and XXXes to factor duplicated setup out into helpers.

Also, instead of a test method "assert that there are no builds with status X" it's probably more helpful to have one that returns whether there are any builds with status X, and use that for assertions directly in your tests. Two advantages:

 * A failure shows up at the bottom of its stack trace, not in a generic method where you have to browse further up the trace to get to the semantic failure.

 * You can show before/after comparisons very effectively. That can help separate bad test setup (which also sometimes tells you what you want to hear) from the change you're really testing for.

With those notes, r=me.

Jeroen

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2010-01-07 05:32:58 +0000
+++ lib/lp/soyuz/configure.zcml 2010-01-12 04:24:18 +0000
@@ -401,7 +401,7 @@
401 <require401 <require
402 permission="launchpad.Edit"402 permission="launchpad.Edit"
403 interface="lp.soyuz.interfaces.archive.IArchiveEdit"403 interface="lp.soyuz.interfaces.archive.IArchiveEdit"
404 set_attributes="description displayname enabled"/>404 set_attributes="description displayname"/>
405 <require405 <require
406 permission="launchpad.Commercial"406 permission="launchpad.Commercial"
407 set_attributes="authorized_size buildd_secret407 set_attributes="authorized_size buildd_secret
408408
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2009-12-08 15:40:23 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2010-01-12 04:24:18 +0000
@@ -1084,6 +1084,12 @@
1084 :param component: An `IComponent` or textual component name.1084 :param component: An `IComponent` or textual component name.
1085 """1085 """
10861086
1087 def enable():
1088 """Enable the archive."""
1089
1090 def disable():
1091 """Disable the archive."""
1092
10871093
1088class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):1094class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):
1089 """Main Archive interface."""1095 """Main Archive interface."""
10901096
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2009-12-24 08:15:46 +0000
+++ lib/lp/soyuz/model/archive.py 2010-01-12 04:24:18 +0000
@@ -31,6 +31,7 @@
31from canonical.database.enumcol import EnumCol31from canonical.database.enumcol import EnumCol
32from canonical.database.sqlbase import (32from canonical.database.sqlbase import (
33 cursor, quote, quote_like, sqlvalues, SQLBase)33 cursor, quote, quote_like, sqlvalues, SQLBase)
34from lp.services.job.interfaces.job import JobStatus
34from lp.soyuz.adapters.packagelocation import PackageLocation35from lp.soyuz.adapters.packagelocation import PackageLocation
35from canonical.launchpad.components.tokens import (36from canonical.launchpad.components.tokens import (
36 create_unique_token_for_table)37 create_unique_token_for_table)
@@ -130,7 +131,8 @@
130 purpose = EnumCol(131 purpose = EnumCol(
131 dbName='purpose', unique=False, notNull=True, schema=ArchivePurpose)132 dbName='purpose', unique=False, notNull=True, schema=ArchivePurpose)
132133
133 enabled = BoolCol(dbName='enabled', notNull=True, default=True)134 _enabled = BoolCol(dbName='enabled', notNull=True, default=True)
135 enabled = property(lambda x: x._enabled)
134136
135 publish = BoolCol(dbName='publish', notNull=True, default=True)137 publish = BoolCol(dbName='publish', notNull=True, default=True)
136138
@@ -1238,6 +1240,38 @@
1238 result_set.config(distinct=True).order_by(SourcePackageRelease.id)1240 result_set.config(distinct=True).order_by(SourcePackageRelease.id)
1239 return result_set1241 return result_set
12401242
1243 def _setBuildStatuses(self, status):
1244 """Update the pending Build Jobs' status for this archive."""
1245
1246 query = """
1247 UPDATE Job SET status = %s
1248 FROM Build, BuildPackageJob, BuildQueue
1249 WHERE
1250 -- insert self.id here
1251 Build.archive = %s
1252 AND BuildPackageJob.build = Build.id
1253 AND BuildPackageJob.job = BuildQueue.job
1254 AND Job.id = BuildQueue.job
1255 -- Build is in state BuildStatus.NEEDSBUILD (0)
1256 AND Build.buildstate = %s;
1257 """ % sqlvalues(status, self, BuildStatus.NEEDSBUILD)
1258
1259 store = Store.of(self)
1260 store.execute(query)
1261
1262 def enable(self):
1263 """See `IArchive`."""
1264 assert self._enabled == False, "This archive is already enabled."
1265 self._enabled = True
1266 self._setBuildStatuses(JobStatus.WAITING)
1267
1268 def disable(self):
1269 """See `IArchive`."""
1270 assert self._enabled == True, "This archive is already disabled."
1271 self._enabled = False
1272 self._setBuildStatuses(JobStatus.SUSPENDED)
1273
1274
1241class ArchiveSet:1275class ArchiveSet:
1242 implements(IArchiveSet)1276 implements(IArchiveSet)
1243 title = "Archives registered in Launchpad"1277 title = "Archives registered in Launchpad"
12441278
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2009-11-06 21:06:38 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2010-01-12 04:24:18 +0000
@@ -3,24 +3,30 @@
33
4"""Test Archive features."""4"""Test Archive features."""
55
6from datetime import datetime6from datetime import datetime, timedelta
7import pytz7import pytz
8import unittest8import unittest
99
10from zope.component import getUtility10from zope.component import getUtility
11from zope.security.proxy import removeSecurityProxy11from zope.security.proxy import removeSecurityProxy
1212
13from canonical.database.sqlbase import sqlvalues
14from canonical.launchpad.webapp.interfaces import (
15 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
13from canonical.testing import LaunchpadZopelessLayer16from canonical.testing import LaunchpadZopelessLayer
1417
15from lp.registry.interfaces.distribution import IDistributionSet18from lp.registry.interfaces.distribution import IDistributionSet
16from lp.registry.interfaces.person import IPersonSet19from lp.registry.interfaces.person import IPersonSet
20from lp.services.job.interfaces.job import JobStatus
17from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose21from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose
18from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat22from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
19from lp.soyuz.interfaces.build import BuildStatus23from lp.soyuz.interfaces.build import BuildStatus
20from lp.soyuz.interfaces.publishing import PackagePublishingStatus24from lp.soyuz.interfaces.publishing import PackagePublishingStatus
25from lp.soyuz.model.build import Build
21from lp.soyuz.tests.test_publishing import SoyuzTestPublisher26from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
22from lp.testing import TestCaseWithFactory27from lp.testing import TestCaseWithFactory
2328
29
24class TestGetPublicationsInArchive(TestCaseWithFactory):30class TestGetPublicationsInArchive(TestCaseWithFactory):
2531
26 layer = LaunchpadZopelessLayer32 layer = LaunchpadZopelessLayer
@@ -382,5 +388,131 @@
382 self.assertIs(388 self.assertIs(
383 self.primary_archive.debug_archive, None)389 self.primary_archive.debug_archive, None)
384390
391
392class TestArchiveEnableDisable(TestCaseWithFactory):
393 """Test the enable and disable methods of Archive."""
394
395 layer = LaunchpadZopelessLayer
396
397 def setUp(self):
398 #XXX: rockstar - 12 Jan 2010 - Bug #506255 - Tidy up these tests!
399 super(TestArchiveEnableDisable, self).setUp()
400
401 self.publisher = SoyuzTestPublisher()
402 self.publisher.prepareBreezyAutotest()
403
404 self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
405 self.archive = getUtility(IArchiveSet).getByDistroPurpose(
406 self.ubuntutest, ArchivePurpose.PRIMARY)
407
408 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
409 sample_data = store.find(Build)
410 for build in sample_data:
411 build.buildstate = BuildStatus.FULLYBUILT
412 store.flush()
413
414 # We test builds that target a primary archive.
415 self.builds = []
416 self.builds.extend(
417 self.publisher.getPubSource(
418 sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
419 archive=self.archive).createMissingBuilds())
420 self.builds.extend(
421 self.publisher.getPubSource(
422 sourcename="firefox",
423 status=PackagePublishingStatus.PUBLISHED,
424 archive=self.archive).createMissingBuilds())
425 self.builds.extend(
426 self.publisher.getPubSource(
427 sourcename="apg", status=PackagePublishingStatus.PUBLISHED,
428 archive=self.archive).createMissingBuilds())
429 self.builds.extend(
430 self.publisher.getPubSource(
431 sourcename="vim", status=PackagePublishingStatus.PUBLISHED,
432 archive=self.archive).createMissingBuilds())
433 self.builds.extend(
434 self.publisher.getPubSource(
435 sourcename="gcc", status=PackagePublishingStatus.PUBLISHED,
436 archive=self.archive).createMissingBuilds())
437 self.builds.extend(
438 self.publisher.getPubSource(
439 sourcename="bison", status=PackagePublishingStatus.PUBLISHED,
440 archive=self.archive).createMissingBuilds())
441 self.builds.extend(
442 self.publisher.getPubSource(
443 sourcename="flex", status=PackagePublishingStatus.PUBLISHED,
444 archive=self.archive).createMissingBuilds())
445 self.builds.extend(
446 self.publisher.getPubSource(
447 sourcename="postgres",
448 status=PackagePublishingStatus.PUBLISHED,
449 archive=self.archive).createMissingBuilds())
450 # Set up the builds for test.
451 score = 1000
452 duration = 0
453 for build in self.builds:
454 score += 1
455 duration += 60
456 bq = build.buildqueue_record
457 bq.lastscore = score
458 bq.estimated_duration = timedelta(seconds=duration)
459
460 def _getBuildJobsByStatus(self, archive, status):
461 # Return the count for archive build jobs with the given status.
462 query = """
463 SELECT COUNT(Job.id)
464 FROM Build, BuildPackageJob, BuildQueue, Job
465 WHERE
466 Build.archive = %s
467 AND BuildPackageJob.build = Build.id
468 AND BuildPackageJob.job = BuildQueue.job
469 AND Job.id = BuildQueue.job
470 AND Build.buildstate = %s
471 AND Job.status = %s;
472 """ % sqlvalues(archive, BuildStatus.NEEDSBUILD, status)
473
474 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
475 return store.execute(query).get_one()[0]
476
477 def assertNoBuildJobsHaveStatus(self, archive, status):
478 # Check that that the jobs attached to this archive do not have this
479 # status.
480 self.assertEqual(self._getBuildJobsByStatus(archive, status), 0)
481
482 def assertHasBuildJobsWithStatus(self, archive, status):
483 # Check that that there are jobs attached to this archive that have
484 # the specified status.
485 self.assertEqual(self._getBuildJobsByStatus(archive, status), 8)
486
487 def test_enableArchive(self):
488 # Enabling an archive should set all the Archive's suspended builds to
489 # WAITING.
490
491 # Disable the archive, because it's currently enabled.
492 self.archive.disable()
493 self.assertHasBuildJobsWithStatus(self.archive, JobStatus.SUSPENDED)
494 self.archive.enable()
495 self.assertNoBuildJobsHaveStatus(self.archive, JobStatus.SUSPENDED)
496 self.assertTrue(self.archive.enabled)
497
498 def test_enableArchiveAlreadyEnabled(self):
499 # Enabling an already enabled Archive should raise an AssertionError.
500 self.assertRaises(AssertionError, self.archive.enable)
501
502 def test_disableArchive(self):
503 # Disabling an archive should set all the Archive's pending bulds to
504 # SUSPENDED.
505 self.assertHasBuildJobsWithStatus(self.archive, JobStatus.WAITING)
506 self.archive.disable()
507 self.assertNoBuildJobsHaveStatus(self.archive, JobStatus.WAITING)
508 self.assertFalse(self.archive.enabled)
509
510 def test_disableArchiveAlreadyDisabled(self):
511 # Disabling an already disabled Archive should raise an
512 # AssertionError.
513 self.archive.disable()
514 self.assertRaises(AssertionError, self.archive.disable)
515
516
385def test_suite():517def test_suite():
386 return unittest.TestLoader().loadTestsFromName(__name__)518 return unittest.TestLoader().loadTestsFromName(__name__)