Merge lp:~abentley/bzr-builder/usage into lp:bzr-builder
- usage
- Merge into trunk
Proposed by
Aaron Bentley
Status: | Merged |
---|---|
Merged at revision: | 110 |
Proposed branch: | lp:~abentley/bzr-builder/usage |
Merge into: | lp:bzr-builder |
Diff against target: |
386 lines (+125/-64) 2 files modified
recipe.py (+58/-25) tests/test_recipe.py (+67/-39) |
To merge this branch: | bzr merge lp:~abentley/bzr-builder/usage |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Westby | Approve | ||
Martin Pool (community) | Approve | ||
Review via email: mp+41230@code.launchpad.net |
Commit message
Provide USAGE in parse errors.
Description of the change
This branch adds usage info to bzr-builder's instruction parsing errors
to make it easier to fix incorrect syntax.
To post a comment you must log in.
Revision history for this message
James Westby (james-w) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'recipe.py' | |||
2 | --- recipe.py 2010-10-27 18:22:03 +0000 | |||
3 | +++ recipe.py 2010-11-18 21:01:47 +0000 | |||
4 | @@ -16,6 +16,7 @@ | |||
5 | 16 | import os | 16 | import os |
6 | 17 | import signal | 17 | import signal |
7 | 18 | import subprocess | 18 | import subprocess |
8 | 19 | from textwrap import dedent | ||
9 | 19 | 20 | ||
10 | 20 | from bzrlib import ( | 21 | from bzrlib import ( |
11 | 21 | branch, | 22 | branch, |
12 | @@ -55,6 +56,10 @@ | |||
13 | 55 | NEST_PART_INSTRUCTION = "nest-part" | 56 | NEST_PART_INSTRUCTION = "nest-part" |
14 | 56 | NEST_INSTRUCTION = "nest" | 57 | NEST_INSTRUCTION = "nest" |
15 | 57 | RUN_INSTRUCTION = "run" | 58 | RUN_INSTRUCTION = "run" |
16 | 59 | USAGE = { | ||
17 | 60 | 'merge': 'merge NAME BRANCH [REVISION]', | ||
18 | 61 | 'nest': 'nest NAME BRANCH TARGET-DIR [REVISION]', | ||
19 | 62 | 'nest-part': 'NAME BRANCH SUBDIR [TARGET-DIR [REVISION]]',} | ||
20 | 58 | 63 | ||
21 | 59 | SAFE_INSTRUCTIONS = [ | 64 | SAFE_INSTRUCTIONS = [ |
22 | 60 | MERGE_INSTRUCTION, NEST_PART_INSTRUCTION, NEST_INSTRUCTION] | 65 | MERGE_INSTRUCTION, NEST_PART_INSTRUCTION, NEST_INSTRUCTION] |
23 | @@ -767,6 +772,14 @@ | |||
24 | 767 | problem=problem) | 772 | problem=problem) |
25 | 768 | 773 | ||
26 | 769 | 774 | ||
27 | 775 | class InstructionParseError(RecipeParseError): | ||
28 | 776 | _fmt = RecipeParseError._fmt + "\nUsage: %(usage)s" | ||
29 | 777 | |||
30 | 778 | def __init__(self, filename, line, char, problem, instruction): | ||
31 | 779 | RecipeParseError.__init__(self, filename, line, char, problem) | ||
32 | 780 | self.usage = USAGE[instruction] | ||
33 | 781 | |||
34 | 782 | |||
35 | 770 | class ForbiddenInstructionError(RecipeParseError): | 783 | class ForbiddenInstructionError(RecipeParseError): |
36 | 771 | 784 | ||
37 | 772 | def __init__(self, filename, line, char, problem, instruction_name=None): | 785 | def __init__(self, filename, line, char, problem, instruction_name=None): |
38 | @@ -858,12 +871,12 @@ | |||
39 | 858 | self.new_line() | 871 | self.new_line() |
40 | 859 | active_branches[-1].run_command(command) | 872 | active_branches[-1].run_command(command) |
41 | 860 | else: | 873 | else: |
44 | 861 | branch_id = self.parse_branch_id() | 874 | branch_id = self.parse_branch_id(instruction) |
45 | 862 | url = self.parse_branch_url() | 875 | url = self.parse_branch_url(instruction) |
46 | 863 | if instruction == NEST_INSTRUCTION: | 876 | if instruction == NEST_INSTRUCTION: |
48 | 864 | location = self.parse_branch_location() | 877 | location = self.parse_branch_location(instruction) |
49 | 865 | if instruction == NEST_PART_INSTRUCTION: | 878 | if instruction == NEST_PART_INSTRUCTION: |
51 | 866 | path = self.parse_subpath() | 879 | path = self.parse_subpath(instruction) |
52 | 867 | target_subdir = self.parse_optional_path() | 880 | target_subdir = self.parse_optional_path() |
53 | 868 | if target_subdir == '': | 881 | if target_subdir == '': |
54 | 869 | target_subdir = None | 882 | target_subdir = None |
55 | @@ -872,7 +885,7 @@ | |||
56 | 872 | revspec = self.parse_optional_revspec() | 885 | revspec = self.parse_optional_revspec() |
57 | 873 | else: | 886 | else: |
58 | 874 | revspec = self.parse_optional_revspec() | 887 | revspec = self.parse_optional_revspec() |
60 | 875 | self.new_line() | 888 | self.new_line(instruction) |
61 | 876 | last_branch = RecipeBranch(branch_id, url, revspec=revspec) | 889 | last_branch = RecipeBranch(branch_id, url, revspec=revspec) |
62 | 877 | if instruction == NEST_INSTRUCTION: | 890 | if instruction == NEST_INSTRUCTION: |
63 | 878 | active_branches[-1].nest_branch(location, last_branch) | 891 | active_branches[-1].nest_branch(location, last_branch) |
64 | @@ -927,12 +940,13 @@ | |||
65 | 927 | self.throw_parse_error("Expecting %s, got '%s'" | 940 | self.throw_parse_error("Expecting %s, got '%s'" |
66 | 928 | % (options_str, instruction)) | 941 | % (options_str, instruction)) |
67 | 929 | 942 | ||
70 | 930 | def parse_branch_id(self): | 943 | def parse_branch_id(self, instruction): |
71 | 931 | self.parse_whitespace("the branch id") | 944 | self.parse_whitespace("the branch id", instruction=instruction) |
72 | 932 | branch_id = self.peek_to_whitespace() | 945 | branch_id = self.peek_to_whitespace() |
73 | 933 | if branch_id is None: | 946 | if branch_id is None: |
74 | 934 | self.throw_parse_error("End of line while looking for the " | 947 | self.throw_parse_error("End of line while looking for the " |
76 | 935 | "branch id") | 948 | "branch id", cls=InstructionParseError, |
77 | 949 | instruction=instruction) | ||
78 | 936 | if branch_id in self.seen_nicks: | 950 | if branch_id in self.seen_nicks: |
79 | 937 | self.throw_parse_error("'%s' was already used to identify " | 951 | self.throw_parse_error("'%s' was already used to identify " |
80 | 938 | "a branch." % branch_id) | 952 | "a branch." % branch_id) |
81 | @@ -940,36 +954,39 @@ | |||
82 | 940 | self.seen_nicks.add(branch_id) | 954 | self.seen_nicks.add(branch_id) |
83 | 941 | return branch_id | 955 | return branch_id |
84 | 942 | 956 | ||
88 | 943 | def parse_branch_url(self): | 957 | def parse_branch_url(self, instruction): |
89 | 944 | self.parse_whitespace("the branch url") | 958 | self.parse_whitespace("the branch url", instruction=instruction) |
90 | 945 | branch_url = self.take_to_whitespace("the branch url") | 959 | branch_url = self.take_to_whitespace("the branch url", instruction) |
91 | 946 | return branch_url | 960 | return branch_url |
92 | 947 | 961 | ||
94 | 948 | def parse_branch_location(self): | 962 | def parse_branch_location(self, instruction): |
95 | 949 | # FIXME: Needs a better term | 963 | # FIXME: Needs a better term |
96 | 950 | self.parse_whitespace("the location to nest") | 964 | self.parse_whitespace("the location to nest") |
97 | 951 | location = self.peek_to_whitespace() | 965 | location = self.peek_to_whitespace() |
98 | 952 | if location is None: | 966 | if location is None: |
99 | 953 | self.throw_parse_error("End of line while looking for the " | 967 | self.throw_parse_error("End of line while looking for the " |
101 | 954 | "location to nest") | 968 | "location to nest", cls=InstructionParseError, |
102 | 969 | instruction=instruction) | ||
103 | 955 | norm_location = os.path.normpath(location) | 970 | norm_location = os.path.normpath(location) |
104 | 956 | if norm_location in self.seen_paths: | 971 | if norm_location in self.seen_paths: |
105 | 957 | self.throw_parse_error("The path '%s' is a duplicate of " | 972 | self.throw_parse_error("The path '%s' is a duplicate of " |
106 | 958 | "the one used on line %d." % (location, | 973 | "the one used on line %d." % (location, |
108 | 959 | self.seen_paths[norm_location])) | 974 | self.seen_paths[norm_location]), |
109 | 975 | InstructionParseError, instruction=instruction) | ||
110 | 960 | if os.path.isabs(norm_location): | 976 | if os.path.isabs(norm_location): |
111 | 961 | self.throw_parse_error("Absolute paths are not allowed: %s" | 977 | self.throw_parse_error("Absolute paths are not allowed: %s" |
113 | 962 | % location) | 978 | % location, InstructionParseError, instruction=instruction) |
114 | 963 | if norm_location.startswith(".."): | 979 | if norm_location.startswith(".."): |
115 | 964 | self.throw_parse_error("Paths outside the current directory " | 980 | self.throw_parse_error("Paths outside the current directory " |
117 | 965 | "are not allowed: %s" % location) | 981 | "are not allowed: %s" % location, |
118 | 982 | cls=InstructionParseError, instruction=instruction) | ||
119 | 966 | self.take_chars(len(location)) | 983 | self.take_chars(len(location)) |
120 | 967 | self.seen_paths[norm_location] = self.line_index + 1 | 984 | self.seen_paths[norm_location] = self.line_index + 1 |
121 | 968 | return location | 985 | return location |
122 | 969 | 986 | ||
126 | 970 | def parse_subpath(self): | 987 | def parse_subpath(self, instruction): |
127 | 971 | self.parse_whitespace("the subpath to merge") | 988 | self.parse_whitespace("the subpath to merge", instruction=instruction) |
128 | 972 | location = self.take_to_whitespace("the subpath to merge") | 989 | location = self.take_to_whitespace("the subpath to merge", instruction) |
129 | 973 | return location | 990 | return location |
130 | 974 | 991 | ||
131 | 975 | def parse_revspec(self): | 992 | def parse_revspec(self): |
132 | @@ -1004,13 +1021,18 @@ | |||
133 | 1004 | def throw_eol(self, expected): | 1021 | def throw_eol(self, expected): |
134 | 1005 | self.throw_parse_error("End of line while looking for '%s'" % expected) | 1022 | self.throw_parse_error("End of line while looking for '%s'" % expected) |
135 | 1006 | 1023 | ||
137 | 1007 | def new_line(self): | 1024 | def new_line(self, instruction=None): |
138 | 1008 | # Jump over any whitespace | 1025 | # Jump over any whitespace |
139 | 1009 | self.parse_whitespace(None, require=False) | 1026 | self.parse_whitespace(None, require=False) |
140 | 1010 | remaining = self.peek_to_whitespace() | 1027 | remaining = self.peek_to_whitespace() |
141 | 1011 | if remaining != None: | 1028 | if remaining != None: |
142 | 1029 | kwargs = {} | ||
143 | 1030 | if instruction is not None: | ||
144 | 1031 | kwargs = { | ||
145 | 1032 | 'cls': InstructionParseError, | ||
146 | 1033 | 'instruction': instruction} | ||
147 | 1012 | self.throw_parse_error("Expecting the end of the line, got '%s'" | 1034 | self.throw_parse_error("Expecting the end of the line, got '%s'" |
149 | 1013 | % remaining) | 1035 | % remaining, **kwargs) |
150 | 1014 | self.index = 0 | 1036 | self.index = 0 |
151 | 1015 | self.line_index += 1 | 1037 | self.line_index += 1 |
152 | 1016 | if self.line_index >= len(self.lines): | 1038 | if self.line_index >= len(self.lines): |
153 | @@ -1076,12 +1098,18 @@ | |||
154 | 1076 | return old_indent_level | 1098 | return old_indent_level |
155 | 1077 | return None | 1099 | return None |
156 | 1078 | 1100 | ||
158 | 1079 | def parse_whitespace(self, looking_for, require=True): | 1101 | def parse_whitespace(self, looking_for, require=True, instruction=None): |
159 | 1080 | if require: | 1102 | if require: |
160 | 1081 | actual = self.peek_char() | 1103 | actual = self.peek_char() |
161 | 1082 | if actual is None: | 1104 | if actual is None: |
162 | 1105 | kwargs = {} | ||
163 | 1106 | if instruction is not None: | ||
164 | 1107 | kwargs = { | ||
165 | 1108 | 'cls': InstructionParseError, | ||
166 | 1109 | 'instruction': instruction, | ||
167 | 1110 | } | ||
168 | 1083 | self.throw_parse_error("End of line while looking for " | 1111 | self.throw_parse_error("End of line while looking for " |
170 | 1084 | "%s" % looking_for) | 1112 | "%s" % looking_for, **kwargs) |
171 | 1085 | if actual not in self.whitespace_chars: | 1113 | if actual not in self.whitespace_chars: |
172 | 1086 | self.throw_parse_error("Expecting whitespace before %s, " | 1114 | self.throw_parse_error("Expecting whitespace before %s, " |
173 | 1087 | "got '%s'." % (looking_for, actual)) | 1115 | "got '%s'." % (looking_for, actual)) |
174 | @@ -1128,11 +1156,16 @@ | |||
175 | 1128 | char = self.peek_char(skip=count) | 1156 | char = self.peek_char(skip=count) |
176 | 1129 | return ret | 1157 | return ret |
177 | 1130 | 1158 | ||
179 | 1131 | def take_to_whitespace(self, looking_for): | 1159 | def take_to_whitespace(self, looking_for, instruction=None): |
180 | 1132 | text = self.peek_to_whitespace() | 1160 | text = self.peek_to_whitespace() |
181 | 1133 | if text is None: | 1161 | if text is None: |
182 | 1162 | kwargs = {} | ||
183 | 1163 | if instruction is not None: | ||
184 | 1164 | kwargs = { | ||
185 | 1165 | 'cls': InstructionParseError, | ||
186 | 1166 | 'instruction': instruction} | ||
187 | 1134 | self.throw_parse_error("End of line while looking for %s" | 1167 | self.throw_parse_error("End of line while looking for %s" |
189 | 1135 | % looking_for) | 1168 | % looking_for, **kwargs) |
190 | 1136 | self.take_chars(len(text)) | 1169 | self.take_chars(len(text)) |
191 | 1137 | return text | 1170 | return text |
192 | 1138 | 1171 | ||
193 | 1139 | 1172 | ||
194 | === modified file 'tests/test_recipe.py' | |||
195 | --- tests/test_recipe.py 2010-10-27 18:22:03 +0000 | |||
196 | +++ tests/test_recipe.py 2010-11-18 21:01:47 +0000 | |||
197 | @@ -33,9 +33,11 @@ | |||
198 | 33 | pull_or_branch, | 33 | pull_or_branch, |
199 | 34 | RecipeParser, | 34 | RecipeParser, |
200 | 35 | RecipeBranch, | 35 | RecipeBranch, |
201 | 36 | InstructionParseError, | ||
202 | 36 | RecipeParseError, | 37 | RecipeParseError, |
203 | 37 | resolve_revisions, | 38 | resolve_revisions, |
204 | 38 | SAFE_INSTRUCTIONS, | 39 | SAFE_INSTRUCTIONS, |
205 | 40 | USAGE, | ||
206 | 39 | ) | 41 | ) |
207 | 40 | 42 | ||
208 | 41 | 43 | ||
209 | @@ -59,6 +61,13 @@ | |||
210 | 59 | self.assertEqual("recipe", exc.filename) | 61 | self.assertEqual("recipe", exc.filename) |
211 | 60 | return exc | 62 | return exc |
212 | 61 | 63 | ||
213 | 64 | def assertInstructionParseError(self, line, char, problem, usage, | ||
214 | 65 | callable, *args, **kwargs): | ||
215 | 66 | exc = self.assertParseError(line, char, problem, callable, *args, | ||
216 | 67 | **kwargs) | ||
217 | 68 | self.assertIsInstance(exc, InstructionParseError), | ||
218 | 69 | self.assertEqual(usage, exc.usage) | ||
219 | 70 | |||
220 | 62 | def check_recipe_branch(self, branch, name, url, revspec=None, | 71 | def check_recipe_branch(self, branch, name, url, revspec=None, |
221 | 63 | num_child_branches=0, revid=None): | 72 | num_child_branches=0, revid=None): |
222 | 64 | self.assertEqual(name, branch.name) | 73 | self.assertEqual(name, branch.name) |
223 | @@ -138,53 +147,66 @@ | |||
224 | 138 | self.basic_header + "http://foo.org/\n" + "cat") | 147 | self.basic_header + "http://foo.org/\n" + "cat") |
225 | 139 | 148 | ||
226 | 140 | def test_rejects_merge_no_name(self): | 149 | def test_rejects_merge_no_name(self): |
230 | 141 | self.assertParseError(3, 7, "End of line while looking for " | 150 | self.assertInstructionParseError(3, 7, "End of line while looking for " |
231 | 142 | "the branch id", self.get_recipe, | 151 | "the branch id", "merge NAME BRANCH [REVISION]", |
232 | 143 | self.basic_header_and_branch + "merge ") | 152 | self.get_recipe, self.basic_header_and_branch + "merge ") |
233 | 153 | |||
234 | 154 | def test_rejects_merge_no_name_no_space(self): | ||
235 | 155 | self.assertInstructionParseError(3, 6, "End of line while looking for " | ||
236 | 156 | "the branch id", "merge NAME BRANCH [REVISION]", | ||
237 | 157 | self.get_recipe, self.basic_header_and_branch + "merge") | ||
238 | 144 | 158 | ||
239 | 145 | def test_rejects_merge_no_url(self): | 159 | def test_rejects_merge_no_url(self): |
243 | 146 | self.assertParseError(3, 11, "End of line while looking for " | 160 | self.assertInstructionParseError(3, 11, "End of line while looking for" |
244 | 147 | "the branch url", self.get_recipe, | 161 | " the branch url", "merge NAME BRANCH [REVISION]", |
245 | 148 | self.basic_header_and_branch + "merge foo ") | 162 | self.get_recipe, self.basic_header_and_branch + "merge foo ") |
246 | 149 | 163 | ||
247 | 150 | def test_rejects_text_at_end_of_merge_line(self): | 164 | def test_rejects_text_at_end_of_merge_line(self): |
250 | 151 | self.assertParseError(3, 17, "Expecting the end of the line, " | 165 | self.assertInstructionParseError(3, 17, |
251 | 152 | "got 'bar'", self.get_recipe, | 166 | "Expecting the end of the line, got 'bar'", |
252 | 167 | 'merge NAME BRANCH [REVISION]', self.get_recipe, | ||
253 | 153 | self.basic_header_and_branch + "merge foo url 2 bar") | 168 | self.basic_header_and_branch + "merge foo url 2 bar") |
254 | 154 | 169 | ||
255 | 155 | def test_rejects_nest_part_no_name(self): | 170 | def test_rejects_nest_part_no_name(self): |
258 | 156 | self.assertParseError(3, 11, "End of line while looking for " | 171 | self.assertInstructionParseError(3, 11, "End of line while looking for" |
259 | 157 | "the branch id", self.get_recipe, | 172 | " the branch id", USAGE['nest-part'], self.get_recipe, |
260 | 158 | self.basic_header_and_branch + "nest-part ") | 173 | self.basic_header_and_branch + "nest-part ") |
261 | 159 | 174 | ||
262 | 160 | def test_rejects_nest_part_no_url(self): | 175 | def test_rejects_nest_part_no_url(self): |
265 | 161 | self.assertParseError(3, 15, "End of line while looking for " | 176 | self.assertInstructionParseError(3, 15, "End of line while looking for" |
266 | 162 | "the branch url", self.get_recipe, | 177 | " the branch url", USAGE['nest-part'], self.get_recipe, |
267 | 163 | self.basic_header_and_branch + "nest-part foo ") | 178 | self.basic_header_and_branch + "nest-part foo ") |
268 | 164 | 179 | ||
269 | 165 | def test_rejects_nest_part_no_subpath(self): | 180 | def test_rejects_nest_part_no_subpath(self): |
272 | 166 | self.assertParseError(3, 22, "End of line while looking for " | 181 | self.assertInstructionParseError(3, 22, "End of line while looking for" |
273 | 167 | "the subpath to merge", self.get_recipe, | 182 | " the subpath to merge", USAGE['nest-part'], self.get_recipe, |
274 | 168 | self.basic_header_and_branch + "nest-part foo url:// ") | 183 | self.basic_header_and_branch + "nest-part foo url:// ") |
275 | 169 | 184 | ||
276 | 185 | def test_rejects_nest_part_no_subpath_no_space(self): | ||
277 | 186 | self.assertInstructionParseError(3, 21, "End of line while looking for" | ||
278 | 187 | " the subpath to merge", USAGE['nest-part'], self.get_recipe, | ||
279 | 188 | self.basic_header_and_branch + "nest-part foo url://") | ||
280 | 189 | |||
281 | 170 | def test_rejects_nest_no_name(self): | 190 | def test_rejects_nest_no_name(self): |
284 | 171 | self.assertParseError(3, 6, "End of line while looking for " | 191 | self.assertInstructionParseError(3, 6, "End of line while looking for" |
285 | 172 | "the branch id", self.get_recipe, | 192 | " the branch id", USAGE['nest'], self.get_recipe, |
286 | 173 | self.basic_header_and_branch + "nest ") | 193 | self.basic_header_and_branch + "nest ") |
287 | 174 | 194 | ||
288 | 175 | def test_rejects_nest_no_url(self): | 195 | def test_rejects_nest_no_url(self): |
292 | 176 | self.assertParseError(3, 10, "End of line while looking for " | 196 | self.assertInstructionParseError(3, 10, |
293 | 177 | "the branch url", self.get_recipe, | 197 | "End of line while looking for the branch url", USAGE['nest'], |
294 | 178 | self.basic_header_and_branch + "nest foo ") | 198 | self.get_recipe, self.basic_header_and_branch + "nest foo ") |
295 | 179 | 199 | ||
296 | 180 | def test_rejects_nest_no_location(self): | 200 | def test_rejects_nest_no_location(self): |
300 | 181 | self.assertParseError(3, 14, "End of line while looking for " | 201 | self.assertInstructionParseError(3, 14, |
301 | 182 | "the location to nest", self.get_recipe, | 202 | "End of line while looking for the location to nest", |
302 | 183 | self.basic_header_and_branch + "nest foo url ") | 203 | USAGE['nest'], self.get_recipe, self.basic_header_and_branch + |
303 | 204 | "nest foo url ") | ||
304 | 184 | 205 | ||
305 | 185 | def test_rejects_text_at_end_of_nest_line(self): | 206 | def test_rejects_text_at_end_of_nest_line(self): |
308 | 186 | self.assertParseError(3, 20, "Expecting the end of the line, " | 207 | self.assertInstructionParseError(3, 20, |
309 | 187 | "got 'baz'", self.get_recipe, | 208 | "Expecting the end of the line, got 'baz'", USAGE['nest'], |
310 | 209 | self.get_recipe, | ||
311 | 188 | self.basic_header_and_branch + "nest foo url bar 2 baz") | 210 | self.basic_header_and_branch + "nest foo url bar 2 baz") |
312 | 189 | 211 | ||
313 | 190 | def test_rejects_indent_after_first_branch(self): | 212 | def test_rejects_indent_after_first_branch(self): |
314 | @@ -443,34 +465,40 @@ | |||
315 | 443 | self.assertEqual("run", exc.instruction_name) | 465 | self.assertEqual("run", exc.instruction_name) |
316 | 444 | 466 | ||
317 | 445 | def test_error_on_duplicate_path(self): | 467 | def test_error_on_duplicate_path(self): |
320 | 446 | exc = self.assertParseError(3, 15, "The path '.' is a duplicate " | 468 | exc = self.assertInstructionParseError(3, 15, |
321 | 447 | "of the one used on line 1.", self.get_recipe, | 469 | "The path '.' is a duplicate of the one used on line 1.", |
322 | 470 | USAGE['nest'], self.get_recipe, | ||
323 | 448 | self.basic_header_and_branch + "nest nest url .\n") | 471 | self.basic_header_and_branch + "nest nest url .\n") |
324 | 449 | 472 | ||
325 | 450 | def test_error_on_duplicate_path_with_another_nest(self): | 473 | def test_error_on_duplicate_path_with_another_nest(self): |
328 | 451 | exc = self.assertParseError(4, 16, "The path 'foo' is a duplicate " | 474 | exc = self.assertInstructionParseError(4, 16, |
329 | 452 | "of the one used on line 3.", self.get_recipe, | 475 | "The path 'foo' is a duplicate of the one used on line 3.", |
330 | 476 | USAGE['nest'], self.get_recipe, | ||
331 | 453 | self.basic_header_and_branch + "nest nest url foo\n" | 477 | self.basic_header_and_branch + "nest nest url foo\n" |
332 | 454 | + "nest nest2 url foo\n") | 478 | + "nest nest2 url foo\n") |
333 | 455 | 479 | ||
334 | 456 | def test_duplicate_path_check_uses_normpath(self): | 480 | def test_duplicate_path_check_uses_normpath(self): |
337 | 457 | exc = self.assertParseError(3, 15, "The path 'foo/..' is a duplicate " | 481 | exc = self.assertInstructionParseError(3, 15, |
338 | 458 | "of the one used on line 1.", self.get_recipe, | 482 | "The path 'foo/..' is a duplicate of the one used on line 1.", |
339 | 483 | USAGE['nest'], self.get_recipe, | ||
340 | 459 | self.basic_header_and_branch + "nest nest url foo/..\n") | 484 | self.basic_header_and_branch + "nest nest url foo/..\n") |
341 | 460 | 485 | ||
342 | 461 | def test_error_absolute_path(self): | 486 | def test_error_absolute_path(self): |
345 | 462 | exc = self.assertParseError(3, 15, "Absolute paths are not allowed: " | 487 | exc = self.assertInstructionParseError(3, 15, |
346 | 463 | "/etc/passwd", self.get_recipe, self.basic_header_and_branch | 488 | "Absolute paths are not allowed: /etc/passwd", USAGE['nest'], |
347 | 489 | self.get_recipe, self.basic_header_and_branch | ||
348 | 464 | + "nest nest url /etc/passwd\n") | 490 | + "nest nest url /etc/passwd\n") |
349 | 465 | 491 | ||
350 | 466 | def test_error_simple_parent_dir(self): | 492 | def test_error_simple_parent_dir(self): |
353 | 467 | exc = self.assertParseError(3, 15, "Paths outside the current " | 493 | exc = self.assertInstructionParseError(3, 15, |
354 | 468 | "directory are not allowed: ../foo", self.get_recipe, | 494 | "Paths outside the current directory are not allowed: ../foo", |
355 | 495 | USAGE['nest'], self.get_recipe, | ||
356 | 469 | self.basic_header_and_branch + "nest nest url ../foo\n") | 496 | self.basic_header_and_branch + "nest nest url ../foo\n") |
357 | 470 | 497 | ||
358 | 471 | def test_error_complex_parent_dir(self): | 498 | def test_error_complex_parent_dir(self): |
361 | 472 | exc = self.assertParseError(3, 15, "Paths outside the current " | 499 | exc = self.assertInstructionParseError(3, 15, |
362 | 473 | "directory are not allowed: ./foo/../..", self.get_recipe, | 500 | "Paths outside the current directory are not allowed:" |
363 | 501 | " ./foo/../..", USAGE['nest'], self.get_recipe, | ||
364 | 474 | self.basic_header_and_branch + "nest nest url ./foo/../..\n") | 502 | self.basic_header_and_branch + "nest nest url ./foo/../..\n") |
365 | 475 | 503 | ||
366 | 476 | 504 | ||
367 | @@ -1064,8 +1092,8 @@ | |||
368 | 1064 | nested_branch1 = RecipeBranch("nested1", "nested1_url", | 1092 | nested_branch1 = RecipeBranch("nested1", "nested1_url", |
369 | 1065 | revspec="tag:foo") | 1093 | revspec="tag:foo") |
370 | 1066 | base_branch.nest_branch(".", nested_branch1) | 1094 | base_branch.nest_branch(".", nested_branch1) |
373 | 1067 | self.assertRaises(RecipeParseError, base_branch.get_recipe_text, | 1095 | self.assertRaises(InstructionParseError, |
374 | 1068 | validate=True) | 1096 | base_branch.get_recipe_text, validate=True) |
375 | 1069 | 1097 | ||
376 | 1070 | def test_str_validates(self): | 1098 | def test_str_validates(self): |
377 | 1071 | base_branch = BaseRecipeBranch("base_url", "1", 0.1) | 1099 | base_branch = BaseRecipeBranch("base_url", "1", 0.1) |
378 | @@ -1073,7 +1101,7 @@ | |||
379 | 1073 | nested_branch1 = RecipeBranch("nested1", "nested1_url", | 1101 | nested_branch1 = RecipeBranch("nested1", "nested1_url", |
380 | 1074 | revspec="tag:foo") | 1102 | revspec="tag:foo") |
381 | 1075 | base_branch.nest_branch(".", nested_branch1) | 1103 | base_branch.nest_branch(".", nested_branch1) |
383 | 1076 | self.assertRaises(RecipeParseError, str, base_branch) | 1104 | self.assertRaises(InstructionParseError, str, base_branch) |
384 | 1077 | 1105 | ||
385 | 1078 | def test_with_nest_part(self): | 1106 | def test_with_nest_part(self): |
386 | 1079 | base_branch = BaseRecipeBranch("base_url", "1", 0.1) | 1107 | base_branch = BaseRecipeBranch("base_url", "1", 0.1) |
That looks reasonable to me, though James might like to review it too.
Needing to pass around the 'instruction' quite so much suggests perhaps splitting out a class that knows the instruction it's parsing?