Merge lp:~rockstar/launchpad/recipe-too-new into lp:launchpad

Proposed by Paul Hummer on 2010-09-22
Status: Merged
Approved by: Aaron Bentley on 2010-09-22
Approved revision: no longer in the source branch.
Merged at revision: 11612
Proposed branch: lp:~rockstar/launchpad/recipe-too-new
Merge into: lp:launchpad
Diff against target: 123 lines (+45/-5)
4 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+6/-0)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+19/-0)
lib/lp/code/model/tests/test_sourcepackagerecipe.py (+8/-5)
lib/lp/code/tests/helpers.py (+12/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/recipe-too-new
Reviewer Review Type Date Requested Status
Aaron Bentley (community) 2010-09-22 Approve on 2010-09-22
Review via email: mp+36379@code.launchpad.net

Description of the change

This branch fixes an issue where users try to use a recipe format that we don't yet support. In this case, we have bzr-builder in source code, but Aaron can't land the support until the buildds have the newer bzr-builder.

I talked this over with Aaron, and we decided to monkey patch the actual NEWEST_VERSION in bzr-builder to a recipe format version we're never going to have (in this case, it's also the frequency of my local HAM radio repeater). This way, we won't have erroneous test failures when we actually add support for 0.3 formatted recipes.

While talking this over with Aaron, I found another potential place where this may occur in tests, and fixed that test as well.

To post a comment you must log in.
Aaron Bentley (abentley) wrote :

The monkey patch needs to use a try/finally block to ensure the new value doesn't leak, or better yet, use a context manager for the purpose.

Other than that, looks fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
2--- lib/lp/code/browser/sourcepackagerecipe.py 2010-09-02 14:28:57 +0000
3+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-09-22 21:13:48 +0000
4@@ -60,6 +60,7 @@
5 BuildAlreadyPending,
6 NoSuchBranch,
7 PrivateBranchRecipe,
8+ TooNewRecipeFormat
9 )
10 from lp.code.interfaces.sourcepackagerecipe import (
11 ISourcePackageRecipe,
12@@ -333,6 +334,11 @@
13 data['recipe_text'], data['description'], data['distros'],
14 data['daily_build_archive'], data['build_daily'])
15 Store.of(source_package_recipe).flush()
16+ except TooNewRecipeFormat:
17+ self.setFieldError(
18+ 'recipe_text',
19+ 'The recipe format version specified is not available.')
20+ return
21 except ForbiddenInstructionError:
22 # XXX: bug=592513 We shouldn't be hardcoding "run" here.
23 self.setFieldError(
24
25=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
26--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-09-20 19:12:35 +0000
27+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-09-22 21:13:48 +0000
28@@ -40,6 +40,7 @@
29 SourcePackageRecipeBuildView,
30 )
31 from lp.code.interfaces.sourcepackagerecipe import MINIMAL_RECIPE_TEXT
32+from lp.code.tests.helpers import recipe_parser_newest_version
33 from lp.registry.interfaces.pocket import PackagePublishingPocket
34 from lp.soyuz.model.processor import ProcessorFamily
35 from lp.testing import (
36@@ -304,6 +305,24 @@
37 self.assertEqual(
38 get_message_text(browser, 2), 'foo is not a branch on Launchpad.')
39
40+ def test_create_recipe_format_too_new(self):
41+ # If the recipe's format version is too new, we should notify the
42+ # user.
43+ product = self.factory.makeProduct(
44+ name='ratatouille', displayname='Ratatouille')
45+ branch = self.factory.makeBranch(
46+ owner=self.chef, product=product, name='veggies')
47+
48+ with recipe_parser_newest_version(145.115):
49+ recipe = dedent(u'''\
50+ # bzr-builder format 145.115 deb-version 0+{revno}
51+ %s
52+ ''') % branch.bzr_identity
53+ browser = self.createRecipe(recipe, branch)
54+ self.assertEqual(
55+ get_message_text(browser, 2),
56+ 'The recipe format version specified is not available.')
57+
58 def test_create_dupe_recipe(self):
59 # You shouldn't be able to create a duplicate recipe owned by the same
60 # person with the same name.
61
62=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
63--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-09-21 19:02:44 +0000
64+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-09-22 21:13:48 +0000
65@@ -53,6 +53,7 @@
66 SourcePackageRecipe,
67 )
68 from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuildJob
69+from lp.code.tests.helpers import recipe_parser_newest_version
70 from lp.registry.interfaces.pocket import PackagePublishingPocket
71 from lp.services.job.interfaces.job import (
72 IJob,
73@@ -246,11 +247,13 @@
74 old_branches, list(sp_recipe.getReferencedBranches()))
75
76 def test_reject_newer_formats(self):
77- builder_recipe = self.factory.makeRecipe()
78- builder_recipe.format = 0.3
79- self.assertRaises(
80- TooNewRecipeFormat,
81- self.factory.makeSourcePackageRecipe, recipe=str(builder_recipe))
82+ with recipe_parser_newest_version(145.115):
83+ builder_recipe = self.factory.makeRecipe()
84+ builder_recipe.format = 145.115
85+ self.assertRaises(
86+ TooNewRecipeFormat,
87+ self.factory.makeSourcePackageRecipe,
88+ recipe=str(builder_recipe))
89
90 def test_requestBuild(self):
91 recipe = self.factory.makeSourcePackageRecipe()
92
93=== modified file 'lib/lp/code/tests/helpers.py'
94--- lib/lp/code/tests/helpers.py 2010-09-15 20:41:46 +0000
95+++ lib/lp/code/tests/helpers.py 2010-09-22 21:13:48 +0000
96@@ -14,11 +14,13 @@
97 ]
98
99
100+from contextlib import contextmanager
101 from datetime import timedelta
102 from difflib import unified_diff
103 from itertools import count
104 import transaction
105
106+from bzrlib.plugins.builder.recipe import RecipeParser
107 from zope.component import getUtility
108 from zope.security.proxy import (
109 isinstance as zisinstance,
110@@ -301,3 +303,13 @@
111 make_project_branch_with_revisions(
112 factory, gen, project, revision_count=num_commits)
113 transaction.commit()
114+
115+
116+@contextmanager
117+def recipe_parser_newest_version(version):
118+ old_version = RecipeParser.NEWEST_VERSION
119+ RecipeParser.NEWEST_VERSION = version
120+ try:
121+ yield
122+ finally:
123+ RecipeParser.NEWEST_VERSION = old_version