Code review comment for lp:~rockstar/launchpad/create-recipe-error-messages

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