launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06006
lp:~allenap/launchpad/builder-rescue-if-lost-ro-crash-bug-906079 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/builder-rescue-if-lost-ro-crash-bug-906079 into lp:launchpad with lp:~allenap/launchpad/translation-templates-build-behaviour-ro-crash-bug-905855 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #906079 in Launchpad itself: "Builder.rescueIfLost writes in a read-only transaction"
https://bugs.launchpad.net/launchpad/+bug/906079
For more details, see:
https://code.launchpad.net/~allenap/launchpad/builder-rescue-if-lost-ro-crash-bug-906079/+merge/86803
Uses BuilddManagerTestFixture extensively in the remaining
lp.buildmaster tests to try and ensure that tests are running in the
correct environment.
--
https://code.launchpad.net/~allenap/launchpad/builder-rescue-if-lost-ro-crash-bug-906079/+merge/86803
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/builder-rescue-if-lost-ro-crash-bug-906079 into lp:launchpad.
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2011-12-23 13:30:34 +0000
+++ lib/lp/buildmaster/manager.py 2011-12-23 13:30:35 +0000
@@ -217,8 +217,9 @@
return defer.succeed(True)
self.logger.info("Cancelling build '%s'" % build.title)
- buildqueue.cancel()
- transaction.commit()
+ with DatabaseTransactionPolicy(read_only=False):
+ buildqueue.cancel()
+ transaction.commit()
d = builder.resumeSlaveHost()
d.addCallback(resume_done)
return d
=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py 2011-12-23 13:30:34 +0000
+++ lib/lp/buildmaster/model/packagebuild.py 2011-12-23 13:30:35 +0000
@@ -293,6 +293,7 @@
job=job, processor=processor,
virtualized=specific_job.virtualized)
Store.of(self).add(queue_entry)
+
return queue_entry
def handleStatus(self, status, librarian, slave_status):
=== modified file 'lib/lp/buildmaster/testing.py'
--- lib/lp/buildmaster/testing.py 2011-12-23 13:30:34 +0000
+++ lib/lp/buildmaster/testing.py 2011-12-23 13:30:35 +0000
@@ -9,6 +9,7 @@
]
from contextlib import contextmanager
+from functools import wraps
import fixtures
import transaction
@@ -42,18 +43,38 @@
self.addCleanup(self.policy.__exit__, None, None, None)
@staticmethod
- @contextmanager
- def extraSetUp():
+ def extraSetUp(func=None):
"""Temporarily enter a read-write transaction to do extra setup.
For example:
- with self.extraSetUp():
+ with BuilddManagerTestFixture.extraSetUp():
removeSecurityProxy(self.build).date_finished = None
On exit it will commit the changes and restore the previous
transaction access mode.
+
+ Alternatively it can be used as a decorator:
+
+ @BuilddManagerTestFixture.extraSetUp
+ def makeSomethingOrOther(self):
+ return ...
+
+ Like with the context manager, on return it will commit the changes
+ and restore the previous transaction access mode.
"""
- with DatabaseTransactionPolicy(read_only=False):
- yield
- transaction.commit()
+ if func is None:
+ @contextmanager
+ def context():
+ with DatabaseTransactionPolicy(read_only=False):
+ yield
+ transaction.commit()
+ return context()
+ else:
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ with DatabaseTransactionPolicy(read_only=False):
+ result = func(*args, **kwargs)
+ transaction.commit()
+ return result
+ return wrapper
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2011-12-23 13:30:34 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2011-12-23 13:30:35 +0000
@@ -36,10 +36,7 @@
IStoreSelector,
MAIN_STORE,
)
-from canonical.testing.layers import (
- DatabaseFunctionalLayer,
- LaunchpadZopelessLayer,
- )
+from canonical.testing.layers import LaunchpadZopelessLayer
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.builder import (
CannotFetchFile,
@@ -57,6 +54,7 @@
)
from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.buildmaster.testing import BuilddManagerTestFixture
from lp.buildmaster.tests.mock_slaves import (
AbortedSlave,
AbortingSlave,
@@ -73,7 +71,6 @@
WaitingSlave,
)
from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.services.job.interfaces.job import JobStatus
from lp.services.log.logger import BufferLogger
from lp.soyuz.enums import (
@@ -94,46 +91,63 @@
class TestBuilderBasics(TestCaseWithFactory):
"""Basic unit tests for `Builder`."""
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestBuilderBasics, self).setUp()
+ self.useFixture(BuilddManagerTestFixture())
def test_providesInterface(self):
# Builder provides IBuilder
- builder = self.factory.makeBuilder()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder()
self.assertProvides(builder, IBuilder)
def test_default_values(self):
- builder = self.factory.makeBuilder()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder()
# Make sure the Storm cache gets the values that the database
# initializes.
flush_database_updates()
+ self.useFixture(BuilddManagerTestFixture())
self.assertEqual(0, builder.failure_count)
def test_getCurrentBuildFarmJob(self):
- bq = self.factory.makeSourcePackageRecipeBuildJob(3333)
- builder = self.factory.makeBuilder()
- bq.markAsBuilding(builder)
+ with BuilddManagerTestFixture.extraSetUp():
+ bq = self.factory.makeSourcePackageRecipeBuildJob(3333)
+ builder = self.factory.makeBuilder()
+ bq.markAsBuilding(builder)
+ self.useFixture(BuilddManagerTestFixture())
self.assertEqual(
bq, builder.getCurrentBuildFarmJob().buildqueue_record)
def test_getBuildQueue(self):
- buildqueueset = getUtility(IBuildQueueSet)
- active_jobs = buildqueueset.getActiveBuildJobs()
- [active_job] = active_jobs
- builder = active_job.builder
+ with BuilddManagerTestFixture.extraSetUp():
+ buildqueueset = getUtility(IBuildQueueSet)
+ active_jobs = buildqueueset.getActiveBuildJobs()
+ [active_job] = active_jobs
+ builder = active_job.builder
bq = builder.getBuildQueue()
self.assertEqual(active_job, bq)
- active_job.builder = None
+ with BuilddManagerTestFixture.extraSetUp():
+ active_job.builder = None
+
bq = builder.getBuildQueue()
self.assertIs(None, bq)
def test_setting_builderok_resets_failure_count(self):
- builder = removeSecurityProxy(self.factory.makeBuilder())
- builder.failure_count = 1
- builder.builderok = False
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = removeSecurityProxy(self.factory.makeBuilder())
+ builder.failure_count = 1
+ builder.builderok = False
+
self.assertEqual(1, builder.failure_count)
- builder.builderok = True
+
+ with BuilddManagerTestFixture.extraSetUp():
+ builder.builderok = True
+
self.assertEqual(0, builder.failure_count)
@@ -145,6 +159,7 @@
def setUp(self):
super(TestBuilder, self).setUp()
self.slave_helper = self.useFixture(SlaveTestHelpers())
+ self.useFixture(BuilddManagerTestFixture())
def test_updateStatus_aborts_lost_and_broken_slave(self):
# A slave that's 'lost' should be aborted; when the slave is
@@ -161,12 +176,14 @@
return d.addBoth(check_slave_status)
def test_resumeSlaveHost_nonvirtual(self):
- builder = self.factory.makeBuilder(virtualized=False)
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder(virtualized=False)
d = builder.resumeSlaveHost()
return assert_fails_with(d, CannotResumeHost)
def test_resumeSlaveHost_no_vmhost(self):
- builder = self.factory.makeBuilder(virtualized=True, vm_host=None)
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder(virtualized=True, vm_host=None)
d = builder.resumeSlaveHost()
return assert_fails_with(d, CannotResumeHost)
@@ -177,7 +194,8 @@
config.push('reset', reset_config)
self.addCleanup(config.pop, 'reset')
- builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
d = builder.resumeSlaveHost()
def got_resume(output):
self.assertEqual(('parp', ''), output)
@@ -189,7 +207,8 @@
vm_resume_command: /bin/false"""
config.push('reset fail', reset_fail_config)
self.addCleanup(config.pop, 'reset fail')
- builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
d = builder.resumeSlaveHost()
return assert_fails_with(d, CannotResumeHost)
@@ -199,11 +218,13 @@
vm_resume_command: /bin/false"""
config.push('reset fail', reset_fail_config)
self.addCleanup(config.pop, 'reset fail')
- builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
- builder.builderok = True
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder(virtualized=True, vm_host="pop")
+ builder.builderok = True
d = builder.handleTimeout(BufferLogger(), 'blah')
return assert_fails_with(d, CannotResumeHost)
+ @BuilddManagerTestFixture.extraSetUp
def _setupBuilder(self):
processor = self.factory.makeProcessor(name="i386")
builder = self.factory.makeBuilder(
@@ -218,6 +239,7 @@
distroseries.nominatedarchindep = das
return builder, distroseries, das
+ @BuilddManagerTestFixture.extraSetUp
def _setupRecipeBuildAndBuilder(self):
# Helper function to make a builder capable of building a
# recipe, returning both.
@@ -226,6 +248,7 @@
distroseries=distroseries)
return builder, build
+ @BuilddManagerTestFixture.extraSetUp
def _setupBinaryBuildAndBuilder(self):
# Helper function to make a builder capable of building a
# binary package, returning both.
@@ -238,7 +261,8 @@
# findAndStartJob finds the next queued job using _findBuildCandidate.
# We don't care about the type of build at all.
builder, build = self._setupRecipeBuildAndBuilder()
- candidate = build.queueBuild()
+ with BuilddManagerTestFixture.extraSetUp():
+ candidate = build.queueBuild()
# _findBuildCandidate is tested elsewhere, we just make sure that
# findAndStartJob delegates to it.
removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
@@ -251,7 +275,8 @@
# and then starts it.
# We don't care about the type of build at all.
builder, build = self._setupRecipeBuildAndBuilder()
- candidate = build.queueBuild()
+ with BuilddManagerTestFixture.extraSetUp():
+ candidate = build.queueBuild()
removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
result=candidate)
d = builder.findAndStartJob()
@@ -264,7 +289,8 @@
# We need to send a ping to the builder to work around a bug
# where sometimes the first network packet sent is dropped.
builder, build = self._setupBinaryBuildAndBuilder()
- candidate = build.queueBuild()
+ with BuilddManagerTestFixture.extraSetUp():
+ candidate = build.queueBuild()
removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
result=candidate)
d = builder.findAndStartJob()
@@ -277,7 +303,8 @@
# Builder.slave is a BuilderSlave that points at the actual Builder.
# The Builder is only ever used in scripts that run outside of the
# security context.
- builder = removeSecurityProxy(self.factory.makeBuilder())
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = removeSecurityProxy(self.factory.makeBuilder())
self.assertEqual(builder.url, builder.slave.url)
def test_recovery_of_aborted_virtual_slave(self):
@@ -347,7 +374,8 @@
# rescueIfLost does not attempt to abort or clean a builder that is
# BUILDING.
building_slave = BuildingSlave()
- builder = MockBuilder("mock_builder", building_slave, TrivialBehavior())
+ builder = MockBuilder(
+ "mock_builder", building_slave, TrivialBehavior())
d = builder.rescueIfLost()
def check_slave_calls(ignored):
self.assertNotIn('abort', building_slave.call_log)
@@ -371,10 +399,11 @@
# too so that we don't traceback when the wrong behaviour tries
# to access a non-existent job.
builder, build = self._setupBinaryBuildAndBuilder()
- candidate = build.queueBuild()
- building_slave = BuildingSlave()
- builder.setSlaveForTesting(building_slave)
- candidate.markAsBuilding(builder)
+ with BuilddManagerTestFixture.extraSetUp():
+ candidate = build.queueBuild()
+ building_slave = BuildingSlave()
+ builder.setSlaveForTesting(building_slave)
+ candidate.markAsBuilding(builder)
# At this point we should see a valid behaviour on the builder:
self.assertFalse(
@@ -382,8 +411,8 @@
builder.current_build_behavior, IdleBuildBehavior))
# Now reset the job and try to rescue the builder.
- candidate.destroySelf()
- self.layer.txn.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ candidate.destroySelf()
builder = getUtility(IBuilderSet)[builder.name]
d = builder.rescueIfLost()
def check_builder(ignored):
@@ -403,13 +432,14 @@
def setUp(self):
super(TestBuilderSlaveStatus, self).setUp()
self.slave_helper = self.useFixture(SlaveTestHelpers())
+ self.builder = self.factory.makeBuilder()
+ self.useFixture(BuilddManagerTestFixture())
def assertStatus(self, slave, builder_status=None,
build_status=None, logtail=False, filemap=None,
dependencies=None):
- builder = self.factory.makeBuilder()
- builder.setSlaveForTesting(slave)
- d = builder.slaveStatus()
+ self.builder.setSlaveForTesting(slave)
+ d = self.builder.slaveStatus()
def got_status(status_dict):
expected = {}
@@ -454,21 +484,19 @@
def test_isAvailable_with_not_builderok(self):
# isAvailable() is a wrapper around slaveStatusSentence()
- builder = self.factory.makeBuilder()
- builder.builderok = False
- d = builder.isAvailable()
+ with BuilddManagerTestFixture.extraSetUp():
+ self.builder.builderok = False
+ d = self.builder.isAvailable()
return d.addCallback(self.assertFalse)
def test_isAvailable_with_slave_fault(self):
- builder = self.factory.makeBuilder()
- builder.setSlaveForTesting(BrokenSlave())
- d = builder.isAvailable()
+ self.builder.setSlaveForTesting(BrokenSlave())
+ d = self.builder.isAvailable()
return d.addCallback(self.assertFalse)
def test_isAvailable_with_slave_idle(self):
- builder = self.factory.makeBuilder()
- builder.setSlaveForTesting(OkSlave())
- d = builder.isAvailable()
+ self.builder.setSlaveForTesting(OkSlave())
+ d = self.builder.isAvailable()
return d.addCallback(self.assertTrue)
@@ -502,6 +530,8 @@
builder.builderok = True
builder.manual = False
+ self.useFixture(BuilddManagerTestFixture())
+
class TestFindBuildCandidateGeneralCases(TestFindBuildCandidateBase):
# Test usage of findBuildCandidate not specific to any archive type.
@@ -510,10 +540,11 @@
# IBuilder._findBuildCandidate identifies if there are builds
# for superseded source package releases in the queue and marks
# the corresponding build record as SUPERSEDED.
- archive = self.factory.makeArchive()
- self.publisher.getPubSource(
- sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
- archive=archive).createMissingBuilds()
+ with BuilddManagerTestFixture.extraSetUp():
+ archive = self.factory.makeArchive()
+ self.publisher.getPubSource(
+ sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
+ archive=archive).createMissingBuilds()
old_candidate = removeSecurityProxy(
self.frog_builder)._findBuildCandidate()
@@ -524,7 +555,8 @@
# Now supersede the source package:
publication = build.current_source_publication
- publication.status = PackagePublishingStatus.SUPERSEDED
+ with BuilddManagerTestFixture.extraSetUp():
+ publication.status = PackagePublishingStatus.SUPERSEDED
# The candidate returned is now a different one:
new_candidate = removeSecurityProxy(
@@ -542,25 +574,24 @@
# PackageBuildJob.postprocessCandidate will attempt to delete
# security builds.
- pub = self.publisher.getPubSource(
- sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
- archive=self.factory.makeArchive(),
- pocket=PackagePublishingPocket.SECURITY)
- pub.createMissingBuilds()
+ with BuilddManagerTestFixture.extraSetUp():
+ pub = self.publisher.getPubSource(
+ sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
+ archive=self.factory.makeArchive(),
+ pocket=PackagePublishingPocket.SECURITY)
+ pub.createMissingBuilds()
+ removeSecurityProxy(self.frog_builder)._findBuildCandidate()
+ # Passes without a "transaction is read-only" error...
transaction.commit()
- with DatabaseTransactionPolicy(read_only=True):
- removeSecurityProxy(self.frog_builder)._findBuildCandidate()
- # The test is that this passes without a "transaction is
- # read-only" error.
- transaction.commit()
def test_acquireBuildCandidate_marks_building(self):
# acquireBuildCandidate() should call _findBuildCandidate and
# mark the build as building.
- archive = self.factory.makeArchive()
- self.publisher.getPubSource(
- sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
- archive=archive).createMissingBuilds()
+ with BuilddManagerTestFixture.extraSetUp():
+ archive = self.factory.makeArchive()
+ self.publisher.getPubSource(
+ sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
+ archive=archive).createMissingBuilds()
candidate = removeSecurityProxy(
self.frog_builder).acquireBuildCandidate()
self.assertEqual(JobStatus.RUNNING, candidate.job.status)
@@ -590,6 +621,8 @@
sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
archive=self.ppa_joe).createMissingBuilds()
+ self.useFixture(BuilddManagerTestFixture())
+
def test_findBuildCandidate_first_build_started(self):
# The allocation rule for PPA dispatching doesn't apply when
# there's only one builder available.
@@ -602,8 +635,9 @@
# If bob is in a failed state the joesppa build is still
# returned.
- self.bob_builder.builderok = False
- self.bob_builder.manual = False
+ with BuilddManagerTestFixture.extraSetUp():
+ self.bob_builder.builderok = False
+ self.bob_builder.manual = False
next_job = removeSecurityProxy(
self.frog_builder)._findBuildCandidate()
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
@@ -615,6 +649,7 @@
ppa_joe_private = False
ppa_jim_private = False
+ @BuilddManagerTestFixture.extraSetUp
def _setBuildsBuildingForArch(self, builds_list, num_builds,
archtag="i386"):
"""Helper function.
@@ -632,7 +667,10 @@
def setUp(self):
"""Publish some builds for the test archive."""
super(TestFindBuildCandidatePPABase, self).setUp()
+ self.extraSetUp()
+ @BuilddManagerTestFixture.extraSetUp
+ def extraSetUp(self):
# Create two PPAs and add some builds to each.
self.ppa_joe = self.factory.makeArchive(
name="joesppa", private=self.ppa_joe_private)
@@ -693,7 +731,8 @@
def test_findBuildCandidate_first_build_finished(self):
# When joe's first ppa build finishes, his fourth i386 build
# will be the next build candidate.
- self.joe_builds[0].status = BuildStatus.FAILEDTOBUILD
+ with BuilddManagerTestFixture.extraSetUp():
+ self.joe_builds[0].status = BuildStatus.FAILEDTOBUILD
next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
self.failUnlessEqual('joesppa', build.archive.name)
@@ -702,9 +741,10 @@
# Disabled archives should not be considered for dispatching
# builds.
disabled_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
- build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(
- disabled_job)
- build.archive.disable()
+ with BuilddManagerTestFixture.extraSetUp():
+ build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(
+ disabled_job)
+ build.archive.disable()
next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
self.assertNotEqual(disabled_job, next_job)
@@ -724,7 +764,8 @@
# dispatched because the builder has to fetch the source files
# from the (password protected) repo area, not the librarian.
pub = build.current_source_publication
- pub.status = PackagePublishingStatus.PENDING
+ with BuilddManagerTestFixture.extraSetUp():
+ pub.status = PackagePublishingStatus.PENDING
candidate = removeSecurityProxy(self.builder4)._findBuildCandidate()
self.assertNotEqual(next_job.id, candidate.id)
@@ -734,6 +775,10 @@
def setUp(self):
"""Publish some builds for the test archive."""
super(TestFindBuildCandidateDistroArchive, self).setUp()
+ self.extraSetUp()
+
+ @BuilddManagerTestFixture.extraSetUp
+ def extraSetUp(self):
# Create a primary archive and publish some builds for the
# queue.
self.non_ppa = self.factory.makeArchive(
@@ -749,7 +794,6 @@
def test_findBuildCandidate_for_non_ppa(self):
# Normal archives are not restricted to serial builds per
# arch.
-
next_job = removeSecurityProxy(
self.frog_builder)._findBuildCandidate()
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
@@ -758,8 +802,9 @@
# Now even if we set the build building, we'll still get the
# second non-ppa build for the same archive as the next candidate.
- build.status = BuildStatus.BUILDING
- build.builder = self.frog_builder
+ with BuilddManagerTestFixture.extraSetUp():
+ build.status = BuildStatus.BUILDING
+ build.builder = self.frog_builder
next_job = removeSecurityProxy(
self.frog_builder)._findBuildCandidate()
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
@@ -776,7 +821,9 @@
self.assertEqual(self.gedit_build.buildqueue_record.lastscore, 2505)
self.assertEqual(self.firefox_build.buildqueue_record.lastscore, 2505)
- recipe_build_job = self.factory.makeSourcePackageRecipeBuildJob(9999)
+ with BuilddManagerTestFixture.extraSetUp():
+ recipe_build_job = (
+ self.factory.makeSourcePackageRecipeBuildJob(9999))
self.assertEqual(recipe_build_job.lastscore, 9999)
@@ -790,6 +837,7 @@
# These tests operate in a "recipe builds only" setting.
# Please see also bug #507782.
+ @BuilddManagerTestFixture.extraSetUp
def clearBuildQueue(self):
"""Delete all `BuildQueue`, XXXJOb and `Job` instances."""
store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
@@ -799,6 +847,10 @@
def setUp(self):
"""Publish some builds for the test archive."""
super(TestFindRecipeBuildCandidates, self).setUp()
+ self.extraSetUp()
+
+ @BuilddManagerTestFixture.extraSetUp
+ def extraSetUp(self):
# Create a primary archive and publish some builds for the
# queue.
self.non_ppa = self.factory.makeArchive(
@@ -844,6 +896,8 @@
self.buildfarmjob = self.build.buildqueue_record.specific_job
+ self.useFixture(BuilddManagerTestFixture())
+
def test_idle_behavior_when_no_current_build(self):
"""We return an idle behavior when there is no behavior specified
nor a current build.
@@ -864,7 +918,8 @@
"""The current behavior is set automatically from the current job."""
# Set the builder attribute on the buildqueue record so that our
# builder will think it has a current build.
- self.build.buildqueue_record.builder = self.builder
+ with BuilddManagerTestFixture.extraSetUp():
+ self.build.buildqueue_record.builder = self.builder
self.assertIsInstance(
self.builder.current_build_behavior, BinaryPackageBuildBehavior)
@@ -1157,15 +1212,17 @@
super(TestSlaveWithLibrarian, self).setUp()
self.slave_helper = self.useFixture(SlaveTestHelpers())
+ self.useFixture(BuilddManagerTestFixture())
+
def test_ensurepresent_librarian(self):
# ensurepresent, when given an http URL for a file will download the
# file from that URL and report that the file is present, and it was
# downloaded.
# Use the Librarian because it's a "convenient" web server.
- lf = self.factory.makeLibraryFileAlias(
- 'HelloWorld.txt', content="Hello World")
- self.layer.txn.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ lf = self.factory.makeLibraryFileAlias(
+ 'HelloWorld.txt', content="Hello World")
self.slave_helper.getServerSlave()
slave = self.slave_helper.getClientSlave()
d = slave.ensurepresent(
@@ -1178,9 +1235,9 @@
# filename made from the sha1 of the content underneath the
# 'filecache' directory.
content = "Hello World"
- lf = self.factory.makeLibraryFileAlias(
- 'HelloWorld.txt', content=content)
- self.layer.txn.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ lf = self.factory.makeLibraryFileAlias(
+ 'HelloWorld.txt', content=content)
expected_url = '%s/filecache/%s' % (
self.slave_helper.BASE_URL, lf.content.sha1)
self.slave_helper.getServerSlave()
@@ -1206,7 +1263,6 @@
def got_files(ignored):
# Called back when getFiles finishes. Make sure all the
# content is as expected.
- got_contents = []
for sha1 in filemap:
local_file = filemap[sha1]
file = open(local_file)
@@ -1222,11 +1278,12 @@
dl = []
for content in contents:
filename = content + '.txt'
- lf = self.factory.makeLibraryFileAlias(filename, content=content)
+ with BuilddManagerTestFixture.extraSetUp():
+ lf = self.factory.makeLibraryFileAlias(
+ filename, content=content)
content_map[lf.content.sha1] = content
fd, filemap[lf.content.sha1] = tempfile.mkstemp()
self.addCleanup(os.remove, filemap[lf.content.sha1])
- self.layer.txn.commit()
d = slave.ensurepresent(lf.content.sha1, lf.http_url, "", "")
dl.append(d)
=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py 2011-12-23 13:30:34 +0000
+++ lib/lp/buildmaster/tests/test_manager.py 2011-12-23 13:30:35 +0000
@@ -14,7 +14,6 @@
assert_fails_with,
AsynchronousDeferredRunTest,
)
-import transaction
from twisted.internet import (
defer,
reactor,
@@ -47,6 +46,7 @@
)
from lp.buildmaster.model.builder import Builder
from lp.buildmaster.model.packagebuild import PackageBuild
+from lp.buildmaster.testing import BuilddManagerTestFixture
from lp.buildmaster.tests.harness import BuilddManagerTestSetup
from lp.buildmaster.tests.mock_slaves import (
BrokenSlave,
@@ -56,7 +56,6 @@
WaitingSlave,
)
from lp.registry.interfaces.distribution import IDistributionSet
-from lp.services.database.transaction_policy import DatabaseTransactionPolicy
from lp.services.log.logger import BufferLogger
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.testing import (
@@ -86,8 +85,6 @@
'bob' builder.
"""
super(TestSlaveScannerScan, self).setUp()
- self.read_only = DatabaseTransactionPolicy(read_only=True)
-
# Creating the required chroots needed for dispatching.
test_publisher = make_publisher()
ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
@@ -95,25 +92,15 @@
test_publisher.setUpDefaultDistroSeries(hoary)
test_publisher.addFakeChroots()
- def _enterReadOnly(self):
- """Go into read-only transaction policy."""
- self.read_only.__enter__()
- self.addCleanup(self._exitReadOnly)
-
- def _exitReadOnly(self):
- """Leave read-only transaction policy."""
- self.read_only.__exit__(None, None, None)
+ self.useFixture(BuilddManagerTestFixture())
def _resetBuilder(self, builder):
"""Reset the given builder and its job."""
-
builder.builderok = True
job = builder.currentjob
if job is not None:
job.reset()
- transaction.commit()
-
def getFreshBuilder(self, slave=None, name=BOB_THE_BUILDER_NAME,
failure_count=0):
"""Return a builder.
@@ -171,12 +158,11 @@
# Obtain a builder. Initialize failure count to 1 so that
# _checkDispatch can make sure that a successful dispatch resets
# the count to 0.
- builder = self.getFreshBuilder(failure_count=1)
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.getFreshBuilder(failure_count=1)
# Run 'scan' and check its result.
- self.layer.txn.commit()
self.layer.switchDbUser(config.builddmaster.dbuser)
- self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkDispatch, builder)
@@ -199,21 +185,19 @@
# When a required chroot is not present the `scan` method
# should not return any `RecordingSlaves` to be processed
# and the builder used should remain active and IDLE.
-
- builder = self.getFreshBuilder()
-
- # Remove hoary/i386 chroot.
- login('foo.bar@xxxxxxxxxxxxx')
- ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
- hoary = ubuntu.getSeries('hoary')
- pocket_chroot = hoary.getDistroArchSeries('i386').getPocketChroot()
- removeSecurityProxy(pocket_chroot).chroot = None
- transaction.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.getFreshBuilder()
+ # Remove hoary/i386 chroot.
+ login('foo.bar@xxxxxxxxxxxxx')
+ ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+ hoary = ubuntu.getSeries('hoary')
+ pocket_chroot = hoary.getDistroArchSeries('i386').getPocketChroot()
+ removeSecurityProxy(pocket_chroot).chroot = None
+
login(ANONYMOUS)
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
- self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.singleCycle)
d.addCallback(self._checkNoDispatch, builder)
@@ -249,13 +233,12 @@
# Disable the sampledata builder
login('foo.bar@xxxxxxxxxxxxx')
- builder.builderok = False
- transaction.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder.builderok = False
login(ANONYMOUS)
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
- self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkJobRescued, builder, job)
@@ -281,9 +264,9 @@
builder = getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME]
login('foo.bar@xxxxxxxxxxxxx')
- builder.builderok = True
- builder.setSlaveForTesting(BuildingSlave(build_id='8-1'))
- transaction.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder.builderok = True
+ builder.setSlaveForTesting(BuildingSlave(build_id='8-1'))
login(ANONYMOUS)
job = builder.currentjob
@@ -291,27 +274,24 @@
# Run 'scan' and check its result.
self.layer.switchDbUser(config.builddmaster.dbuser)
- self._enterReadOnly()
scanner = self._getScanner()
d = defer.maybeDeferred(scanner.scan)
d.addCallback(self._checkJobUpdated, builder, job)
return d
def test_scan_with_nothing_to_dispatch(self):
- builder = self.factory.makeBuilder()
- builder.setSlaveForTesting(OkSlave())
- transaction.commit()
- self._enterReadOnly()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.factory.makeBuilder()
+ builder.setSlaveForTesting(OkSlave())
scanner = self._getScanner(builder_name=builder.name)
d = scanner.scan()
return d.addCallback(self._checkNoDispatch, builder)
def test_scan_with_manual_builder(self):
# Reset sampledata builder.
- builder = self.getFreshBuilder()
- builder.manual = True
- transaction.commit()
- self._enterReadOnly()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.getFreshBuilder()
+ builder.manual = True
scanner = self._getScanner()
d = scanner.scan()
d.addCallback(self._checkNoDispatch, builder)
@@ -319,10 +299,9 @@
def test_scan_with_not_ok_builder(self):
# Reset sampledata builder.
- builder = self.getFreshBuilder()
- builder.builderok = False
- transaction.commit()
- self._enterReadOnly()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.getFreshBuilder()
+ builder.builderok = False
scanner = self._getScanner()
d = scanner.scan()
# Because the builder is not ok, we can't use _checkNoDispatch.
@@ -331,9 +310,8 @@
return d
def test_scan_of_broken_slave(self):
- builder = self.getFreshBuilder(slave=BrokenSlave())
- transaction.commit()
- self._enterReadOnly()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder = self.getFreshBuilder(slave=BrokenSlave())
scanner = self._getScanner(builder_name=builder.name)
d = scanner.scan()
return assert_fails_with(d, xmlrpclib.Fault)
@@ -350,16 +328,17 @@
def failing_scan():
return defer.fail(Exception("fake exception"))
- scanner = self._getScanner()
- scanner.scan = failing_scan
- self.patch(manager_module, 'assessFailureCounts', FakeMethod())
- builder = getUtility(IBuilderSet)[scanner.builder_name]
+ with BuilddManagerTestFixture.extraSetUp():
+ scanner = self._getScanner()
+ scanner.scan = failing_scan
+ self.patch(manager_module, 'assessFailureCounts', FakeMethod())
+ builder = getUtility(IBuilderSet)[scanner.builder_name]
- builder.failure_count = builder_count
- builder.currentjob.specific_job.build.failure_count = job_count
- # The _scanFailed() calls abort, so make sure our existing
- # failure counts are persisted.
- self.layer.txn.commit()
+ builder.failure_count = builder_count
+ builder.currentjob.specific_job.build.failure_count = job_count
+ # The _scanFailed() calls abort, so make sure our existing failure
+ # counts are persisted by exiting the extraSetUp() context (which
+ # commits).
# singleCycle() calls scan() which is our fake one that throws an
# exception.
@@ -405,9 +384,9 @@
scanner = self._getScanner()
scanner.scan = failing_scan
builder = getUtility(IBuilderSet)[scanner.builder_name]
- builder.failure_count = Builder.FAILURE_THRESHOLD
- builder.currentjob.reset()
- self.layer.txn.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder.failure_count = Builder.FAILURE_THRESHOLD
+ builder.currentjob.reset()
d = scanner.singleCycle()
@@ -428,17 +407,19 @@
# Reset sampledata builder.
builder = removeSecurityProxy(
getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME])
- self._resetBuilder(builder)
- self.assertEqual(0, builder.failure_count)
- builder.setSlaveForTesting(slave)
- builder.vm_host = "fake_vm_host"
+ with BuilddManagerTestFixture.extraSetUp():
+ self._resetBuilder(builder)
+ self.assertEqual(0, builder.failure_count)
+ builder.setSlaveForTesting(slave)
+ builder.vm_host = "fake_vm_host"
scanner = self._getScanner()
# Get the next job that will be dispatched.
job = removeSecurityProxy(builder._findBuildCandidate())
- job.virtualized = True
- builder.virtualized = True
+ with BuilddManagerTestFixture.extraSetUp():
+ job.virtualized = True
+ builder.virtualized = True
d = scanner.singleCycle()
def check(ignored):
@@ -470,19 +451,20 @@
# Set the sample data builder building with the slave from above.
builder = getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME]
login('foo.bar@xxxxxxxxxxxxx')
- builder.builderok = True
- # For now, we can only cancel virtual builds.
- builder.virtualized = True
- builder.vm_host = "fake_vm_host"
- builder.setSlaveForTesting(slave)
- transaction.commit()
+ with BuilddManagerTestFixture.extraSetUp():
+ builder.builderok = True
+ # For now, we can only cancel virtual builds.
+ builder.virtualized = True
+ builder.vm_host = "fake_vm_host"
+ builder.setSlaveForTesting(slave)
login(ANONYMOUS)
buildqueue = builder.currentjob
self.assertBuildingJob(buildqueue, builder)
# Now set the build to CANCELLING.
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
- build.status = BuildStatus.CANCELLING
+ with BuilddManagerTestFixture.extraSetUp():
+ build.status = BuildStatus.CANCELLING
# Run 'scan' and check its results.
self.layer.switchDbUser(config.builddmaster.dbuser)
@@ -513,19 +495,22 @@
# not affect the functioning builder.
clock = task.Clock()
- broken_builder = self.getFreshBuilder(
- slave=BrokenSlave(), name=BOB_THE_BUILDER_NAME)
- broken_scanner = self._getScanner(builder_name=broken_builder.name)
- good_builder = self.getFreshBuilder(
- slave=WaitingSlave(), name=FROG_THE_BUILDER_NAME)
- good_build = self.factory.makeBinaryPackageBuild(
- distroarchseries=self.factory.makeDistroArchSeries())
-
- # The good build is being handled by the good builder.
- buildqueue = good_build.queueBuild()
- buildqueue.builder = good_builder
-
- removeSecurityProxy(good_build.build_farm_job).date_started = UTC_NOW
+ with BuilddManagerTestFixture.extraSetUp():
+ broken_builder = self.getFreshBuilder(
+ slave=BrokenSlave(), name=BOB_THE_BUILDER_NAME)
+ broken_scanner = self._getScanner(
+ builder_name=broken_builder.name)
+ good_builder = self.getFreshBuilder(
+ slave=WaitingSlave(), name=FROG_THE_BUILDER_NAME)
+ good_build = self.factory.makeBinaryPackageBuild(
+ distroarchseries=self.factory.makeDistroArchSeries())
+
+ # The good build is being handled by the good builder.
+ buildqueue = good_build.queueBuild()
+ buildqueue.builder = good_builder
+
+ removeSecurityProxy(
+ good_build.build_farm_job).date_started = UTC_NOW
# The good builder requests information from a successful build,
# and up receiving it, updates the build's metadata.
@@ -569,9 +554,12 @@
self.scanner.builder = self.builder
self.scanner.logger.name = 'slave-scanner'
+ self.useFixture(BuilddManagerTestFixture())
+
def test_ignores_nonvirtual(self):
# If the builder is nonvirtual make sure we return False.
- self.builder.virtualized = False
+ with BuilddManagerTestFixture.extraSetUp():
+ self.builder.virtualized = False
d = self.scanner.checkCancellation(self.builder)
return d.addCallback(self.assertFalse)
@@ -579,7 +567,8 @@
# If the builder has no buildqueue associated,
# make sure we return False.
buildqueue = self.builder.currentjob
- buildqueue.reset()
+ with BuilddManagerTestFixture.extraSetUp():
+ buildqueue.reset()
d = self.scanner.checkCancellation(self.builder)
return d.addCallback(self.assertFalse)
@@ -587,7 +576,8 @@
# If the active build is not in a CANCELLING state, ignore it.
buildqueue = self.builder.currentjob
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
- build.status = BuildStatus.BUILDING
+ with BuilddManagerTestFixture.extraSetUp():
+ build.status = BuildStatus.BUILDING
d = self.scanner.checkCancellation(self.builder)
return d.addCallback(self.assertFalse)
@@ -601,11 +591,13 @@
return defer.succeed((None, None, 0))
slave = OkSlave()
slave.resume = fake_resume
- self.builder.vm_host = "fake_vm_host"
- self.builder.setSlaveForTesting(slave)
- buildqueue = self.builder.currentjob
- build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
- build.status = BuildStatus.CANCELLING
+
+ with BuilddManagerTestFixture.extraSetUp():
+ self.builder.vm_host = "fake_vm_host"
+ self.builder.setSlaveForTesting(slave)
+ buildqueue = self.builder.currentjob
+ build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
+ build.status = BuildStatus.CANCELLING
def check(result):
self.assertEqual(1, call_counter.call_count)
=== modified file 'lib/lp/services/database/tests/test_transaction_policy.py'
--- lib/lp/services/database/tests/test_transaction_policy.py 2011-12-23 13:30:34 +0000
+++ lib/lp/services/database/tests/test_transaction_policy.py 2011-12-23 13:30:35 +0000
@@ -41,6 +41,14 @@
self.factory.makePerson(name=name)
return name
+ def writeToDatabaseAndCommit(self):
+ """Write an object to the database and commit.
+
+ :return: A token that `hasDatabaseBeenWrittenTo` can look for.
+ """
+ self.writeToDatabase()
+ transaction.commit()
+
def hasDatabaseBeenWrittenTo(self, test_token):
"""Is the object made by `writeToDatabase` present in the database?
@@ -216,28 +224,36 @@
def test_policy_can_span_transactions(self):
# It's okay to commit within a policy; the policy will still
# apply to the next transaction inside the same policy.
- def write_and_commit():
- self.writeToDatabase()
- transaction.commit()
-
test_token = self.writeToDatabase()
transaction.commit()
with DatabaseTransactionPolicy(read_only=True):
self.hasDatabaseBeenWrittenTo(test_token)
transaction.commit()
- self.assertRaises(InternalError, write_and_commit)
+ self.assertRaises(InternalError, self.writeToDatabaseAndCommit)
transaction.abort()
def test_policy_survives_abort(self):
# Even after aborting the initial transaction, a transaction
# policy still applies.
- def write_and_commit():
- self.writeToDatabase()
- transaction.commit()
-
with DatabaseTransactionPolicy(read_only=True):
self.readFromDatabase()
transaction.abort()
- self.assertRaises(InternalError, write_and_commit)
+ self.assertRaises(InternalError, self.writeToDatabaseAndCommit)
transaction.abort()
+
+ def test_readOnly(self):
+ # DatabaseTransactionPolicy.readOnly() is a function decorator that
+ # applies a read-only policy for the duration of the function call.
+ writeToDatabaseAndCommit = DatabaseTransactionPolicy.readOnly(
+ self.writeToDatabaseAndCommit)
+ with DatabaseTransactionPolicy(read_only=False):
+ self.assertRaises(InternalError, writeToDatabaseAndCommit)
+
+ def test_readWrite(self):
+ # DatabaseTransactionPolicy.readWrite() is a function decorator that
+ # applies a read-write policy for the duration of the function call.
+ writeToDatabaseAndCommit = DatabaseTransactionPolicy.readWrite(
+ self.writeToDatabaseAndCommit)
+ with DatabaseTransactionPolicy(read_only=True):
+ writeToDatabaseAndCommit() # No exception.
=== modified file 'lib/lp/services/database/transaction_policy.py'
--- lib/lp/services/database/transaction_policy.py 2011-12-23 13:30:34 +0000
+++ lib/lp/services/database/transaction_policy.py 2011-12-23 13:30:35 +0000
@@ -8,6 +8,8 @@
'DatabaseTransactionPolicy',
]
+from functools import wraps
+
from psycopg2.extensions import TRANSACTION_STATUS_IDLE
import transaction
from zope.component import getUtility
@@ -195,3 +197,19 @@
"""
self.store.execute(
"SET %s TO %s" % (self.db_switch, quote(read_only)))
+
+ @classmethod
+ def readOnly(cls, func):
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ with cls(read_only=True):
+ return func(*args, **kwargs)
+ return wrapper
+
+ @classmethod
+ def readWrite(cls, func):
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ with cls(read_only=False):
+ return func(*args, **kwargs)
+ return wrapper
