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

Proposed by Paul Hummer on 2010-01-12
Status: Merged
Approved by: Jeroen T. Vermeulen on 2010-01-12
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 2010-01-12 Approve on 2010-01-12
Review via email: mp+17191@code.launchpad.net
To post a comment you must log in.
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

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
1=== modified file 'lib/lp/soyuz/configure.zcml'
2--- lib/lp/soyuz/configure.zcml 2010-01-07 05:32:58 +0000
3+++ lib/lp/soyuz/configure.zcml 2010-01-12 04:24:18 +0000
4@@ -401,7 +401,7 @@
5 <require
6 permission="launchpad.Edit"
7 interface="lp.soyuz.interfaces.archive.IArchiveEdit"
8- set_attributes="description displayname enabled"/>
9+ set_attributes="description displayname"/>
10 <require
11 permission="launchpad.Commercial"
12 set_attributes="authorized_size buildd_secret
13
14=== modified file 'lib/lp/soyuz/interfaces/archive.py'
15--- lib/lp/soyuz/interfaces/archive.py 2009-12-08 15:40:23 +0000
16+++ lib/lp/soyuz/interfaces/archive.py 2010-01-12 04:24:18 +0000
17@@ -1084,6 +1084,12 @@
18 :param component: An `IComponent` or textual component name.
19 """
20
21+ def enable():
22+ """Enable the archive."""
23+
24+ def disable():
25+ """Disable the archive."""
26+
27
28 class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):
29 """Main Archive interface."""
30
31=== modified file 'lib/lp/soyuz/model/archive.py'
32--- lib/lp/soyuz/model/archive.py 2009-12-24 08:15:46 +0000
33+++ lib/lp/soyuz/model/archive.py 2010-01-12 04:24:18 +0000
34@@ -31,6 +31,7 @@
35 from canonical.database.enumcol import EnumCol
36 from canonical.database.sqlbase import (
37 cursor, quote, quote_like, sqlvalues, SQLBase)
38+from lp.services.job.interfaces.job import JobStatus
39 from lp.soyuz.adapters.packagelocation import PackageLocation
40 from canonical.launchpad.components.tokens import (
41 create_unique_token_for_table)
42@@ -130,7 +131,8 @@
43 purpose = EnumCol(
44 dbName='purpose', unique=False, notNull=True, schema=ArchivePurpose)
45
46- enabled = BoolCol(dbName='enabled', notNull=True, default=True)
47+ _enabled = BoolCol(dbName='enabled', notNull=True, default=True)
48+ enabled = property(lambda x: x._enabled)
49
50 publish = BoolCol(dbName='publish', notNull=True, default=True)
51
52@@ -1238,6 +1240,38 @@
53 result_set.config(distinct=True).order_by(SourcePackageRelease.id)
54 return result_set
55
56+ def _setBuildStatuses(self, status):
57+ """Update the pending Build Jobs' status for this archive."""
58+
59+ query = """
60+ UPDATE Job SET status = %s
61+ FROM Build, BuildPackageJob, BuildQueue
62+ WHERE
63+ -- insert self.id here
64+ Build.archive = %s
65+ AND BuildPackageJob.build = Build.id
66+ AND BuildPackageJob.job = BuildQueue.job
67+ AND Job.id = BuildQueue.job
68+ -- Build is in state BuildStatus.NEEDSBUILD (0)
69+ AND Build.buildstate = %s;
70+ """ % sqlvalues(status, self, BuildStatus.NEEDSBUILD)
71+
72+ store = Store.of(self)
73+ store.execute(query)
74+
75+ def enable(self):
76+ """See `IArchive`."""
77+ assert self._enabled == False, "This archive is already enabled."
78+ self._enabled = True
79+ self._setBuildStatuses(JobStatus.WAITING)
80+
81+ def disable(self):
82+ """See `IArchive`."""
83+ assert self._enabled == True, "This archive is already disabled."
84+ self._enabled = False
85+ self._setBuildStatuses(JobStatus.SUSPENDED)
86+
87+
88 class ArchiveSet:
89 implements(IArchiveSet)
90 title = "Archives registered in Launchpad"
91
92=== modified file 'lib/lp/soyuz/tests/test_archive.py'
93--- lib/lp/soyuz/tests/test_archive.py 2009-11-06 21:06:38 +0000
94+++ lib/lp/soyuz/tests/test_archive.py 2010-01-12 04:24:18 +0000
95@@ -3,24 +3,30 @@
96
97 """Test Archive features."""
98
99-from datetime import datetime
100+from datetime import datetime, timedelta
101 import pytz
102 import unittest
103
104 from zope.component import getUtility
105 from zope.security.proxy import removeSecurityProxy
106
107+from canonical.database.sqlbase import sqlvalues
108+from canonical.launchpad.webapp.interfaces import (
109+ IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
110 from canonical.testing import LaunchpadZopelessLayer
111
112 from lp.registry.interfaces.distribution import IDistributionSet
113 from lp.registry.interfaces.person import IPersonSet
114+from lp.services.job.interfaces.job import JobStatus
115 from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose
116 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
117 from lp.soyuz.interfaces.build import BuildStatus
118 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
119+from lp.soyuz.model.build import Build
120 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
121 from lp.testing import TestCaseWithFactory
122
123+
124 class TestGetPublicationsInArchive(TestCaseWithFactory):
125
126 layer = LaunchpadZopelessLayer
127@@ -382,5 +388,131 @@
128 self.assertIs(
129 self.primary_archive.debug_archive, None)
130
131+
132+class TestArchiveEnableDisable(TestCaseWithFactory):
133+ """Test the enable and disable methods of Archive."""
134+
135+ layer = LaunchpadZopelessLayer
136+
137+ def setUp(self):
138+ #XXX: rockstar - 12 Jan 2010 - Bug #506255 - Tidy up these tests!
139+ super(TestArchiveEnableDisable, self).setUp()
140+
141+ self.publisher = SoyuzTestPublisher()
142+ self.publisher.prepareBreezyAutotest()
143+
144+ self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
145+ self.archive = getUtility(IArchiveSet).getByDistroPurpose(
146+ self.ubuntutest, ArchivePurpose.PRIMARY)
147+
148+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
149+ sample_data = store.find(Build)
150+ for build in sample_data:
151+ build.buildstate = BuildStatus.FULLYBUILT
152+ store.flush()
153+
154+ # We test builds that target a primary archive.
155+ self.builds = []
156+ self.builds.extend(
157+ self.publisher.getPubSource(
158+ sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
159+ archive=self.archive).createMissingBuilds())
160+ self.builds.extend(
161+ self.publisher.getPubSource(
162+ sourcename="firefox",
163+ status=PackagePublishingStatus.PUBLISHED,
164+ archive=self.archive).createMissingBuilds())
165+ self.builds.extend(
166+ self.publisher.getPubSource(
167+ sourcename="apg", status=PackagePublishingStatus.PUBLISHED,
168+ archive=self.archive).createMissingBuilds())
169+ self.builds.extend(
170+ self.publisher.getPubSource(
171+ sourcename="vim", status=PackagePublishingStatus.PUBLISHED,
172+ archive=self.archive).createMissingBuilds())
173+ self.builds.extend(
174+ self.publisher.getPubSource(
175+ sourcename="gcc", status=PackagePublishingStatus.PUBLISHED,
176+ archive=self.archive).createMissingBuilds())
177+ self.builds.extend(
178+ self.publisher.getPubSource(
179+ sourcename="bison", status=PackagePublishingStatus.PUBLISHED,
180+ archive=self.archive).createMissingBuilds())
181+ self.builds.extend(
182+ self.publisher.getPubSource(
183+ sourcename="flex", status=PackagePublishingStatus.PUBLISHED,
184+ archive=self.archive).createMissingBuilds())
185+ self.builds.extend(
186+ self.publisher.getPubSource(
187+ sourcename="postgres",
188+ status=PackagePublishingStatus.PUBLISHED,
189+ archive=self.archive).createMissingBuilds())
190+ # Set up the builds for test.
191+ score = 1000
192+ duration = 0
193+ for build in self.builds:
194+ score += 1
195+ duration += 60
196+ bq = build.buildqueue_record
197+ bq.lastscore = score
198+ bq.estimated_duration = timedelta(seconds=duration)
199+
200+ def _getBuildJobsByStatus(self, archive, status):
201+ # Return the count for archive build jobs with the given status.
202+ query = """
203+ SELECT COUNT(Job.id)
204+ FROM Build, BuildPackageJob, BuildQueue, Job
205+ WHERE
206+ Build.archive = %s
207+ AND BuildPackageJob.build = Build.id
208+ AND BuildPackageJob.job = BuildQueue.job
209+ AND Job.id = BuildQueue.job
210+ AND Build.buildstate = %s
211+ AND Job.status = %s;
212+ """ % sqlvalues(archive, BuildStatus.NEEDSBUILD, status)
213+
214+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
215+ return store.execute(query).get_one()[0]
216+
217+ def assertNoBuildJobsHaveStatus(self, archive, status):
218+ # Check that that the jobs attached to this archive do not have this
219+ # status.
220+ self.assertEqual(self._getBuildJobsByStatus(archive, status), 0)
221+
222+ def assertHasBuildJobsWithStatus(self, archive, status):
223+ # Check that that there are jobs attached to this archive that have
224+ # the specified status.
225+ self.assertEqual(self._getBuildJobsByStatus(archive, status), 8)
226+
227+ def test_enableArchive(self):
228+ # Enabling an archive should set all the Archive's suspended builds to
229+ # WAITING.
230+
231+ # Disable the archive, because it's currently enabled.
232+ self.archive.disable()
233+ self.assertHasBuildJobsWithStatus(self.archive, JobStatus.SUSPENDED)
234+ self.archive.enable()
235+ self.assertNoBuildJobsHaveStatus(self.archive, JobStatus.SUSPENDED)
236+ self.assertTrue(self.archive.enabled)
237+
238+ def test_enableArchiveAlreadyEnabled(self):
239+ # Enabling an already enabled Archive should raise an AssertionError.
240+ self.assertRaises(AssertionError, self.archive.enable)
241+
242+ def test_disableArchive(self):
243+ # Disabling an archive should set all the Archive's pending bulds to
244+ # SUSPENDED.
245+ self.assertHasBuildJobsWithStatus(self.archive, JobStatus.WAITING)
246+ self.archive.disable()
247+ self.assertNoBuildJobsHaveStatus(self.archive, JobStatus.WAITING)
248+ self.assertFalse(self.archive.enabled)
249+
250+ def test_disableArchiveAlreadyDisabled(self):
251+ # Disabling an already disabled Archive should raise an
252+ # AssertionError.
253+ self.archive.disable()
254+ self.assertRaises(AssertionError, self.archive.disable)
255+
256+
257 def test_suite():
258 return unittest.TestLoader().loadTestsFromName(__name__)