Merge lp:~frankban/lpsetup/interactive-execution into lp:lpsetup
- interactive-execution
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Francesco Banconi |
Approved revision: | 83 |
Merged at revision: | 65 |
Proposed branch: | lp:~frankban/lpsetup/interactive-execution |
Merge into: | lp:lpsetup |
Diff against target: |
985 lines (+501/-11) 18 files modified
lpsetup/argparser.py (+58/-2) lpsetup/handlers.py (+1/-0) lpsetup/subcommands/finish_inithost.py (+5/-0) lpsetup/subcommands/inithost.py (+25/-0) lpsetup/subcommands/initlxc.py (+22/-1) lpsetup/subcommands/initrepo.py (+9/-0) lpsetup/subcommands/install_lxc.py (+21/-3) lpsetup/subcommands/update.py (+13/-0) lpsetup/tests/examples.py (+15/-0) lpsetup/tests/integration/test_init_host.py (+1/-1) lpsetup/tests/integration/test_install_lxc.py (+1/-1) lpsetup/tests/subcommands/test_initrepo.py (+1/-0) lpsetup/tests/subcommands/test_smoke.py (+28/-0) lpsetup/tests/test_argparser.py (+73/-1) lpsetup/tests/test_handlers.py (+6/-2) lpsetup/tests/test_utils.py (+117/-0) lpsetup/tests/utils.py (+45/-0) lpsetup/utils.py (+60/-0) |
To merge this branch: | bzr merge lp:~frankban/lpsetup/interactive-execution |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Francesco Banconi (community) | Approve | ||
Benji York (community) | code | Approve | |
Review via email: mp+117062@code.launchpad.net |
Commit message
Interactive and dry command execution.
Description of the change
This branch adds interactive and dry execution for lpsetup commands.
The diff is very long, I am sorry.
== Changes ==
Fixed *ArgumentParser
Added *StepsBasedSubC
Introduced steps' descriptions: the *description* attributes of each step are collected and used as command description. If the attribute is not found, the description is an empty string. It is possible to format description using using the Python's builtin templating system *string.Template* supporting $-based substitutions. The current namespace is used as context for substitutions.
Implemented the *confirm* function, and the RawInputReturning context manager that is used to temporarily mock raw_input so that it is possible to easily test *confirm*.
Added descriptions for all the relevant steps.
Added a --dry option to force the command to just display steps' descriptions and then exit.
Updated *handle_testing* to automatically imply a non-interactive run.
Updated *install_lxc* to pass --yes to the interactive sub commands re-invoked from inside the LXC.
Added --yes to integration tests.
Benji York (benji) wrote : | # |
Francesco Banconi (frankban) wrote : | # |
Thanks for all your comments Benji. Replies follow.
> The use of $templates for the descriptions was a good choice. It keeps
> them simpler than having to have a function for each, but provides the
> dynamism we need to be able to include runtime values.
Exactly.
> We might want an assertion that no un-replaced substitutions are left in
> the description after substitution takes place. I wouldn't be surprised
> if we typo a name now and then and it would keep those from persisting
> very long.
Didn't thought about typos. I think what you are suggesting can be implemented just replacing Template.
> The careful indention of description strings only for them to be
> dedented in the future seems odd. This:
>
> fetch.description = """Set up a Launchpad repository in $repository,
> retrieving the source code from $source."""
>
> or this:
>
> fetch.description = ('Set up a Launchpad repository in $repository,'
> 'retrieving the source code from $source.')
>
> better matches prevailing Python style than this:
>
> fetch.description = """
> Set up a Launchpad repository in $repository,
> retrieving the source code from $source.
> """
I will change the style of the descriptions.
> I don't feel strongly about it, but normally see dry-run options spelled
> "--dry-run" I don't recall seeing one spelled "--dry".
>
> Having the namespace attribute as "dry" is a bit terse. Spelling it
> "dry_run" would help when reading the code.
Agreed.
>
> I'm surprised that the bit about inverting the boolean if store_false is
> used (in lpsetup/
> didn't look deeply enough into it to understand why, but I just wanted
> to point it out in case it made you realize anything.
Unfortunately that's necessary. Normally you have flags like:
parser.
And the old code to regenerate the args from the namespace worked in this case, because the logic was:
if the value is boolean, and the value is True, then add the corresponding argument
But what about:
parser.
In this case, following the logic above, --yes is not re-added to the arguments, because interactive will be False.
That's why, when store_false is used, we need to invert the logic: if value if False, add the argument. To keep the existing code as is, I decided to just invert the value before the check.
> Having to do self.is_
> namespace.
> all commands have the option, so sometimes the attribute won't be there.
> I wonder if it would be better to always have the attribute available,
> even if it can not be true would be better.
namespace.
Benji York (benji) wrote : | # |
On Fri, Jul 27, 2012 at 1:13 PM, Francesco Banconi
<email address hidden> wrote:
> Thanks for all your comments Benji. Replies follow.
[snip lots of good stuff that doesn't need comment]
>> Having to do self.is_
>> namespace.
>> all commands have the option, so sometimes the attribute won't be there.
>> I wonder if it would be better to always have the attribute available,
>> even if it can not be true would be better.
>
> namespace.
>
> is_interactive = getattr(namespace, 'interactive', False)
>
> This can be done in the run() method, or, maybe better, in init_namespace.
>
> I decided to create a method to allow sub commands to override that method and decide themselves if they are interactive. However, this machinery is not used, and, as I think you suggested, having the calculated value in the namespace could be nice. Having this context, what you'd suggest?
Could we have a handler that sets namespace.
there is otherwise no value? Hmm, I can't check right now, but
doesn't argparse's add_option take a "default" argument. That seems
like it would work.
>> The fact that steps can have empty descriptions feels like a trap.
>> Perhaps it should be an error for that to happen.
>
> I am confused here, could you please deepen this argument
I'm worried that someone will add a step that does something dramatic
-- say "delete the entire hard disk" for effect -- and forgets to add
a description. Maybe we could have a foo.description = None so if
there is really no description we have to be explicit about it.
>> It feels like there should be some whitespace between the step function
>> definitions and the description attribute assignment for the functions.
>> I don't know if the pep8 formatting checker will warn about them or not.
>
> I don't know either, but, AFAICT, tarmac doesn't complain about the steps
> already defining their name like that in tests.examples.
I would like at least one blank line separating them.
>> The way descriptions are presented is a little hard to read. Here is
>> some output from my system:
>
>> If we wanted to be extra fancy in get_step_
>> the current terminal width and wrap to that.
>
> These suggestions definitely improve the description readability, I will try
> to add these changes in this branch.
Cool.
--
Benji York
Francesco Banconi (frankban) wrote : | # |
Benji, I updated this branch following you suggestions:
- fIxed steps' descriptions indentation and formatting, e.g.::
$ ./lp-setup init-host --dry-run
This command will perform the following actions:
* Update your system and install necessary deb packages (ssh bzr apache2.2-common).
* Create the user frankban if it does not exist.
* Create Apache document roots for launchpad and enable required Apache modules (proxy proxy_http rewrite ssl
deflate headers).
* Set up hosts file for Launchpad (/etc/hosts).
* Set up the user's ssh directory (/home/
* Create, if it does not exist, the ssh key /home/frankban/
frankban.
* Add bazaar.
* Set up bazaar authentication: Francesco Banconi <email address hidden>.
* Set up Launchpad user id: frankban.
* Add required APT repositories and install Launchpad dependencies: launchpad-
developer-
- descriptions are now wrapped at terminal size
- simplified the confirm function, updated the corresponding tests
- s/dry/dry_run
Also updated the descriptions handling, in particular:
- an error is raised if placeholders are missing in the steps' descriptions
- an error is raised for missing descriptions
These changes convinced me to implement a --dry-run smoke test, so that we can ensure our steps are correctly described.
- *interactive* now is always an attribute of namespace, even if the sub command does not support interactive execution. => Removed the is_interactive method.
Benji York (benji) wrote : | # |
The branch looks good. The wrapping to the terminal width is especially nice.
Launchpad QA Bot (lpqabot) wrote : | # |
Attempt to merge into lp:lpsetup failed due to conflicts:
text conflict in lpsetup/
Launchpad QA Bot (lpqabot) wrote : | # |
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.
Francesco Banconi (frankban) : | # |
Launchpad QA Bot (lpqabot) wrote : | # |
The attempt to merge lp:~frankban/lpsetup/interactive-execution into lp:lpsetup failed. Below is the output from the failed tests.
nose.plugins.cover: ERROR: Coverage not available: unable to import coverage module
.......
=======
ERROR: test_subcommand
-------
Traceback (most recent call last):
File "/home/
retcode = main([name, '--dry-run'] + args)
File "/home/
return args.main(args)
File "/home/
return self.callback(
File "/home/
description = self.get_
File "/home/
descriptions = [get_step_
File "/home/
width = get_terminal_
File "/home/
return int(run('stty', 'size').split()[1])
File "/usr/lib/
raise exception
CalledProcessError: Command '['stty', 'size']' returned non-zero exit status 1
=======
ERROR: test_command_
-------
Traceback (most recent call last):
File "/home/
self.
File "/home/
return namespace.
File "/home/
return self.callback(
File "/home/
description = self.get_
File "/home/
descriptions = [get_step_
File "/home/
width = get_terminal_
File "/home/
return int(run('stty', 'size').split()[1])
File "/usr/lib/
raise exception
CalledProcessError: Command '['stty', '...
Francesco Banconi (frankban) : | # |
Launchpad QA Bot (lpqabot) wrote : | # |
The attempt to merge lp:~frankban/lpsetup/interactive-execution into lp:lpsetup failed. Below is the output from the failed tests.
nose.plugins.cover: ERROR: Coverage not available: unable to import coverage module
.......
=======
ERROR: test_subcommand
-------
Traceback (most recent call last):
File "/home/
retcode = main([name, '--dry-run'] + args)
File "/home/
return args.main(args)
File "/home/
return self.callback(
File "/home/
description = self.get_
File "/home/
descriptions = [get_step_
File "/home/
width = get_terminal_
File "/home/
raise ValueError(str(err) + ' - ' + err.output)
ValueError: Command '['stty', 'size']' returned non-zero exit status 1 - stty: standard input: Invalid argument
=======
ERROR: test_command_
-------
Traceback (most recent call last):
File "/home/
self.
File "/home/
return namespace.
File "/home/
return self.callback(
File "/home/
description = self.get_
File "/home/
descriptions = [get_step_
File "/home/
width = get_terminal_
File "/home/
raise ValueError(str(err) + ' - ' + err.output)
ValueError: Command '['stty', 'size']' returned non-zero exit status 1 - stty: standard input: Invalid argument
=======
ERROR: test_dry_run ...
- 82. By Francesco Banconi
-
get_terminal_width does not raise an error when the terminal is not found.
- 83. By Francesco Banconi
-
Checkpoint
Francesco Banconi (frankban) : | # |
Preview Diff
1 | === modified file 'lpsetup/argparser.py' |
2 | --- lpsetup/argparser.py 2012-06-27 12:00:03 +0000 |
3 | +++ lpsetup/argparser.py 2012-07-31 08:52:19 +0000 |
4 | @@ -17,6 +17,10 @@ |
5 | import sys |
6 | |
7 | from lpsetup.exceptions import ValidationError |
8 | +from lpsetup.utils import ( |
9 | + confirm, |
10 | + get_step_description, |
11 | + ) |
12 | |
13 | |
14 | class ArgumentParser(argparse.ArgumentParser): |
15 | @@ -106,12 +110,19 @@ |
16 | dest = action.dest |
17 | option_strings = action.option_strings |
18 | value = getattr(namespace, dest, None) |
19 | + isbool = isinstance(value, bool) |
20 | + # If the value is boolean and the action is 'store_false', we |
21 | + # invert the value. This way the following `if value:` block |
22 | + # is executed if the original value is False, and the argument |
23 | + # is correctly added. |
24 | + if isbool and isinstance(action, argparse._StoreFalseAction): |
25 | + value = not value |
26 | if value: |
27 | if option_strings: |
28 | args.append(option_strings[0]) |
29 | if isinstance(value, list): |
30 | args.extend(value) |
31 | - elif not isinstance(value, bool): |
32 | + elif not isbool: |
33 | args.append(value) |
34 | return args |
35 | |
36 | @@ -332,6 +343,7 @@ |
37 | ... trace.append('step2 received {0} and {1}'.format(foo, bar)) |
38 | |
39 | >>> class SubCommand(StepsBasedSubCommand): |
40 | + ... has_interactive_run = False |
41 | ... steps = ( |
42 | ... (step1, 'foo'), |
43 | ... (step2, 'foo', 'bar'), |
44 | @@ -419,9 +431,11 @@ |
45 | """ |
46 | |
47 | steps = () |
48 | + has_interactive_run = True |
49 | |
50 | def add_arguments(self, parser): |
51 | super(StepsBasedSubCommand, self).add_arguments(parser) |
52 | + # Add steps related arguments. |
53 | step_names = [self._get_step_name(i[0]) for i in self.steps] |
54 | parser.add_argument( |
55 | '-s', '--steps', nargs='+', choices=step_names, |
56 | @@ -429,6 +443,23 @@ |
57 | parser.add_argument( |
58 | '--skip-steps', nargs='+', choices=step_names, |
59 | help='Skip one or more internal functions.') |
60 | + # Add developer interaction related arguments. |
61 | + if self.has_interactive_run: |
62 | + parser.add_argument( |
63 | + '-y', '--yes', action='store_false', dest='interactive', |
64 | + help='Assume yes to all queries.') |
65 | + parser.add_argument( |
66 | + '--dry-run', action='store_true', help='Dry run.') |
67 | + |
68 | + def init_namespace(self, namespace): |
69 | + """Override to add *namespace.interactive*. |
70 | + |
71 | + This is done to ensure *interactive* is always present as an |
72 | + attribute of *namespace*, even if self.has_interactive_run is False. |
73 | + """ |
74 | + super(StepsBasedSubCommand, self).init_namespace(namespace) |
75 | + if not self.has_interactive_run: |
76 | + namespace.interactive = False |
77 | |
78 | def _get_step_name(self, step): |
79 | """Return the string representation of a step callable. |
80 | @@ -487,9 +518,34 @@ |
81 | """Default callable used to run a `step`, using given `args`.""" |
82 | return step(*args) |
83 | |
84 | + def get_steps_description(self, namespace, steps): |
85 | + """Retrieve steps' descriptions from the given *steps*. |
86 | + |
87 | + Return a string containing all the descriptions. |
88 | + """ |
89 | + context = namespace.__dict__ |
90 | + descriptions = [get_step_description(i[1], **context) for i in steps] |
91 | + return '\n'.join(filter(None, descriptions)) |
92 | + |
93 | def run(self, namespace): |
94 | + steps = self.get_steps(namespace) |
95 | + if namespace.dry_run or namespace.interactive: |
96 | + # Collect and display the description of each step. |
97 | + description = self.get_steps_description(namespace, steps) |
98 | + if description: |
99 | + print 'This command will perform the following actions:\n' |
100 | + print description + '\n' |
101 | + # Quit without errors if this is a dry run. |
102 | + if namespace.dry_run: |
103 | + return |
104 | + # If this is not a dry run, then it is an interactive one. |
105 | + # Prompt the user for confirmation to proceed and quit if |
106 | + # requested (in this case with exit code 1). |
107 | + if not confirm(): |
108 | + return 1 |
109 | + # Execute all the steps. |
110 | default_step_runner = self._call_step |
111 | - for step_name, step, args in self.get_steps(namespace): |
112 | + for step_name, step, args in steps: |
113 | # Run the step using a dynamic dispatcher. |
114 | step_runner = getattr( |
115 | self, 'call_' + step_name, default_step_runner) |
116 | |
117 | === modified file 'lpsetup/handlers.py' |
118 | --- lpsetup/handlers.py 2012-07-23 15:01:16 +0000 |
119 | +++ lpsetup/handlers.py 2012-07-31 08:52:19 +0000 |
120 | @@ -280,6 +280,7 @@ |
121 | if getattr(namespace, 'testing', False): |
122 | namespace.create_scripts = True |
123 | namespace.install_subunit = True |
124 | + namespace.interactive = False |
125 | namespace.no_checkout = True |
126 | namespace.stop_lxc = True |
127 | namespace.use_http = True |
128 | |
129 | === modified file 'lpsetup/subcommands/finish_inithost.py' |
130 | --- lpsetup/subcommands/finish_inithost.py 2012-07-20 10:40:43 +0000 |
131 | +++ lpsetup/subcommands/finish_inithost.py 2012-07-31 08:52:19 +0000 |
132 | @@ -53,6 +53,11 @@ |
133 | pwd_database = pwd.getpwnam(user) |
134 | os.chown('/srv/launchpad.dev/', pwd_database.pw_uid, pwd_database.pw_gid) |
135 | |
136 | +setup_launchpad.description = """Set up the Launchpad database: this will \ |
137 | + destroy any other Postgres db! |
138 | + Make and install Launchpad (buildout dependencies, Apache configuration). |
139 | +""" |
140 | + |
141 | |
142 | class SubCommand(argparser.StepsBasedSubCommand): |
143 | """Finish the initialization of a Launchpad development host. |
144 | |
145 | === modified file 'lpsetup/subcommands/inithost.py' |
146 | --- lpsetup/subcommands/inithost.py 2012-07-24 15:27:08 +0000 |
147 | +++ lpsetup/subcommands/inithost.py 2012-07-31 08:52:19 +0000 |
148 | @@ -140,6 +140,11 @@ |
149 | if not user_exists(user): |
150 | call('useradd', '-m', '-s', '/bin/bash', '-U', user) |
151 | |
152 | +initialize_base.description = """Update your system and install necessary \ |
153 | + deb packages ({packages}). |
154 | + Create the user $user if it does not exist. |
155 | +""".format(packages=' '.join(BASE_PACKAGES)) |
156 | + |
157 | |
158 | def initialize(user): |
159 | """Initialize host machine.""" |
160 | @@ -170,6 +175,12 @@ |
161 | for line in lines: |
162 | file_append(HOSTS_FILE, line) |
163 | |
164 | +initialize.description = initialize_base.description + """ Create Apache \ |
165 | + document roots for launchpad and enable required Apache \ |
166 | + modules ({modules}). |
167 | + Set up hosts file for Launchpad ({hosts}). |
168 | +""".format(modules=LP_APACHE_MODULES, hosts=HOSTS_FILE) |
169 | + |
170 | |
171 | def setup_home( |
172 | user, full_name, email, lpuser, private_key, public_key, valid_ssh_keys, |
173 | @@ -196,6 +207,14 @@ |
174 | if valid_ssh_keys: |
175 | subprocess.call(['bzr', 'launchpad-login', lpuser]) |
176 | |
177 | +setup_home.description = """Set up the user's ssh directory ($home_dir/.ssh). |
178 | + Create, if it does not exist, the ssh key $ssh_key_path and authorize \ |
179 | + this key for the user $user. |
180 | + Add bazaar.launchpad.net to known hosts. |
181 | + Set up bazaar authentication: $full_name <$email>. |
182 | + Set up Launchpad user id: $lpuser. |
183 | +""" |
184 | + |
185 | |
186 | def initialize_lxc(): |
187 | """Initialize LXC container. |
188 | @@ -223,6 +242,8 @@ |
189 | make_backup(networking) |
190 | render_to_file('networking.conf', {}, networking) |
191 | |
192 | +initialize_lxc.description = None |
193 | + |
194 | |
195 | def get_distro(): |
196 | return run('lsb_release', '-cs').strip() |
197 | @@ -240,6 +261,10 @@ |
198 | # Install base and Launchpad deb packages. |
199 | apt_get_install(*LP_PACKAGES, LC_ALL='C', caller=call) |
200 | |
201 | +setup_apt.description = """Add required APT repositories and install \ |
202 | + Launchpad dependencies: {dependencies}. |
203 | +""".format(dependencies=' '.join(LP_PACKAGES)) |
204 | + |
205 | |
206 | class SubCommand(argparser.StepsBasedSubCommand): |
207 | """Prepare a machine to run Launchpad. May be an LXC container or not.""" |
208 | |
209 | === modified file 'lpsetup/subcommands/initlxc.py' |
210 | --- lpsetup/subcommands/initlxc.py 2012-07-24 15:34:58 +0000 |
211 | +++ lpsetup/subcommands/initlxc.py 2012-07-31 08:52:19 +0000 |
212 | @@ -64,6 +64,11 @@ |
213 | # entropy exhaustion during automated parallel tests. |
214 | apt_get_install('haveged', caller=call) |
215 | |
216 | +initialize.description = inithost.initialize_base.description + """Install \ |
217 | + haveged in order to fill /dev/random, avoiding entropy exhaustion \ |
218 | + during automated parallel tests. |
219 | +""" |
220 | + |
221 | |
222 | def create_lxc(lxc_name, lxc_arch, lxc_os, user, install_subunit): |
223 | """Create the LXC named `lxc_name` sharing `user` home directory. |
224 | @@ -121,17 +126,27 @@ |
225 | '/var/lib/lxc/{0}/fstab'.format(lxc_name), |
226 | 'none dev/shm tmpfs defaults 0 0\n') |
227 | |
228 | +create_lxc.description = """Create an LXC container named $lxc_name \ |
229 | + ($lxc_os $lxc_arch), bind mounting $home_dir, and disabling apparmor \ |
230 | + profiles for lxc so that we don't have problems installing Postgres. |
231 | + Allow root access to the container. |
232 | +""" |
233 | + |
234 | |
235 | def start_lxc(lxc_name): |
236 | """Start the lxc instance named `lxc_name`.""" |
237 | call('lxc-start', '-n', lxc_name, '-d') |
238 | |
239 | +start_lxc.description = 'Start the LXC container named $lxc_name.\n' |
240 | + |
241 | |
242 | def wait_for_lxc(lxc_name, ssh_key_path): |
243 | """Try to ssh into the LXC container named `lxc_name`.""" |
244 | retry_ssh = retry(subprocess.CalledProcessError)(ssh) |
245 | retry_ssh(lxc_name, 'true', key=ssh_key_path) |
246 | |
247 | +wait_for_lxc.description = None |
248 | + |
249 | |
250 | def install_lpsetup_in_lxc(lxc_name, ssh_key_path, lxc_os, |
251 | user, home_dir, lpsetup_branch=None): |
252 | @@ -188,16 +203,20 @@ |
253 | # must be done as root. |
254 | shutil.rmtree(dest) |
255 | |
256 | +install_lpsetup_in_lxc.description = None |
257 | + |
258 | |
259 | def inithost_in_lxc(lxc_name, ssh_key_path, user, email, full_name, lpuser, |
260 | private_key, public_key, ssh_key_name, home_dir): |
261 | """Prepare the Launchpad environment inside an LXC.""" |
262 | # Use ssh to call this script from inside the container. |
263 | - args = ['init-host', '-u', user, '-E', email, '-f', full_name, |
264 | + args = ['init-host', '--yes', '-u', user, '-E', email, '-f', full_name, |
265 | '-l', lpuser, '-S', ssh_key_name, '--skip-steps', 'setup_home'] |
266 | cmd = this_command(home_dir, args) |
267 | ssh(lxc_name, cmd, key=ssh_key_path) |
268 | |
269 | +inithost_in_lxc.description = 'Initialize the LXC instance $lxc_name.\n' |
270 | + |
271 | |
272 | def stop_lxc(lxc_name, ssh_key_path): |
273 | """Stop the lxc instance named `lxc_name`.""" |
274 | @@ -205,6 +224,8 @@ |
275 | if not lxc_stopped(lxc_name): |
276 | subprocess.call(['lxc-stop', '-n', lxc_name]) |
277 | |
278 | +stop_lxc.description = None |
279 | + |
280 | |
281 | class SubCommand(inithost.SubCommand): |
282 | """Create an LXC container suitable for later installing a Launchpad |
283 | |
284 | === modified file 'lpsetup/subcommands/initrepo.py' |
285 | --- lpsetup/subcommands/initrepo.py 2012-07-20 10:40:43 +0000 |
286 | +++ lpsetup/subcommands/initrepo.py 2012-07-31 08:52:19 +0000 |
287 | @@ -83,6 +83,10 @@ |
288 | msg = 'Error: unable to create the lightweight checkout: ' |
289 | raise exceptions.ExecutionError(msg + err.output) |
290 | |
291 | +fetch.description = """Set up a Launchpad repository in $repository, \ |
292 | + retrieving the source code from $source. |
293 | +""" |
294 | + |
295 | |
296 | def setup_bzr_locations( |
297 | lpuser, repository, branch_name, template=LP_BZR_LOCATIONS): |
298 | @@ -112,6 +116,11 @@ |
299 | f.write(get_file_header() + '\n') |
300 | parser.write(f) |
301 | |
302 | +setup_bzr_locations.description = """If bzr+ssh is used, update bazaar \ |
303 | + locations ($home_dir/.bazaar/locations.conf) to include repository \ |
304 | + $repository and branch $branch_name. |
305 | +""" |
306 | + |
307 | |
308 | class SubCommand(argparser.StepsBasedSubCommand): |
309 | """Get the Launchpad code and dependent source code.""" |
310 | |
311 | === modified file 'lpsetup/subcommands/install_lxc.py' |
312 | --- lpsetup/subcommands/install_lxc.py 2012-07-24 14:07:50 +0000 |
313 | +++ lpsetup/subcommands/install_lxc.py 2012-07-31 08:52:19 +0000 |
314 | @@ -69,6 +69,11 @@ |
315 | with open(procfile, 'w') as f: |
316 | f.write('0\n') |
317 | |
318 | +create_scripts.description = """If requested, create helper script \ |
319 | + /usr/locl/bin/lp-setup-*: they can be used to build Launchpad and \ |
320 | + start a parallel test run. |
321 | +""" |
322 | + |
323 | |
324 | def cmd_in_lxc(lxc_name, ssh_key_path, home_dir, args, as_user=None): |
325 | cmd = this_command(home_dir, args) |
326 | @@ -81,7 +86,7 @@ |
327 | args = [ |
328 | 'init-repo', '--source', source, |
329 | '--branch-name', branch_name, '--checkout-name', checkout_name, |
330 | - '--repository', repository, |
331 | + '--repository', repository, '--yes', |
332 | ] |
333 | if use_http: |
334 | args.append('--use-http') |
335 | @@ -89,6 +94,10 @@ |
336 | args.append('--no-checkout') |
337 | cmd_in_lxc(lxc_name, ssh_key_path, home_dir, args, as_user=user) |
338 | |
339 | +init_repo_in_lxc.description = """Initialize the Launchpad repository \ |
340 | + inside the LXC container $lxc_name. |
341 | +""" + initrepo.fetch.description + initrepo.setup_bzr_locations.description |
342 | + |
343 | |
344 | def update_in_lxc( |
345 | lxc_name, ssh_key_path, home_dir, user, external_path, |
346 | @@ -98,12 +107,21 @@ |
347 | args.append('--use-http') |
348 | cmd_in_lxc(lxc_name, ssh_key_path, home_dir, args, as_user=user) |
349 | |
350 | +update_in_lxc.description = ( |
351 | + update.initialize_directories.description + |
352 | + update.update_dependencies.description + |
353 | + update.update_tree.description) |
354 | + |
355 | |
356 | def finish_inithost_in_lxc( |
357 | lxc_name, ssh_key_path, home_dir, user, target_dir): |
358 | - args = ['finish-init-host', target_dir, '--user', user] |
359 | + args = ['finish-init-host', target_dir, '--user', user, '--yes'] |
360 | cmd_in_lxc(lxc_name, ssh_key_path, home_dir, args) |
361 | |
362 | +finish_inithost_in_lxc.description = """Set up the database, make and \ |
363 | + install Launchpad inside the LXC instance $lxc_name. |
364 | +""" |
365 | + |
366 | |
367 | class SubCommand(initlxc.SubCommand): |
368 | """Completely sets up an LXC environment with Launchpad using the sandbox |
369 | @@ -154,4 +172,4 @@ |
370 | parser.add_argument( |
371 | '--testing', action='store_true', |
372 | help='Alias for --create-scripts --install-subunit --no-checkout ' |
373 | - '--stop_lxc --use-http.') |
374 | + '--stop_lxc --use-http --yes.') |
375 | |
376 | === modified file 'lpsetup/subcommands/update.py' |
377 | --- lpsetup/subcommands/update.py 2012-07-20 10:40:43 +0000 |
378 | +++ lpsetup/subcommands/update.py 2012-07-31 08:52:19 +0000 |
379 | @@ -29,6 +29,10 @@ |
380 | for dir_ in ['eggs', 'yui', 'sourcecode']: |
381 | mkdirs(os.path.join(target_dir, external_path, dir_)) |
382 | |
383 | +initialize_directories.description = """Initialize the eggs, yui, and \ |
384 | + sourcecode directories in $target_dir. |
385 | +""" |
386 | + |
387 | |
388 | def update_dependencies(target_dir, external_path, use_http): |
389 | """Update the external dependencies.""" |
390 | @@ -54,12 +58,20 @@ |
391 | '--target', target_dir, |
392 | '--parent', external_path) |
393 | |
394 | +update_dependencies.description = """Update Launchpad external \ |
395 | + dependencies in $target_dir. |
396 | +""" |
397 | + |
398 | |
399 | def update_tree(target_dir): |
400 | """Update the tree at target_dir with the latest LP code.""" |
401 | with cd(target_dir): |
402 | run('bzr', 'pull') |
403 | |
404 | +update_tree.description = """Update the tree at $target_dir with the \ |
405 | + latest LP code. |
406 | +""" |
407 | + |
408 | |
409 | class SubCommand(argparser.StepsBasedSubCommand): |
410 | """Updates an existing Launchpad development environment. |
411 | @@ -67,6 +79,7 @@ |
412 | Gets new versions of Launchpad source and external sources. |
413 | """ |
414 | |
415 | + has_interactive_run = False |
416 | steps = ( |
417 | (initialize_directories, 'target_dir', 'external_path'), |
418 | (update_dependencies, 'target_dir', 'external_path', 'use_http'), |
419 | |
420 | === modified file 'lpsetup/tests/examples.py' |
421 | --- lpsetup/tests/examples.py 2012-06-27 09:20:50 +0000 |
422 | +++ lpsetup/tests/examples.py 2012-07-31 08:52:19 +0000 |
423 | @@ -25,9 +25,16 @@ |
424 | |
425 | def step2(foo, bar): |
426 | print 'step2 received {0} and {1}'.format(foo, bar) |
427 | + |
428 | step2.step_name = 'mystep' |
429 | |
430 | |
431 | +def step_with_description(foo): |
432 | + pass |
433 | + |
434 | +step_with_description.description = 'step description' |
435 | + |
436 | + |
437 | def bad_step(foo): |
438 | raise subprocess.CalledProcessError(1, 'command') |
439 | |
440 | @@ -48,6 +55,7 @@ |
441 | class StepsBasedSubCommand(argparser.StepsBasedSubCommand): |
442 | """An example steps based sub command.""" |
443 | |
444 | + has_interactive_run = False |
445 | steps = ( |
446 | (step1, 'foo'), |
447 | (step2, 'foo', 'bar'), |
448 | @@ -76,3 +84,10 @@ |
449 | print 'running step1 with {args} while bar is {bar}'.format( |
450 | args=','.join(args), bar=namespace.bar) |
451 | step(*args) |
452 | + |
453 | + |
454 | +class InteractiveStepsBasedSubCommand(StepsBasedSubCommand): |
455 | + """An example interactive steps based sub command.""" |
456 | + |
457 | + has_interactive_run = True |
458 | + steps = [(step_with_description, 'foo')] |
459 | |
460 | === modified file 'lpsetup/tests/integration/test_init_host.py' |
461 | --- lpsetup/tests/integration/test_init_host.py 2012-07-25 13:11:25 +0000 |
462 | +++ lpsetup/tests/integration/test_init_host.py 2012-07-31 08:52:19 +0000 |
463 | @@ -27,7 +27,7 @@ |
464 | # Since the most common scenario is to have bzr whoami setup, we do |
465 | # that instead of providing the arguments directly to lpsetup. |
466 | super(InitHostTest, self).do_test() |
467 | - self.on_remote('cd lpsetup; ./lp-setup init-host') |
468 | + self.on_remote('cd lpsetup; ./lp-setup init-host --yes') |
469 | |
470 | |
471 | if __name__ == '__main__': |
472 | |
473 | === modified file 'lpsetup/tests/integration/test_install_lxc.py' |
474 | --- lpsetup/tests/integration/test_install_lxc.py 2012-07-27 12:12:46 +0000 |
475 | +++ lpsetup/tests/integration/test_install_lxc.py 2012-07-31 08:52:19 +0000 |
476 | @@ -95,7 +95,7 @@ |
477 | lp_source = 'http://bazaar.launchpad.net/~yellow/launchpad/stub' |
478 | cmd = ( |
479 | 'lpsetup/lp-setup install-lxc --use-http ' |
480 | - '--source {} -B {} -r {}'.format( |
481 | + '--source {} -B {} -r {} --yes'.format( |
482 | lp_source, branch_location, self.repo)) |
483 | self.on_remote(cmd) |
484 | |
485 | |
486 | === modified file 'lpsetup/tests/subcommands/test_initrepo.py' |
487 | --- lpsetup/tests/subcommands/test_initrepo.py 2012-07-19 14:17:15 +0000 |
488 | +++ lpsetup/tests/subcommands/test_initrepo.py 2012-07-31 08:52:19 +0000 |
489 | @@ -96,6 +96,7 @@ |
490 | '--repository', self.repository, |
491 | '--branch-name', branch_name, |
492 | '--checkout-name', checkout_name, |
493 | + '--yes', |
494 | ) |
495 | self.branch = os.path.join(self.repository, branch_name) |
496 | self.checkout = os.path.join(self.repository, checkout_name) |
497 | |
498 | === modified file 'lpsetup/tests/subcommands/test_smoke.py' |
499 | --- lpsetup/tests/subcommands/test_smoke.py 2012-06-26 15:02:09 +0000 |
500 | +++ lpsetup/tests/subcommands/test_smoke.py 2012-07-31 08:52:19 +0000 |
501 | @@ -10,6 +10,11 @@ |
502 | subcommands, |
503 | ) |
504 | |
505 | +from lpsetup.tests.utils import ( |
506 | + capture_output, |
507 | + root_not_needed, |
508 | + ) |
509 | + |
510 | |
511 | class SmokeTest(unittest.TestCase): |
512 | """Tests which are intended to do a quick check for broken subcommands.""" |
513 | @@ -19,3 +24,26 @@ |
514 | # function and verify that a non-error exit code is returned. |
515 | for subcommand, callable in subcommands: |
516 | self.assertEqual(main([subcommand, '--help']), 0) |
517 | + |
518 | + def test_subcommand_smoke_via_dry_run(self): |
519 | + # Perform a basic smoke test by running each subcommand's dry run |
520 | + # and verify that a non-error exit code is returned. |
521 | + required_args = ['-f', 'Example User', '-E', 'email@example.com'] |
522 | + name_args_map = { |
523 | + 'finish-init-host': [], |
524 | + 'init-host': required_args, |
525 | + 'init-lxc': required_args, |
526 | + 'init-repo': [], |
527 | + 'install-lxc': required_args, |
528 | + 'update': [], |
529 | + } |
530 | + name_class_map = dict(subcommands) |
531 | + warning = 'This command will perform the following actions:' |
532 | + for name, args in name_args_map.items(): |
533 | + subcommand = name_class_map[name] |
534 | + with capture_output() as output: |
535 | + with root_not_needed(subcommand): |
536 | + retcode = main([name, '--dry-run'] + args) |
537 | + self.assertFalse(retcode) |
538 | + # Ensure sub command description is printed to stdout. |
539 | + self.assertIn(warning, output.getvalue()) |
540 | |
541 | === modified file 'lpsetup/tests/test_argparser.py' |
542 | --- lpsetup/tests/test_argparser.py 2012-06-27 09:20:50 +0000 |
543 | +++ lpsetup/tests/test_argparser.py 2012-07-31 08:52:19 +0000 |
544 | @@ -4,6 +4,7 @@ |
545 | |
546 | """Tests for the argparser module.""" |
547 | |
548 | +from contextlib import nested |
549 | import subprocess |
550 | import unittest |
551 | |
552 | @@ -13,6 +14,7 @@ |
553 | from lpsetup.tests.utils import ( |
554 | capture_error, |
555 | capture_output, |
556 | + RawInputReturning, |
557 | SubCommandTestMixin, |
558 | ) |
559 | |
560 | @@ -61,6 +63,27 @@ |
561 | args = self.parser.get_args_from_namespace(namespace) |
562 | self.assertSequenceEqual(['--foo', 'changed', 'spam'], args) |
563 | |
564 | + def test_args_from_namespace_with_multiple_values(self): |
565 | + # Ensure *get_args_from_namespace* correcty handles options |
566 | + # accepting multiple values. |
567 | + self.parser.add_argument('foo') |
568 | + self.parser.add_argument('--bar', nargs='+') |
569 | + namespace = self.parser.parse_args('foo --bar eggs spam'.split()) |
570 | + namespace.bar.append('another argument') |
571 | + args = self.parser.get_args_from_namespace(namespace) |
572 | + expected = ['foo', '--bar', 'eggs', 'spam', 'another argument'] |
573 | + self.assertSequenceEqual(expected, args) |
574 | + |
575 | + def test_args_from_namespace_with_boolean_values(self): |
576 | + # Ensure *get_args_from_namespace* correcty handles options |
577 | + # accepting boolean values. |
578 | + self.parser.add_argument('--foo', action='store_true') |
579 | + self.parser.add_argument('--bar', action='store_false') |
580 | + expected = ['--foo', '--bar'] |
581 | + namespace = self.parser.parse_args(expected) |
582 | + args = self.parser.get_args_from_namespace(namespace) |
583 | + self.assertSequenceEqual(expected, args) |
584 | + |
585 | def test_help_subcommand(self): |
586 | # Ensure the help sub command is added if other commands exist. |
587 | self.parser.register_subcommand('foo', self.get_sub_command()) |
588 | @@ -170,7 +193,9 @@ |
589 | def test_failing_step(self): |
590 | # Ensure the steps execution is stopped if a step raises |
591 | # `subprocess.CalledProcessError`. |
592 | - with self.assertRaises(subprocess.CalledProcessError): |
593 | + with nested( |
594 | + capture_output(), |
595 | + self.assertRaises(subprocess.CalledProcessError)): |
596 | self.parse_and_call_main('--foo', 'eggs') |
597 | |
598 | |
599 | @@ -189,3 +214,50 @@ |
600 | 'step2 received eggs and spam' |
601 | ] |
602 | self.check_output(expected, output) |
603 | + |
604 | + |
605 | +class InteractiveStepsBasedSubCommandTest( |
606 | + SubCommandTestMixin, unittest.TestCase): |
607 | + |
608 | + sub_command_class = examples.InteractiveStepsBasedSubCommand |
609 | + step_description = examples.step_with_description.description |
610 | + |
611 | + def test_command_description(self): |
612 | + # Ensure the command description is generated collecting steps' |
613 | + # descriptions. |
614 | + with capture_output() as output: |
615 | + with RawInputReturning('yes'): |
616 | + self.parse_and_call_main() |
617 | + self.assertIn(self.step_description, output.getvalue()) |
618 | + |
619 | + def test_interactive_execution_granted(self): |
620 | + # Ensure the command executes if the user confirms to proceed. |
621 | + with nested(capture_output(), RawInputReturning('yes')): |
622 | + retcode = self.parse_and_call_main() |
623 | + self.assertFalse(retcode) |
624 | + |
625 | + def test_interactive_execution_denied(self): |
626 | + # Ensure the command exits with an error if the user denies execution. |
627 | + with nested(capture_output(), RawInputReturning('no')): |
628 | + retcode = self.parse_and_call_main() |
629 | + self.assertEqual(1, retcode) |
630 | + |
631 | + def test_assume_yes(self): |
632 | + # Ensure confirmation is not asked if `--yes` is provided. |
633 | + with capture_output(): |
634 | + with RawInputReturning('') as cm: |
635 | + self.parse_and_call_main('--yes') |
636 | + self.assertEqual(0, cm.call_count) |
637 | + |
638 | + def test_dry_run(self): |
639 | + # Ensure a dry run is never interactive, exits without errors and |
640 | + # prints out the command description. |
641 | + with capture_output() as output: |
642 | + with RawInputReturning('') as cm: |
643 | + retcode = self.parse_and_call_main('--dry-run') |
644 | + # Confirm has not been called. |
645 | + self.assertEqual(0, cm.call_count) |
646 | + # The command exits without errors. |
647 | + self.assertFalse(retcode) |
648 | + # The command description is displayed. |
649 | + self.assertIn(self.step_description, output.getvalue()) |
650 | |
651 | === modified file 'lpsetup/tests/test_handlers.py' |
652 | --- lpsetup/tests/test_handlers.py 2012-07-23 15:01:16 +0000 |
653 | +++ lpsetup/tests/test_handlers.py 2012-07-31 08:52:19 +0000 |
654 | @@ -319,17 +319,21 @@ |
655 | |
656 | def test_true(self): |
657 | # Ensure aliased options are set to True if testing is True. |
658 | - namespace = argparse.Namespace(testing=True, **self.ctx) |
659 | + namespace = argparse.Namespace( |
660 | + testing=True, interactive=True, **self.ctx) |
661 | handle_testing(namespace) |
662 | for key in self.ctx: |
663 | self.assertTrue(getattr(namespace, key)) |
664 | + self.assertFalse(namespace.interactive) |
665 | |
666 | def test_false(self): |
667 | # Ensure no changes are made to aliased options if testing is False. |
668 | - namespace = argparse.Namespace(testing=False, **self.ctx) |
669 | + namespace = argparse.Namespace( |
670 | + testing=False, interactive=True, **self.ctx) |
671 | handle_testing(namespace) |
672 | for key, value in self.ctx.items(): |
673 | self.assertEqual(value, getattr(namespace, key)) |
674 | + self.assertTrue(namespace.interactive) |
675 | |
676 | |
677 | class HandleUserTest(HandlersTestMixin, unittest.TestCase): |
678 | |
679 | === modified file 'lpsetup/tests/test_utils.py' |
680 | --- lpsetup/tests/test_utils.py 2012-07-18 14:27:32 +0000 |
681 | +++ lpsetup/tests/test_utils.py 2012-07-31 08:52:19 +0000 |
682 | @@ -6,6 +6,7 @@ |
683 | |
684 | from datetime import datetime |
685 | import getpass |
686 | +import itertools |
687 | import os |
688 | import shutil |
689 | import sys |
690 | @@ -19,13 +20,20 @@ |
691 | LXC_LP_TEST_DIR_PATTERN, |
692 | LXC_NAME, |
693 | ) |
694 | +from lpsetup.tests.utils import ( |
695 | + capture_output, |
696 | + RawInputReturning, |
697 | + ) |
698 | from lpsetup.utils import ( |
699 | ConfigParser, |
700 | + confirm, |
701 | get_container_path, |
702 | get_file_header, |
703 | get_lxc_gateway, |
704 | get_network_interfaces, |
705 | get_running_containers, |
706 | + get_step_description, |
707 | + get_terminal_width, |
708 | render_to_file, |
709 | retry, |
710 | Scrubber, |
711 | @@ -48,6 +56,36 @@ |
712 | self.assertEqual('value2', items['option2:colon']) |
713 | |
714 | |
715 | +class ConfirmTest(unittest.TestCase): |
716 | + |
717 | + def test_yes(self): |
718 | + # Ensure *confirm* returns True if the response is 'yes'. |
719 | + for response in ('y', 'Y', 'yes', 'Yes'): |
720 | + with RawInputReturning(response) as cm: |
721 | + self.assertTrue(confirm()) |
722 | + self.assertEqual(1, cm.call_count) |
723 | + |
724 | + def test_no(self): |
725 | + # Ensure *confirm* returns False if the response is 'no'. |
726 | + for response in ('n', 'N', 'no', 'No'): |
727 | + with RawInputReturning(response) as cm: |
728 | + self.assertFalse(confirm()) |
729 | + self.assertEqual(1, cm.call_count) |
730 | + |
731 | + def test_default(self): |
732 | + # Ensure *confirm* returns False if no input is given. |
733 | + with RawInputReturning('') as cm: |
734 | + self.assertFalse(confirm()) |
735 | + self.assertEqual(1, cm.call_count) |
736 | + |
737 | + def test_wrong_input(self): |
738 | + # Ensure the question is asked again if the answer is wrong. |
739 | + with capture_output(): |
740 | + with RawInputReturning('Nope', 'Yep', 'y') as cm: |
741 | + self.assertTrue(confirm()) |
742 | + self.assertEqual(3, cm.call_count) |
743 | + |
744 | + |
745 | class GetContainerPathTest(unittest.TestCase): |
746 | |
747 | def test_root_path(self): |
748 | @@ -169,6 +207,85 @@ |
749 | self.assertRunning([], ['c1', 'c2', 'c3']) |
750 | |
751 | |
752 | +class GetStepDescriptionTest(unittest.TestCase): |
753 | + |
754 | + def assertStartsWith(self, prefix, content): |
755 | + """Assert that content starts with prefix.""" |
756 | + self.assertTrue( |
757 | + content.startswith(prefix), |
758 | + '%r does not start with %r' % (content, prefix)) |
759 | + |
760 | + def get_step(self, description=None): |
761 | + step = lambda: None |
762 | + step.description = description |
763 | + return step |
764 | + |
765 | + def test_with_context(self): |
766 | + # Ensure the description is correctly retrieved and formatted. |
767 | + step = self.get_step('This step will do $stuff.') |
768 | + description = get_step_description(step, stuff='nothing') |
769 | + self.assertEqual('* This step will do nothing.', description) |
770 | + |
771 | + def test_without_context(self): |
772 | + # The description can be still retrieved if no context is provided. |
773 | + original = 'This step will do $stuff.' |
774 | + description = get_step_description(self.get_step(original)) |
775 | + self.assertEqual('* ' + original, description) |
776 | + |
777 | + def test_without_placeholder(self): |
778 | + # Ensure an error is raised if a placeholder is missing. |
779 | + expected = 'This step will do $stuff.' |
780 | + with self.assertRaises(KeyError): |
781 | + get_step_description(self.get_step(expected), foo='bar') |
782 | + |
783 | + def test_missing_description(self): |
784 | + # Ensure an empty string is returned if the description is None. |
785 | + description = get_step_description(self.get_step()) |
786 | + self.assertEqual('', description) |
787 | + |
788 | + def test_no_description(self): |
789 | + # Ensure an AttrubuteError is raised if the description is not found. |
790 | + with self.assertRaises(AttributeError): |
791 | + get_step_description(lambda: None) |
792 | + |
793 | + def test_dedent(self): |
794 | + # Ensure the description is correctly dedented. |
795 | + original = """ |
796 | + Hi there! |
797 | + """ |
798 | + description = get_step_description(self.get_step(original)) |
799 | + self.assertEqual('* Hi there!', description) |
800 | + |
801 | + def test_empty_lines(self): |
802 | + # Ensure empty lines in description are removed. |
803 | + original = 'Hello.\n \nGoodbye.' |
804 | + description = get_step_description(self.get_step(original)) |
805 | + self.assertEqual('* Hello.\n* Goodbye.', description) |
806 | + |
807 | + def test_wrapping(self): |
808 | + # Ensure the description is correctly wrapped. |
809 | + width = get_terminal_width() |
810 | + elements = itertools.cycle('Lorem ipsum dolor sit amet.') |
811 | + original = ''.join(itertools.islice(elements, width + 1)) |
812 | + description = get_step_description(self.get_step(original)) |
813 | + lines = description.splitlines() |
814 | + self.assertEqual(2, len(lines)) |
815 | + first_line, second_line = lines |
816 | + self.assertStartsWith('* ', first_line) |
817 | + self.assertStartsWith(' ', second_line) |
818 | + |
819 | + |
820 | +class GetTerminalSizeTest(unittest.TestCase): |
821 | + |
822 | + def test_is_integer(self): |
823 | + # Ensure the returned value is an integer number. |
824 | + self.assertIsInstance(get_terminal_width(), int) |
825 | + |
826 | + def test_positive_values(self): |
827 | + # Ensure the returned value is greater than 0. |
828 | + self.assertGreater(get_terminal_width(), 0) |
829 | + |
830 | + |
831 | class RenderToFileTest(unittest.TestCase): |
832 | |
833 | def setUp(self): |
834 | |
835 | === modified file 'lpsetup/tests/utils.py' |
836 | --- lpsetup/tests/utils.py 2012-07-19 14:17:15 +0000 |
837 | +++ lpsetup/tests/utils.py 2012-07-31 08:52:19 +0000 |
838 | @@ -124,6 +124,51 @@ |
839 | lpuser is None, 'You need to set up a Launchpad login to run this test.') |
840 | |
841 | |
842 | +class RawInputReturning(object): |
843 | + """Mocks the *raw_input* builtin function. |
844 | + |
845 | + This context manager takes one or more pre-defined answers and |
846 | + keeps track of mocked *raw_input* call count. |
847 | + """ |
848 | + def __init__(self, *args): |
849 | + self.answers = iter(args) |
850 | + self._builtin = __import__('__builtin__') |
851 | + self.call_count = 0 |
852 | + self.original = self._builtin.raw_input |
853 | + |
854 | + def __enter__(self): |
855 | + self._builtin.raw_input = self.input |
856 | + return self |
857 | + |
858 | + def __exit__(self, exc_type, exc_val, exc_tb): |
859 | + self._builtin.raw_input = self.original |
860 | + |
861 | + def input(self, question): |
862 | + self.call_count += 1 |
863 | + return self.answers.next() |
864 | + |
865 | + |
866 | +@contextmanager |
867 | +def root_not_needed(subcommand): |
868 | + """Temporarily set to False the *needs_root* flag of *subcommand*.""" |
869 | + original, subcommand.needs_root = subcommand.needs_root, False |
870 | + try: |
871 | + yield |
872 | + finally: |
873 | + subcommand.needs_root = original |
874 | + |
875 | + |
876 | +class RootNotNeededTest(unittest.TestCase): |
877 | + |
878 | + def test_context_manager(self): |
879 | + # Ensure the context manager temporarily sets to False *needs_root*. |
880 | + subcommand = type( |
881 | + 'SubCommand', (argparser.BaseSubCommand,), {'needs_root': True}) |
882 | + with root_not_needed(subcommand): |
883 | + self.assertFalse(subcommand.needs_root) |
884 | + self.assertTrue(subcommand.needs_root) |
885 | + |
886 | + |
887 | class SubCommandTestMixin(object): |
888 | |
889 | sub_command_class = examples.SubCommand |
890 | |
891 | === modified file 'lpsetup/utils.py' |
892 | --- lpsetup/utils.py 2012-07-24 14:07:50 +0000 |
893 | +++ lpsetup/utils.py 2012-07-31 08:52:19 +0000 |
894 | @@ -7,11 +7,14 @@ |
895 | __metaclass__ = type |
896 | __all__ = [ |
897 | 'call', |
898 | + 'confirm', |
899 | 'get_container_path', |
900 | 'get_file_header', |
901 | 'get_lxc_gateway', |
902 | 'get_network_interfaces', |
903 | 'get_running_containers', |
904 | + 'get_step_description', |
905 | + 'get_terminal_width', |
906 | 'lxc_in_state', |
907 | 'lxc_ip', |
908 | 'lxc_stopped', |
909 | @@ -35,6 +38,7 @@ |
910 | import re |
911 | import subprocess |
912 | import shutil |
913 | +import string |
914 | import sys |
915 | import textwrap |
916 | import time |
917 | @@ -79,6 +83,23 @@ |
918 | ) |
919 | |
920 | |
921 | +def confirm(): |
922 | + """Ask a yes/no confirmation to proceed. |
923 | + |
924 | + Return True if the answer is 'yes', False otherwise. |
925 | + When the user presses Enter without typing anything, False is assumed. |
926 | + """ |
927 | + while True: |
928 | + response = raw_input('Do you want to proceed? [y/N] ').lower() |
929 | + if not response: |
930 | + return False |
931 | + if response in ('y', 'yes'): |
932 | + return True |
933 | + if response in ('n', 'no'): |
934 | + return False |
935 | + print("I didn't understand you. Please specify '(y)es' or'(n)o'.") |
936 | + |
937 | + |
938 | def get_container_path(lxc_name, path='', base_path=LXC_PATH): |
939 | """Return the path of LXC container called `lxc_name`. |
940 | |
941 | @@ -148,6 +169,45 @@ |
942 | visited[container] = 1 |
943 | |
944 | |
945 | +def get_step_description(step, **kwargs): |
946 | + """Retrieve and format step description from the given *step* callable. |
947 | + |
948 | + *kwargs*, if provided, will be used as context to format the description. |
949 | + Formatting is done using the Python's builtin templating system |
950 | + *string.Template* supporting $-based substitutions. |
951 | + |
952 | + If placeholders are missing from *kwargs* an error will be raised. |
953 | + """ |
954 | + description = step.description |
955 | + if not description: |
956 | + # This step has no description, nothing else to do. |
957 | + return '' |
958 | + if kwargs: |
959 | + s = string.Template(description) |
960 | + description = s.substitute(**kwargs) |
961 | + # Remove multiple spaces from lines. |
962 | + lines = [' '.join(line.split()) for line in description.splitlines()] |
963 | + # Retrieve all the non empty lines. |
964 | + lines = filter(None, lines) |
965 | + # For each line, wrap the contents. Note that we can't wrap the text of |
966 | + # the entire paragraph because we want to preserve existing new lines. |
967 | + width = get_terminal_width() |
968 | + return '\n'.join(textwrap.fill( |
969 | + line, width=width, initial_indent='* ', subsequent_indent=' ') |
970 | + for line in lines) |
971 | + |
972 | + |
973 | +def get_terminal_width(): |
974 | + """Return the terminal width (number of columns).""" |
975 | + try: |
976 | + width = run('stty', 'size').split()[1] |
977 | + except subprocess.CalledProcessError: |
978 | + # No input available for stty, falling back to $COLUMNS. |
979 | + # If $COLUMNS is not exported, return a default value. |
980 | + width = os.getenv('COLUMNS', 79) |
981 | + return int(width) |
982 | + |
983 | + |
984 | def lxc_in_state(state, lxc_name, timeout=30): |
985 | """Return True if the LXC named `lxc_name` is in state `state`. |
986 |
I like the dry run and description functionality.
The use of $templates for the descriptions was a good choice. It keeps
them simpler than having to have a function for each, but provides the
dynamism we need to be able to include runtime values.
We might want an assertion that no un-replaced substitutions are left in
the description after substitution takes place. I wouldn't be surprised
if we typo a name now and then and it would keep those from persisting
very long.
The careful indention of description strings only for them to be
dedented in the future seems odd. This:
fetch.description = """Set up a Launchpad repository in $repository,
retrieving the source code from $source."""
or this:
fetch.description = ('Set up a Launchpad repository in $repository,'
'retrieving the source code from $source.')
better matches prevailing Python style than this:
fetch.description = """
Set up a Launchpad repository in $repository,
retrieving the source code from $source.
"""
I don't feel strongly about it, but normally see dry-run options spelled
"--dry-run" I don't recall seeing one spelled "--dry".
Having the namespace attribute as "dry" is a bit terse. Spelling it
"dry_run" would help when reading the code.
I'm surprised that the bit about inverting the boolean if store_false is argparser. py, line 19 of the diff) is necessary. I
used (in lpsetup/
didn't look deeply enough into it to understand why, but I just wanted
to point it out in case it made you realize anything.
Having to do self.is_ interactive( namespace) instead of something like is_interactive is unfortunate. I think I see why: because not
namespace.
all commands have the option, so sometimes the attribute won't be there.
I wonder if it would be better to always have the attribute available,
even if it can not be true would be better.
The fact that steps can have empty descriptions feels like a trap.
Perhaps it should be an error for that to happen.
The confirm function is never called with a question other than "Do you
want to proceed?" and the "suffix" parameter is never used so it could
be simplified. Some tests would go away too.
It feels like there should be some whitespace between the step function
definitions and the description attribute assignment for the functions.
I don't know if the pep8 formatting checker will warn about them or not.
The way descriptions are presented is a little hard to read. Here is
some output from my system:
Update your system and install necessary deb packages (ssh bzr .ssh/id_ rsa. launchpad. net to known hosts. database- dependencies- 9.1 launchpad- developer- depe...
apache2.2-common).
Create the user benji if it does not exist.
Create Apache document roots for launchpad and enable required Apache
modules (proxy proxy_http rewrite ssl deflate headers).
Set up hosts file for Launchpad (/etc/hosts).
Set up the user's ssh directory (/home/benji/.ssh).
Create, if it does not exist, the ssh key /home/benji/
Authorize this key for the user benji.
Add bazaar.
Set up bazaar authentication: Benji York <email address hidden>.
Set up Launchpad user id: benji.
Add required APT repositories and install Launchpad dependencies:
launchpad-