# HG changeset patch # User cmlenz # Date 1187300524 0 # Node ID 201f467e0ec1692acfba83f2e57f33554660c54e # Parent 74c51f648466fd2c9fbc68bf0ad7934da20cd556 More unit tests for admin module. diff --git a/bitten/admin.py b/bitten/admin.py --- a/bitten/admin.py +++ b/bitten/admin.py @@ -40,23 +40,8 @@ master = BuildMaster(self.env) if req.method == 'POST': - changed = False - build_all = 'build_all' in req.args - if build_all != master.build_all: - self.config['bitten'].set('build_all', - build_all and 'yes' or 'no') - changed = True - adjust_timestamps = 'adjust_timestamps' in req.args - if adjust_timestamps != master.adjust_timestamps: - self.config['bitten'].set('adjust_timestamps', - adjust_timestamps and 'yes' or 'no') - changed = True - slave_timeout = int(req.args.get('slave_timeout', 0)) - if slave_timeout != master.slave_timeout: - self.config['bitten'].set('slave_timeout', str(slave_timeout)) - changed = True - if changed: - self.config.save() + self._save_config_changes(req, master) + req.redirect(req.abs_href.admin(cat, page)) req.hdf['admin.master'] = { 'build_all': master.build_all, @@ -65,6 +50,33 @@ } return 'bitten_admin_master.cs', None + # Internal methods + + def _save_config_changes(self, req, master): + changed = False + + build_all = 'build_all' in req.args + if build_all != master.build_all: + self.config['bitten'].set('build_all', + build_all and 'yes' or 'no') + changed = True + + adjust_timestamps = 'adjust_timestamps' in req.args + if adjust_timestamps != master.adjust_timestamps: + self.config['bitten'].set('adjust_timestamps', + adjust_timestamps and 'yes' or 'no') + changed = True + + slave_timeout = int(req.args.get('slave_timeout', 0)) + if slave_timeout != master.slave_timeout: + self.config['bitten'].set('slave_timeout', str(slave_timeout)) + changed = True + + if changed: + self.config.save() + + return master + class BuildConfigurationsAdminPageProvider(Component): """Web administration panel for configuring the build master.""" @@ -86,52 +98,8 @@ if req.method == 'POST': if 'save' in req.args: - name = req.args.get('name') - if not name: - raise TracError('Missing required field "name"', - 'Missing field') - if not re.match(r'^[\w.-]+$', name): - raise TracError('The field "name" may only contain ' - 'letters, digits, periods, or dashes.', - 'Invalid field') - - path = req.args.get('path', '') - repos = self.env.get_repository(req.authname) - max_rev = req.args.get('max_rev') or None - try: - node = repos.get_node(path, max_rev) - assert node.isdir, '%s is not a directory' % node.path - except (AssertionError, TracError), e: - raise TracError(e, 'Invalid repository path') - if req.args.get('min_rev'): - try: - repos.get_node(path, req.args.get('min_rev')) - except TracError, e: - raise TracError(e, - 'Invalid value for oldest revision') - - recipe_xml = req.args.get('recipe', '') - if recipe_xml: - try: - Recipe(xmlio.parse(recipe_xml)).validate() - except xmlio.ParseError, e: - raise TracError('Failure parsing recipe: %s' % e, - 'Invalid recipe') - except InvalidRecipeError, e: - raise TracError(e, 'Invalid recipe') - - config.name = name - config.path = repos.normalize_path(path) - config.recipe = recipe_xml - config.min_rev = req.args.get('min_rev') - config.max_rev = req.args.get('max_rev') - config.label = req.args.get('label', '') - config.description = req.args.get('description', '') - config.update() - req.redirect(self.env.href.admin(cat, page)) - - elif 'cancel' in req.args: - req.redirect(self.env.href.admin(cat, page)) + self._save_config(req, config) + req.redirect(req.abs_href.admin(cat, page)) data['config'] = { 'name': config.name, 'label': config.label or config.name, @@ -149,39 +117,14 @@ else: if req.method == 'POST': - # Add configuration - if 'add' in req.args: - config = BuildConfig(self.env) - config.name = req.args.get('name') - config.label = req.args.get('label', config.name) - config.path = req.args.get('path') - config.insert() - req.redirect(self.env.href.admin(cat, page, config.name)) - - # Remove configurations - elif 'remove' in req.args: - sel = req.args.get('sel') - sel = isinstance(sel, list) and sel or [sel] - if not sel: - raise TracError('No configuration selected') - db = self.env.get_db_cnx() - for name in sel: - config = BuildConfig.fetch(self.env, name, db=db) - config.delete(db=db) - db.commit() - req.redirect(self.env.href.admin(cat, page)) - - # Set active state - elif 'apply' in req.args: - active = req.args.get('active') - active = isinstance(active, list) and active or [active] - db = self.env.get_db_cnx() - for config in BuildConfig.select(self.env, db=db, - include_inactive=True): - config.active = config.name in active - config.update(db=db) - db.commit() - req.redirect(self.env.href.admin(cat, page)) + if 'add' in req.args: # Add build config + config = self._create_config(req) + req.redirect(req.abs_href.admin(cat, page, config.name)) + elif 'remove' in req.args: # Remove selected build configs + self._remove_configs(req) + elif 'apply' in req.args: # Update active state of configs + self._activate_configs(req) + req.redirect(req.abs_href.admin(cat, page)) configs = [] for config in BuildConfig.select(self.env, include_inactive=True): @@ -195,3 +138,80 @@ req.hdf['admin'] = data return 'bitten_admin_configs.cs', None + + # Internal methods + + def _activate_configs(self, req): + active = req.args.get('active') + active = isinstance(active, list) and active or [active] + db = self.env.get_db_cnx() + for config in BuildConfig.select(self.env, db=db, + include_inactive=True): + config.active = config.name in active + config.update(db=db) + db.commit() + + def _create_config(self, req): + req.perm.assert_permission('BUILD_CREATE') + config = BuildConfig(self.env) + config.name = req.args.get('name') + config.label = req.args.get('label', config.name) + config.path = req.args.get('path') + config.insert() + return config + + def _remove_configs(self, req): + req.perm.assert_permission('BUILD_DELETE') + sel = req.args.get('sel') + if not sel: + raise TracError('No configuration selected') + sel = isinstance(sel, list) and sel or [sel] + db = self.env.get_db_cnx() + for name in sel: + config = BuildConfig.fetch(self.env, name, db=db) + if not config: + raise TracError('Configuration %r not found' % name) + config.delete(db=db) + db.commit() + + def _update_config(self, req, config): + name = req.args.get('name') + if not name: + raise TracError('Missing required field "name"', 'Missing field') + if not re.match(r'^[\w.-]+$', name): + raise TracError('The field "name" may only contain letters, ' + 'digits, periods, or dashes.', 'Invalid field') + + path = req.args.get('path', '') + repos = self.env.get_repository(req.authname) + max_rev = req.args.get('max_rev') or None + try: + node = repos.get_node(path, max_rev) + assert node.isdir, '%s is not a directory' % node.path + except (AssertionError, TracError), e: + raise TracError(e, 'Invalid repository path') + if req.args.get('min_rev'): + try: + repos.get_node(path, req.args.get('min_rev')) + except TracError, e: + raise TracError(e, + 'Invalid value for oldest revision') + + recipe_xml = req.args.get('recipe', '') + if recipe_xml: + try: + Recipe(xmlio.parse(recipe_xml)).validate() + except xmlio.ParseError, e: + raise TracError('Failure parsing recipe: %s' % e, + 'Invalid recipe') + except InvalidRecipeError, e: + raise TracError(e, 'Invalid recipe') + + config.name = name + config.path = repos.normalize_path(path) + config.recipe = recipe_xml + config.min_rev = req.args.get('min_rev') + config.max_rev = req.args.get('max_rev') + config.label = req.args.get('label', '') + config.description = req.args.get('description', '') + config.update() diff --git a/bitten/model.py b/bitten/model.py --- a/bitten/model.py +++ b/bitten/model.py @@ -141,10 +141,10 @@ config = BuildConfig(env) config.name = config._old_name = name config.path = row[0] or '' - config.active = row[1] and True or False + config.active = bool(row[1]) config.recipe = row[2] or '' - config.min_rev = row[3] or '' - config.max_rev = row[4] or '' + config.min_rev = row[3] or None + config.max_rev = row[4] or None config.label = row[5] or '' config.description = row[6] or '' return config diff --git a/bitten/tests/admin.py b/bitten/tests/admin.py --- a/bitten/tests/admin.py +++ b/bitten/tests/admin.py @@ -13,7 +13,7 @@ from trac.core import TracError from trac.db import DatabaseManager -from trac.perm import PermissionCache, PermissionSystem +from trac.perm import PermissionCache, PermissionError, PermissionSystem from trac.test import EnvironmentStub, Mock from trac.versioncontrol import Repository from trac.web.clearsilver import HDFWrapper @@ -68,14 +68,15 @@ self.assertEqual([], list(provider.get_admin_pages(req))) def test_process_get_request(self): - provider = BuildMasterAdminPageProvider(self.env) - data = {} req = Mock(method='GET', hdf=data, perm=PermissionCache(self.env, 'joe')) + + provider = BuildMasterAdminPageProvider(self.env) template_name, content_type = provider.process_admin_request( req, 'bitten', 'master', '' ) + self.assertEqual('bitten_admin_master.cs', template_name) self.assertEqual(None, content_type) assert 'admin.master' in data @@ -85,6 +86,28 @@ 'build_all': False, }, data['admin.master']) + def test_process_config_changes(self): + redirected_to = [] + def redirect(url): + redirected_to.append(url) + raise RequestDone + req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + abs_href=Href('http://example.org/'), redirect=redirect, + args={'slave_timeout': '60', 'adjust_timestamps': ''}) + + provider = BuildMasterAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'master', '') + self.fail('Expected RequestDone') + + except RequestDone: + self.assertEqual('http://example.org/admin/bitten/master', + redirected_to[0]) + section = self.env.config['bitten'] + self.assertEqual(60, section.getint('slave_timeout')) + self.assertEqual(True, section.getbool('adjust_timestamps')) + self.assertEqual(False, section.getbool('build_all')) + class BuildConfigurationsAdminPageProviderTestCase(unittest.TestCase): @@ -103,6 +126,8 @@ # Set up permissions self.env.config.set('trac', 'permission_store', 'DefaultPermissionStore') + PermissionSystem(self.env).grant_permission('joe', 'BUILD_CREATE') + PermissionSystem(self.env).grant_permission('joe', 'BUILD_DELETE') PermissionSystem(self.env).grant_permission('joe', 'BUILD_MODIFY') # Hook up a dummy repository @@ -129,18 +154,187 @@ self.assertEqual([], list(provider.get_admin_pages(req))) def test_process_get_request_overview_empty(self): - provider = BuildConfigurationsAdminPageProvider(self.env) - data = {} req = Mock(method='GET', hdf=data, perm=PermissionCache(self.env, 'joe')) + + provider = BuildConfigurationsAdminPageProvider(self.env) template_name, content_type = provider.process_admin_request( req, 'bitten', 'configs', '' ) + self.assertEqual('bitten_admin_configs.cs', template_name) self.assertEqual(None, content_type) self.assertEqual([], data['admin']['configs']) + def test_process_view_configs(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + BuildConfig(self.env, name='bar', label='Bar', path='branches/bar', + min_rev='123', max_rev='456').insert() + + data = {} + req = Mock(method='GET', hdf=data, href=Href('/'), + perm=PermissionCache(self.env, 'joe')) + + provider = BuildConfigurationsAdminPageProvider(self.env) + template_name, content_type = provider.process_admin_request( + req, 'bitten', 'configs', '' + ) + + self.assertEqual('bitten_admin_configs.cs', template_name) + self.assertEqual(None, content_type) + configs = data['admin']['configs'] + self.assertEqual(2, len(configs)) + self.assertEqual({ + 'name': 'bar', 'href': '/admin/bitten/configs/bar', + 'label': 'Bar', 'min_rev': '123', 'max_rev': '456', + 'path': 'branches/bar', 'active': False + }, configs[0]) + self.assertEqual({ + 'name': 'foo', 'href': '/admin/bitten/configs/foo', + 'label': 'Foo', 'min_rev': None, 'max_rev': None, + 'path': 'branches/foo', 'active': True + }, configs[1]) + + def test_process_view_config(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + TargetPlatform(self.env, config='foo', name='any').insert() + + data = {} + req = Mock(method='GET', hdf=data, href=Href('/'), + perm=PermissionCache(self.env, 'joe')) + + provider = BuildConfigurationsAdminPageProvider(self.env) + template_name, content_type = provider.process_admin_request( + req, 'bitten', 'configs', 'foo' + ) + + self.assertEqual('bitten_admin_configs.cs', template_name) + self.assertEqual(None, content_type) + config = data['admin']['config'] + self.assertEqual({ + 'name': 'foo', 'label': 'Foo', 'description': '', 'recipe': '', + 'path': 'branches/foo', 'min_rev': None, 'max_rev': None, + 'active': True, 'platforms': [{ + 'href': '/admin/bitten/configs/foo/1', + 'name': 'any', + 'id': 1 + }] + }, config) + + def test_process_add_config(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + + redirected_to = [] + def redirect(url): + redirected_to.append(url) + raise RequestDone + req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + abs_href=Href('http://example.org/'), redirect=redirect, + args={'add': '', 'name': 'bar', 'label': 'Bar'}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected RequestDone') + + except RequestDone: + self.assertEqual('http://example.org/admin/bitten/configs/bar', + redirected_to[0]) + config = BuildConfig.fetch(self.env, name='bar') + self.assertEqual('Bar', config.label) + + def test_process_add_config_no_perms(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + PermissionSystem(self.env).revoke_permission('joe', 'BUILD_CREATE') + + req = Mock(method='POST', + perm=PermissionCache(self.env, 'joe'), + args={'add': '', 'name': 'bar', 'label': 'Bar'}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected PermissionError') + + except PermissionError: + pass + + def test_process_remove_config(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + BuildConfig(self.env, name='bar', label='Bar', path='branches/bar', + min_rev='123', max_rev='456').insert() + + redirected_to = [] + def redirect(url): + redirected_to.append(url) + raise RequestDone + req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + abs_href=Href('http://example.org/'), redirect=redirect, + args={'remove': '', 'sel': 'bar'}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected RequestDone') + + except RequestDone: + self.assertEqual('http://example.org/admin/bitten/configs', + redirected_to[0]) + assert not BuildConfig.fetch(self.env, name='bar') + + def test_process_remove_config_no_selection(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + + req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + args={'remove': ''}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected TracError') + + except TracError, e: + self.assertEqual('No configuration selected', e.message) + + def test_process_remove_config_bad_selection(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + + req = Mock(method='POST', perm=PermissionCache(self.env, 'joe'), + args={'remove': '', 'sel': 'baz'}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected TracError') + + except TracError, e: + self.assertEqual("Configuration 'baz' not found", e.message) + + def test_process_remove_config_no_perms(self): + BuildConfig(self.env, name='foo', label='Foo', path='branches/foo', + active=True).insert() + PermissionSystem(self.env).revoke_permission('joe', 'BUILD_DELETE') + + req = Mock(method='POST', + perm=PermissionCache(self.env, 'joe'), + args={'remove': '', 'sel': 'bar'}) + + provider = BuildConfigurationsAdminPageProvider(self.env) + try: + provider.process_admin_request(req, 'bitten', 'configs', '') + self.fail('Expected PermissionError') + + except PermissionError: + pass + def suite(): suite = unittest.TestSuite()