Mercurial > bitten > bitten-test
changeset 606:07ac9218f649
0.6dev: Fixing numerous problems related to `BuildQueue.should_delete_build()`:
* Crash when trying to write `TargetPlatform().name` to log when a target platform may no longer exist (#212, #391).
* Delete pending builds that have a platform that is since deleted (#212, thanks dabrahams).
* When not `build_all` delete pending builds if a newer build already exists (#264, thanks J?r?me Berger). This adds `min_rev_time` and `max_rev_time` criteria to `Build.select()` to enable time-based selection of builds.
* Remove old-style logging as none of the nice log messages actually ended up in the environment log.
Added 4 tests for each of the checks performed in `BuildQueue.should_delete_build()`.
| author | osimons |
|---|---|
| date | Thu, 30 Jul 2009 23:28:53 +0000 |
| parents | c94481bc4646 |
| children | 5d396356bf7a |
| files | bitten/model.py bitten/queue.py bitten/tests/queue.py |
| diffstat | 3 files changed, 102 insertions(+), 19 deletions(-) [+] |
line wrap: on
line diff
--- a/bitten/model.py +++ b/bitten/model.py @@ -489,7 +489,7 @@ fetch = classmethod(fetch) def select(cls, env, config=None, rev=None, platform=None, slave=None, - status=None, db=None): + status=None, db=None, min_rev_time=None, max_rev_time=None): """Retrieve existing builds from the database that match the specified criteria. """ @@ -507,6 +507,10 @@ where_clauses.append(("slave=%s", slave)) if status is not None: where_clauses.append(("status=%s", status)) + if min_rev_time is not None: + where_clauses.append(("rev_time>=%s", min_rev_time)) + if max_rev_time is not None: + where_clauses.append(("rev_time<=%s", max_rev_time)) if where_clauses: where = "WHERE " + " AND ".join([wc[0] for wc in where_clauses]) else:
--- a/bitten/queue.py +++ b/bitten/queue.py @@ -20,7 +20,6 @@ """ from itertools import ifilter -import logging import re import time @@ -30,8 +29,6 @@ __docformat__ = 'restructuredtext en' -log = logging.getLogger('bitten.queue') - def collect_changes(repos, config, db=None): """Collect all changes for a build configuration that either have already @@ -127,7 +124,7 @@ :return: the allocated build, or `None` if no build was found :rtype: `Build` """ - log.debug('Checking for pending builds...') + self.log.debug('Checking for pending builds...') db = self.env.get_db_cnx() repos = self.env.get_repository() @@ -286,18 +283,25 @@ db.commit() def should_delete_build(self, build, repos): + config = BuildConfig.fetch(self.env, build.config) + + platform = TargetPlatform.fetch(self.env, build.platform) + # Platform may or may not exist anymore - get safe name for logging + platform_name = platform and platform.name \ + or 'unknown platform "%s"' % build.platform + + # Drop build if platform no longer exists + if not platform: + self.log.info('Dropping build of configuration "%s" at ' + 'revision [%s] on %s because the platform no longer ' + 'exists', config.name, build.rev, platform_name) + return True + # Ignore pending builds for deactived build configs - config = BuildConfig.fetch(self.env, build.config) if not config.active: - target_platform = TargetPlatform.fetch(self.env, build.platform) - if target_platform: - target_platform_name = '"%s"' % (target_platform.name,) - else: - target_platform_name = 'unknown platform "%s"' % (build.platform,) - log.info('Dropping build of configuration "%s" at ' + self.log.info('Dropping build of configuration "%s" at ' 'revision [%s] on %s because the configuration is ' - 'deactivated', config.name, build.rev, - target_platform_name) + 'deactivated', config.name, build.rev, platform_name) return True # Stay within the revision limits of the build config @@ -305,12 +309,18 @@ config.min_rev)) \ or (config.max_rev and repos.rev_older_than(config.max_rev, build.rev)): - # This minimum and/or maximum revision has changed since - # this build was enqueued, so drop it - log.info('Dropping build of configuration "%s" at revision [%s] on ' + self.log.info('Dropping build of configuration "%s" at revision [%s] on ' '"%s" because it is outside of the revision range of the ' - 'configuration', config.name, build.rev, - TargetPlatform.fetch(self.env, build.platform).name) + 'configuration', config.name, build.rev, platform_name) + return True + + # If not 'build_all', drop if a more recent revision is available + if not self.build_all and \ + len(list(Build.select(self.env, config=build.config, + min_rev_time=build.rev_time, platform=build.platform))) > 1: + self.log.info('Dropping build of configuration "%s" at revision [%s] ' + 'on "%s" because a more recent build exists', + config.name, build.rev, platform_name) return True return False
--- a/bitten/tests/queue.py +++ b/bitten/tests/queue.py @@ -261,6 +261,75 @@ self.assertEqual(platform2.id, builds[5].platform) self.assertEqual('120', builds[5].rev) + def test_should_delete_build_platform_dont_exist(self): + messages = [] + self.env.log = Mock(info=lambda msg, *args: messages.append(msg)) + config = BuildConfig(self.env, 'test', active=True) + config.insert() + build = Build(self.env, config=config.name, rev=42, + platform="no-stuff", rev_time=123456) + build.insert() + queue = BuildQueue(self.env, build_all=True) + + self.assertEqual(True, queue.should_delete_build(build, self.repos)) + self.assert_("platform no longer exists" in messages[0]) + + def test_should_delete_build_config_deactivated(self): + messages = [] + self.env.log = Mock(info=lambda msg, *args: messages.append(msg)) + config = BuildConfig(self.env, 'test', active=False) + config.insert() + platform = TargetPlatform(self.env, config='test', name='stuff') + platform.insert() + build = Build(self.env, config=config.name, rev=42, + platform=platform.id, rev_time=123456) + build.insert() + queue = BuildQueue(self.env, build_all=True) + + self.assertEqual(True, queue.should_delete_build(build, self.repos)) + self.assert_("configuration is deactivated" in messages[0]) + + def test_should_delete_build_outside_revision_range(self): + messages = [] + self.env.log = Mock(info=lambda msg, *args: messages.append(msg)) + self.repos.rev_older_than = lambda rev1, rev2: rev1 < rev2 + config = BuildConfig(self.env, 'test', active=True, min_rev=120, + max_rev=123) + config.insert() + platform = TargetPlatform(self.env, config='test', name='stuff') + platform.insert() + build1 = Build(self.env, config=config.name, rev=42, + platform=platform.id, rev_time=123456) + build1.insert() + build2 = Build(self.env, config=config.name, rev=10042, + platform=platform.id, rev_time=123456) + build2.insert() + queue = BuildQueue(self.env, build_all=True) + + self.assertEqual(True, queue.should_delete_build(build1, self.repos)) + self.assertEqual(True, queue.should_delete_build(build2, self.repos)) + self.assert_("outside of the revision range" in messages[0]) + self.assert_("outside of the revision range" in messages[1]) + + def test_should_delete_build_old_with_not_buildall(self): + messages = [] + self.env.log = Mock(info=lambda msg, *args: messages.append(msg)) + config = BuildConfig(self.env, 'test', active=True) + config.insert() + platform = TargetPlatform(self.env, config='test', name='stuff') + platform.insert() + build1 = Build(self.env, config=config.name, rev=42, + platform=platform.id, rev_time=123456) + build1.insert() + build2 = Build(self.env, config=config.name, rev=43, + platform=platform.id, rev_time=123457, + slave='slave') + build2.insert() + queue = BuildQueue(self.env, build_all=False) + + self.assertEqual(True, queue.should_delete_build(build1, self.repos)) + self.assert_("more recent build exists" in messages[0]) + def test_reset_orphaned_builds(self): BuildConfig(self.env, 'test').insert() platform = TargetPlatform(self.env, config='test', name='Foo')