Launchpad itself

Merge lp:~rockstar/launchpad/create-recipe-error-messages into lp:launchpad

  1. create-recipe-error-messages
  2. Merge into devel
Proposed by Paul Hummer
Status: Merged
Approved by: Paul Hummer
Approved revision: no longer in the source branch.
Merged at revision: 11154
Proposed branch: lp:~rockstar/launchpad/create-recipe-error-messages
Merge into: lp:launchpad
Diff against target: 46 lines (+17/-4)
2 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+3/-2)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+14/-2)
To merge this branch: bzr merge lp:~rockstar/launchpad/create-recipe-error-messages
Reviewer Review Type Date Requested Status
Jonathan Lange (community) code Approve
Review via email: mp+30253@code.launchpad.net

Description of the change

This branch is a very small fix to pass the bzr-builder exception on to the user when creating a source package recipe. It's bug #604725. I had to change the test a bit so that I could actually construct a real recipe. In fact, since I was sitting next to thumper at the time he found this bug (we knew it existed, but hadn't encountered it yet), I made the test very specific to his exact use case.

I _think_ this test will need to be changed when we update bzr-builder next, because James made some other changes to the exceptions in bzr-builder recently.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

On Mon, Jul 19, 2010 at 12:41 PM, Paul Hummer <email address hidden> wrote:
> Paul Hummer has proposed merging lp:~rockstar/launchpad/create-recipe-error-messages into lp:launchpad/devel.
>
> Requested reviews:
>  Jonathan Lange (jml): code
> Related bugs:
>  #604725 Parse errors creating recipe don't show the exception message
>  https://bugs.launchpad.net/bugs/604725
>
>
> This branch is a very small fix to pass the bzr-builder exception on to the user when creating a source package recipe.  It's bug #604725.  I had to change the test a bit so that I could actually construct a real recipe.  In fact, since I was sitting next to thumper at the time he found this bug (we knew it existed, but hadn't encountered it yet), I made the test very specific to his exact use case.

Hey Paul,

This branch looks great. Only one question:

...
> === modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
> --- lib/lp/code/browser/tests/test_sourcepackagerecipe.py       2010-07-13 10:28:38 +0000
> +++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py       2010-07-19 10:41:11 +0000
> @@ -192,10 +192,26 @@
>     def test_create_recipe_bad_text(self):
>         # If a user tries to create source package recipe with bad text, they
>         # should get an error.
> -        browser = self.createRecipe('Foo bar baz')
> +        product = self.factory.makeProduct(
> +            name='ratatouille', displayname='Ratatouille')
> +        branch = self.factory.makeBranch(
> +            owner=self.chef, product=product, name='veggies')
> +        package_branch = self.factory.makeBranch(
> +            owner=self.chef, product=product, name='packaging')
> +

Why is the common product, branch name and branch owner relevant?

Why not:
    branch = self.factory.makeBranch()
    package_branch = self.factory.makeBranch()
?

jml

Revision history for this message
Jonathan Lange (jml) wrote :

See above.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2010-07-13 10:28:38 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-07-19 10:55:04 +0000
@@ -294,10 +294,11 @@
294 try:294 try:
295 parser = RecipeParser(data['recipe_text'])295 parser = RecipeParser(data['recipe_text'])
296 parser.parse()296 parser.parse()
297 except RecipeParseError:297 except RecipeParseError, error:
298 self.setFieldError(298 self.setFieldError(
299 'recipe_text',299 'recipe_text',
300 'The recipe text is not a valid bzr-builder recipe.')300 'The recipe text is not a valid bzr-builder recipe. '
301 '%(error)s' % {'error': error.problem})
301302
302303
303class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):304class SourcePackageRecipeAddView(RecipeTextValidatorMixin, LaunchpadFormView):
304305
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-07-13 10:28:38 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-07-19 10:55:04 +0000
@@ -192,10 +192,22 @@
192 def test_create_recipe_bad_text(self):192 def test_create_recipe_bad_text(self):
193 # If a user tries to create source package recipe with bad text, they193 # If a user tries to create source package recipe with bad text, they
194 # should get an error.194 # should get an error.
195 browser = self.createRecipe('Foo bar baz')195 branch = self.factory.makeBranch(name='veggies')
196 package_branch = self.factory.makeBranch(name='packaging')
197
198 browser = self.createRecipe(
199 dedent('''
200 # bzr-builder format 0.2 deb-version 0+{revno}
201 %(branch)s
202 merge %(package_branch)s
203 ''' % {
204 'branch': branch.bzr_identity,
205 'package_branch': package_branch.bzr_identity,}),
206 branch=branch)
196 self.assertEqual(207 self.assertEqual(
197 get_message_text(browser, 2),208 get_message_text(browser, 2),
198 'The recipe text is not a valid bzr-builder recipe.')209 "The recipe text is not a valid bzr-builder recipe. "
210 "End of line while looking for '#'")
199211
200 def test_create_recipe_no_distroseries(self):212 def test_create_recipe_no_distroseries(self):
201 browser = self.getViewBrowser(self.makeBranch(), '+new-recipe')213 browser = self.getViewBrowser(self.makeBranch(), '+new-recipe')