Merge lp:~ack/landscape-client/refactor-prompt-functions into lp:~landscape/landscape-client/trunk
- refactor-prompt-functions
- Merge into trunk
Proposed by
Alberto Donato
Status: | Merged |
---|---|
Approved by: | Alberto Donato |
Approved revision: | 943 |
Merged at revision: | 938 |
Proposed branch: | lp:~ack/landscape-client/refactor-prompt-functions |
Merge into: | lp:~landscape/landscape-client/trunk |
Diff against target: |
807 lines (+190/-168) 2 files modified
landscape/configuration.py (+52/-47) landscape/tests/test_configuration.py (+138/-121) |
To merge this branch: | bzr merge lp:~ack/landscape-client/refactor-prompt-functions |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Bogdana Vereha (community) | Approve | ||
Free Ekanayaka (community) | Approve | ||
🤖 Landscape Builder | test results | Approve | |
Review via email: mp+318905@code.launchpad.net |
Commit message
Purely refactoring branch, it moves prompt_yes_no and show_help functions out of the script class, and reuse prompt_yes_no instead of duplicated code.
Description of the change
Purely refactoring branch, it moves prompt_yes_no and show_help functions out of the script class, and reuse prompt_yes_no instead of duplicated code.
Diff is big because of mock changes in tests.
Testing instructions:
run unittests
To post a comment you must log in.
Revision history for this message
🤖 Landscape Builder (landscape-builder) : | # |
review:
Abstain
(executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote : | # |
review:
Approve
(test results)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'landscape/configuration.py' |
2 | --- landscape/configuration.py 2017-02-24 09:38:26 +0000 |
3 | +++ landscape/configuration.py 2017-03-03 12:43:19 +0000 |
4 | @@ -22,10 +22,13 @@ |
5 | from landscape.lib.twisted_util import gather_results |
6 | from landscape.lib.fetch import fetch, FetchError |
7 | from landscape.lib.bootstrap import BootstrapList, BootstrapDirectory |
8 | +from landscape.lib.persist import Persist |
9 | from landscape.reactor import LandscapeReactor |
10 | from landscape.broker.registration import RegistrationError |
11 | from landscape.broker.config import BrokerConfiguration |
12 | from landscape.broker.amp import RemoteBrokerConnector |
13 | +from landscape.broker.registration import Identity |
14 | +from landscape.broker.service import BrokerService |
15 | |
16 | |
17 | class ConfigurationError(Exception): |
18 | @@ -47,6 +50,27 @@ |
19 | stream.flush() |
20 | |
21 | |
22 | +def show_help(text): |
23 | + """Display help text.""" |
24 | + lines = text.strip().splitlines() |
25 | + print_text("\n" + "".join([line.strip() + "\n" for line in lines])) |
26 | + |
27 | + |
28 | +def prompt_yes_no(message, default=True): |
29 | + """Prompt for a yes/no question and return the answer as bool.""" |
30 | + default_msg = "[Y/n]" if default else "[y/N]" |
31 | + while True: |
32 | + value = raw_input("{} {}: ".format(message, default_msg)).lower() |
33 | + if value: |
34 | + if value.startswith("n"): |
35 | + return False |
36 | + if value.startswith("y"): |
37 | + return True |
38 | + show_help("Invalid input.") |
39 | + else: |
40 | + return default |
41 | + |
42 | + |
43 | def get_invalid_users(users): |
44 | """ |
45 | Process a string with a list of comma separated usernames, this returns |
46 | @@ -183,10 +207,6 @@ |
47 | def __init__(self, config): |
48 | self.config = config |
49 | |
50 | - def show_help(self, text): |
51 | - lines = text.strip().splitlines() |
52 | - print_text("\n" + "".join([line.strip() + "\n" for line in lines])) |
53 | - |
54 | def prompt_get_input(self, msg, required): |
55 | """Prompt the user on the terminal for a value |
56 | |
57 | @@ -199,7 +219,7 @@ |
58 | return value |
59 | elif not required: |
60 | break |
61 | - self.show_help("This option is required to configure Landscape.") |
62 | + show_help("This option is required to configure Landscape.") |
63 | |
64 | def prompt(self, option, msg, required=False): |
65 | """Prompt the user on the terminal for a value. |
66 | @@ -239,37 +259,20 @@ |
67 | value2 = getpass.getpass("Please confirm: ") |
68 | if value: |
69 | if value != value2: |
70 | - self.show_help("Keys must match.") |
71 | + show_help("Keys must match.") |
72 | else: |
73 | setattr(self.config, option, value) |
74 | break |
75 | elif default or not required: |
76 | break |
77 | else: |
78 | - self.show_help("This option is required to configure " |
79 | - "Landscape.") |
80 | - |
81 | - def prompt_yes_no(self, message, default=True): |
82 | - if default: |
83 | - default_msg = " [Y/n]" |
84 | - else: |
85 | - default_msg = " [y/N]" |
86 | - while True: |
87 | - value = raw_input(message + default_msg).lower() |
88 | - if value: |
89 | - if value.startswith("n"): |
90 | - return False |
91 | - if value.startswith("y"): |
92 | - return True |
93 | - self.show_help("Invalid input.") |
94 | - else: |
95 | - return default |
96 | + show_help("This option is required to configure Landscape.") |
97 | |
98 | def query_computer_title(self): |
99 | if "computer_title" in self.config.get_command_line_options(): |
100 | return |
101 | |
102 | - self.show_help( |
103 | + show_help( |
104 | """ |
105 | The computer title you provide will be used to represent this |
106 | computer in the Landscape user interface. It's important to use |
107 | @@ -283,7 +286,7 @@ |
108 | if "account_name" in self.config.get_command_line_options(): |
109 | return |
110 | |
111 | - self.show_help( |
112 | + show_help( |
113 | """ |
114 | You must now specify the name of the Landscape account you |
115 | want to register this computer with. Your account name is shown |
116 | @@ -297,7 +300,7 @@ |
117 | if "registration_key" in command_line_options: |
118 | return |
119 | |
120 | - self.show_help( |
121 | + show_help( |
122 | """ |
123 | A registration key may be associated with your Landscape |
124 | account to prevent unauthorized registration attempts. This |
125 | @@ -316,7 +319,7 @@ |
126 | if "http_proxy" in options and "https_proxy" in options: |
127 | return |
128 | |
129 | - self.show_help( |
130 | + show_help( |
131 | """ |
132 | The Landscape client communicates with the server over HTTP and |
133 | HTTPS. If your network requires you to use a proxy to access HTTP |
134 | @@ -337,7 +340,7 @@ |
135 | raise ConfigurationError("Unknown system users: %s" % |
136 | ", ".join(invalid_users)) |
137 | return |
138 | - self.show_help( |
139 | + show_help( |
140 | """ |
141 | Landscape has a feature which enables administrators to run |
142 | arbitrary scripts on machines under their control. By default this |
143 | @@ -351,10 +354,10 @@ |
144 | if included_plugins == [""]: |
145 | included_plugins = [] |
146 | default = "ScriptExecution" in included_plugins |
147 | - if self.prompt_yes_no(msg, default=default): |
148 | + if prompt_yes_no(msg, default=default): |
149 | if "ScriptExecution" not in included_plugins: |
150 | included_plugins.append("ScriptExecution") |
151 | - self.show_help( |
152 | + show_help( |
153 | """ |
154 | By default, scripts are restricted to the 'landscape' and |
155 | 'nobody' users. Please enter a comma-delimited list of users |
156 | @@ -368,8 +371,8 @@ |
157 | if not invalid_users: |
158 | break |
159 | else: |
160 | - self.show_help("Unknown system users: %s" % |
161 | - ",".join(invalid_users)) |
162 | + show_help("Unknown system users: {}".format( |
163 | + ",".join(invalid_users))) |
164 | self.config.script_users = None |
165 | else: |
166 | if "ScriptExecution" in included_plugins: |
167 | @@ -382,8 +385,9 @@ |
168 | if "access_group" in options: |
169 | return # an access group is already provided, don't ask for one |
170 | |
171 | - self.show_help("You may provide an access group for this computer " |
172 | - "e.g. webservers.") |
173 | + show_help( |
174 | + "You may provide an access group for this computer " |
175 | + "e.g. webservers.") |
176 | self.prompt("access_group", "Access group", False) |
177 | |
178 | def _get_invalid_tags(self, tagnames): |
179 | @@ -407,19 +411,19 @@ |
180 | ", ".join(invalid_tags)) |
181 | return |
182 | |
183 | - self.show_help("You may provide tags for this computer e.g. " |
184 | - "server,precise.") |
185 | + show_help( |
186 | + "You may provide tags for this computer e.g. server,precise.") |
187 | while True: |
188 | self.prompt("tags", "Tags", False) |
189 | if self._get_invalid_tags(self.config.tags): |
190 | - self.show_help("Tag names may only contain alphanumeric " |
191 | - "characters.") |
192 | + show_help( |
193 | + "Tag names may only contain alphanumeric characters.") |
194 | self.config.tags = None # Reset for the next prompt |
195 | else: |
196 | break |
197 | |
198 | def show_header(self): |
199 | - self.show_help( |
200 | + show_help( |
201 | """ |
202 | This script will interactively set up the Landscape client. It will |
203 | ask you a few questions about this computer and your Landscape |
204 | @@ -526,10 +530,11 @@ |
205 | if config.silent: |
206 | setup_init_script_and_start_client() |
207 | elif not sysvconfig.is_configured_to_run(): |
208 | - answer = raw_input("\nThe Landscape client must be started " |
209 | - "on boot to operate correctly.\n\n" |
210 | - "Start Landscape client on boot? (Y/n): ") |
211 | - if not answer.upper().startswith("N"): |
212 | + answer = prompt_yes_no( |
213 | + "\nThe Landscape client must be started " |
214 | + "on boot to operate correctly.\n\n" |
215 | + "Start Landscape client on boot?") |
216 | + if answer: |
217 | setup_init_script_and_start_client() |
218 | else: |
219 | sys.exit("Aborting Landscape configuration") |
220 | @@ -774,9 +779,9 @@ |
221 | report_registration_outcome(result, print=print) |
222 | sys.exit(determine_exit_code(result)) |
223 | else: |
224 | - answer = raw_input("\nRequest a new registration for " |
225 | - "this computer now? (Y/n): ") |
226 | - if not answer.upper().startswith("N"): |
227 | + answer = prompt_yes_no( |
228 | + "\nRequest a new registration for this computer now?") |
229 | + if answer: |
230 | result = register(config, reactor) |
231 | report_registration_outcome(result, print=print) |
232 | sys.exit(determine_exit_code(result)) |
233 | |
234 | === modified file 'landscape/tests/test_configuration.py' |
235 | --- landscape/tests/test_configuration.py 2017-02-24 10:09:22 +0000 |
236 | +++ landscape/tests/test_configuration.py 2017-03-03 12:43:19 +0000 |
237 | @@ -15,7 +15,7 @@ |
238 | from landscape.configuration import ( |
239 | print_text, LandscapeSetupScript, LandscapeSetupConfiguration, |
240 | register, setup, main, setup_init_script_and_start_client, |
241 | - ConfigurationError, |
242 | + ConfigurationError, prompt_yes_no, show_help, |
243 | ImportOptionError, store_public_key_data, |
244 | bootstrap_tree, got_connection, success, failure, exchange_failure, |
245 | handle_registration_errors, done, got_error, report_registration_outcome, |
246 | @@ -217,6 +217,50 @@ |
247 | self.assertEqual("Hi!END", stdout.getvalue()) |
248 | |
249 | |
250 | +class PromptYesNoTest(unittest.TestCase): |
251 | + |
252 | + def test_prompt_yes_no(self): |
253 | + """ |
254 | + prompt_yes_no prompts a question and returns a boolean with the answer. |
255 | + """ |
256 | + comparisons = [("Y", True), |
257 | + ("y", True), |
258 | + ("yEs", True), |
259 | + ("YES", True), |
260 | + ("n", False), |
261 | + ("N", False), |
262 | + ("No", False), |
263 | + ("no", False), |
264 | + ("", True)] |
265 | + |
266 | + for input_string, result in comparisons: |
267 | + with mock.patch("__builtin__.raw_input", |
268 | + return_value=input_string) as mock_raw_input: |
269 | + prompt_yes_no("Foo") |
270 | + mock_raw_input.assert_called_once_with("Foo [Y/n]: ") |
271 | + |
272 | + @mock.patch("__builtin__.raw_input", return_value="") |
273 | + def test_prompt_yes_no_default(self, mock_raw_input): |
274 | + self.assertFalse(prompt_yes_no("Foo", default=False)) |
275 | + mock_raw_input.assert_called_once_with("Foo [y/N]: ") |
276 | + |
277 | + @mock.patch("__builtin__.raw_input", side_effect=("x", "n")) |
278 | + @mock.patch("landscape.configuration.show_help") |
279 | + def test_prompt_yes_no_invalid(self, mock_show_help, mock_raw_input): |
280 | + self.assertFalse(prompt_yes_no("Foo")) |
281 | + mock_show_help.assert_called_once_with("Invalid input.") |
282 | + calls = [mock.call("Foo [Y/n]: "), mock.call("Foo [Y/n]: ")] |
283 | + mock_raw_input.assert_has_calls(calls) |
284 | + |
285 | + |
286 | +class ShowHelpTest(unittest.TestCase): |
287 | + |
288 | + @mock.patch("landscape.configuration.print_text") |
289 | + def test_show_help(self, mock_print_text): |
290 | + show_help("\n\n \n Hello \n \n world! \n \n\n") |
291 | + mock_print_text.assert_called_once_with("\nHello\n\nworld!\n") |
292 | + |
293 | + |
294 | class LandscapeSetupScriptTest(LandscapeTest): |
295 | |
296 | def setUp(self): |
297 | @@ -228,11 +272,6 @@ |
298 | self.config = MyLandscapeSetupConfiguration() |
299 | self.script = LandscapeSetupScript(self.config) |
300 | |
301 | - @mock.patch("landscape.configuration.print_text") |
302 | - def test_show_help(self, mock_print_text): |
303 | - self.script.show_help("\n\n \n Hello \n \n world! \n \n\n") |
304 | - mock_print_text.assert_called_once_with("\nHello\n\nworld!\n") |
305 | - |
306 | @mock.patch("__builtin__.raw_input", return_value="Desktop") |
307 | def test_prompt_simple(self, mock_raw_input): |
308 | self.script.prompt("computer_title", "Message") |
309 | @@ -248,10 +287,10 @@ |
310 | self.assertEqual(self.config.computer_title, "default") |
311 | |
312 | @mock.patch("__builtin__.raw_input", side_effect=("", "Desktop")) |
313 | - def test_prompt_with_required(self, mock_raw_input): |
314 | - self.script.show_help = mock.Mock() |
315 | + @mock.patch("landscape.configuration.show_help") |
316 | + def test_prompt_with_required(self, mock_show_help, mock_raw_input): |
317 | self.script.prompt("computer_title", "Message", True) |
318 | - self.script.show_help.assert_called_once_with( |
319 | + mock_show_help.assert_called_once_with( |
320 | "This option is required to configure Landscape.") |
321 | |
322 | calls = [mock.call("Message: "), mock.call("Message: ")] |
323 | @@ -286,71 +325,41 @@ |
324 | mock_getpass.assert_has_calls(calls) |
325 | self.assertEqual(self.config.registration_key, "password") |
326 | |
327 | + @mock.patch("landscape.configuration.show_help") |
328 | @mock.patch("landscape.configuration.getpass.getpass", |
329 | side_effect=("password", "", "password", "password")) |
330 | - def test_password_prompt_simple_non_matching(self, mock_getpass): |
331 | - self.script.show_help = mock.Mock() |
332 | - |
333 | + def test_password_prompt_simple_non_matching(self, mock_getpass, |
334 | + mock_show_help): |
335 | self.script.password_prompt("registration_key", "Password") |
336 | |
337 | calls = [mock.call("Password: "), mock.call("Please confirm: "), |
338 | mock.call("Password: "), mock.call("Please confirm: ")] |
339 | mock_getpass.assert_has_calls(calls) |
340 | - self.script.show_help.assert_called_once_with("Keys must match.") |
341 | + mock_show_help.assert_called_once_with("Keys must match.") |
342 | self.assertEqual(self.config.registration_key, "password") |
343 | |
344 | + @mock.patch("landscape.configuration.show_help") |
345 | @mock.patch("landscape.configuration.getpass.getpass", |
346 | side_effect=("", "password", "password")) |
347 | - def test_password_prompt_simple_matching_required(self, mock_getpass): |
348 | - self.script.show_help = mock.Mock() |
349 | - |
350 | + def test_password_prompt_simple_matching_required(self, mock_getpass, |
351 | + mock_show_help): |
352 | self.script.password_prompt("registration_key", "Password", True) |
353 | |
354 | calls = [mock.call("Password: "), mock.call("Password: "), |
355 | mock.call("Please confirm: ")] |
356 | mock_getpass.assert_has_calls(calls) |
357 | - self.script.show_help.assert_called_once_with( |
358 | + mock_show_help.assert_called_once_with( |
359 | "This option is required to configure Landscape.") |
360 | self.assertEqual(self.config.registration_key, "password") |
361 | |
362 | - def test_prompt_yes_no(self): |
363 | - comparisons = [("Y", True), |
364 | - ("y", True), |
365 | - ("yEs", True), |
366 | - ("YES", True), |
367 | - ("n", False), |
368 | - ("N", False), |
369 | - ("No", False), |
370 | - ("no", False), |
371 | - ("", True)] |
372 | - |
373 | - for input_string, result in comparisons: |
374 | - with mock.patch("__builtin__.raw_input", |
375 | - return_value=input_string) as mock_raw_input: |
376 | - self.script.prompt_yes_no("Foo") |
377 | - mock_raw_input.assert_called_once_with("Foo [Y/n]") |
378 | - |
379 | - @mock.patch("__builtin__.raw_input", return_value="") |
380 | - def test_prompt_yes_no_default(self, mock_raw_input): |
381 | - self.assertFalse(self.script.prompt_yes_no("Foo", default=False)) |
382 | - mock_raw_input.assert_called_once_with("Foo [y/N]") |
383 | - |
384 | - @mock.patch("__builtin__.raw_input", side_effect=("x", "n")) |
385 | - def test_prompt_yes_no_invalid(self, mock_raw_input): |
386 | - self.script.show_help = mock.Mock() |
387 | - self.assertFalse(self.script.prompt_yes_no("Foo")) |
388 | - self.script.show_help.assert_called_once_with("Invalid input.") |
389 | - calls = [mock.call("Foo [Y/n]"), mock.call("Foo [Y/n]")] |
390 | - mock_raw_input.assert_has_calls(calls) |
391 | - |
392 | - def test_query_computer_title(self): |
393 | + @mock.patch("landscape.configuration.show_help") |
394 | + def test_query_computer_title(self, mock_show_help): |
395 | help_snippet = "The computer title you" |
396 | - self.script.show_help = mock.Mock() |
397 | self.script.prompt = mock.Mock() |
398 | self.script.query_computer_title() |
399 | self.script.prompt.assert_called_once_with( |
400 | "computer_title", "This computer's title", True) |
401 | - [call] = self.script.show_help.mock_calls |
402 | + [call] = mock_show_help.mock_calls |
403 | self.assertTrue(call.strip().startswith(help_snippet)) |
404 | |
405 | @mock.patch("__builtin__.raw_input") |
406 | @@ -360,14 +369,14 @@ |
407 | self.script.query_computer_title() |
408 | mock_raw_input.assert_not_called() |
409 | |
410 | - def test_query_account_name(self): |
411 | + @mock.patch("landscape.configuration.show_help") |
412 | + def test_query_account_name(self, mock_show_help): |
413 | help_snippet = "You must now specify the name of the Landscape account" |
414 | - self.script.show_help = mock.Mock() |
415 | self.script.prompt = mock.Mock() |
416 | self.script.query_account_name() |
417 | self.script.prompt.assert_called_once_with( |
418 | "account_name", "Account name", True) |
419 | - [call] = self.script.show_help.mock_calls |
420 | + [call] = mock_show_help.mock_calls |
421 | self.assertTrue(call.strip().startswith(help_snippet)) |
422 | |
423 | self.script.query_account_name() |
424 | @@ -378,14 +387,14 @@ |
425 | self.script.query_account_name() |
426 | mock_raw_input.assert_not_called() |
427 | |
428 | - def test_query_registration_key(self): |
429 | + @mock.patch("landscape.configuration.show_help") |
430 | + def test_query_registration_key(self, mock_show_help): |
431 | help_snippet = "A registration key may be" |
432 | - self.script.show_help = mock.Mock() |
433 | self.script.password_prompt = mock.Mock() |
434 | self.script.query_registration_key() |
435 | self.script.password_prompt.assert_called_once_with( |
436 | "registration_key", "Account registration key") |
437 | - [call] = self.script.show_help.mock_calls |
438 | + [call] = mock_show_help.mock_calls |
439 | self.assertTrue(call.strip().startswith(help_snippet)) |
440 | |
441 | @mock.patch("landscape.configuration.getpass.getpass") |
442 | @@ -395,16 +404,16 @@ |
443 | self.script.query_registration_key() |
444 | mock_getpass.assert_not_called() |
445 | |
446 | - def test_query_proxies(self): |
447 | + @mock.patch("landscape.configuration.show_help") |
448 | + def test_query_proxies(self, mock_show_help): |
449 | help_snippet = "The Landscape client communicates" |
450 | - self.script.show_help = mock.Mock() |
451 | self.script.prompt = mock.Mock() |
452 | |
453 | self.script.query_proxies() |
454 | calls = [mock.call("http_proxy", "HTTP proxy URL"), |
455 | mock.call("https_proxy", "HTTPS proxy URL")] |
456 | self.script.prompt.assert_has_calls(calls) |
457 | - [call] = self.script.show_help.mock_calls |
458 | + [call] = mock_show_help.mock_calls |
459 | self.assertTrue(call.strip().startswith(help_snippet)) |
460 | |
461 | @mock.patch("__builtin__.raw_input") |
462 | @@ -414,53 +423,53 @@ |
463 | self.script.query_proxies() |
464 | mock_raw_input.assert_not_called() |
465 | |
466 | - def test_query_http_proxy_defined_on_command_line(self): |
467 | + @mock.patch("landscape.configuration.show_help") |
468 | + def test_query_http_proxy_defined_on_command_line(self, mock_show_help): |
469 | help_snippet = "The Landscape client communicates" |
470 | - self.script.show_help = mock.Mock() |
471 | self.script.prompt = mock.Mock() |
472 | |
473 | self.config.load_command_line(["--http-proxy", "localhost:8080"]) |
474 | self.script.query_proxies() |
475 | - [call] = self.script.show_help.mock_calls |
476 | + [call] = mock_show_help.mock_calls |
477 | self.assertTrue(call.strip().startswith(help_snippet)) |
478 | |
479 | - def test_query_https_proxy_defined_on_command_line(self): |
480 | + @mock.patch("landscape.configuration.show_help") |
481 | + def test_query_https_proxy_defined_on_command_line(self, mock_show_help): |
482 | help_snippet = "The Landscape client communicates" |
483 | - self.script.show_help = mock.Mock() |
484 | self.script.prompt = mock.Mock() |
485 | self.config.load_command_line(["--https-proxy", "localhost:8443"]) |
486 | self.script.query_proxies() |
487 | self.script.prompt.assert_called_once_with( |
488 | "http_proxy", "HTTP proxy URL") |
489 | - [call] = self.script.show_help.mock_calls |
490 | + [call] = mock_show_help.mock_calls |
491 | self.assertTrue(call.strip().startswith(help_snippet)) |
492 | |
493 | - def test_query_script_plugin_no(self): |
494 | + @mock.patch("landscape.configuration.show_help") |
495 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=False) |
496 | + def test_query_script_plugin_no(self, mock_prompt_yes_no, mock_show_help): |
497 | help_snippet = "Landscape has a feature which enables administrators" |
498 | - self.script.show_help = mock.Mock() |
499 | - self.script.prompt_yes_no = mock.Mock(return_value=False) |
500 | |
501 | self.script.query_script_plugin() |
502 | self.assertEqual(self.config.include_manager_plugins, "") |
503 | - self.script.prompt_yes_no.assert_called_once_with( |
504 | + mock_prompt_yes_no.assert_called_once_with( |
505 | "Enable script execution?", default=False) |
506 | - [call] = self.script.show_help.mock_calls |
507 | + [call] = mock_show_help.mock_calls |
508 | self.assertTrue(call.strip().startswith(help_snippet)) |
509 | |
510 | - def test_query_script_plugin_yes(self): |
511 | + @mock.patch("landscape.configuration.show_help") |
512 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=True) |
513 | + def test_query_script_plugin_yes(self, mock_prompt_yes_no, mock_show_help): |
514 | """ |
515 | If the user *does* want script execution, then the script asks which |
516 | users to enable it for. |
517 | """ |
518 | help_snippet = "Landscape has a feature which enables administrators" |
519 | - self.script.show_help = mock.Mock() |
520 | - self.script.prompt_yes_no = mock.Mock(return_value=True) |
521 | self.script.prompt = mock.Mock() |
522 | |
523 | self.script.query_script_plugin() |
524 | - self.script.prompt_yes_no.assert_called_once_with( |
525 | + mock_prompt_yes_no.assert_called_once_with( |
526 | "Enable script execution?", default=False) |
527 | - first_call, second_call = self.script.show_help.mock_calls |
528 | + first_call, second_call = mock_show_help.mock_calls |
529 | self.assertTrue(first_call.strip().startswith(help_snippet)) |
530 | self.assertTrue(second_call.strip().startswith( |
531 | "By default, scripts are restricted")) |
532 | @@ -470,56 +479,58 @@ |
533 | self.assertEqual(self.config.include_manager_plugins, |
534 | "ScriptExecution") |
535 | |
536 | - def test_disable_script_plugin(self): |
537 | + @mock.patch("landscape.configuration.show_help") |
538 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=False) |
539 | + def test_disable_script_plugin(self, mock_prompt_yes_no, mock_show_help): |
540 | """ |
541 | Answering NO to enabling the script plugin while it's already enabled |
542 | will disable it. |
543 | """ |
544 | self.config.include_manager_plugins = "ScriptExecution" |
545 | help_snippet = "Landscape has a feature which enables administrators" |
546 | - self.script.show_help = mock.Mock() |
547 | - self.script.prompt_yes_no = mock.Mock(return_value=False) |
548 | |
549 | self.script.query_script_plugin() |
550 | - self.script.prompt_yes_no.assert_called_once_with( |
551 | + mock_prompt_yes_no.assert_called_once_with( |
552 | "Enable script execution?", default=True) |
553 | self.assertEqual(self.config.include_manager_plugins, "") |
554 | - [call] = self.script.show_help.mock_calls |
555 | + [call] = mock_show_help.mock_calls |
556 | self.assertTrue(call.strip().startswith(help_snippet)) |
557 | |
558 | - def test_disabling_script_plugin_leaves_existing_inclusions(self): |
559 | + @mock.patch("landscape.configuration.show_help") |
560 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=False) |
561 | + def test_disabling_script_plugin_leaves_existing_inclusions( |
562 | + self, mock_prompt_yes_no, mock_show_help): |
563 | """ |
564 | Disabling the script execution plugin doesn't remove other included |
565 | plugins. |
566 | """ |
567 | self.config.include_manager_plugins = "FooPlugin, ScriptExecution" |
568 | - self.script.show_help = mock.Mock() |
569 | - self.script.prompt_yes_no = mock.Mock(return_value=False) |
570 | |
571 | self.script.query_script_plugin() |
572 | - self.script.prompt_yes_no.assert_called_once_with( |
573 | + mock_prompt_yes_no.assert_called_once_with( |
574 | "Enable script execution?", default=True) |
575 | self.assertEqual(self.config.include_manager_plugins, "FooPlugin") |
576 | - self.script.show_help.assert_called_once_with(mock.ANY) |
577 | + mock_show_help.assert_called_once_with(mock.ANY) |
578 | |
579 | - def test_enabling_script_plugin_leaves_existing_inclusions(self): |
580 | + @mock.patch("landscape.configuration.show_help") |
581 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=True) |
582 | + def test_enabling_script_plugin_leaves_existing_inclusions( |
583 | + self, mock_prompt_yes_no, mock_show_help): |
584 | """ |
585 | Enabling the script execution plugin doesn't remove other included |
586 | plugins. |
587 | """ |
588 | self.config.include_manager_plugins = "FooPlugin" |
589 | |
590 | - self.script.show_help = mock.Mock() |
591 | - self.script.prompt_yes_no = mock.Mock(return_value=True) |
592 | self.script.prompt = mock.Mock() |
593 | |
594 | self.script.query_script_plugin() |
595 | - self.script.prompt_yes_no.assert_called_once_with( |
596 | + mock_prompt_yes_no.assert_called_once_with( |
597 | "Enable script execution?", default=False) |
598 | |
599 | self.script.prompt.assert_called_once_with( |
600 | "script_users", "Script users") |
601 | - self.assertEqual(2, self.script.show_help.call_count) |
602 | + self.assertEqual(2, mock_show_help.call_count) |
603 | self.assertEqual(self.config.include_manager_plugins, |
604 | "FooPlugin, ScriptExecution") |
605 | |
606 | @@ -534,9 +545,10 @@ |
607 | "ScriptExecution") |
608 | self.assertEqual(self.config.script_users, "root, nobody") |
609 | |
610 | - def test_query_script_manager_plugins_defined_on_command_line(self): |
611 | - self.script.show_help = mock.Mock() |
612 | - self.script.prompt_yes_no = mock.Mock(return_value=True) |
613 | + @mock.patch("landscape.configuration.show_help") |
614 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=True) |
615 | + def test_query_script_manager_plugins_defined_on_command_line( |
616 | + self, mock_prompt_yes_no, mock_show_help): |
617 | self.script.prompt = mock.Mock() |
618 | |
619 | self.config.load_command_line( |
620 | @@ -544,17 +556,19 @@ |
621 | self.script.query_script_plugin() |
622 | self.script.prompt.assert_called_once_with( |
623 | "script_users", "Script users") |
624 | - self.assertEqual(2, self.script.show_help.call_count) |
625 | + self.assertEqual(2, mock_show_help.call_count) |
626 | self.assertEqual(self.config.include_manager_plugins, |
627 | "FooPlugin, ScriptExecution") |
628 | |
629 | + @mock.patch("landscape.configuration.show_help") |
630 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=True) |
631 | @mock.patch("landscape.configuration.pwd.getpwnam", return_value=None) |
632 | - def test_query_script_users_defined_on_command_line(self, mock_getpwnam): |
633 | + def test_query_script_users_defined_on_command_line(self, mock_getpwnam, |
634 | + mock_prompt_yes_no, |
635 | + mock_show_help): |
636 | """ |
637 | Confirm with the user for users specified for the ScriptPlugin. |
638 | """ |
639 | - self.script.show_help = mock.Mock() |
640 | - self.script.prompt_yes_no = mock.Mock(return_value=True) |
641 | self.script.prompt_get_input = mock.Mock(return_value=None) |
642 | |
643 | self.config.include_manager_plugins = "FooPlugin" |
644 | @@ -564,11 +578,11 @@ |
645 | self.script.query_script_plugin() |
646 | |
647 | mock_getpwnam.assert_called_with("landscape") |
648 | - self.script.prompt_yes_no.assert_called_once_with( |
649 | + mock_prompt_yes_no.assert_called_once_with( |
650 | "Enable script execution?", default=False) |
651 | self.script.prompt_get_input.assert_called_once_with( |
652 | "Script users [root, nobody, landscape]: ", False) |
653 | - self.assertEqual(2, self.script.show_help.call_count) |
654 | + self.assertEqual(2, mock_show_help.call_count) |
655 | self.assertEqual(self.config.script_users, |
656 | "root, nobody, landscape") |
657 | |
658 | @@ -617,53 +631,56 @@ |
659 | "--include-manager-plugins", "ScriptPlugin"]) |
660 | self.assertRaises(ConfigurationError, self.script.query_script_plugin) |
661 | |
662 | - def test_invalid_user_entered_by_user(self): |
663 | + @mock.patch("landscape.configuration.show_help") |
664 | + @mock.patch("landscape.configuration.prompt_yes_no", return_value=True) |
665 | + def test_invalid_user_entered_by_user(self, mock_prompt_yes_no, |
666 | + mock_show_help): |
667 | """ |
668 | If an invalid user is entered on the command line the user should be |
669 | informed and prompted again. |
670 | """ |
671 | help_snippet = "Landscape has a feature which enables administrators" |
672 | - self.script.show_help = mock.Mock() |
673 | - self.script.prompt_yes_no = mock.Mock(return_value=True) |
674 | self.script.prompt_get_input = mock.Mock( |
675 | side_effect=(u"nonexistent", u"root")) |
676 | |
677 | self.script.query_script_plugin() |
678 | self.assertEqual(self.config.script_users, "root") |
679 | - first_call, second_call, third_call = self.script.show_help.mock_calls |
680 | + first_call, second_call, third_call = mock_show_help.mock_calls |
681 | self.assertTrue(first_call.strip().startswith(help_snippet)) |
682 | self.assertTrue(second_call.strip().startswith( |
683 | "By default, scripts are restricted")) |
684 | self.assertTrue(third_call.strip().startswith( |
685 | "Unknown system users: nonexistsent")) |
686 | |
687 | - def test_tags_not_defined_on_command_line(self): |
688 | + @mock.patch("landscape.configuration.show_help") |
689 | + def test_tags_not_defined_on_command_line(self, mock_show_help): |
690 | """ |
691 | If tags are not provided, the user should be prompted for them. |
692 | """ |
693 | help_snippet = ("You may provide tags for this computer e.g. " |
694 | "server,precise.") |
695 | - self.script.show_help = mock.Mock() |
696 | self.script.prompt = mock.Mock() |
697 | |
698 | self.script.query_tags() |
699 | self.script.prompt.assert_called_once_with( |
700 | "tags", "Tags", False) |
701 | - [call] = self.script.show_help.mock_calls |
702 | + [call] = mock_show_help.mock_calls |
703 | self.assertTrue(call.strip().startswith(help_snippet)) |
704 | |
705 | - def test_invalid_tags_entered_by_user(self): |
706 | + @mock.patch("landscape.configuration.show_help") |
707 | + @mock.patch("landscape.configuration.prompt_yes_no") |
708 | + def test_invalid_tags_entered_by_user(self, mock_prompt_yes_no, |
709 | + mock_show_help): |
710 | """ |
711 | If tags are not provided, the user should be prompted for them, and |
712 | they should be valid tags, if not the user should be prompted for them |
713 | again. |
714 | """ |
715 | - self.script.show_help = mock.Mock() |
716 | self.script.prompt_get_input = mock.Mock( |
717 | side_effect=(u"<script>alert();</script>", u"london")) |
718 | |
719 | self.script.query_tags() |
720 | - first_call, second_call = self.script.show_help.mock_calls |
721 | + first_call, second_call = mock_show_help.mock_calls |
722 | self.assertTrue( |
723 | first_call.strip().startswith("You may provide tags for this " |
724 | "computer e.g. server,precise.")) |
725 | @@ -693,16 +710,16 @@ |
726 | self.assertRaises(ConfigurationError, self.script.query_tags) |
727 | mock_raw_input.assert_not_called() |
728 | |
729 | - def test_access_group_not_defined_on_command_line(self): |
730 | + @mock.patch("landscape.configuration.show_help") |
731 | + def test_access_group_not_defined_on_command_line(self, mock_show_help): |
732 | """ |
733 | If an access group is not provided, the user should be prompted for it. |
734 | """ |
735 | help_snippet = ("You may provide an access group for this computer " |
736 | "e.g. webservers.") |
737 | - self.script.show_help = mock.Mock() |
738 | self.script.prompt = mock.Mock() |
739 | self.script.query_access_group() |
740 | - [call] = self.script.show_help.mock_calls |
741 | + [call] = mock_show_help.mock_calls |
742 | self.assertTrue(call.strip().startswith(help_snippet)) |
743 | |
744 | @mock.patch("__builtin__.raw_input") |
745 | @@ -716,11 +733,11 @@ |
746 | self.assertEqual(self.config.access_group, u"webservers") |
747 | mock_raw_input.assert_not_called() |
748 | |
749 | - def test_show_header(self): |
750 | + @mock.patch("landscape.configuration.show_help") |
751 | + def test_show_header(self, mock_show_help): |
752 | help_snippet = "This script will" |
753 | - self.script.show_help = mock.Mock() |
754 | self.script.show_header() |
755 | - [call] = self.script.show_help.mock_calls |
756 | + [call] = mock_show_help.mock_calls |
757 | self.assertTrue(call.strip().startswith(help_snippet)) |
758 | |
759 | def test_run(self): |
760 | @@ -1095,7 +1112,7 @@ |
761 | main(["-c", self.make_working_config()], print=noop_print) |
762 | mock_register.assert_not_called() |
763 | mock_raw_input.assert_called_once_with( |
764 | - "\nRequest a new registration for this computer now? (Y/n): ") |
765 | + "\nRequest a new registration for this computer now? [Y/n]: ") |
766 | |
767 | @mock.patch("landscape.configuration.register", return_value="success") |
768 | @mock.patch("landscape.configuration.setup") |
769 | @@ -1136,7 +1153,7 @@ |
770 | mock_setup.assert_called_once_with(mock.ANY) |
771 | mock_register.assert_called_once_with(mock.ANY, mock.ANY) |
772 | mock_raw_input.assert_called_once_with( |
773 | - "\nRequest a new registration for this computer now? (Y/n): ") |
774 | + "\nRequest a new registration for this computer now? [Y/n]: ") |
775 | self.assertEqual( |
776 | [("Please wait...", sys.stdout), |
777 | ("System successfully registered.", sys.stdout)], |
778 | @@ -1161,7 +1178,7 @@ |
779 | mock_setup.assert_called_once_with(mock.ANY) |
780 | mock_register.assert_called_once_with(mock.ANY, mock.ANY) |
781 | mock_raw_input.assert_called_once_with( |
782 | - "\nRequest a new registration for this computer now? (Y/n): ") |
783 | + "\nRequest a new registration for this computer now? [Y/n]: ") |
784 | |
785 | # Note that the error is output via sys.stderr. |
786 | self.assertEqual( |
787 | @@ -1249,9 +1266,9 @@ |
788 | mock_raw_input.assert_any_call( |
789 | "\nThe Landscape client must be started " |
790 | "on boot to operate correctly.\n\n" |
791 | - "Start Landscape client on boot? (Y/n): ") |
792 | + "Start Landscape client on boot? [Y/n]: ") |
793 | mock_raw_input.assert_called_with( |
794 | - "\nRequest a new registration for this computer now? (Y/n): ") |
795 | + "\nRequest a new registration for this computer now? [Y/n]: ") |
796 | |
797 | @mock.patch("landscape.configuration.print_text") |
798 | @mock.patch("landscape.configuration.SysVConfig") |
799 | @@ -1307,7 +1324,7 @@ |
800 | mock_setup.assert_called_once_with(mock.ANY) |
801 | mock_register.assert_called_once_with(mock.ANY, mock.ANY) |
802 | mock_raw_input.assert_called_once_with( |
803 | - "\nRequest a new registration for this computer now? (Y/n): ") |
804 | + "\nRequest a new registration for this computer now? [Y/n]: ") |
805 | |
806 | @mock.patch("landscape.configuration.SysVConfig") |
807 | def test_setup_init_script_and_start_client( |
Command: TRIAL_ARGS=-j4 make check /ci.lscape. net/job/ latch-test- precise/ 870/
Result: Success
Revno: 943
Branch: lp:~ack/landscape-client/refactor-prompt-functions
Jenkins: https:/