Mercurial > bitten > bitten-test
changeset 247:b28285d3ceec
Add validation for build configurations, and in particular for build recipes. Closes #48.
| author | cmlenz |
|---|---|
| date | Tue, 04 Oct 2005 10:12:08 +0000 |
| parents | ebfe660f7cb1 |
| children | 15d8d8c809f0 |
| files | bitten/recipe.py bitten/tests/recipe.py bitten/trac_ext/tests/web_ui.py bitten/trac_ext/web_ui.py |
| diffstat | 4 files changed, 198 insertions(+), 26 deletions(-) [+] |
line wrap: on
line diff
--- a/bitten/recipe.py +++ b/bitten/recipe.py @@ -11,6 +11,10 @@ import logging import os import re +try: + set +except NameError: + from sets import Set as set from pkg_resources import WorkingSet from bitten.build import BuildError @@ -124,9 +128,37 @@ assert isinstance(xml, xmlio.ParsedElement) self.ctxt = Context(basedir, config) self._root = xml - self.description = self._root.attr.get('description') def __iter__(self): """Provide an iterator over the individual steps of the recipe.""" for child in self._root.children('step'): yield Step(child) + + def validate(self): + if self._root.name != 'build': + raise InvalidRecipeError, 'Root element must be <build>' + steps = list(self._root.children()) + if not steps: + raise InvalidRecipeError, 'Recipe defines no build steps' + + step_ids = set() + for step in steps: + if step.name != 'step': + raise InvalidRecipeError, 'Only <step> elements allowed ' \ + 'at top level of recipe' + if not step.attr.get('id'): + raise InvalidRecipeError, 'Steps must have an "id" attribute' + + if step.attr['id'] in step_ids: + raise InvalidRecipeError, 'Duplicate step ID "%s"' \ + % step.attr['id'] + step_ids.add(step.attr['id']) + + cmds = list(step.children()) + if not cmds: + raise InvalidRecipeError, 'Step "%s" has no recipe commands' \ + % step.attr['id'] + for cmd in cmds: + if len(list(cmd.children())): + raise InvalidRecipeError, 'Recipe command <%s> has ' \ + 'nested content' % cmd.name
--- a/bitten/tests/recipe.py +++ b/bitten/tests/recipe.py @@ -12,7 +12,7 @@ import tempfile import unittest -from bitten.recipe import Recipe +from bitten.recipe import Recipe, InvalidRecipeError from bitten.util import xmlio @@ -25,9 +25,8 @@ shutil.rmtree(self.basedir) def test_empty_recipe(self): - xml = xmlio.parse('<build description="test"/>') + xml = xmlio.parse('<build/>') recipe = Recipe(xml, basedir=self.basedir) - self.assertEqual('test', recipe.description) self.assertEqual(self.basedir, recipe.ctxt.basedir) steps = list(recipe) self.assertEqual(0, len(steps)) @@ -43,6 +42,63 @@ self.assertEqual('Bar', steps[0].description) self.assertEqual('fail', steps[0].onerror) + def test_validate_bad_root(self): + xml = xmlio.parse('<foo></foo>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_no_steps(self): + xml = xmlio.parse('<build></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_child_not_step(self): + xml = xmlio.parse('<build><foo/></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_child_not_step(self): + xml = xmlio.parse('<build><foo/></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_step_without_id(self): + xml = xmlio.parse('<build><step><cmd/></step></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_step_with_empty_id(self): + xml = xmlio.parse('<build><step id=""><cmd/></step></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_step_without_commands(self): + xml = xmlio.parse('<build><step id="test"/></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_step_with_command_children(self): + xml = xmlio.parse('<build><step id="test">' + '<somecmd><child1/><child2/></somecmd>' + '</step></build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_step_with_duplicate_id(self): + xml = xmlio.parse('<build>' + '<step id="test"><somecmd></somecmd></step>' + '<step id="test"><othercmd></othercmd></step>' + '</build>') + recipe = Recipe(xml, basedir=self.basedir) + self.assertRaises(InvalidRecipeError, recipe.validate) + + def test_validate_successful(self): + xml = xmlio.parse('<build>' + '<step id="foo"><somecmd></somecmd></step>' + '<step id="bar"><othercmd></othercmd></step>' + '</build>') + recipe = Recipe(xml, basedir=self.basedir) + recipe.validate() def suite(): return unittest.makeSuite(RecipeTestCase, 'test')
--- a/bitten/trac_ext/tests/web_ui.py +++ b/bitten/trac_ext/tests/web_ui.py @@ -11,6 +11,7 @@ import tempfile import unittest +from trac.core import TracError from trac.perm import PermissionCache, PermissionSystem from trac.test import EnvironmentStub, Mock from trac.versioncontrol import Repository @@ -39,9 +40,13 @@ 'DefaultPermissionStore') # Hook up a dummy repository - self.repos = Mock(get_node=lambda path: Mock(get_history=lambda: []), - sync=lambda: None) - self.env.get_repository = lambda x: self.repos + self.repos = Mock( + get_node=lambda path, rev=None: Mock(get_history=lambda: [], + isdir=True), + normalize_path=lambda path: path, + sync=lambda: None + ) + self.env.get_repository = lambda authname=None: self.repos def tearDown(self): shutil.rmtree(self.env.path) @@ -169,6 +174,56 @@ self.assertEqual('test/trunk', config.path) self.assertEqual('Bla bla', config.description) + def test_new_config_submit_without_name(self): + PermissionSystem(self.env).grant_permission('joe', 'BUILD_ADMIN') + req = Mock(Request, method='POST', cgi_location='', path_info='/build', + hdf=HDFWrapper(), perm=PermissionCache(self.env, 'joe'), + args={'action': 'new', 'name': '', 'path': 'test/trunk', + 'label': 'Test', 'description': 'Bla bla'}) + + module = BuildConfigController(self.env) + assert module.match_request(req) + self.assertRaises(TracError, module.process_request, req) + + def test_new_config_submit_invalid_path(self): + PermissionSystem(self.env).grant_permission('joe', 'BUILD_ADMIN') + req = Mock(Request, method='POST', cgi_location='', path_info='/build', + hdf=HDFWrapper(), perm=PermissionCache(self.env, 'joe'), + args={'action': 'new', 'name': 'test', 'path': 'test/trunk', + 'label': 'Test', 'description': 'Bla bla'}) + + def get_node(path, rev=None): + raise TracError, 'No such node' + self.repos = Mock(get_node=get_node) + + module = BuildConfigController(self.env) + assert module.match_request(req) + self.assertRaises(TracError, module.process_request, req) + + def test_new_config_submit_with_non_wellformed_recipe(self): + PermissionSystem(self.env).grant_permission('joe', 'BUILD_ADMIN') + req = Mock(Request, method='POST', cgi_location='', path_info='/build', + hdf=HDFWrapper(), perm=PermissionCache(self.env, 'joe'), + args={'action': 'new', 'name': 'test', 'path': 'test/trunk', + 'label': 'Test', 'description': 'Bla bla', + 'recipe': '<build><step>'}) + + module = BuildConfigController(self.env) + assert module.match_request(req) + self.assertRaises(TracError, module.process_request, req) + + def test_new_config_submit_with_invalid_recipe(self): + PermissionSystem(self.env).grant_permission('joe', 'BUILD_ADMIN') + req = Mock(Request, method='POST', cgi_location='', path_info='/build', + hdf=HDFWrapper(), perm=PermissionCache(self.env, 'joe'), + args={'action': 'new', 'name': 'test', 'path': 'test/trunk', + 'label': 'Test', 'description': 'Bla bla', + 'recipe': '<build><step/></build>'}) + + module = BuildConfigController(self.env) + assert module.match_request(req) + self.assertRaises(TracError, module.process_request, req) + def test_new_config_cancel(self): PermissionSystem(self.env).grant_permission('joe', 'BUILD_ADMIN') redirected_to = []
--- a/bitten/trac_ext/web_ui.py +++ b/bitten/trac_ext/web_ui.py @@ -21,7 +21,9 @@ from bitten.model import BuildConfig, TargetPlatform, Build, BuildStep, \ BuildLog, Report from bitten.queue import collect_changes +from bitten.recipe import Recipe, InvalidRecipeError from bitten.trac_ext.api import ILogFormatter, IReportSummarizer +from bitten.util import xmlio _status_label = {Build.IN_PROGRESS: 'in progress', Build.SUCCESS: 'completed', @@ -156,17 +158,12 @@ config_name = req.args.get('name') - assert not BuildConfig.fetch(self.env, config_name), \ - 'A build configuration with the name "%s" already exists' \ - % config_name + if BuildConfig.fetch(self.env, config_name): + raise TracError('A build configuration with the name "%s" already ' + 'exists' % config_name, 'Duplicate name') - config = BuildConfig(self.env, name=config_name, - path=req.args.get('path', ''), - recipe=req.args.get('recipe', ''), - min_rev=req.args.get('min_rev', ''), - max_rev=req.args.get('max_rev', ''), - label=req.args.get('label', ''), - description=req.args.get('description')) + config = BuildConfig(self.env) + self._process_config(req, config) config.insert() req.redirect(self.env.href.build(config.name)) @@ -197,7 +194,10 @@ req.redirect(self.env.href.build(config_name)) config = BuildConfig.fetch(self.env, config_name) - assert config, 'Build configuration "%s" does not exist' % config_name + if not config: + # FIXME: 404 + raise TracError('Build configuration "%s" does not exist' + % config_name, 'Object not found') if 'activate' in req.args: config.active = True @@ -206,18 +206,47 @@ config.active = False else: - # TODO: Validate recipe, repository path, etc - config.name = req.args.get('name') - config.path = req.args.get('path', '') - config.recipe = req.args.get('recipe', '') - 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', '') + self._process_config(req, config) config.update() req.redirect(self.env.href.build(config.name)) + def _process_config(self, req, config): + if not req.args.get('name'): + raise TracError('Missing required field "name"', 'Missing 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 = req.args.get('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', '') + def _do_create_platform(self, req, config_name): """Create a new target platform.""" req.perm.assert_permission('BUILD_MODIFY')