Merge lp:~frankban/lpsetup/out-of-control into lp:lpsetup
- out-of-control
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Francesco Banconi |
Approved revision: | 96 |
Merged at revision: | 71 |
Proposed branch: | lp:~frankban/lpsetup/out-of-control |
Merge into: | lp:lpsetup |
Diff against target: |
3094 lines (+1356/-1056) 24 files modified
lpsetup/__init__.py (+1/-1) lpsetup/argparser.py (+338/-427) lpsetup/cli.py (+137/-19) lpsetup/handlers.py (+1/-1) lpsetup/subcommands/finish_inithost.py (+2/-5) lpsetup/subcommands/inithost.py (+14/-16) lpsetup/subcommands/initlxc.py (+20/-22) lpsetup/subcommands/initrepo.py (+11/-12) lpsetup/subcommands/install_lxc.py (+14/-17) lpsetup/subcommands/update.py (+6/-3) lpsetup/tests/examples.py (+0/-93) lpsetup/tests/subcommands/test_finish_inithost.py (+2/-2) lpsetup/tests/subcommands/test_inithost.py (+3/-7) lpsetup/tests/subcommands/test_initlxc.py (+2/-2) lpsetup/tests/subcommands/test_initrepo.py (+1/-2) lpsetup/tests/subcommands/test_install_lxc.py (+2/-2) lpsetup/tests/subcommands/test_smoke.py (+7/-13) lpsetup/tests/subcommands/test_update.py (+1/-2) lpsetup/tests/subcommands/test_version.py (+17/-15) lpsetup/tests/test_argparser.py (+310/-212) lpsetup/tests/test_cli.py (+295/-0) lpsetup/tests/test_utils.py (+11/-73) lpsetup/tests/utils.py (+144/-73) lpsetup/utils.py (+17/-37) |
To merge this branch: | bzr merge lp:~frankban/lpsetup/out-of-control |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Brad Crittenden (community) | code | Abstain | |
Review via email: mp+118982@code.launchpad.net |
Commit message
Argparser/
Description of the change
= Summary =
This branch is the result of a previous experimental branch. The goal is to reduce the inversion of control taking place in the argparser/
Before:
We had a customized *ArgumentParser* being able to register subcommands.
Subcommands were subclasses of *BaseSubCommand*, registered using
*ArgumentPa
framework took care of the process of validating, restarting as root and
actual running each sub command.
In this branch:
The *ArgumentParser* is more or less a default *argparse.
The only change there is a method override done to store actions in
a "public" attribute. Subcommands are just objects implementing a pre-defined
protocol, that is described in the branch. So subcommands can be instances,
or just modules.
The process of registering and running a subcommand is now handled by functions
in `cli.py`, and the code flow should be easier to follow because several
actions are now executed in a procedural way: prepare the parser, add common
arguments, add user-interaction related arguments, restart as root, namespace
initialization and validation, ...
However, the inversion of control is still present, but it's moved forward to
the point of executing actual subcommand stuff: i.e. when *subcommand.run* is
called. As a consequence, while the subcommand base classes are still there,
they are now more lightweight, and they represent only a flexible and easy way
to implement the subcommand protocol. They still introduce and handle the
concepts of handlers and steps, which are more well-documented in the code.
If we will decide in the future to follow the path of further reducing the inversion
of control (e.g. getting rid of subcommands base classes), this branch could be
considered as an incremental step.
For now, my opinion is that this could be a good trade off between the need for simplicity
and the generality/
This branch adds documentation and massively reorganizes code: methods are now
functions, tests have been changed accordingly, some unused modules have been removed,
etc... so the diff is HUGE, and I am very sorry.
The integration test `test_install_
== Other changes ==
Introduced a basic documentation for subcommands.
Modified *ArgumentParser* and base subcommand classes as described above. Also
removed no longer valid doctests. The code exercised by removed doctests is fully
covered by unittests.
*subcommand.
descriptions in a generic way: steps based subcommands override that method so that
the resulting description is created collecting steps' descriptions.
The `argparser` module now defines several functions explicitly called by cli:
*add_common_
The `cli `module itself now contains some helper functions:
- *get_parse*r returns an argument parser with all subcommands registered
- *prepare_parser* takes a parser (or a subparser) and a subcommand, adds common
arguments to the parsers and registers a callback pointing to the subcommand.
Note that the callback is a closure: that way we can immediately regain the
control of the code flow. Also note that this function can be used passing an
arbitrary parser. Usually a subparser is passed, but providing a normal
*
separate subcommands (e.g. `lp-init-host` or `lp-init-repo`).
- *run*: taking care of:
- namespace initialization and validation
- interactive arguments handling
- executing a dry run, if requested
- restarting the same sub command as root, if required
- actual sub command execution
Removed the subcommand dynamic dispatcher: RIP.
This is how we usually define steps::
steps = [(step1, 'arg1', 'arg2'), (step2...)]
In this branch:
steps = [(step1, ['arg1', 'arg2']), (step2, []...)]
So the second item of each sequence is now a sequence of arg names, and a third
optional element is also allowed: a filter function that takes a namespace and must
return True if the step can be executed, False otherwise. As a side note, thanks
to this change we can now be more precise when collecting steps' descriptions.
Updated all the subcommands accordingly.
s/needs_
Removed the `lpsetup.
Now we have two factory test mixins in `lpsetup.
steps or subcommands when needed in tests.
Added a new context manager (*call_
This is used to test the "restart as root" functions.
Removed the *root_not_needed* context manager: it was only used in dry run smoke tests:
now the dry execution does not want to restart as root, because it is handled before
`sudo` is called.
Removed no longer used *SubCommandTest
the latter can be safely removed once we get rid of the copy/paste smoke tests.
Moved *get_step_
Bumped version up a little.
- 85. By Francesco Banconi
-
s/dry execution/dry run execution/
- 86. By Francesco Banconi
-
s/sub command/subcommand/
- 87. By Francesco Banconi
-
Fixes to get_terminal_width.
Francesco Banconi (frankban) wrote : | # |
Thanks for your comments Brad, replies follow.
> * This change increases our LOC from 5948 to 6243 (excluding
> distribute_
Yes, we have an increase of ~200 LOC in tests.
> * s/dry execution/dry run execution/
Fixed.
> * Please pick one of "subcommand" or "sub command" and standardize. I prefer
> "subcommand".
Right, done.
> * Running within an Emacs shell (useful for Emacs users to run pdb) results
> in:
>
> ]0;sapa:
> update --dry-run
> Traceback (most recent call last):
> ValueError: invalid width 0 (must be > 0)
It seems that emacs sets the terminal width to 0. I've changed `utils.
handle this case. I've spotted another error in that function: in same cases it was possible to crash
the program setting an invalid $COLUMNS. Fixed.
> * Why do needs_root and get_handlers take a namespace? It is not used. YAGNI?
You are right, I think they were used (or at least needs_root) in some old revision.
If we agree to drop the namespace, we could also eliminate needs_root as a callable and just use
root_required.
> * I'm glad to see some methods on BaseSubCommand that never should have been
> methods (those which never reference 'self') have been moved to functions.
>
> The approach you've taken here seems quite sane. You made the obvious choices
> but didn't go to the point where de-factoring (I made that up!) would cause
> you to have to violate DRY principles.
>
> Honestly, though, as an experiment I find it to be too complicated to be
> compelling. A 3,000 line diff is not something that can be easily digested
> and so I find it hard to find the lessons in the work. I'm glad you did it,
> and I hope you learned along the way, but this branch is not one that we as a
> group can look at as a guiding example. You've done nothing wrong but were
> super ambitious.
If the code seems complicated, than the experiment can be considered a glorious fail!
If you mean the diff is hard to follow:
I've tried to minimize the effort (and the diff) of this experiment, and I agree
with you, 3000 lines are too much. However, I don't know if this is the result of me
wanting to do too many things at once, or just a consequence of the goal of this branch:
I had to move a lot of code and tests to convert a class based framework to be function
based.
Benji York (benji) wrote : | # |
This branch is very nice.
I was worried by Brad's observation that the resulting code was longer
than the original so I investigated a little. It turns out that there
were several tests added during the refactoring (which is unsurprising).
Excluding test additions the amount of code stayed basically the same (a
total reduction of 24 LOC).
The best way I could come up with writing a review was to intersperse my
comments with the diff. For easy searching, my comments are marked with
the string "~~". For this comment I have trimmed large parts of the
diff, but the full version with my inline comments can be found at
http://
Several of the comments/
improvements, so feel free to take or leave those as you wish.
=== modified file 'lpsetup/
--- lpsetup/
+++ lpsetup/
@@ -2,25 +2,158 @@
# Copyright 2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Argparse wrapper to add support for sub commands."""
+"""Helpers and functions to create subcommands.
+
+
+= Subcommands =
+
+A subcommand can be any object implementing the following protocol:
+
+ Required names:
+
+ - help: the command line help of the subcommand.
+
+ - get_description
+ and must return the description of the subcommand as a string that
+ will be printed during interactive or dry run execution.
+ Note that this name must only be defined if the subcommand supports
+ interactive or dry run (see below in "Optional names").
+
+ - needs_root(
+ requires the user to be root, False otherwise.
+
+ - run(namespace): a callable actually executing the subcommand.
+
+ Optional names:
+
+ - add_arguments(
+ add subcommand specific arguments to that parser.
+
+ - has_dry_run: True if the subcommand supports dry run execution,
+ False otherwise. If not defined, False is considered.
~~ Are there any commands that don't support dry-run? If so, could they be
made to in some trivial way so that some of the dry-run logic could be removed?
+== BaseSubCommand ==
+
+This base class implements:
+
+ - help: as an empty string.
~~ Would None be a better "nothing" value?
+ - prepare_
+
+ This base subcommand introduces the concept of handlers. Handlers are
+ callables receiving the current namespace. They can manipulate the
+ namespace adding or changing names. They can also validate the
+ namespace, e.g. when the validation depends on multiple args.
+ If something is wrong with the arguments, a handler should raise a
+ *ValidationError*.
+
+ Subcommands can store handlers in the *handlers* attribute as a
+ sequence, or can dynamically set up handlers overriding the
+ *get_handlers* method.
~~ Do we really need two ...
Benji York (benji) wrote : | # |
A comment from Gary made me realize that get_terminal_width should really check stty first (it seems to be most likely to be right, if it returns anything at all) and then COLUMNS and then default.
How does that sound?
- 88. By Francesco Banconi
-
Removed the YAGNI needs_root. Now only root_required, if defined, is taken into consideration.
- 89. By Francesco Banconi
-
Help is None.
- 90. By Francesco Banconi
-
Help as None in the description.
- 91. By Francesco Banconi
-
Improved handlers description.
- 92. By Francesco Banconi
-
BaseSubCommand.
get_description returns an empty string. - 93. By Francesco Banconi
-
Improved namespace handling description.
- 94. By Francesco Banconi
-
Fixed steps description.
- 95. By Francesco Banconi
-
Simplified resolve_steps.
Francesco Banconi (frankban) wrote : | # |
Thanks for this great review Benji.
> ~~ Are there any commands that don't support dry-run? If so, could they be
> made to in some trivial way so that some of the dry-run logic could be
> removed?
The only subcommand not supporting dry run is `version`. It's not diffcult to add a version subcommand in `cli.get_parser` (in the same way `help` is already added). Doing that, we can assume all the subcommands support dry run and totally remove the `has_dry_run` logic.
I am inclined to create another card for this change, do you agree?
> + - help: as an empty string.
>
> ~~ Would None be a better "nothing" value?
Fixed.
>
> + - prepare_
> +
> + This base subcommand introduces the concept of handlers. Handlers are
> + callables receiving the current namespace. They can manipulate the
> + namespace adding or changing names. They can also validate the
> + namespace, e.g. when the validation depends on multiple args.
> + If something is wrong with the arguments, a handler should raise a
> + *ValidationError*.
> +
> + Subcommands can store handlers in the *handlers* attribute as a
> + sequence, or can dynamically set up handlers overriding the
> + *get_handlers* method.
>
> ~~ Do we really need two ways? Which gets priority? How do they interact?
By default, get_handlers returns self.handlers, so get_handlers gets priority.
Subcommands can define handlers or override get_handlers if required (e.g.: when handlers must be dynamically resolved).
> Is "get_handlers" part of the subcommand protocol? Should it be defined
> above?
No, it is something introduced by BaseSubCommand. The protocol does not know anything about handlers. BaseSubCommand implements prepare_namespace (part of the protocol) to make use of handlers. I improved the docstring to better describe the relation between handlers and get_handlers.
> + - needs_root(
> +
> + returning *subcommand.
> + This way subcommands can just set up the *root_required* attribute
> + if they don't need to calculate that value dynamically.
>
> ~~ Defining a method that returns a constant is pretty easy too. Maybe we
> should just have the method.
Brad, in his review, made me realize needs_root(
> + - get_description
>
> ~~ Similar to the above: should we just have the method and not an attribute
> too?
get_description returning help generates confusion. They have different pourposes: help is used in `lp-setup subcommand -h` (or `lp-setup help subcommand`); get_description is used to warn the user during interactive or dry run executions. I've changed the default implementation to just return an empty string: this way I hope to avoid the confusion.
> It is too bad the namespace is mutated as we go, otherwise we could pass it
> into the subcommand at construction time and then these could be properties
> instead.
The namespace...
- 96. By Francesco Banconi
-
Better description of get_terminal_width logic.
Preview Diff
1 | === modified file 'lpsetup/__init__.py' |
2 | --- lpsetup/__init__.py 2012-06-28 10:33:46 +0000 |
3 | +++ lpsetup/__init__.py 2012-08-13 12:44:19 +0000 |
4 | @@ -9,7 +9,7 @@ |
5 | 'get_version', |
6 | ] |
7 | |
8 | -VERSION = (0, 2, 2) |
9 | +VERSION = (0, 2, 3) |
10 | |
11 | |
12 | def get_version(): |
13 | |
14 | === modified file 'lpsetup/argparser.py' |
15 | --- lpsetup/argparser.py 2012-07-30 12:39:54 +0000 |
16 | +++ lpsetup/argparser.py 2012-08-13 12:44:19 +0000 |
17 | @@ -2,25 +2,158 @@ |
18 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
19 | # GNU Affero General Public License version 3 (see the file LICENSE). |
20 | |
21 | -"""Argparse wrapper to add support for sub commands.""" |
22 | +"""Helpers and functions to create subcommands. |
23 | + |
24 | + |
25 | += Subcommands = |
26 | + |
27 | +A subcommand can be any object implementing the following protocol: |
28 | + |
29 | + Required names: |
30 | + |
31 | + - help: the command line help of the subcommand. |
32 | + |
33 | + - get_description(namespace): a callable that receives a namespace |
34 | + and must return the description of the subcommand as a string that |
35 | + will be printed during interactive or dry run execution. |
36 | + Note that this name must only be defined if the subcommand supports |
37 | + interactive or dry run (see below in "Optional names"). |
38 | + Also note that the namespace is passed so that the callable can |
39 | + retrieve data from it, but the callable itself should not mutate the |
40 | + namespace. |
41 | + |
42 | + - run(namespace): a callable actually executing the subcommand. |
43 | + Again, the namespace should not be mutataed by this callable. |
44 | + |
45 | + Optional names: |
46 | + |
47 | + - add_arguments(parser): a callable that receives a parser and can |
48 | + add subcommand specific arguments to that parser. |
49 | + |
50 | + - has_dry_run: True if the subcommand supports dry run execution, |
51 | + False otherwise. If not defined, False is considered. |
52 | + |
53 | + - has_interactive_run: True if the subcommand supports interactive |
54 | + execution, False otherwise. If not defined, the subcommand is |
55 | + considered non-interactive. |
56 | + |
57 | + - prepare_namespace(namespace): a callable that receives a namespace |
58 | + object. A subcommand can define this callable to further initialize |
59 | + or validate the namespace as needed. A ValidationError can be |
60 | + raised by *prepare_namespace* to stop the subcommand execution. |
61 | + This is the only place where the namespace can be mutated. |
62 | + |
63 | + - root_required: True if the subcommand requires the user to be root, |
64 | + False otherwise. If not defined, False is considered. |
65 | + |
66 | +However, this module provides two classes that can be used to automate the |
67 | +process in several ways. |
68 | + |
69 | + |
70 | +== BaseSubCommand == |
71 | + |
72 | +This base class implements: |
73 | + |
74 | + - help: as None. |
75 | + |
76 | + - prepare_namespace(namespace): |
77 | + |
78 | + This base subcommand introduces the concept of handlers. Handlers are |
79 | + callables receiving the current namespace. They can manipulate the |
80 | + namespace adding or changing names. They can also validate the |
81 | + namespace, e.g. when the validation depends on multiple args. |
82 | + If something is wrong with the arguments, a handler should raise a |
83 | + *ValidationError*. |
84 | + |
85 | + Subcommands can store handlers in the *handlers* attribute as a |
86 | + sequence, or can dynamically set up handlers overriding the |
87 | + *get_handlers* method. By default, *get_handlers* returns |
88 | + *self.handlers*. |
89 | + |
90 | + In any case, the *prepare_namespace* implementation of this class |
91 | + calls the handlers in the order they are provided by *get_handlers*. |
92 | + |
93 | + - get_description(namespace): returning an empty string. |
94 | + |
95 | +All the required names in the subcommands protocol are implemented by |
96 | +*BaseSubCommand* but *run*, that must be overridden by subclasses to |
97 | +do actual stuff. |
98 | + |
99 | + |
100 | +== StepsBasedSubCommand == |
101 | + |
102 | +Instances of this class (subclass of *BaseSubCommand*) can take advantage of |
103 | +the integrated "steps" machinery. The *run* method is implemented to execute |
104 | +steps in order. The main advantage of basing a subcommand on steps is the |
105 | +possibility of executing only a subset of the subcommand, including or |
106 | +skipping some steps. This allows more granularity when the subcommand is run. |
107 | +Note that this base class is a subclass of *BaseSubCommand*. |
108 | +In particular, this base class implements: |
109 | + |
110 | + - steps: |
111 | + |
112 | + a sequence of *(callable, arg-names, filter-callable)* where: |
113 | + |
114 | + - callable is a callable step that will be run passing collected *args* |
115 | + - *arg-names* are the names of namespace attributes, used to retrieve |
116 | + the corresponding values and generate a tuple of *args* to be |
117 | + passed to the step |
118 | + - *filter-callable* is an OPTIONAL function that takes the namespace |
119 | + and returns True if the step must be run, False otherwise. |
120 | + If not provided, the step is run. |
121 | + |
122 | + Example: run *mystep*, passing *foo* and *bar* values, |
123 | + only if the namespace contains 'foo' and 'foo' is True:: |
124 | + |
125 | + def my_step(foo, bar): |
126 | + pass |
127 | + |
128 | + steps = [ |
129 | + (my_step, ['foo', 'bar'], |
130 | + lambda namespace: getattr(namespace, 'foo', False)) |
131 | + ] |
132 | + |
133 | + - add_arguments(parser): |
134 | + |
135 | + adds `--steps` and `--skip-steps` arguments to the parser. They can |
136 | + be used to include or exclude steps during subcommand's execution. |
137 | + |
138 | + - get_description(namespace): |
139 | + |
140 | + collects steps' descriptions so that the subcommand description |
141 | + is based on the descriptions of its steps. |
142 | + |
143 | + - run(namespace): |
144 | + |
145 | + implemented to resolve the steps to be run and to invoke them |
146 | + in the order they are provided. |
147 | + |
148 | + - has_dry_run and has_interactive_run are also set to True because usually |
149 | + a steps based subcommand supports interactive and dry run execution. |
150 | +""" |
151 | |
152 | __metaclass__ = type |
153 | __all__ = [ |
154 | - 'StepsBasedSubCommand', |
155 | + 'add_common_arguments', |
156 | 'ArgumentParser', |
157 | 'BaseSubCommand', |
158 | + 'get_args_from_namespace', |
159 | + 'get_step_description', |
160 | + 'get_step_name', |
161 | + 'init_namespace', |
162 | + 'resolve_steps', |
163 | + 'restart_as_root', |
164 | + 'StepsBasedSubCommand', |
165 | ] |
166 | |
167 | import argparse |
168 | import os |
169 | +import string |
170 | import subprocess |
171 | import sys |
172 | +import textwrap |
173 | |
174 | -from lpsetup.exceptions import ValidationError |
175 | -from lpsetup.utils import ( |
176 | - confirm, |
177 | - get_step_description, |
178 | - ) |
179 | +from lpsetup.utils import get_terminal_width |
180 | |
181 | |
182 | class ArgumentParser(argparse.ArgumentParser): |
183 | @@ -28,7 +161,6 @@ |
184 | |
185 | def __init__(self, *args, **kwargs): |
186 | self.actions = [] |
187 | - self.subparsers = None |
188 | super(ArgumentParser, self).__init__(*args, **kwargs) |
189 | |
190 | def add_argument(self, *args, **kwargs): |
191 | @@ -43,213 +175,31 @@ |
192 | action = super(ArgumentParser, self).add_argument(*args, **kwargs) |
193 | self.actions.append(action) |
194 | |
195 | - def register_subcommand(self, name, subcommand_class, callback=None): |
196 | - """Add a subcommand to this parser. |
197 | - |
198 | - A sub command is registered giving the sub command `name` and class:: |
199 | - |
200 | - >>> parser = ArgumentParser() |
201 | - |
202 | - >>> class SubCommand(BaseSubCommand): |
203 | - ... def run(self, namespace): |
204 | - ... return self.name |
205 | - |
206 | - >>> parser = ArgumentParser() |
207 | - >>> _ = parser.register_subcommand('foo', SubCommand) |
208 | - |
209 | - The `main` method of the subcommand class is added to namespace, and |
210 | - can be used to actually handle the sub command execution. |
211 | - |
212 | - >>> namespace = parser.parse_args(['foo']) |
213 | - >>> namespace.main(namespace) |
214 | - 'foo' |
215 | - |
216 | - A `callback` callable can also be provided to handle the subcommand |
217 | - execution:: |
218 | - |
219 | - >>> callback = lambda namespace: 'custom callback' |
220 | - |
221 | - >>> parser = ArgumentParser() |
222 | - >>> _ = parser.register_subcommand( |
223 | - ... 'bar', SubCommand, callback=callback) |
224 | - |
225 | - >>> namespace = parser.parse_args(['bar']) |
226 | - >>> namespace.main(namespace) |
227 | - 'custom callback' |
228 | - """ |
229 | - if self.subparsers is None: |
230 | - self.subparsers = self.add_subparsers( |
231 | - title='subcommands', |
232 | - help='Each subcommand accepts --h or --help to describe it.') |
233 | - subcommand = subcommand_class(name, callback=callback) |
234 | - parser = self.subparsers.add_parser( |
235 | - subcommand.name, help=subcommand.help) |
236 | - subcommand.add_arguments(parser) |
237 | - parser.set_defaults(main=subcommand.main, get_parser=lambda: parser) |
238 | - return subcommand |
239 | - |
240 | - def get_args_from_namespace(self, namespace): |
241 | - """Return a list of arguments taking values from `namespace`. |
242 | - |
243 | - Having a parser defined as usual:: |
244 | - |
245 | - >>> parser = ArgumentParser() |
246 | - >>> _ = parser.add_argument('--foo') |
247 | - >>> _ = parser.add_argument('bar') |
248 | - >>> namespace = parser.parse_args('--foo eggs spam'.split()) |
249 | - |
250 | - It is possible to recreate the argument list taking values from |
251 | - a different namespace:: |
252 | - |
253 | - >>> namespace.foo = 'changed' |
254 | - >>> parser.get_args_from_namespace(namespace) |
255 | - ['--foo', 'changed', 'spam'] |
256 | - """ |
257 | - args = [] |
258 | - for action in self.actions: |
259 | - dest = action.dest |
260 | - option_strings = action.option_strings |
261 | - value = getattr(namespace, dest, None) |
262 | - isbool = isinstance(value, bool) |
263 | - # If the value is boolean and the action is 'store_false', we |
264 | - # invert the value. This way the following `if value:` block |
265 | - # is executed if the original value is False, and the argument |
266 | - # is correctly added. |
267 | - if isbool and isinstance(action, argparse._StoreFalseAction): |
268 | - value = not value |
269 | - if value: |
270 | - if option_strings: |
271 | - args.append(option_strings[0]) |
272 | - if isinstance(value, list): |
273 | - args.extend(value) |
274 | - elif not isbool: |
275 | - args.append(value) |
276 | - return args |
277 | - |
278 | - def _run_help(self, namespace): |
279 | - """Help sub command callback.""" |
280 | - command = namespace.command |
281 | - help = self.prefix_chars + 'h' |
282 | - args = [help] if command is None else [command, help] |
283 | - self.parse_args(args) |
284 | - |
285 | - def _add_help_subcommand(self): |
286 | - """Add an help sub command to this parser.""" |
287 | - name = 'help' |
288 | - choices = self.subparsers.choices.keys() |
289 | - if name not in choices: |
290 | - choices.append(name) |
291 | - parser = self.subparsers.add_parser( |
292 | - name, help='More help on a command.') |
293 | - parser.add_argument('command', nargs='?', choices=choices) |
294 | - parser.set_defaults(main=self._run_help) |
295 | - |
296 | - def parse_args(self, *args, **kwargs): |
297 | - """Override to add an help sub command. |
298 | - |
299 | - The help sub command is added only if other sub commands exist:: |
300 | - |
301 | - >>> stderr, sys.stderr = sys.stderr, sys.stdout |
302 | - >>> parser = ArgumentParser() |
303 | - >>> parser.parse_args(['help']) |
304 | - Traceback (most recent call last): |
305 | - SystemExit: 2 |
306 | - >>> sys.stderr = stderr |
307 | - |
308 | - >>> class SubCommand(BaseSubCommand): pass |
309 | - >>> _ = parser.register_subcommand('command', SubCommand) |
310 | - >>> namespace = parser.parse_args(['help']) |
311 | - >>> namespace.main(namespace) |
312 | - Traceback (most recent call last): |
313 | - SystemExit: 0 |
314 | - """ |
315 | - if self.subparsers is not None: |
316 | - self._add_help_subcommand() |
317 | - return super(ArgumentParser, self).parse_args(*args, **kwargs) |
318 | - |
319 | |
320 | class BaseSubCommand(object): |
321 | - """Base class defining a sub command. |
322 | - |
323 | - Objects of this class can be used to easily add sub commands to this |
324 | - script as plugins, providing arguments, validating them, restarting |
325 | - as root if needed. |
326 | - |
327 | - Override `add_arguments()` to add arguments, `handlers` to add |
328 | - namespace handlers, and `run()` to manage sub command execution:: |
329 | - |
330 | - >>> def handler(namespace): |
331 | - ... namespace.bar = True |
332 | - |
333 | - >>> class SubCommand(BaseSubCommand): |
334 | - ... help = 'Sub command example.' |
335 | - ... handlers = (handler,) |
336 | - ... |
337 | - ... def add_arguments(self, parser): |
338 | - ... super(SubCommand, self).add_arguments(parser) |
339 | - ... parser.add_argument('--foo') |
340 | - ... |
341 | - ... def run(self, namespace): |
342 | - ... return namespace |
343 | - |
344 | - Namespace handlers can be used to initialize and/or validate the |
345 | - arguments namespace. |
346 | - |
347 | - Register the sub command using `ArgumentParser.register_subcommand`:: |
348 | - |
349 | - >>> parser = ArgumentParser() |
350 | - >>> sub_command = parser.register_subcommand('spam', SubCommand) |
351 | - |
352 | - Now the subcommand has a name:: |
353 | - |
354 | - >>> sub_command.name |
355 | - 'spam' |
356 | - |
357 | - The sub command can be executed using `namespace.main()`:: |
358 | - |
359 | - >>> namespace = parser.parse_args('spam --foo eggs'.split()) |
360 | - >>> namespace = namespace.main(namespace) |
361 | - >>> namespace.foo |
362 | - 'eggs' |
363 | - >>> namespace.bar |
364 | - True |
365 | - |
366 | - The help attribute of sub command instances is used to generate |
367 | - the command usage message:: |
368 | - |
369 | - >>> help = parser.format_help() |
370 | - >>> 'spam' in help |
371 | - True |
372 | - >>> 'Sub command example.' in help |
373 | - True |
374 | + """Base class defining a subcommand. |
375 | + |
376 | + Objects of this class can be used to easily add subcommands to this |
377 | + script as plug-ins, providing arguments and validating them. |
378 | + |
379 | + See the docstring of this module for further details. |
380 | """ |
381 | |
382 | - help = '' |
383 | - needs_root = False |
384 | + help = None |
385 | handlers = () |
386 | |
387 | - def __init__(self, name, callback=None): |
388 | - self.name = name |
389 | - self.callback = callback or self.run |
390 | - |
391 | def __repr__(self): |
392 | return '<{klass}: {name}>'.format( |
393 | - klass=self.__class__.__name__, name=self.name) |
394 | - |
395 | - def init_namespace(self, namespace): |
396 | - """Add `run_as_root` and `euid` names to the given `namespace`.""" |
397 | - euid = os.geteuid() |
398 | - namespace.euid, namespace.run_as_root = euid, not euid |
399 | - |
400 | - def get_handlers(self, namespace): |
401 | - """Return an iterable of namespace handlers for this sub command. |
402 | - |
403 | - Subclasses can override this to dynamically change handlers |
404 | - based on the given `namespace`. |
405 | - """ |
406 | - return self.handlers |
407 | - |
408 | - def prepare_namespace(self, parser, namespace): |
409 | + klass=self.__class__.__name__, name=self.help) |
410 | + |
411 | + def add_arguments(self, parser): |
412 | + """Here subclasses can add subcommand specific args to the parser.""" |
413 | + pass |
414 | + |
415 | + def get_description(self, namespace): |
416 | + return '' |
417 | + |
418 | + def prepare_namespace(self, namespace): |
419 | """Prepare the current namespace running namespace handlers. |
420 | |
421 | The method `self.get_handlers` can return an iterable of objects |
422 | @@ -258,77 +208,29 @@ |
423 | each other, or on the current environment. |
424 | |
425 | Each handler is a callable object, takes the current namespace |
426 | - and can raise ValidationError if the arguments are not valid:: |
427 | - |
428 | - >>> import sys |
429 | - >>> stderr, sys.stderr = sys.stderr, sys.stdout |
430 | - >>> def handler(namespace): |
431 | - ... raise ValidationError('nothing is going on') |
432 | - >>> sub_command = BaseSubCommand('foo') |
433 | - >>> sub_command.handlers = [handler] |
434 | - >>> sub_command.prepare_namespace( |
435 | - ... ArgumentParser(), argparse.Namespace()) |
436 | - Traceback (most recent call last): |
437 | - SystemExit: 2 |
438 | - >>> sys.stderr = stderr |
439 | + and can raise ValidationError if the arguments are not valid. |
440 | """ |
441 | for handler in self.get_handlers(namespace): |
442 | - try: |
443 | - handler(namespace) |
444 | - except ValidationError as err: |
445 | - parser.error(err) |
446 | - |
447 | - def get_needs_root(self, namespace): |
448 | - """Return True if root is needed to run this subcommand. |
449 | - |
450 | - Subclasses can override this to dynamically change this value |
451 | - based on the given `namespace`. |
452 | - """ |
453 | - return self.needs_root |
454 | - |
455 | - def restart_as_root(self, parser, namespace): |
456 | - """Restart this script using *sudo*. |
457 | - |
458 | - The arguments are recreated using the given `namespace`. |
459 | - """ |
460 | - args = parser.get_args_from_namespace(namespace) |
461 | - return subprocess.call(['sudo', sys.argv[0], self.name] + args) |
462 | - |
463 | - def main(self, namespace): |
464 | - """Entry point for subcommand execution. |
465 | - |
466 | - This method takes care of: |
467 | - |
468 | - - current argparse subparser retrieval |
469 | - - namespace initialization |
470 | - - namespace handling/validation |
471 | - - script restart as root (if this sub command needs to be run as root) |
472 | - |
473 | - If everything is ok the sub command callback is called passing |
474 | - the initialized/validated namespace. |
475 | - """ |
476 | - parser = namespace.get_parser() |
477 | - self.init_namespace(namespace) |
478 | - self.prepare_namespace(parser, namespace) |
479 | - if self.get_needs_root(namespace) and not namespace.run_as_root: |
480 | - return self.restart_as_root(parser, namespace) |
481 | - return self.callback(namespace) |
482 | + handler(namespace) |
483 | |
484 | def run(self, namespace): |
485 | - """Default sub command callback. |
486 | + """Default subcommand callback. |
487 | |
488 | - Subclasses must either implement this method or provide another |
489 | - callback during sub command registration. |
490 | + This method must be implemented by subclasses. |
491 | """ |
492 | raise NotImplementedError |
493 | |
494 | - def add_arguments(self, parser): |
495 | - """Here subclasses can add arguments to the subparser.""" |
496 | - pass |
497 | + def get_handlers(self, namespace): |
498 | + """Return an iterable of namespace handlers for this subcommand. |
499 | + |
500 | + Subclasses can override this to dynamically change handlers |
501 | + based on the given *namespace*. |
502 | + """ |
503 | + return self.handlers |
504 | |
505 | |
506 | class StepsBasedSubCommand(BaseSubCommand): |
507 | - """A sub command that uses "steps" to handle its execution. |
508 | + """A subcommand that uses "steps" to handle its execution. |
509 | |
510 | Steps are callables stored in the `steps` attribute, together |
511 | with the arguments they expect. Those arguments are strings |
512 | @@ -345,8 +247,8 @@ |
513 | >>> class SubCommand(StepsBasedSubCommand): |
514 | ... has_interactive_run = False |
515 | ... steps = ( |
516 | - ... (step1, 'foo'), |
517 | - ... (step2, 'foo', 'bar'), |
518 | + ... (step1, ['foo']), |
519 | + ... (step2, ['foo', 'bar']), |
520 | ... ) |
521 | ... |
522 | ... def add_arguments(self, parser): |
523 | @@ -355,12 +257,13 @@ |
524 | ... parser.add_argument('--bar') |
525 | |
526 | This class implements a `run` method that executes steps in the |
527 | - order they are provided:: |
528 | + order they are provided. |
529 | |
530 | + >>> subcommand = SubCommand() |
531 | >>> parser = ArgumentParser() |
532 | - >>> _ = parser.register_subcommand('sub', SubCommand) |
533 | - >>> namespace = parser.parse_args('sub --foo eggs --bar spam'.split()) |
534 | - >>> namespace.main(namespace) |
535 | + >>> subcommand.add_arguments(parser) |
536 | + >>> namespace = parser.parse_args('--foo eggs --bar spam'.split()) |
537 | + >>> subcommand.run(namespace) |
538 | >>> trace |
539 | ['step1 received eggs', 'step2 received eggs and spam'] |
540 | |
541 | @@ -369,8 +272,8 @@ |
542 | |
543 | >>> trace = [] |
544 | |
545 | - >>> namespace = parser.parse_args('sub --foo eggs -s step1'.split()) |
546 | - >>> namespace.main(namespace) |
547 | + >>> namespace = parser.parse_args('--foo eggs -s step1'.split()) |
548 | + >>> subcommand.run(namespace) |
549 | >>> trace |
550 | ['step1 received eggs'] |
551 | |
552 | @@ -380,173 +283,181 @@ |
553 | >>> trace = [] |
554 | |
555 | >>> namespace = parser.parse_args( |
556 | - ... 'sub --foo eggs --skip-steps step1'.split()) |
557 | - >>> namespace.main(namespace) |
558 | + ... '--foo eggs --skip-steps step1'.split()) |
559 | + >>> subcommand.run(namespace) |
560 | >>> trace |
561 | ['step2 received eggs and None'] |
562 | - |
563 | - The steps' execution is stopped if a step raises an exception. |
564 | - |
565 | - >>> trace = [] |
566 | - |
567 | - >>> def erroneous_step(foo): |
568 | - ... raise subprocess.CalledProcessError(1, 'command') |
569 | - |
570 | - >>> class SubCommandWithErrors(SubCommand): |
571 | - ... steps = ( |
572 | - ... (step1, 'foo'), |
573 | - ... (erroneous_step, 'foo'), |
574 | - ... (step2, 'foo', 'bar'), |
575 | - ... ) |
576 | - |
577 | - >>> parser = ArgumentParser() |
578 | - >>> _ = parser.register_subcommand('sub', SubCommandWithErrors) |
579 | - >>> namespace = parser.parse_args('sub --foo eggs'.split()) |
580 | - >>> namespace.main(namespace) |
581 | - Traceback (most recent call last): |
582 | - CalledProcessError: Command 'command' returned non-zero exit status 1 |
583 | - |
584 | - The step `step2` is not executed:: |
585 | - |
586 | - >>> trace |
587 | - ['step1 received eggs'] |
588 | - |
589 | - A dynamic dispatcher takes care of running the steps. This way, a |
590 | - sub command can easily wrap or override the provided steps, defining a |
591 | - method named 'call_[step name]':: |
592 | - |
593 | - >>> trace = [] |
594 | - |
595 | - >>> class DynamicSubCommand(SubCommand): |
596 | - ... def call_step1(self, namespace, step, args): |
597 | - ... trace.append('running step') |
598 | - ... step(*args) |
599 | - |
600 | - >>> parser = ArgumentParser() |
601 | - >>> _ = parser.register_subcommand('sub', DynamicSubCommand) |
602 | - >>> namespace = parser.parse_args('sub --foo eggs --bar spam'.split()) |
603 | - >>> namespace.main(namespace) |
604 | - >>> trace |
605 | - ['running step', 'step1 received eggs', 'step2 received eggs and spam'] |
606 | """ |
607 | |
608 | steps = () |
609 | + has_dry_run = True |
610 | has_interactive_run = True |
611 | |
612 | def add_arguments(self, parser): |
613 | - super(StepsBasedSubCommand, self).add_arguments(parser) |
614 | - # Add steps related arguments. |
615 | - step_names = [self._get_step_name(i[0]) for i in self.steps] |
616 | + """Add steps related arguments.""" |
617 | + step_names = [get_step_name(i[0]) for i in self.steps] |
618 | parser.add_argument( |
619 | '-s', '--steps', nargs='+', choices=step_names, |
620 | help='Call one or more internal functions.') |
621 | parser.add_argument( |
622 | '--skip-steps', nargs='+', choices=step_names, |
623 | help='Skip one or more internal functions.') |
624 | - # Add developer interaction related arguments. |
625 | - if self.has_interactive_run: |
626 | - parser.add_argument( |
627 | - '-y', '--yes', action='store_false', dest='interactive', |
628 | - help='Assume yes to all queries.') |
629 | - parser.add_argument( |
630 | - '--dry-run', action='store_true', help='Dry run.') |
631 | - |
632 | - def init_namespace(self, namespace): |
633 | - """Override to add *namespace.interactive*. |
634 | - |
635 | - This is done to ensure *interactive* is always present as an |
636 | - attribute of *namespace*, even if self.has_interactive_run is False. |
637 | - """ |
638 | - super(StepsBasedSubCommand, self).init_namespace(namespace) |
639 | - if not self.has_interactive_run: |
640 | - namespace.interactive = False |
641 | - |
642 | - def _get_step_name(self, step): |
643 | - """Return the string representation of a step callable. |
644 | - |
645 | - The name is retrieved using attributes lookup for `step_name` |
646 | - and then `__name__`:: |
647 | - |
648 | - >>> def step1(): |
649 | - ... pass |
650 | - >>> step1.step_name = 'mystep' |
651 | - |
652 | - >>> def step2(): |
653 | - ... pass |
654 | - |
655 | - >>> sub_command = StepsBasedSubCommand('foo') |
656 | - >>> sub_command._get_step_name(step1) |
657 | - 'mystep' |
658 | - >>> sub_command._get_step_name(step2) |
659 | - 'step2' |
660 | - """ |
661 | - try: |
662 | - return step.step_name |
663 | - except AttributeError: |
664 | - return step.__name__ |
665 | - |
666 | - def _include_step(self, step_name, namespace): |
667 | - """Return True if the given *step_name* must be run, False otherwise. |
668 | - |
669 | - A step is included in the command execution if the step included in |
670 | - `--steps` (or steps is None) and is not included in `--skip-steps`. |
671 | - """ |
672 | - steps_to_skip = namespace.skip_steps or [] |
673 | - steps_to_run = namespace.steps |
674 | - # If explicitly told to skip a step, then skip it. |
675 | - if step_name in steps_to_skip: |
676 | - return False |
677 | - # If no list of steps was provided then any non-skipped are to be run. |
678 | - if steps_to_run is None: |
679 | - return True |
680 | - # A list of steps to run was provided, so the step has to be included |
681 | - # in order to be run. |
682 | - return step_name in steps_to_run |
683 | - |
684 | - def get_steps(self, namespace): |
685 | - """Return a list of *(step_name, step_callable, step_args)* tuples.""" |
686 | - steps = [] |
687 | - for step_arg_names in self.steps: |
688 | - step, arg_names = step_arg_names[0], step_arg_names[1:] |
689 | - step_name = self._get_step_name(step) |
690 | - if self._include_step(step_name, namespace): |
691 | - args = [getattr(namespace, i) for i in arg_names] |
692 | - steps.append((step_name, step, args)) |
693 | - return steps |
694 | - |
695 | - def _call_step(self, namespace, step, args): |
696 | - """Default callable used to run a `step`, using given `args`.""" |
697 | - return step(*args) |
698 | - |
699 | - def get_steps_description(self, namespace, steps): |
700 | - """Retrieve steps' descriptions from the given *steps*. |
701 | - |
702 | - Return a string containing all the descriptions. |
703 | - """ |
704 | + |
705 | + def get_description(self, namespace): |
706 | + """Collect steps' descriptions and return them as a single string.""" |
707 | context = namespace.__dict__ |
708 | - descriptions = [get_step_description(i[1], **context) for i in steps] |
709 | + steps = resolve_steps(self.steps, namespace) |
710 | + descriptions = [get_step_description(i[0], **context) for i in steps] |
711 | return '\n'.join(filter(None, descriptions)) |
712 | |
713 | def run(self, namespace): |
714 | - steps = self.get_steps(namespace) |
715 | - if namespace.dry_run or namespace.interactive: |
716 | - # Collect and display the description of each step. |
717 | - description = self.get_steps_description(namespace, steps) |
718 | - if description: |
719 | - print 'This command will perform the following actions:\n' |
720 | - print description + '\n' |
721 | - # Quit without errors if this is a dry run. |
722 | - if namespace.dry_run: |
723 | - return |
724 | - # If this is not a dry run, then it is an interactive one. |
725 | - # Prompt the user for confirmation to proceed and quit if |
726 | - # requested (in this case with exit code 1). |
727 | - if not confirm(): |
728 | - return 1 |
729 | - # Execute all the steps. |
730 | - default_step_runner = self._call_step |
731 | - for step_name, step, args in steps: |
732 | - # Run the step using a dynamic dispatcher. |
733 | - step_runner = getattr( |
734 | - self, 'call_' + step_name, default_step_runner) |
735 | - step_runner(namespace, step, args) |
736 | + """Execute the steps in the order they are provided.""" |
737 | + for step, args in resolve_steps(self.steps, namespace): |
738 | + step(*args) |
739 | + |
740 | + |
741 | +def add_common_arguments( |
742 | + parser, has_dry_run=False, has_interactive_run=False): |
743 | + """Add to given *parser* the arguments that all subcommands have in common. |
744 | + |
745 | + The common arguments are: |
746 | + |
747 | + - `-y` or `--yes`: non-interactive execution. |
748 | + - `--dry-run`: dry run execution. |
749 | + |
750 | + Ensure the resulting namespace always contains *interactive* and *dry_run* |
751 | + names, even if *has_interactive_run* or *has_dry_run* are False. |
752 | + """ |
753 | + if has_dry_run: |
754 | + parser.add_argument('--dry-run', action='store_true', help='Dry run.') |
755 | + else: |
756 | + parser.set_defaults(dry_run=False) |
757 | + if has_interactive_run: |
758 | + parser.add_argument( |
759 | + '-y', '--yes', action='store_false', dest='interactive', |
760 | + help='Assume yes to all queries.') |
761 | + else: |
762 | + parser.set_defaults(interactive=False) |
763 | + |
764 | + |
765 | +def get_args_from_namespace(parser, namespace): |
766 | + """Return a list of arguments taking values from `namespace`.""" |
767 | + args = [] |
768 | + for action in parser.actions: |
769 | + dest = action.dest |
770 | + option_strings = action.option_strings |
771 | + value = getattr(namespace, dest, None) |
772 | + isbool = isinstance(value, bool) |
773 | + # If the value is boolean and the action is 'store_false', we |
774 | + # invert the value. This way the following `if value:` block |
775 | + # is executed if the original value is False, and the argument |
776 | + # is correctly added. |
777 | + if isbool and isinstance(action, argparse._StoreFalseAction): |
778 | + value = not value |
779 | + if value: |
780 | + if option_strings: |
781 | + args.append(option_strings[0]) |
782 | + if isinstance(value, list): |
783 | + args.extend(value) |
784 | + elif not isbool: |
785 | + args.append(value) |
786 | + return args |
787 | + |
788 | + |
789 | +def init_namespace(namespace): |
790 | + """Add `run_as_root` and `euid` names to the given *namespace*.""" |
791 | + euid = os.geteuid() |
792 | + namespace.euid, namespace.run_as_root = euid, not euid |
793 | + |
794 | + |
795 | +def restart_as_root(parser, namespace): |
796 | + """Restart this script using *sudo*. |
797 | + |
798 | + The arguments are recreated using the given `namespace`. |
799 | + """ |
800 | + cmd = ['sudo', sys.argv[0]] + parser.prog.split()[1:] |
801 | + namespace.interactive = False |
802 | + args = get_args_from_namespace(parser, namespace) |
803 | + return subprocess.call(cmd + args) |
804 | + |
805 | + |
806 | +def get_step_name(step): |
807 | + """Return a string representing the name of the given *step*.""" |
808 | + try: |
809 | + return step.step_name |
810 | + except AttributeError: |
811 | + return step.__name__ |
812 | + |
813 | + |
814 | +def resolve_steps(steps, namespace): |
815 | + """Return a list of *(step_callable, step_args)* tuples. |
816 | + |
817 | + The argument *steps* is a sequence of |
818 | + *(step_callable, arg_names, filter_callable)*. |
819 | + |
820 | + The *arg_names* are strings representing arguments in the namespace. |
821 | + The *filter_callable* is optional. It must take the namespace and return |
822 | + True if the corresponding step must be included, False otherwise. |
823 | + |
824 | + If the namespace contains the `steps` or `skip_steps` names, they are |
825 | + taken into consideration to resolve the steps that must be run. |
826 | + |
827 | + In essence, a step is returned as part of the sequence if: |
828 | + |
829 | + - is not included in *namespace.skip_steps* |
830 | + - is not excluded from *namespace.step* (if *namespace.step* is not |
831 | + defined or is None, all the steps are considered as included) |
832 | + - the provided filter callable returns True (if not present, the |
833 | + step is included by default) |
834 | + """ |
835 | + steps_to_skip = getattr(namespace, 'skip_steps', None) or [] |
836 | + steps_to_run = getattr(namespace, 'steps', None) |
837 | + |
838 | + def get_step(step_sequence): |
839 | + if len(step_sequence) == 3: |
840 | + # If filter_callable is present, just return the given sequence. |
841 | + return step_sequence |
842 | + # Otherwise, add a dummy filter_callable that always returns True. |
843 | + return list(step_sequence) + [lambda namespace: True] |
844 | + |
845 | + step_args = [] |
846 | + for step_callable, arg_names, should_run_step in map(get_step, steps): |
847 | + step_name = get_step_name(step_callable) |
848 | + # Include step if is not explicitly told to skip it. |
849 | + if step_name not in steps_to_skip: |
850 | + # If no list of steps was provided then any non-skipped are to be |
851 | + # run, otherwise the step has to be included in order to be run. |
852 | + if (steps_to_run is None) or (step_name in steps_to_run): |
853 | + # The step filter function, if present, can deny the execution. |
854 | + if should_run_step(namespace): |
855 | + args = [getattr(namespace, i) for i in arg_names] |
856 | + step_args.append((step_callable, args)) |
857 | + return step_args |
858 | + |
859 | + |
860 | +def get_step_description(step, **kwargs): |
861 | + """Retrieve and format step description from the given *step* callable. |
862 | + |
863 | + *kwargs*, if provided, will be used as context to format the description. |
864 | + Formatting is done using the Python's built-in templating system |
865 | + *string.Template* supporting $-based substitutions. |
866 | + |
867 | + If placeholders are missing from *kwargs* an error will be raised. |
868 | + """ |
869 | + description = step.description |
870 | + if not description: |
871 | + # This step has no description, nothing else to do. |
872 | + return '' |
873 | + if kwargs: |
874 | + s = string.Template(description) |
875 | + description = s.substitute(**kwargs) |
876 | + # Remove multiple spaces from lines. |
877 | + lines = [' '.join(line.split()) for line in description.splitlines()] |
878 | + # Retrieve all the non empty lines. |
879 | + lines = filter(None, lines) |
880 | + # For each line, wrap the contents. Note that we can't wrap the text of |
881 | + # the entire paragraph because we want to preserve existing new lines. |
882 | + width = get_terminal_width() |
883 | + return '\n'.join(textwrap.fill( |
884 | + line, width=width, initial_indent='* ', subsequent_indent=' ') |
885 | + for line in lines) |
886 | |
887 | === modified file 'lpsetup/cli.py' |
888 | --- lpsetup/cli.py 2012-07-09 15:19:13 +0000 |
889 | +++ lpsetup/cli.py 2012-08-13 12:44:19 +0000 |
890 | @@ -6,7 +6,10 @@ |
891 | |
892 | __metaclass__ = type |
893 | __all__ = [ |
894 | + 'get_parser', |
895 | 'main', |
896 | + 'prepare_parser', |
897 | + 'run', |
898 | ] |
899 | |
900 | import sys |
901 | @@ -24,30 +27,145 @@ |
902 | update, |
903 | version, |
904 | ) |
905 | - |
906 | - |
907 | -subcommands = [ |
908 | - ('finish-init-host', finish_inithost.SubCommand), |
909 | - ('init-host', inithost.SubCommand), |
910 | - ('init-lxc', initlxc.SubCommand), |
911 | - ('init-repo', initrepo.SubCommand), |
912 | - ('install-lxc', install_lxc.SubCommand), |
913 | - ('update', update.SubCommand), |
914 | - ('version', version.SubCommand), |
915 | +from lpsetup.utils import confirm |
916 | + |
917 | + |
918 | +SUBCOMMANDS = [ |
919 | + ('finish-init-host', finish_inithost.SubCommand()), |
920 | + ('init-host', inithost.SubCommand()), |
921 | + ('init-lxc', initlxc.SubCommand()), |
922 | + ('init-repo', initrepo.SubCommand()), |
923 | + ('install-lxc', install_lxc.SubCommand()), |
924 | + ('update', update.SubCommand()), |
925 | + ('version', version.SubCommand()), |
926 | ] |
927 | |
928 | |
929 | -def main(args=None): |
930 | +def run(parser, subcommand, namespace): |
931 | + """Run the *subcommand* using the given *namespace*. |
932 | + |
933 | + This function takes care of:: |
934 | + |
935 | + - namespace initialization and validation |
936 | + - interactive arguments handling |
937 | + - executing a dry run, if requested |
938 | + - restarting the same subcommand as root, if required |
939 | + - actual subcommand execution |
940 | + """ |
941 | + # Initialize the namespace. |
942 | + argparser.init_namespace(namespace) |
943 | + |
944 | + # Prepare the namespace. Each subcommand can define a *prepare_namespace* |
945 | + # method that can raise a ValidationError if the namespace is not valid. |
946 | + try: |
947 | + subcommand.prepare_namespace(namespace) |
948 | + except exceptions.ValidationError as err: |
949 | + parser.error(err) |
950 | + except AttributeError: |
951 | + pass |
952 | + |
953 | + # Handle user interaction and dry run. |
954 | + if namespace.dry_run or namespace.interactive: |
955 | + # Display the subcommand description. |
956 | + command_description = subcommand.get_description(namespace) |
957 | + if command_description: |
958 | + print 'This command will perform the following actions:\n' |
959 | + print command_description + '\n' |
960 | + # Quit without errors if this is a dry run. |
961 | + if namespace.dry_run: |
962 | + return |
963 | + # If this is not a dry run, then it is an interactive one. |
964 | + # Prompt the user for confirmation to proceed and quit if |
965 | + # requested (in this case with exit code 1). |
966 | + try: |
967 | + proceed = confirm() |
968 | + except KeyboardInterrupt: |
969 | + print '\nQuitting.' |
970 | + return 1 |
971 | + if not proceed: |
972 | + return 1 |
973 | + |
974 | + # Restart as root if needed. |
975 | + if not namespace.run_as_root and getattr( |
976 | + subcommand, 'root_required', False): |
977 | + return argparser.restart_as_root(parser, namespace) |
978 | + |
979 | + # Execute the command. |
980 | + try: |
981 | + return subcommand.run(namespace) |
982 | + except exceptions.ExecutionError as err: |
983 | + return err |
984 | + |
985 | + |
986 | +def prepare_parser(parser, subcommand): |
987 | + """Add common and subcommand specific arguments to the given *parser*. |
988 | + |
989 | + Also add to the parser defaults a *main* function that, taking a parsed |
990 | + namespace, executes the current subcommand with the current parser. |
991 | + """ |
992 | + def main(namespace): |
993 | + return run(parser, subcommand, namespace) |
994 | + |
995 | + # Store a reference to the current subcommand entry point. |
996 | + parser.set_defaults(main=main) |
997 | + # Add user interaction related arguments. |
998 | + argparser.add_common_arguments( |
999 | + parser, |
1000 | + has_dry_run=getattr(subcommand, 'has_dry_run', False), |
1001 | + has_interactive_run=getattr(subcommand, 'has_interactive_run', False), |
1002 | + ) |
1003 | + # Add subcommand specific arguments, if requested. |
1004 | + try: |
1005 | + subcommand.add_arguments(parser) |
1006 | + except AttributeError: |
1007 | + pass |
1008 | + |
1009 | + |
1010 | +def get_parser(subcommands): |
1011 | + """Return the argument parser with all subcommands registered. |
1012 | + |
1013 | + Also add the help subcommand. |
1014 | + """ |
1015 | + parser = argparser.ArgumentParser(description=description) |
1016 | + subparsers = parser.add_subparsers( |
1017 | + title='subcommands', |
1018 | + help='Each subcommand accepts --h or --help to describe it.') |
1019 | + for name, subcommand in subcommands: |
1020 | + # Register the subcommand. |
1021 | + help = subcommand.help |
1022 | + subparser = subparsers.add_parser(name, description=help, help=help) |
1023 | + # Prepare the subcommand arguments. |
1024 | + prepare_parser(subparser, subcommand) |
1025 | + |
1026 | + # Add the *help* subcommand. |
1027 | + def main(namespace): |
1028 | + command = namespace.command |
1029 | + help = parser.prefix_chars + 'h' |
1030 | + args = [help] if command is None else [command, help] |
1031 | + parser.parse_args(args) |
1032 | + |
1033 | + choices = dict(subcommands).keys() |
1034 | + help = 'More help on a command.' |
1035 | + subparser = subparsers.add_parser('help', description=help, help=help) |
1036 | + subparser.add_argument('command', nargs='?', choices=choices) |
1037 | + subparser.set_defaults(main=main) |
1038 | + |
1039 | + return parser |
1040 | + |
1041 | + |
1042 | +def main(args=None, parser=None): |
1043 | + # Retrieve the command line arguments. |
1044 | if args is None: |
1045 | args = sys.argv[1:] |
1046 | - parser = argparser.ArgumentParser(description=description) |
1047 | - for name, klass in subcommands: |
1048 | - parser.register_subcommand(name, klass) |
1049 | + # Get the parser. |
1050 | + if parser is None: |
1051 | + parser = get_parser(SUBCOMMANDS) |
1052 | + # Parse the command line arguments. |
1053 | try: |
1054 | - args = parser.parse_args(args) |
1055 | + namespace = parser.parse_args(args) |
1056 | + # Run the subcommand. Each subparser adds to namespace a main |
1057 | + # function that basically execute run() passing the corresponding |
1058 | + # subcommand, sub parser and the namespace itself. |
1059 | + return namespace.main(namespace) |
1060 | except SystemExit as err: |
1061 | return err.code |
1062 | - try: |
1063 | - return args.main(args) |
1064 | - except (exceptions.ExecutionError, KeyboardInterrupt) as err: |
1065 | - return err |
1066 | |
1067 | === modified file 'lpsetup/handlers.py' |
1068 | --- lpsetup/handlers.py 2012-08-06 15:12:28 +0000 |
1069 | +++ lpsetup/handlers.py 2012-08-13 12:44:19 +0000 |
1070 | @@ -2,7 +2,7 @@ |
1071 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1072 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1073 | |
1074 | -"""Sub command arguments handling and validation.""" |
1075 | +"""Subcommand arguments handling and validation.""" |
1076 | |
1077 | __metaclass__ = type |
1078 | __all__ = [ |
1079 | |
1080 | === modified file 'lpsetup/subcommands/finish_inithost.py' |
1081 | --- lpsetup/subcommands/finish_inithost.py 2012-08-01 14:42:01 +0000 |
1082 | +++ lpsetup/subcommands/finish_inithost.py 2012-08-13 12:44:19 +0000 |
1083 | @@ -69,13 +69,10 @@ |
1084 | """ |
1085 | |
1086 | help = __doc__ |
1087 | - |
1088 | - needs_root = True |
1089 | - |
1090 | + root_required = True |
1091 | steps = ( |
1092 | - (setup_launchpad, 'user', 'target_dir'), |
1093 | + (setup_launchpad, ['user', 'target_dir']), |
1094 | ) |
1095 | - |
1096 | handlers = (handle_user, handle_target_dir) |
1097 | |
1098 | def add_arguments(self, parser): |
1099 | |
1100 | === modified file 'lpsetup/subcommands/inithost.py' |
1101 | --- lpsetup/subcommands/inithost.py 2012-08-08 20:25:02 +0000 |
1102 | +++ lpsetup/subcommands/inithost.py 2012-08-13 12:44:19 +0000 |
1103 | @@ -8,7 +8,9 @@ |
1104 | __all__ = [ |
1105 | 'initialize', |
1106 | 'initialize_base', |
1107 | + 'initialize_lxc', |
1108 | 'setup_apt', |
1109 | + 'setup_home', |
1110 | 'SubCommand', |
1111 | ] |
1112 | |
1113 | @@ -214,7 +216,7 @@ |
1114 | def initialize_lxc(): |
1115 | """Initialize LXC container. |
1116 | |
1117 | - Note that this step is guarded by the call_initialize_lxc method so that |
1118 | + Note that this step is guarded by a filter callable method so that |
1119 | it is only called when lpsetup is running in a container. |
1120 | """ |
1121 | lxc_os = get_distro() |
1122 | @@ -263,15 +265,16 @@ |
1123 | |
1124 | class SubCommand(argparser.StepsBasedSubCommand): |
1125 | """Prepare a machine to run Launchpad. May be an LXC container or not.""" |
1126 | - initialize_step = (initialize, 'user') |
1127 | - |
1128 | - setup_home_step = (setup_home, |
1129 | - 'user', 'full_name', 'email', 'lpuser', |
1130 | - 'valid_ssh_keys', 'ssh_key_path') |
1131 | - |
1132 | - initialize_lxc_step = (initialize_lxc, ) |
1133 | - |
1134 | - setup_apt_step = (setup_apt, ) |
1135 | + initialize_step = (initialize, ['user']) |
1136 | + |
1137 | + setup_home_step = ( |
1138 | + setup_home, ['user', 'full_name', 'email', 'lpuser', |
1139 | + 'valid_ssh_keys', 'ssh_key_path']) |
1140 | + |
1141 | + initialize_lxc_step = ( |
1142 | + initialize_lxc, [], lambda namespace: running_in_container()) |
1143 | + |
1144 | + setup_apt_step = (setup_apt, []) |
1145 | |
1146 | steps = ( |
1147 | initialize_step, |
1148 | @@ -281,7 +284,7 @@ |
1149 | ) |
1150 | |
1151 | help = __doc__ |
1152 | - needs_root = True |
1153 | + root_required = True |
1154 | handlers = ( |
1155 | handle_user, |
1156 | handle_lpuser_as_username, |
1157 | @@ -289,11 +292,6 @@ |
1158 | handle_ssh_keys, |
1159 | ) |
1160 | |
1161 | - def call_initialize_lxc(self, namespace, step, args): |
1162 | - """Caller that only initializes LXC if we are in an LXC.""" |
1163 | - if running_in_container(): |
1164 | - return step(*args) |
1165 | - |
1166 | def add_arguments(self, parser): |
1167 | super(SubCommand, self).add_arguments(parser) |
1168 | parser.add_argument( |
1169 | |
1170 | === modified file 'lpsetup/subcommands/initlxc.py' |
1171 | --- lpsetup/subcommands/initlxc.py 2012-08-08 20:25:02 +0000 |
1172 | +++ lpsetup/subcommands/initlxc.py 2012-08-13 12:44:19 +0000 |
1173 | @@ -12,6 +12,7 @@ |
1174 | __all__ = [ |
1175 | 'create_lxc', |
1176 | 'inithost_in_lxc', |
1177 | + 'initialize', |
1178 | 'install_lpsetup_in_lxc', |
1179 | 'start_lxc', |
1180 | 'stop_lxc', |
1181 | @@ -224,7 +225,7 @@ |
1182 | if not lxc_stopped(lxc_name): |
1183 | subprocess.call(['lxc-stop', '-n', lxc_name]) |
1184 | |
1185 | -stop_lxc.description = None |
1186 | +stop_lxc.description = 'Stop the LXC instance $lxc_name.\n' |
1187 | |
1188 | |
1189 | class SubCommand(inithost.SubCommand): |
1190 | @@ -232,23 +233,25 @@ |
1191 | development environment. |
1192 | """ |
1193 | |
1194 | - create_lxc_step = (create_lxc, |
1195 | - 'lxc_name', 'lxc_arch', 'lxc_os', 'user', 'install_subunit') |
1196 | - start_lxc_step = (start_lxc, |
1197 | - 'lxc_name') |
1198 | - wait_for_lxc_step = (wait_for_lxc, |
1199 | - 'lxc_name', 'ssh_key_path') |
1200 | - install_lpsetup_in_lxc_step = (install_lpsetup_in_lxc, |
1201 | - 'lxc_name', 'ssh_key_path', 'lxc_os', 'user', 'home_dir', |
1202 | - 'lpsetup_branch') |
1203 | - inithost_in_lxc_step = (inithost_in_lxc, |
1204 | - 'lxc_name', 'ssh_key_path', 'user', 'email', 'full_name', 'lpuser', |
1205 | - 'ssh_key_name', 'home_dir') |
1206 | - stop_lxc_step = (stop_lxc, |
1207 | - 'lxc_name', 'ssh_key_path') |
1208 | + create_lxc_step = ( |
1209 | + create_lxc, |
1210 | + ['lxc_name', 'lxc_arch', 'lxc_os', 'user', 'install_subunit']) |
1211 | + start_lxc_step = (start_lxc, ['lxc_name']) |
1212 | + wait_for_lxc_step = (wait_for_lxc, ['lxc_name', 'ssh_key_path']) |
1213 | + install_lpsetup_in_lxc_step = ( |
1214 | + install_lpsetup_in_lxc, |
1215 | + ['lxc_name', 'ssh_key_path', 'lxc_os', 'user', 'home_dir', |
1216 | + 'lpsetup_branch']) |
1217 | + inithost_in_lxc_step = ( |
1218 | + inithost_in_lxc, |
1219 | + ['lxc_name', 'ssh_key_path', 'user', 'email', 'full_name', 'lpuser', |
1220 | + 'ssh_key_name', 'home_dir']) |
1221 | + stop_lxc_step = ( |
1222 | + stop_lxc, ['lxc_name', 'ssh_key_path'], |
1223 | + lambda namespace: namespace.stop_lxc) |
1224 | |
1225 | base_steps = ( |
1226 | - (initialize, 'user', 'install_haveged'), |
1227 | + (initialize, ['user', 'install_haveged']), |
1228 | inithost.SubCommand.setup_home_step, |
1229 | create_lxc_step, |
1230 | start_lxc_step, |
1231 | @@ -259,12 +262,7 @@ |
1232 | steps = base_steps + (stop_lxc_step,) |
1233 | |
1234 | help = __doc__ |
1235 | - needs_root = True |
1236 | - |
1237 | - def call_stop_lxc(self, namespace, step, args): |
1238 | - """Run the `stop_lxc` step only if the related flag is set.""" |
1239 | - if namespace.stop_lxc: |
1240 | - return step(*args) |
1241 | + root_required = True |
1242 | |
1243 | def add_arguments(self, parser): |
1244 | super(SubCommand, self).add_arguments(parser) |
1245 | |
1246 | === modified file 'lpsetup/subcommands/initrepo.py' |
1247 | --- lpsetup/subcommands/initrepo.py 2012-08-08 20:25:02 +0000 |
1248 | +++ lpsetup/subcommands/initrepo.py 2012-08-13 12:44:19 +0000 |
1249 | @@ -15,6 +15,8 @@ |
1250 | |
1251 | __metaclass__ = type |
1252 | __all__ = [ |
1253 | + 'fetch', |
1254 | + 'setup_bzr_locations', |
1255 | 'SubCommand', |
1256 | ] |
1257 | |
1258 | @@ -93,8 +95,8 @@ |
1259 | lpuser, repository, branch_name, template=LP_BZR_LOCATIONS): |
1260 | """Set up bazaar locations. |
1261 | |
1262 | - Note that this step is guarded by the *call_setup_bzr_locations* method |
1263 | - so that it is only called when the user Launchpad login can be retrieved. |
1264 | + Note that this step is guarded by a filter callable so that it is only |
1265 | + called when the user Launchpad login can be retrieved. |
1266 | """ |
1267 | context = { |
1268 | 'branch_dir': os.path.join(repository, branch_name), |
1269 | @@ -119,8 +121,8 @@ |
1270 | f.write(wrap_contents(contents.getvalue())) |
1271 | contents.close() |
1272 | |
1273 | -setup_bzr_locations.description = """If bzr+ssh is used, update bazaar \ |
1274 | - locations ($home_dir/.bazaar/locations.conf) to include repository \ |
1275 | +setup_bzr_locations.description = """Update bazaar locations \ |
1276 | + ($home_dir/.bazaar/locations.conf) to include repository \ |
1277 | $repository and branch $branch_name. |
1278 | """ |
1279 | |
1280 | @@ -130,9 +132,10 @@ |
1281 | |
1282 | steps = ( |
1283 | (fetch, |
1284 | - 'source', 'repository', 'branch_name', 'checkout_name', |
1285 | - 'no_checkout'), |
1286 | - (setup_bzr_locations, 'lpuser', 'repository', 'branch_name'), |
1287 | + ['source', 'repository', 'branch_name', 'checkout_name', |
1288 | + 'no_checkout']), |
1289 | + (setup_bzr_locations, ['lpuser', 'repository', 'branch_name'], |
1290 | + lambda namespace: namespace.lpuser is not None), |
1291 | ) |
1292 | |
1293 | help = __doc__ |
1294 | @@ -143,11 +146,7 @@ |
1295 | handlers.handle_branch_and_checkout, |
1296 | handlers.handle_source, |
1297 | ) |
1298 | - |
1299 | - def call_setup_bzr_locations(self, namespace, step, args): |
1300 | - """Caller that only sets up bzr locations if lpuser exists.""" |
1301 | - if namespace.lpuser is not None: |
1302 | - return step(*args) |
1303 | + root_required = False |
1304 | |
1305 | @staticmethod |
1306 | def add_common_arguments(parser): |
1307 | |
1308 | === modified file 'lpsetup/subcommands/install_lxc.py' |
1309 | --- lpsetup/subcommands/install_lxc.py 2012-08-09 12:15:53 +0000 |
1310 | +++ lpsetup/subcommands/install_lxc.py 2012-08-13 12:44:19 +0000 |
1311 | @@ -7,7 +7,10 @@ |
1312 | __metaclass__ = type |
1313 | __all__ = [ |
1314 | 'create_scripts', |
1315 | + 'finish_inithost_in_lxc', |
1316 | + 'init_repo_in_lxc', |
1317 | 'SubCommand', |
1318 | + 'update_in_lxc', |
1319 | ] |
1320 | |
1321 | import os |
1322 | @@ -69,7 +72,7 @@ |
1323 | with open(procfile, 'w') as f: |
1324 | f.write('0\n') |
1325 | |
1326 | -create_scripts.description = """If requested, create helper script \ |
1327 | +create_scripts.description = """Create helper scripts \ |
1328 | /usr/local/bin/lp-setup-*: they can be used to build Launchpad and \ |
1329 | start a parallel test run. |
1330 | """ |
1331 | @@ -125,25 +128,24 @@ |
1332 | |
1333 | |
1334 | class SubCommand(initlxc.SubCommand): |
1335 | - """Completely sets up an LXC environment with Launchpad using the sandbox |
1336 | - development model. |
1337 | - """ |
1338 | + """Completely sets up an LXC environment with Launchpad.""" |
1339 | |
1340 | steps = initlxc.SubCommand.base_steps + ( |
1341 | # Run on host: |
1342 | - (create_scripts, 'lxc_name', 'ssh_key_path', 'user'), |
1343 | + (create_scripts, ['lxc_name', 'ssh_key_path', 'user'], |
1344 | + lambda namespace: namespace.create_scripts), |
1345 | # Run inside the container: |
1346 | (init_repo_in_lxc, |
1347 | - 'lxc_name', 'ssh_key_path', 'home_dir', 'user', 'source', 'use_http', |
1348 | - 'branch_name', 'checkout_name', 'repository', 'no_checkout'), |
1349 | - (update_in_lxc, 'lxc_name', 'ssh_key_path', 'home_dir', 'user', |
1350 | - 'external_path', 'target_dir', 'lp_source_deps', 'use_http'), |
1351 | - (finish_inithost_in_lxc, 'lxc_name', 'ssh_key_path', 'home_dir', |
1352 | - 'user', 'target_dir'), |
1353 | + ['lxc_name', 'ssh_key_path', 'home_dir', 'user', 'source', 'use_http', |
1354 | + 'branch_name', 'checkout_name', 'repository', 'no_checkout']), |
1355 | + (update_in_lxc, |
1356 | + ['lxc_name', 'ssh_key_path', 'home_dir', 'user', 'external_path', |
1357 | + 'target_dir', 'lp_source_deps', 'use_http']), |
1358 | + (finish_inithost_in_lxc, |
1359 | + ['lxc_name', 'ssh_key_path', 'home_dir', 'user', 'target_dir']), |
1360 | # Run on host: |
1361 | initlxc.SubCommand.stop_lxc_step, |
1362 | ) |
1363 | - |
1364 | help = __doc__ |
1365 | |
1366 | def get_handlers(self, namespace): |
1367 | @@ -156,11 +158,6 @@ |
1368 | handle_target_from_repository, |
1369 | ) |
1370 | |
1371 | - def call_create_scripts(self, namespace, step, args): |
1372 | - """Run the `create_scripts` step only if the related flag is set.""" |
1373 | - if namespace.create_scripts: |
1374 | - return step(*args) |
1375 | - |
1376 | def add_arguments(self, parser): |
1377 | super(SubCommand, self).add_arguments(parser) |
1378 | # Inherit arguments from subcommands we depend upon. |
1379 | |
1380 | === modified file 'lpsetup/subcommands/update.py' |
1381 | --- lpsetup/subcommands/update.py 2012-08-02 14:44:32 +0000 |
1382 | +++ lpsetup/subcommands/update.py 2012-08-13 12:44:19 +0000 |
1383 | @@ -6,7 +6,10 @@ |
1384 | |
1385 | __metaclass__ = type |
1386 | __all__ = [ |
1387 | + 'initialize_directories', |
1388 | 'SubCommand', |
1389 | + 'update_dependencies', |
1390 | + 'update_tree', |
1391 | ] |
1392 | |
1393 | import os.path |
1394 | @@ -83,10 +86,10 @@ |
1395 | |
1396 | has_interactive_run = False |
1397 | steps = ( |
1398 | - (initialize_directories, 'target_dir', 'external_path'), |
1399 | + (initialize_directories, ['target_dir', 'external_path']), |
1400 | (update_dependencies, |
1401 | - 'target_dir', 'external_path', 'use_http', 'lp_source_deps'), |
1402 | - (update_tree, 'target_dir'), |
1403 | + ['target_dir', 'external_path', 'use_http', 'lp_source_deps']), |
1404 | + (update_tree, ['target_dir']), |
1405 | ) |
1406 | help = __doc__ |
1407 | handlers = ( |
1408 | |
1409 | === removed file 'lpsetup/tests/examples.py' |
1410 | --- lpsetup/tests/examples.py 2012-07-30 10:05:19 +0000 |
1411 | +++ lpsetup/tests/examples.py 1970-01-01 00:00:00 +0000 |
1412 | @@ -1,93 +0,0 @@ |
1413 | -#!/usr/bin/env python |
1414 | -# Copyright 2012 Canonical Ltd. This software is licensed under the |
1415 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
1416 | - |
1417 | -"""Example objects used in tests.""" |
1418 | - |
1419 | -__metaclass__ = type |
1420 | -__all__ = [ |
1421 | - 'step1', |
1422 | - 'step2', |
1423 | - 'StepsBasedSubCommand', |
1424 | - 'StepsBasedSubCommandWithErrors', |
1425 | - 'bad_step', |
1426 | - 'SubCommand', |
1427 | - ] |
1428 | - |
1429 | -import subprocess |
1430 | - |
1431 | -from lpsetup import argparser |
1432 | - |
1433 | - |
1434 | -def step1(foo): |
1435 | - print 'step1 received ' + foo |
1436 | - |
1437 | - |
1438 | -def step2(foo, bar): |
1439 | - print 'step2 received {0} and {1}'.format(foo, bar) |
1440 | - |
1441 | -step2.step_name = 'mystep' |
1442 | - |
1443 | - |
1444 | -def step_with_description(foo): |
1445 | - pass |
1446 | - |
1447 | -step_with_description.description = 'step description' |
1448 | - |
1449 | - |
1450 | -def bad_step(foo): |
1451 | - raise subprocess.CalledProcessError(1, 'command') |
1452 | - |
1453 | - |
1454 | -class SubCommand(argparser.BaseSubCommand): |
1455 | - """An example sub command.""" |
1456 | - |
1457 | - help = 'Sub command example.' |
1458 | - |
1459 | - def add_arguments(self, parser): |
1460 | - super(SubCommand, self).add_arguments(parser) |
1461 | - parser.add_argument('--foo') |
1462 | - |
1463 | - def run(self, namespace): |
1464 | - return namespace |
1465 | - |
1466 | - |
1467 | -class StepsBasedSubCommand(argparser.StepsBasedSubCommand): |
1468 | - """An example steps based sub command.""" |
1469 | - |
1470 | - has_interactive_run = False |
1471 | - steps = ( |
1472 | - (step1, 'foo'), |
1473 | - (step2, 'foo', 'bar'), |
1474 | - ) |
1475 | - |
1476 | - def add_arguments(self, parser): |
1477 | - super(StepsBasedSubCommand, self).add_arguments(parser) |
1478 | - parser.add_argument('--foo') |
1479 | - parser.add_argument('--bar') |
1480 | - |
1481 | - |
1482 | -class StepsBasedSubCommandWithErrors(StepsBasedSubCommand): |
1483 | - """An example steps based sub command (containing a failing step).""" |
1484 | - |
1485 | - steps = ( |
1486 | - (step1, 'foo'), |
1487 | - (bad_step, 'foo'), |
1488 | - (step2, 'foo', 'bar'), |
1489 | - ) |
1490 | - |
1491 | - |
1492 | -class DynamicStepsBasedSubCommand(StepsBasedSubCommand): |
1493 | - """An example steps based sub command (using internal step dispatcher).""" |
1494 | - |
1495 | - def call_step1(self, namespace, step, args): |
1496 | - print 'running step1 with {args} while bar is {bar}'.format( |
1497 | - args=','.join(args), bar=namespace.bar) |
1498 | - step(*args) |
1499 | - |
1500 | - |
1501 | -class InteractiveStepsBasedSubCommand(StepsBasedSubCommand): |
1502 | - """An example interactive steps based sub command.""" |
1503 | - |
1504 | - has_interactive_run = True |
1505 | - steps = [(step_with_description, 'foo')] |
1506 | |
1507 | === modified file 'lpsetup/tests/subcommands/test_finish_inithost.py' |
1508 | --- lpsetup/tests/subcommands/test_finish_inithost.py 2012-07-20 10:40:43 +0000 |
1509 | +++ lpsetup/tests/subcommands/test_finish_inithost.py 2012-08-13 12:44:19 +0000 |
1510 | @@ -2,7 +2,7 @@ |
1511 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1512 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1513 | |
1514 | -"""Tests for the finish-init-host sub command.""" |
1515 | +"""Tests for the finish-init-host subcommand.""" |
1516 | |
1517 | import unittest |
1518 | |
1519 | @@ -26,7 +26,7 @@ |
1520 | |
1521 | class FinishInitHostTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1522 | |
1523 | - sub_command_class = finish_inithost.SubCommand |
1524 | + subcommand_class = finish_inithost.SubCommand |
1525 | expected_arguments = get_arguments() |
1526 | expected_handlers = (handlers.handle_user, handlers.handle_target_dir) |
1527 | |
1528 | |
1529 | === modified file 'lpsetup/tests/subcommands/test_inithost.py' |
1530 | --- lpsetup/tests/subcommands/test_inithost.py 2012-08-06 10:04:17 +0000 |
1531 | +++ lpsetup/tests/subcommands/test_inithost.py 2012-08-13 12:44:19 +0000 |
1532 | @@ -2,7 +2,7 @@ |
1533 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1534 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1535 | |
1536 | -"""Tests for the inithost sub command.""" |
1537 | +"""Tests for the inithost subcommand.""" |
1538 | |
1539 | import os |
1540 | import shutil |
1541 | @@ -39,7 +39,7 @@ |
1542 | |
1543 | class InithostSmokeTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1544 | |
1545 | - sub_command_class = inithost.SubCommand |
1546 | + subcommand_class = inithost.SubCommand |
1547 | expected_arguments = get_arguments() |
1548 | expected_handlers = ( |
1549 | handlers.handle_user, |
1550 | @@ -47,11 +47,7 @@ |
1551 | handlers.handle_userdata, |
1552 | handlers.handle_ssh_keys, |
1553 | ) |
1554 | - expected_steps = ( |
1555 | - initialize_step, |
1556 | - setup_home_step, |
1557 | - initialize_lxc_step, |
1558 | - setup_apt_step,) |
1559 | + expected_steps = (initialize_step, setup_home_step, setup_apt_step) |
1560 | needs_root = True |
1561 | |
1562 | |
1563 | |
1564 | === modified file 'lpsetup/tests/subcommands/test_initlxc.py' |
1565 | --- lpsetup/tests/subcommands/test_initlxc.py 2012-08-06 09:37:04 +0000 |
1566 | +++ lpsetup/tests/subcommands/test_initlxc.py 2012-08-13 12:44:19 +0000 |
1567 | @@ -2,7 +2,7 @@ |
1568 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1569 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1570 | |
1571 | -"""Tests for the initlxc sub command.""" |
1572 | +"""Tests for the initlxc subcommand.""" |
1573 | |
1574 | import random |
1575 | import unittest |
1576 | @@ -46,7 +46,7 @@ |
1577 | |
1578 | class InitLxcTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1579 | |
1580 | - sub_command_class = initlxc.SubCommand |
1581 | + subcommand_class = initlxc.SubCommand |
1582 | expected_arguments = get_arguments() |
1583 | expected_handlers = ( |
1584 | handlers.handle_user, |
1585 | |
1586 | === modified file 'lpsetup/tests/subcommands/test_initrepo.py' |
1587 | --- lpsetup/tests/subcommands/test_initrepo.py 2012-07-25 15:57:26 +0000 |
1588 | +++ lpsetup/tests/subcommands/test_initrepo.py 2012-08-13 12:44:19 +0000 |
1589 | @@ -48,8 +48,7 @@ |
1590 | @skip_if_no_lpuser |
1591 | class InitrepoTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1592 | |
1593 | - sub_command_name = 'init-repo' |
1594 | - sub_command_class = initrepo.SubCommand |
1595 | + subcommand_class = initrepo.SubCommand |
1596 | expected_arguments = get_arguments() |
1597 | expected_handlers = ( |
1598 | handlers.handle_user, |
1599 | |
1600 | === modified file 'lpsetup/tests/subcommands/test_install_lxc.py' |
1601 | --- lpsetup/tests/subcommands/test_install_lxc.py 2012-08-01 20:37:15 +0000 |
1602 | +++ lpsetup/tests/subcommands/test_install_lxc.py 2012-08-13 12:44:19 +0000 |
1603 | @@ -2,7 +2,7 @@ |
1604 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1605 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1606 | |
1607 | -"""Tests for the install-lxc sub command.""" |
1608 | +"""Tests for the install-lxc subcommand.""" |
1609 | |
1610 | import unittest |
1611 | |
1612 | @@ -48,7 +48,7 @@ |
1613 | |
1614 | class InstallLxcTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1615 | |
1616 | - sub_command_class = install_lxc.SubCommand |
1617 | + subcommand_class = install_lxc.SubCommand |
1618 | expected_arguments = get_arguments() |
1619 | expected_handlers = test_initlxc.InitLxcTest.expected_handlers + ( |
1620 | handlers.handle_testing, |
1621 | |
1622 | === modified file 'lpsetup/tests/subcommands/test_smoke.py' |
1623 | --- lpsetup/tests/subcommands/test_smoke.py 2012-07-30 13:37:25 +0000 |
1624 | +++ lpsetup/tests/subcommands/test_smoke.py 2012-08-13 12:44:19 +0000 |
1625 | @@ -1,19 +1,16 @@ |
1626 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1627 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1628 | |
1629 | -"""Smoke tests for the sub commands.""" |
1630 | +"""Smoke tests for the subcommands.""" |
1631 | |
1632 | import unittest |
1633 | |
1634 | from lpsetup.cli import ( |
1635 | main, |
1636 | - subcommands, |
1637 | + SUBCOMMANDS, |
1638 | ) |
1639 | |
1640 | -from lpsetup.tests.utils import ( |
1641 | - capture_output, |
1642 | - root_not_needed, |
1643 | - ) |
1644 | +from lpsetup.tests.utils import capture_output |
1645 | |
1646 | |
1647 | class SmokeTest(unittest.TestCase): |
1648 | @@ -22,8 +19,8 @@ |
1649 | def test_subcommand_smoke_via_help(self): |
1650 | # Perform a basic smoke test by running each subcommand's --help |
1651 | # function and verify that a non-error exit code is returned. |
1652 | - for subcommand, callable in subcommands: |
1653 | - self.assertEqual(main([subcommand, '--help']), 0) |
1654 | + for name, _ in SUBCOMMANDS: |
1655 | + self.assertEqual(main([name, '--help']), 0) |
1656 | |
1657 | def test_subcommand_smoke_via_dry_run(self): |
1658 | # Perform a basic smoke test by running each subcommand's dry run |
1659 | @@ -37,13 +34,10 @@ |
1660 | 'install-lxc': required_args, |
1661 | 'update': [], |
1662 | } |
1663 | - name_class_map = dict(subcommands) |
1664 | warning = 'This command will perform the following actions:' |
1665 | for name, args in name_args_map.items(): |
1666 | - subcommand = name_class_map[name] |
1667 | with capture_output() as output: |
1668 | - with root_not_needed(subcommand): |
1669 | - retcode = main([name, '--dry-run'] + args) |
1670 | + retcode = main([name, '--dry-run'] + args) |
1671 | self.assertFalse(retcode) |
1672 | - # Ensure sub command description is printed to stdout. |
1673 | + # Ensure subcommand description is printed to stdout. |
1674 | self.assertIn(warning, output.getvalue()) |
1675 | |
1676 | === modified file 'lpsetup/tests/subcommands/test_update.py' |
1677 | --- lpsetup/tests/subcommands/test_update.py 2012-08-01 16:17:15 +0000 |
1678 | +++ lpsetup/tests/subcommands/test_update.py 2012-08-13 12:44:19 +0000 |
1679 | @@ -33,8 +33,7 @@ |
1680 | |
1681 | class UpdateTest(StepsBasedSubCommandTestMixin, unittest.TestCase): |
1682 | |
1683 | - sub_command_name = 'update' |
1684 | - sub_command_class = update.SubCommand |
1685 | + subcommand_class = update.SubCommand |
1686 | expected_arguments = get_arguments() |
1687 | expected_handlers = (handlers.handle_user, handlers.handle_target_dir) |
1688 | expected_steps = ( |
1689 | |
1690 | === modified file 'lpsetup/tests/subcommands/test_version.py' |
1691 | --- lpsetup/tests/subcommands/test_version.py 2012-06-25 10:41:46 +0000 |
1692 | +++ lpsetup/tests/subcommands/test_version.py 2012-08-13 12:44:19 +0000 |
1693 | @@ -2,24 +2,26 @@ |
1694 | # Copyright 2012 Canonical Ltd. This software is licensed under the |
1695 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1696 | |
1697 | -"""Tests for the version sub command.""" |
1698 | +"""Tests for the version subcommand.""" |
1699 | |
1700 | +import argparse |
1701 | +import os |
1702 | +import sys |
1703 | import unittest |
1704 | |
1705 | from lpsetup import get_version |
1706 | from lpsetup.subcommands import version |
1707 | -from lpsetup.tests.utils import ( |
1708 | - capture_output, |
1709 | - SubCommandTestMixin, |
1710 | - ) |
1711 | - |
1712 | - |
1713 | -class VersionTest(SubCommandTestMixin, unittest.TestCase): |
1714 | - |
1715 | - sub_command_class = version.SubCommand |
1716 | - |
1717 | - def test_sub_command(self): |
1718 | +from lpsetup.tests.utils import capture_output |
1719 | + |
1720 | + |
1721 | +class VersionTest(unittest.TestCase): |
1722 | + |
1723 | + def test_subcommand(self): |
1724 | + # Ensure the version subcommand prints to stdout |
1725 | + # the actual app name and version. |
1726 | + subcommand = version.SubCommand() |
1727 | with capture_output() as output: |
1728 | - self.parse_and_call_main() |
1729 | - bits = output.getvalue().strip().split() |
1730 | - self.assertEqual('v' + get_version(), bits[1]) |
1731 | + subcommand.run(argparse.Namespace()) |
1732 | + app_name, app_version = output.getvalue().strip().split() |
1733 | + self.assertEqual(os.path.basename(sys.argv[0]), app_name) |
1734 | + self.assertEqual('v' + get_version(), app_version) |
1735 | |
1736 | === modified file 'lpsetup/tests/test_argparser.py' |
1737 | --- lpsetup/tests/test_argparser.py 2012-07-30 12:04:54 +0000 |
1738 | +++ lpsetup/tests/test_argparser.py 2012-08-13 12:44:19 +0000 |
1739 | @@ -4,30 +4,69 @@ |
1740 | |
1741 | """Tests for the argparser module.""" |
1742 | |
1743 | -from contextlib import nested |
1744 | -import subprocess |
1745 | +import argparse |
1746 | +from collections import OrderedDict |
1747 | +import itertools |
1748 | +import sys |
1749 | import unittest |
1750 | |
1751 | from lpsetup import argparser |
1752 | +from lpsetup.utils import get_terminal_width |
1753 | from lpsetup.exceptions import ValidationError |
1754 | -from lpsetup.tests import examples |
1755 | from lpsetup.tests.utils import ( |
1756 | - capture_error, |
1757 | - capture_output, |
1758 | - RawInputReturning, |
1759 | - SubCommandTestMixin, |
1760 | + call_replaced_by, |
1761 | + ParserTestMixin, |
1762 | + StepsFactoryTestMixin, |
1763 | + SubCommandFactoryTestMixin, |
1764 | ) |
1765 | |
1766 | |
1767 | -class ArgumentParserTest(unittest.TestCase): |
1768 | - |
1769 | - def setUp(self): |
1770 | - self.parser = argparser.ArgumentParser() |
1771 | - |
1772 | - def get_sub_command(self): |
1773 | - return type( |
1774 | - 'SubCommand', (argparser.BaseSubCommand,), |
1775 | - {'run': lambda self, namespace: self.name}) |
1776 | +class AddCommonArgumentsTest(ParserTestMixin, unittest.TestCase): |
1777 | + |
1778 | + def test_interactive_supported(self): |
1779 | + # Ensure the resulting *namespace.interactive* is True if the sub |
1780 | + # command is interactive and `--yes` is not provided. |
1781 | + argparser.add_common_arguments(self.parser, has_interactive_run=True) |
1782 | + namespace = self.parser.parse_args([]) |
1783 | + self.assertTrue(namespace.interactive) |
1784 | + |
1785 | + def test_interactive_not_supported(self): |
1786 | + # Ensure the resulting *namespace.interactive* is False if the sub |
1787 | + # command does not support interactive execution. |
1788 | + argparser.add_common_arguments(self.parser, has_interactive_run=False) |
1789 | + namespace = self.parser.parse_args([]) |
1790 | + self.assertFalse(namespace.interactive) |
1791 | + |
1792 | + def test_assume_yes_provided(self): |
1793 | + # Ensure the resulting *namespace.interactive* is False if the sub |
1794 | + # command is interactive but `--yes` is provided. |
1795 | + argparser.add_common_arguments(self.parser, has_interactive_run=True) |
1796 | + namespace = self.parser.parse_args(['--yes']) |
1797 | + self.assertFalse(namespace.interactive) |
1798 | + |
1799 | + def test_dry_run_supported(self): |
1800 | + # Ensure the resulting *namespace.dry_run* is False if the sub |
1801 | + # command supports dry run execution but `--dry-run` is not provided. |
1802 | + argparser.add_common_arguments(self.parser, has_dry_run=True) |
1803 | + namespace = self.parser.parse_args([]) |
1804 | + self.assertFalse(namespace.dry_run) |
1805 | + |
1806 | + def test_dry_run_not_supported(self): |
1807 | + # Ensure the resulting *namespace.dry_run* is False if the sub |
1808 | + # command does not support dry run execution. |
1809 | + argparser.add_common_arguments(self.parser, has_dry_run=False) |
1810 | + namespace = self.parser.parse_args([]) |
1811 | + self.assertFalse(namespace.dry_run) |
1812 | + |
1813 | + def test_dry_run_provided(self): |
1814 | + # Ensure the resulting *namespace.dry_run* is True if the sub |
1815 | + # command supports dry run execution and `--dry-run` is provided. |
1816 | + argparser.add_common_arguments(self.parser, has_dry_run=True) |
1817 | + namespace = self.parser.parse_args(['--dry-run']) |
1818 | + self.assertTrue(namespace.dry_run) |
1819 | + |
1820 | + |
1821 | +class ArgumentParserTest(ParserTestMixin, unittest.TestCase): |
1822 | |
1823 | def test_add_argument(self): |
1824 | # Ensure actions are stored in a "public" instance attribute. |
1825 | @@ -36,228 +75,287 @@ |
1826 | dests = [action.dest for action in self.parser.actions] |
1827 | self.assertListEqual(['help', 'arg1', 'arg2'], dests) |
1828 | |
1829 | - def test_register_subcommand(self): |
1830 | - # The `main` method of the subcommand class is added to namespace, |
1831 | - # and can be used to actually handle the sub command execution. |
1832 | - name = 'foo' |
1833 | - self.parser.register_subcommand(name, self.get_sub_command()) |
1834 | - namespace = self.parser.parse_args([name]) |
1835 | - self.assertEqual(name, namespace.main(namespace)) |
1836 | - |
1837 | - def test_register_subcommand_providing_handler(self): |
1838 | - # Ensure a `handler` callable can also be provided to handle the |
1839 | - # subcommand execution. |
1840 | - callback = lambda namespace: 'custom handler' |
1841 | - self.parser.register_subcommand( |
1842 | - 'bar', self.get_sub_command(), callback=callback) |
1843 | - namespace = self.parser.parse_args(['bar']) |
1844 | - self.assertEqual(callback(namespace), namespace.main(namespace)) |
1845 | - |
1846 | - def test_get_args_from_namespace(self): |
1847 | + |
1848 | +class BaseSubCommandTest(SubCommandFactoryTestMixin, unittest.TestCase): |
1849 | + |
1850 | + def setUp(self): |
1851 | + self.namespace = argparse.Namespace() |
1852 | + |
1853 | + def _successful_handler(self, namespace): |
1854 | + namespace.bar = True |
1855 | + |
1856 | + def _failing_handler(self, namespace): |
1857 | + raise ValidationError('nothing is going on') |
1858 | + |
1859 | + def test_get_description(self): |
1860 | + # The default subcommand description is an empty string. |
1861 | + subcommand = self.make_subcommand() |
1862 | + self.assertEqual('', subcommand.get_description(self.namespace)) |
1863 | + |
1864 | + def test_prepare_namespace(self): |
1865 | + # The handlers are executed when *subcommand.prepare_namespace* |
1866 | + # is invoked. |
1867 | + subcommand = self.make_subcommand(handlers=[self._successful_handler]) |
1868 | + subcommand.prepare_namespace(self.namespace) |
1869 | + self.assertTrue(self.namespace.bar) |
1870 | + |
1871 | + def test_failing_handler(self): |
1872 | + # Ensure a validation error is raised by *subcommand.prepare_namespace* |
1873 | + # if the namespace is not valid. |
1874 | + subcommand = self.make_subcommand(handlers=[self._failing_handler]) |
1875 | + with self.assertRaises(ValidationError) as cm: |
1876 | + subcommand.prepare_namespace(self.namespace) |
1877 | + self.assertIn('nothing is going on', str(cm.exception)) |
1878 | + |
1879 | + |
1880 | +class GetArgsFromNamespaceTest(ParserTestMixin, unittest.TestCase): |
1881 | + |
1882 | + def test_different_namespace(self): |
1883 | # It is possible to recreate the argument list taking values from |
1884 | # a different namespace. |
1885 | self.parser.add_argument('--foo') |
1886 | self.parser.add_argument('bar') |
1887 | namespace = self.parser.parse_args('--foo eggs spam'.split()) |
1888 | namespace.foo = 'changed' |
1889 | - args = self.parser.get_args_from_namespace(namespace) |
1890 | + args = argparser.get_args_from_namespace(self.parser, namespace) |
1891 | self.assertSequenceEqual(['--foo', 'changed', 'spam'], args) |
1892 | |
1893 | - def test_args_from_namespace_with_multiple_values(self): |
1894 | - # Ensure *get_args_from_namespace* correcty handles options |
1895 | + def test_with_multiple_values(self): |
1896 | + # Ensure *get_args_from_namespace* correctly handles options |
1897 | # accepting multiple values. |
1898 | self.parser.add_argument('foo') |
1899 | self.parser.add_argument('--bar', nargs='+') |
1900 | namespace = self.parser.parse_args('foo --bar eggs spam'.split()) |
1901 | namespace.bar.append('another argument') |
1902 | - args = self.parser.get_args_from_namespace(namespace) |
1903 | + args = argparser.get_args_from_namespace(self.parser, namespace) |
1904 | expected = ['foo', '--bar', 'eggs', 'spam', 'another argument'] |
1905 | self.assertSequenceEqual(expected, args) |
1906 | |
1907 | - def test_args_from_namespace_with_boolean_values(self): |
1908 | - # Ensure *get_args_from_namespace* correcty handles options |
1909 | + def test_boolean_values(self): |
1910 | + # Ensure *get_args_from_namespace* correctly handles options |
1911 | # accepting boolean values. |
1912 | self.parser.add_argument('--foo', action='store_true') |
1913 | self.parser.add_argument('--bar', action='store_false') |
1914 | expected = ['--foo', '--bar'] |
1915 | namespace = self.parser.parse_args(expected) |
1916 | - args = self.parser.get_args_from_namespace(namespace) |
1917 | + args = argparser.get_args_from_namespace(self.parser, namespace) |
1918 | self.assertSequenceEqual(expected, args) |
1919 | |
1920 | - def test_help_subcommand(self): |
1921 | - # Ensure the help sub command is added if other commands exist. |
1922 | - self.parser.register_subcommand('foo', self.get_sub_command()) |
1923 | - namespace = self.parser.parse_args(['help']) |
1924 | - with self.assertRaises(SystemExit) as cm: |
1925 | - with capture_output() as output: |
1926 | - namespace.main(namespace) |
1927 | - self.assertEqual(0, cm.exception.code) |
1928 | - self.assertIn('usage:', output.getvalue()) |
1929 | - |
1930 | - def test_missing_help_subcommand(self): |
1931 | - # Ensure the help sub command is missing if no other commands exist. |
1932 | - with self.assertRaises(SystemExit) as cm: |
1933 | - with capture_error() as error: |
1934 | - self.parser.parse_args(['help']) |
1935 | - self.assertEqual(2, cm.exception.code) |
1936 | - self.assertIn('unrecognized arguments: help', error.getvalue()) |
1937 | - |
1938 | - |
1939 | -class BaseSubCommandTest(SubCommandTestMixin, unittest.TestCase): |
1940 | - |
1941 | - def set_handlers(self, *args): |
1942 | - self.sub_command.handlers = args |
1943 | - |
1944 | - def _successful_handler(self, namespace): |
1945 | - namespace.bar = True |
1946 | - |
1947 | - def _failing_handler(self, namespace): |
1948 | - raise ValidationError('nothing is going on') |
1949 | - |
1950 | - def test_name(self): |
1951 | - # Ensure a registered sub command has a name. |
1952 | - self.assertEqual(self.sub_command_name, self.sub_command.name) |
1953 | - |
1954 | - def test_arguments(self): |
1955 | - # Ensure the sub command arguments are correctly handled. |
1956 | - namespace = self.parse_and_call_main('--foo', 'eggs') |
1957 | - self.assertEqual('eggs', namespace.foo) |
1958 | - |
1959 | - def test_successful_validation(self): |
1960 | - # Ensure attached handlers are called by the default callback. |
1961 | - self.set_handlers(self._successful_handler) |
1962 | - namespace = self.parse_and_call_main() |
1963 | - self.assertTrue(namespace.bar) |
1964 | - |
1965 | - def test_failing_validation(self): |
1966 | - # Ensure `ValidationError` stops the command execution. |
1967 | - self.set_handlers(self._failing_handler) |
1968 | - with self.assertRaises(SystemExit) as cm: |
1969 | - with capture_error() as error: |
1970 | - self.parse_and_call_main() |
1971 | - self.assertEqual(2, cm.exception.code) |
1972 | - self.assertIn('nothing is going on', error.getvalue()) |
1973 | - |
1974 | - def test_help(self): |
1975 | - # The help attribute of sub command instances is used to generate |
1976 | - # the command usage message. |
1977 | - help = self.parser.format_help() |
1978 | - self.assertIn(self.sub_command.name, help) |
1979 | - self.assertIn(self.sub_command.help, help) |
1980 | - |
1981 | - def test_init_namespace(self): |
1982 | + |
1983 | +class GetStepNameTest(StepsFactoryTestMixin, unittest.TestCase): |
1984 | + |
1985 | + def test_named_step(self): |
1986 | + # Ensure the *step_name* attribute of a step, if present, is returned. |
1987 | + step = self.make_named_step('mystep') |
1988 | + self.assertEqual('mystep', argparser.get_step_name(step)) |
1989 | + |
1990 | + def test_function(self): |
1991 | + # If the *step_name* attribute is not present, *__name__* is used. |
1992 | + step = self.make_step() |
1993 | + self.assertEqual(step.__name__, argparser.get_step_name(step)) |
1994 | + |
1995 | + |
1996 | +class GetStepDescriptionTest(StepsFactoryTestMixin, unittest.TestCase): |
1997 | + |
1998 | + def assertStartsWith(self, prefix, content): |
1999 | + """Assert that content starts with prefix.""" |
2000 | + self.assertTrue( |
2001 | + content.startswith(prefix), |
2002 | + '%r does not start with %r' % (content, prefix)) |
2003 | + |
2004 | + def test_with_context(self): |
2005 | + # Ensure the description is correctly retrieved and formatted. |
2006 | + step = self.make_described_step('This step will do $stuff.') |
2007 | + description = argparser.get_step_description(step, stuff='nothing') |
2008 | + self.assertEqual('* This step will do nothing.', description) |
2009 | + |
2010 | + def test_without_context(self): |
2011 | + # The description can be still retrieved if no context is provided. |
2012 | + original = 'This step will do $stuff.' |
2013 | + step = self.make_described_step(original) |
2014 | + description = argparser.get_step_description(step) |
2015 | + self.assertEqual('* ' + original, description) |
2016 | + |
2017 | + def test_without_placeholder(self): |
2018 | + # Ensure an error is raised if a placeholder is missing. |
2019 | + step = self.make_described_step('This step will do $stuff.') |
2020 | + with self.assertRaises(KeyError): |
2021 | + argparser.get_step_description(step, foo='bar') |
2022 | + |
2023 | + def test_missing_description(self): |
2024 | + # Ensure an empty string is returned if the description is None. |
2025 | + step = self.make_described_step(None) |
2026 | + description = argparser.get_step_description(step) |
2027 | + self.assertEqual('', description) |
2028 | + |
2029 | + def test_no_description(self): |
2030 | + # Ensure an AttrubuteError is raised if the description is not found. |
2031 | + with self.assertRaises(AttributeError): |
2032 | + argparser.get_step_description(self.make_step()) |
2033 | + |
2034 | + def test_dedent(self): |
2035 | + # Ensure the description is correctly dedented. |
2036 | + original = """ |
2037 | + Hi there! |
2038 | + """ |
2039 | + step = self.make_described_step(original) |
2040 | + description = argparser.get_step_description(step) |
2041 | + self.assertEqual('* Hi there!', description) |
2042 | + |
2043 | + def test_empty_lines(self): |
2044 | + # Ensure empty lines in description are removed. |
2045 | + step = self.make_described_step('Hello.\n \nGoodbye.') |
2046 | + description = argparser.get_step_description(step) |
2047 | + self.assertEqual('* Hello.\n* Goodbye.', description) |
2048 | + |
2049 | + def test_wrapping(self): |
2050 | + # Ensure the description is correctly wrapped. |
2051 | + width = get_terminal_width() |
2052 | + elements = itertools.cycle('Lorem ipsum dolor sit amet.') |
2053 | + original = ''.join(itertools.islice(elements, width + 1)) |
2054 | + step = self.make_described_step(original) |
2055 | + description = argparser.get_step_description(step) |
2056 | + lines = description.splitlines() |
2057 | + self.assertEqual(2, len(lines)) |
2058 | + first_line, second_line = lines |
2059 | + self.assertStartsWith('* ', first_line) |
2060 | + self.assertStartsWith(' ', second_line) |
2061 | + |
2062 | + |
2063 | +class InitNamespaceTest(unittest.TestCase): |
2064 | + |
2065 | + def test_user_info_in_namespace(self): |
2066 | # The namespace is initialized with current user info. |
2067 | - namespace = self.parse() |
2068 | + namespace = argparse.Namespace() |
2069 | + argparser.init_namespace(namespace) |
2070 | self.assertIsInstance(namespace.euid, int) |
2071 | self.assertIsInstance(namespace.run_as_root, bool) |
2072 | |
2073 | |
2074 | -class StepsBasedSubCommandTest(SubCommandTestMixin, unittest.TestCase): |
2075 | - |
2076 | - sub_command_class = examples.StepsBasedSubCommand |
2077 | - |
2078 | - def test_steps(self): |
2079 | - # Ensure steps are executed in the order they are provided. |
2080 | - with capture_output() as output: |
2081 | - self.parse_and_call_main('--foo', 'eggs', '--bar', 'spam') |
2082 | - self.check_output( |
2083 | - ['step1 received eggs', 'step2 received eggs and spam'], |
2084 | - output) |
2085 | - |
2086 | - def test_steps_flag(self): |
2087 | - # A special argument `-s` or `--steps` is automatically added to the |
2088 | - # parser. It can be used to execute only one or a subset of steps. |
2089 | - with capture_output() as output: |
2090 | - self.parse_and_call_main('--foo', 'eggs', '-s', 'step1') |
2091 | - self.check_output(['step1 received eggs'], output) |
2092 | - |
2093 | - def test_skip_steps_flag(self): |
2094 | - # A special argument `--skip-steps` is automatically added to the |
2095 | - # parser. It can be used to skip one or more steps. |
2096 | - with capture_output() as output: |
2097 | - self.parse_and_call_main('--foo', 'eggs', '--skip-steps', 'step1') |
2098 | - self.check_output(['step2 received eggs and None'], output) |
2099 | - |
2100 | - def test_step_name(self): |
2101 | - # Ensure the string representation of a step is correctly retrieved. |
2102 | - method = self.sub_command._get_step_name |
2103 | - self.assertEqual('step1', method(examples.step1)) |
2104 | - self.assertEqual('mystep', method(examples.step2)) |
2105 | - |
2106 | - |
2107 | -class StepsBasedSubCommandWithErrorsTest( |
2108 | - SubCommandTestMixin, unittest.TestCase): |
2109 | - |
2110 | - sub_command_class = examples.StepsBasedSubCommandWithErrors |
2111 | - |
2112 | - def test_failing_step(self): |
2113 | - # Ensure the steps execution is stopped if a step raises |
2114 | - # `subprocess.CalledProcessError`. |
2115 | - with nested( |
2116 | - capture_output(), |
2117 | - self.assertRaises(subprocess.CalledProcessError)): |
2118 | - self.parse_and_call_main('--foo', 'eggs') |
2119 | - |
2120 | - |
2121 | -class DynamicStepsBasedSubCommandTest(SubCommandTestMixin, unittest.TestCase): |
2122 | - |
2123 | - sub_command_class = examples.DynamicStepsBasedSubCommand |
2124 | - |
2125 | - def test_dynamic_dispatcher(self): |
2126 | - # The test runner calls a function named 'call_[step name]' if it is |
2127 | - # defined. |
2128 | - with capture_output() as output: |
2129 | - self.parse_and_call_main('--foo', 'eggs', '--bar', 'spam') |
2130 | - expected = [ |
2131 | - 'running step1 with eggs while bar is spam', |
2132 | - 'step1 received eggs', |
2133 | - 'step2 received eggs and spam' |
2134 | +class ResolveStepsTest(StepsFactoryTestMixin, unittest.TestCase): |
2135 | + |
2136 | + def setUp(self): |
2137 | + self.step1 = self.make_step('step1') |
2138 | + self.step2 = self.make_step('step2') |
2139 | + self.step3 = self.make_named_step('step3') |
2140 | + self.steps = [(self.step1, []), (self.step2, []), (self.step3, [])] |
2141 | + |
2142 | + def resolve(self, steps, namespace): |
2143 | + """Resolve the steps and return them as an ordered dict.""" |
2144 | + return OrderedDict(argparser.resolve_steps(steps, namespace)) |
2145 | + |
2146 | + def test_arguments_resolving(self): |
2147 | + # Ensure the steps arguments are correctly resolved using |
2148 | + # the current namespace. |
2149 | + steps = [(self.step1, ['arg1', 'arg2']), (self.step2, ['arg3'])] |
2150 | + namespace = argparse.Namespace(arg1='foo', arg2='bar', arg3='spam') |
2151 | + steps_dict = self.resolve(steps, namespace) |
2152 | + self.assertListEqual(['foo', 'bar'], steps_dict[self.step1]) |
2153 | + self.assertListEqual(['spam'], steps_dict[self.step2]) |
2154 | + |
2155 | + def test_skip_steps(self): |
2156 | + # Ensure steps are correctly skipped if requested. |
2157 | + namespace = argparse.Namespace(skip_steps=['step1', 'step3']) |
2158 | + steps = self.resolve(self.steps, namespace).keys() |
2159 | + self.assertEqual(1, len(steps)) |
2160 | + self.assertIs(self.step2, steps[0]) |
2161 | + |
2162 | + def test_include_steps(self): |
2163 | + # Ensure not explicitly included steps are excluded. |
2164 | + namespace = argparse.Namespace(steps=['step1', 'step3']) |
2165 | + steps = self.resolve(self.steps, namespace).keys() |
2166 | + self.assertListEqual([self.step1, self.step3], steps) |
2167 | + |
2168 | + def test_include_skipped_steps(self): |
2169 | + # Ensure it's not possible to include a step already skipped. |
2170 | + namespace = argparse.Namespace( |
2171 | + steps=['step1'], skip_steps=['step1', 'step2', 'step3']) |
2172 | + self.assertEqual({}, self.resolve(self.steps, namespace)) |
2173 | + |
2174 | + def test_filter_function(self): |
2175 | + # The filter function can grant or deny the step execution. |
2176 | + steps = [ |
2177 | + (self.step1, [], lambda namespace: namespace.run_step1), |
2178 | + (self.step2, [], lambda namespace: True) |
2179 | ] |
2180 | - self.check_output(expected, output) |
2181 | - |
2182 | - |
2183 | -class InteractiveStepsBasedSubCommandTest( |
2184 | - SubCommandTestMixin, unittest.TestCase): |
2185 | - |
2186 | - sub_command_class = examples.InteractiveStepsBasedSubCommand |
2187 | - step_description = examples.step_with_description.description |
2188 | - |
2189 | - def test_command_description(self): |
2190 | - # Ensure the command description is generated collecting steps' |
2191 | - # descriptions. |
2192 | - with capture_output() as output: |
2193 | - with RawInputReturning('yes'): |
2194 | - self.parse_and_call_main() |
2195 | - self.assertIn(self.step_description, output.getvalue()) |
2196 | - |
2197 | - def test_interactive_execution_granted(self): |
2198 | - # Ensure the command executes if the user confirms to proceed. |
2199 | - with nested(capture_output(), RawInputReturning('yes')): |
2200 | - retcode = self.parse_and_call_main() |
2201 | - self.assertFalse(retcode) |
2202 | - |
2203 | - def test_interactive_execution_denied(self): |
2204 | - # Ensure the command exits with an error if the user denies execution. |
2205 | - with nested(capture_output(), RawInputReturning('no')): |
2206 | - retcode = self.parse_and_call_main() |
2207 | - self.assertEqual(1, retcode) |
2208 | - |
2209 | - def test_assume_yes(self): |
2210 | - # Ensure confirmation is not asked if `--yes` is provided. |
2211 | - with capture_output(): |
2212 | - with RawInputReturning('') as cm: |
2213 | - self.parse_and_call_main('--yes') |
2214 | - self.assertEqual(0, cm.call_count) |
2215 | - |
2216 | - def test_dry_run(self): |
2217 | - # Ensure a dry run is never interactive, exits without errors and |
2218 | - # prints out the command description. |
2219 | - with capture_output() as output: |
2220 | - with RawInputReturning('') as cm: |
2221 | - retcode = self.parse_and_call_main('--dry-run') |
2222 | - # Confirm has not been called. |
2223 | - self.assertEqual(0, cm.call_count) |
2224 | - # The command exits without errors. |
2225 | - self.assertFalse(retcode) |
2226 | - # The command description is displayed. |
2227 | - self.assertIn(self.step_description, output.getvalue()) |
2228 | + namespace = argparse.Namespace(run_step1=False) |
2229 | + steps = self.resolve(steps, namespace).keys() |
2230 | + self.assertEqual(1, len(steps)) |
2231 | + self.assertIs(self.step2, steps[0]) |
2232 | + |
2233 | + |
2234 | +class RestartAsRootTest(ParserTestMixin, unittest.TestCase): |
2235 | + |
2236 | + def test_restart_normal(self): |
2237 | + # Ensure an arbitrary command can be correctly restarted using `sudo`. |
2238 | + args = ['--foo', 'bar'] |
2239 | + self.parser.add_argument(args[0]) |
2240 | + namespace = self.parser.parse_args(args) |
2241 | + with call_replaced_by(lambda cmd: cmd): |
2242 | + cmd = argparser.restart_as_root(self.parser, namespace) |
2243 | + expected = ['sudo', sys.argv[0]] + args |
2244 | + self.assertListEqual(expected, cmd) |
2245 | + |
2246 | + def test_restart_subcommand(self): |
2247 | + # Ensure a subcommand is correctly restarted using `sudo`. |
2248 | + name = 'subcommand-name' |
2249 | + subparsers = self.parser.add_subparsers() |
2250 | + subparser = subparsers.add_parser(name) |
2251 | + namespace = self.parser.parse_args([name]) |
2252 | + with call_replaced_by(lambda cmd: cmd): |
2253 | + cmd = argparser.restart_as_root(subparser, namespace) |
2254 | + expected = ['sudo', sys.argv[0], name] |
2255 | + self.assertListEqual(expected, cmd) |
2256 | + |
2257 | + |
2258 | +class StepsBasedSubCommandTest( |
2259 | + SubCommandFactoryTestMixin, StepsFactoryTestMixin, unittest.TestCase): |
2260 | + |
2261 | + def make_subcommand_with_steps(self, *args): |
2262 | + """Create a steps based subcommand containing given steps. |
2263 | + |
2264 | + Each step takes no arguments. |
2265 | + """ |
2266 | + steps = [(step, []) for step in args] |
2267 | + return self.make_steps_based_subcommand(steps=steps) |
2268 | + |
2269 | + def test_add_arguments(self): |
2270 | + # The special arguments `--steps` and `--skip-steps` are |
2271 | + # automatically added to the parser. |
2272 | + step1 = self.make_named_step('step1') |
2273 | + step2 = self.make_named_step('step2') |
2274 | + step3 = self.make_named_step('step3') |
2275 | + subcommand = self.make_subcommand_with_steps(step1, step2, step3) |
2276 | + parser = argparse.ArgumentParser() |
2277 | + subcommand.add_arguments(parser) |
2278 | + steps_to_skip = ['step1', 'step2'] |
2279 | + steps_to_run = ['step3'] |
2280 | + namespace = parser.parse_args( |
2281 | + ['--skip-steps'] + steps_to_skip + ['--steps'] + steps_to_run) |
2282 | + self.assertListEqual(steps_to_skip, namespace.skip_steps) |
2283 | + self.assertListEqual(steps_to_run, namespace.steps) |
2284 | + |
2285 | + def test_get_description(self): |
2286 | + # Ensure the subcommand's description includes the description |
2287 | + # of each step. |
2288 | + step1 = self.make_described_step('step1 description') |
2289 | + step2 = self.make_described_step(None) |
2290 | + step3 = self.make_described_step('step3 description') |
2291 | + subcommand = self.make_subcommand_with_steps(step1, step2, step3) |
2292 | + description = subcommand.get_description(argparse.Namespace()) |
2293 | + # A step with no description is ignored. |
2294 | + step1_description, step3_description = description.split('\n') |
2295 | + self.assertIn(argparser.get_step_description(step1), step1_description) |
2296 | + self.assertIn(argparser.get_step_description(step3), step3_description) |
2297 | + |
2298 | + def test_run(self): |
2299 | + # Ensure the subcommand *run* method executes the steps |
2300 | + # in the order they are provided. |
2301 | + executed = [] |
2302 | + # Each step adds its name to *executed* when is run. |
2303 | + step1 = lambda: executed.append('step1') |
2304 | + step2 = lambda: executed.append('step2') |
2305 | + step3 = lambda: executed.append('step3') |
2306 | + subcommand = self.make_subcommand_with_steps(step1, step2, step3) |
2307 | + subcommand.run(argparse.Namespace()) |
2308 | + self.assertListEqual(['step1', 'step2', 'step3'], executed) |
2309 | |
2310 | === added file 'lpsetup/tests/test_cli.py' |
2311 | --- lpsetup/tests/test_cli.py 1970-01-01 00:00:00 +0000 |
2312 | +++ lpsetup/tests/test_cli.py 2012-08-13 12:44:19 +0000 |
2313 | @@ -0,0 +1,295 @@ |
2314 | +#!/usr/bin/env python |
2315 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
2316 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
2317 | + |
2318 | +"""Tests for the cli module.""" |
2319 | + |
2320 | +from contextlib import nested |
2321 | +import unittest |
2322 | + |
2323 | +from lpsetup import ( |
2324 | + cli, |
2325 | + exceptions, |
2326 | + ) |
2327 | +from lpsetup.tests.utils import ( |
2328 | + call_replaced_by, |
2329 | + capture_error, |
2330 | + capture_output, |
2331 | + ParserTestMixin, |
2332 | + RawInputReturning, |
2333 | + SubCommandFactoryTestMixin, |
2334 | + ) |
2335 | + |
2336 | + |
2337 | +class GetParserTest(SubCommandFactoryTestMixin, unittest.TestCase): |
2338 | + |
2339 | + def test_base_help(self): |
2340 | + # Ensure the help subcommand is correctly added to the parser. |
2341 | + parser = cli.get_parser([]) |
2342 | + namespace = parser.parse_args(['help']) |
2343 | + with self.assertRaises(SystemExit) as cm: |
2344 | + with capture_output() as output: |
2345 | + namespace.main(namespace) |
2346 | + self.assertEqual(0, cm.exception.code) |
2347 | + self.assertIn('usage:', output.getvalue()) |
2348 | + |
2349 | + def test_subcommand_in_global_help(self): |
2350 | + # The help attribute of subcommands is used to generate |
2351 | + # the base command usage message. |
2352 | + subcommand = self.make_subcommand(help='help message') |
2353 | + parser = cli.get_parser([('foo', subcommand)]) |
2354 | + help = parser.format_help() |
2355 | + self.assertIn(subcommand.help, help) |
2356 | + self.assertIn('foo', help) |
2357 | + |
2358 | + def test_help_subcommand(self): |
2359 | + # Ensure the help subcommand correctly show help messages retrieving |
2360 | + # info from other subcommands. |
2361 | + name = 'foo' |
2362 | + subcommand = self.make_subcommand( |
2363 | + help='subcommand help message', |
2364 | + add_arguments=lambda self, parser: parser.add_argument('--foo')) |
2365 | + parser = cli.get_parser([(name, subcommand)]) |
2366 | + namespace = parser.parse_args(['help', name]) |
2367 | + with self.assertRaises(SystemExit) as cm: |
2368 | + with capture_output() as output: |
2369 | + namespace.main(namespace) |
2370 | + output_value = output.getvalue() |
2371 | + # The help subcommand exits without errors. |
2372 | + self.assertEqual(0, cm.exception.code) |
2373 | + # The subcommand help is present in the output. |
2374 | + self.assertIn(subcommand.help, output_value) |
2375 | + # The subcommand name is displayed as well. |
2376 | + self.assertIn(name, output_value) |
2377 | + # The help subcommand also prints arguments info. |
2378 | + self.assertIn('--foo', output_value) |
2379 | + |
2380 | + def test_subcommands(self): |
2381 | + # Ensure sub parsers are added for each registered subcommand. |
2382 | + subcommands = ( |
2383 | + ('foo', self.make_subcommand_returning('foo')), |
2384 | + ('bar', self.make_subcommand_returning('bar')), |
2385 | + ) |
2386 | + parser = cli.get_parser(subcommands) |
2387 | + for name in dict(subcommands): |
2388 | + namespace = parser.parse_args([name]) |
2389 | + self.assertEqual(name, namespace.main(namespace)) |
2390 | + |
2391 | + |
2392 | +class PrepareParserTestMixin(ParserTestMixin, SubCommandFactoryTestMixin): |
2393 | + |
2394 | + def prepare_and_parse(self, subcommand, args=()): |
2395 | + """Prepare and parse *self.parser* passing given *subcommand*. |
2396 | + |
2397 | + Return the resulting namespace. |
2398 | + """ |
2399 | + cli.prepare_parser(self.parser, subcommand) |
2400 | + return self.parser.parse_args(args) |
2401 | + |
2402 | + |
2403 | +class PrepareParserTest(PrepareParserTestMixin, unittest.TestCase): |
2404 | + |
2405 | + def test_main_function_stored(self): |
2406 | + # Ensure *prepare_parser* stores the main function in the parser |
2407 | + # defaults, and that the main function calls *subcommand.run*. |
2408 | + subcommand = self.make_subcommand_returning('foo') |
2409 | + namespace = self.prepare_and_parse(subcommand) |
2410 | + self.assertEqual('foo', namespace.main(namespace)) |
2411 | + |
2412 | + def test_common_arguments(self): |
2413 | + # Ensure common arguments are added to the parser and to the resulting |
2414 | + # namespace. |
2415 | + subcommand = self.make_subcommand( |
2416 | + has_dry_run=True, has_interactive_run=True) |
2417 | + args = ('--dry-run', '--yes') |
2418 | + namespace = self.prepare_and_parse(subcommand, args) |
2419 | + self.assertTrue(namespace.dry_run) |
2420 | + self.assertFalse(namespace.interactive) |
2421 | + |
2422 | + def test_subcommand_arguments(self): |
2423 | + # Ensure subcommand specific arguments are added to the parser as well. |
2424 | + subcommand = self.make_subcommand( |
2425 | + add_arguments=lambda self, parser: parser.add_argument('--foo')) |
2426 | + args = ('--foo', 'bar') |
2427 | + namespace = self.prepare_and_parse(subcommand, args) |
2428 | + self.assertEqual('bar', namespace.foo) |
2429 | + |
2430 | + |
2431 | +class RunTest(PrepareParserTestMixin, unittest.TestCase): |
2432 | + |
2433 | + def assertHasAttr(self, obj, attr): |
2434 | + """Assert given *obj* has *attr*.""" |
2435 | + self.assertTrue( |
2436 | + hasattr(obj, attr), |
2437 | + '%r does not have %r attribute' % (obj, attr)) |
2438 | + |
2439 | + def make_described_subcommand(self, **kwargs): |
2440 | + """Create a subcommand having a *get_description* method. |
2441 | + |
2442 | + The created subcommand returns None. |
2443 | + """ |
2444 | + get_description = lambda self, namespace: 'subcommand description' |
2445 | + return self.make_subcommand_returning( |
2446 | + None, get_description=get_description, **kwargs) |
2447 | + |
2448 | + def test_init_namespace(self): |
2449 | + # Ensure *argparser.init_namespace* is invoked by *run*. |
2450 | + subcommand = self.make_subcommand_returning('foo') |
2451 | + namespace = self.prepare_and_parse(subcommand) |
2452 | + cli.run(self.parser, subcommand, namespace) |
2453 | + self.assertHasAttr(namespace, 'euid') |
2454 | + self.assertHasAttr(namespace, 'run_as_root') |
2455 | + |
2456 | + def test_prepare_namespace(self): |
2457 | + # Ensure *subcommand.prepare_namespace*, if defined, is invoked. |
2458 | + def prepare_namespace(self, namespace): |
2459 | + namespace.prepared = True |
2460 | + |
2461 | + subcommand = self.make_subcommand_returning( |
2462 | + 'foo', prepare_namespace=prepare_namespace) |
2463 | + namespace = self.prepare_and_parse(subcommand) |
2464 | + cli.run(self.parser, subcommand, namespace) |
2465 | + self.assertTrue(namespace.prepared) |
2466 | + |
2467 | + def test_validation_error_in_prepare_namespace(self): |
2468 | + # Ensure the execution is stopped if *prepare_namespace* raises a |
2469 | + # ValidationError. |
2470 | + msg = 'Nothing is really going on.' |
2471 | + |
2472 | + def prepare_namespace(self, namespace): |
2473 | + raise exceptions.ValidationError(msg) |
2474 | + |
2475 | + subcommand = self.make_subcommand(prepare_namespace=prepare_namespace) |
2476 | + namespace = self.prepare_and_parse(subcommand) |
2477 | + with self.assertRaises(SystemExit) as cm: |
2478 | + with capture_error() as error: |
2479 | + cli.run(self.parser, subcommand, namespace) |
2480 | + # The program exits with an error. |
2481 | + self.assertEqual(2, cm.exception.code) |
2482 | + # The validation error is printed to stderr. |
2483 | + self.assertIn(msg, error.getvalue()) |
2484 | + |
2485 | + def test_dry_run(self): |
2486 | + # The subcommand's description is printed to stdout and the execution |
2487 | + # is stopped without errors if this is a dry run. |
2488 | + subcommand = self.make_described_subcommand(has_dry_run=True) |
2489 | + namespace = self.prepare_and_parse(subcommand, ['--dry-run']) |
2490 | + with capture_output() as output: |
2491 | + retcode = cli.run(self.parser, subcommand, namespace) |
2492 | + # The command description is displayed. |
2493 | + self.assertIn(subcommand.get_description(namespace), output.getvalue()) |
2494 | + # The command exited without errors. |
2495 | + self.assertIsNone(retcode) |
2496 | + |
2497 | + def test_dry_run_is_never_interactive(self): |
2498 | + # Ensure a dry run is never interactive. |
2499 | + subcommand = self.make_described_subcommand( |
2500 | + has_dry_run=True, has_interactive_run=True) |
2501 | + namespace = self.prepare_and_parse(subcommand, ['--dry-run']) |
2502 | + with nested(capture_output(), RawInputReturning('')) as cms: |
2503 | + cli.run(self.parser, subcommand, namespace) |
2504 | + # The confirm function has not been called. |
2505 | + self.assertEqual(0, cms[1].call_count) |
2506 | + |
2507 | + def test_dry_run_never_requires_root(self): |
2508 | + # Ensure a dry run never requires to restart as root. |
2509 | + subcommand = self.make_described_subcommand( |
2510 | + root_required=True, has_dry_run=True) |
2511 | + namespace = self.prepare_and_parse(subcommand, ['--dry-run']) |
2512 | + with capture_output(): |
2513 | + retcode = cli.run(self.parser, subcommand, namespace) |
2514 | + # The command exited without errors and without a restart as root. |
2515 | + self.assertIsNone(retcode) |
2516 | + |
2517 | + def test_interactive_execution_granted(self): |
2518 | + # Ensure the execution continues if the user confirms to proceed. |
2519 | + subcommand = self.make_subcommand_returning( |
2520 | + None, has_interactive_run=True) |
2521 | + namespace = self.prepare_and_parse(subcommand) |
2522 | + with nested(capture_output(), RawInputReturning('yes')) as cms: |
2523 | + retcode = cli.run(self.parser, subcommand, namespace) |
2524 | + # The command exited without errors. |
2525 | + self.assertIsNone(retcode) |
2526 | + # The user confirmed the execution. |
2527 | + self.assertEqual(1, cms[1].call_count) |
2528 | + |
2529 | + def test_interactive_execution_denied(self): |
2530 | + # Ensure the command exits with an error if the user denies execution. |
2531 | + subcommand = self.make_subcommand_returning( |
2532 | + None, has_interactive_run=True) |
2533 | + namespace = self.prepare_and_parse(subcommand) |
2534 | + with nested(capture_output(), RawInputReturning('no')): |
2535 | + retcode = cli.run(self.parser, subcommand, namespace) |
2536 | + self.assertEqual(1, retcode) |
2537 | + |
2538 | + def test_assume_yes(self): |
2539 | + # Ensure confirmation is not asked if `--yes` is provided. |
2540 | + subcommand = self.make_subcommand_returning( |
2541 | + None, has_interactive_run=True) |
2542 | + namespace = self.prepare_and_parse(subcommand, ['--yes']) |
2543 | + with nested(capture_output(), RawInputReturning('')) as cms: |
2544 | + retcode = cli.run(self.parser, subcommand, namespace) |
2545 | + # The confirm function has not been called. |
2546 | + self.assertEqual(0, cms[1].call_count) |
2547 | + # The command exited without errors. |
2548 | + self.assertIsNone(retcode) |
2549 | + |
2550 | + def test_restart_as_root(self): |
2551 | + # Ensure the execution is restarted as root if required. |
2552 | + subcommand = self.make_described_subcommand(root_required=True) |
2553 | + namespace = self.prepare_and_parse(subcommand) |
2554 | + with call_replaced_by(lambda cmd: cmd): |
2555 | + cmd = cli.run(self.parser, subcommand, namespace) |
2556 | + self.assertEqual('sudo', cmd[0]) |
2557 | + |
2558 | + def test_execution_error_in_run(self): |
2559 | + # An error is returned if the subcommand's *run()* |
2560 | + # raises an ExecutionError |
2561 | + execution_error = exceptions.ExecutionError() |
2562 | + |
2563 | + def run(self, namespace): |
2564 | + raise execution_error |
2565 | + |
2566 | + subcommand = self.make_subcommand(run=run) |
2567 | + namespace = self.prepare_and_parse(subcommand) |
2568 | + error = cli.run(self.parser, subcommand, namespace) |
2569 | + self.assertIs(execution_error, error) |
2570 | + |
2571 | + |
2572 | +class MainTest(PrepareParserTestMixin, unittest.TestCase): |
2573 | + |
2574 | + def test_normal_execution(self): |
2575 | + # Ensure *main* executes the main function as it is found |
2576 | + # in the namespace, and return its return value. |
2577 | + self.parser.set_defaults(main=lambda namespace: 1) |
2578 | + self.assertEqual(1, cli.main([], self.parser)) |
2579 | + |
2580 | + def test_subcommand_execution(self): |
2581 | + # Ensure *main* returns the subcommand's *run()* return value. |
2582 | + subcommand = self.make_subcommand_returning(1) |
2583 | + parser = cli.get_parser([('foo', subcommand)]) |
2584 | + self.assertEqual(1, cli.main(['foo'], parser)) |
2585 | + |
2586 | + def test_help(self): |
2587 | + # Ensure *main* returns a non-error exit code if help is invoked. |
2588 | + with capture_output(): |
2589 | + self.assertEqual(0, cli.main(['-h'], self.parser)) |
2590 | + |
2591 | + def test_subcommand_help(self): |
2592 | + # Ensure *main* returns a non-error exit code if a subcommand's help |
2593 | + # is requested. |
2594 | + subcommand = self.make_subcommand(help='help message') |
2595 | + parser = cli.get_parser([('foo', subcommand)]) |
2596 | + with capture_output() as output: |
2597 | + retcode = cli.main(['help', 'foo'], parser) |
2598 | + self.assertEqual(0, retcode) |
2599 | + self.assertIn(subcommand.help, output.getvalue()) |
2600 | + |
2601 | + def test_invalid_arguments(self): |
2602 | + # Ensure *main* returns an error exit code if wrong arguments are |
2603 | + # provided. |
2604 | + invalid_args = 'invalid arguments' |
2605 | + with capture_error() as error: |
2606 | + retcode = cli.main(invalid_args.split(), self.parser) |
2607 | + self.assertEqual(2, retcode) |
2608 | + self.assertIn(invalid_args, error.getvalue()) |
2609 | |
2610 | === modified file 'lpsetup/tests/test_utils.py' |
2611 | --- lpsetup/tests/test_utils.py 2012-08-09 14:32:10 +0000 |
2612 | +++ lpsetup/tests/test_utils.py 2012-08-13 12:44:19 +0000 |
2613 | @@ -6,7 +6,6 @@ |
2614 | |
2615 | from datetime import datetime |
2616 | import getpass |
2617 | -import itertools |
2618 | import os |
2619 | import shutil |
2620 | import sys |
2621 | @@ -14,6 +13,8 @@ |
2622 | import tempfile |
2623 | import unittest |
2624 | |
2625 | +from shelltoolbox import environ |
2626 | + |
2627 | from lpsetup import get_version |
2628 | from lpsetup.settings import ( |
2629 | LXC_LP_DIR_PATTERN, |
2630 | @@ -32,7 +33,6 @@ |
2631 | get_lxc_gateway, |
2632 | get_network_interfaces, |
2633 | get_running_containers, |
2634 | - get_step_description, |
2635 | get_terminal_width, |
2636 | render_to_file, |
2637 | retry, |
2638 | @@ -207,83 +207,21 @@ |
2639 | self.assertRunning([], ['c1', 'c2', 'c3']) |
2640 | |
2641 | |
2642 | -class GetStepDescriptionTest(unittest.TestCase): |
2643 | - |
2644 | - def assertStartsWith(self, prefix, content): |
2645 | - """Assert that content starts with prefix.""" |
2646 | - self.assertTrue( |
2647 | - content.startswith(prefix), |
2648 | - '%r does not start with %r' % (content, prefix)) |
2649 | - |
2650 | - def get_step(self, description=None): |
2651 | - step = lambda: None |
2652 | - step.description = description |
2653 | - return step |
2654 | - |
2655 | - def test_with_context(self): |
2656 | - # Ensure the description is correctly retrieved and formatted. |
2657 | - step = self.get_step('This step will do $stuff.') |
2658 | - description = get_step_description(step, stuff='nothing') |
2659 | - self.assertEqual('* This step will do nothing.', description) |
2660 | - |
2661 | - def test_without_context(self): |
2662 | - # The description can be still retrieved if no context is provided. |
2663 | - original = 'This step will do $stuff.' |
2664 | - description = get_step_description(self.get_step(original)) |
2665 | - self.assertEqual('* ' + original, description) |
2666 | - |
2667 | - def test_without_placeholder(self): |
2668 | - # Ensure an error is raised if a placeholder is missing. |
2669 | - expected = 'This step will do $stuff.' |
2670 | - with self.assertRaises(KeyError): |
2671 | - get_step_description(self.get_step(expected), foo='bar') |
2672 | - |
2673 | - def test_missing_description(self): |
2674 | - # Ensure an empty string is returned if the description is None. |
2675 | - description = get_step_description(self.get_step()) |
2676 | - self.assertEqual('', description) |
2677 | - |
2678 | - def test_no_description(self): |
2679 | - # Ensure an AttrubuteError is raised if the description is not found. |
2680 | - with self.assertRaises(AttributeError): |
2681 | - get_step_description(lambda: None) |
2682 | - |
2683 | - def test_dedent(self): |
2684 | - # Ensure the description is correctly dedented. |
2685 | - original = """ |
2686 | - Hi there! |
2687 | - """ |
2688 | - description = get_step_description(self.get_step(original)) |
2689 | - self.assertEqual('* Hi there!', description) |
2690 | - |
2691 | - def test_empty_lines(self): |
2692 | - # Ensure empty lines in description are removed. |
2693 | - original = 'Hello.\n \nGoodbye.' |
2694 | - description = get_step_description(self.get_step(original)) |
2695 | - self.assertEqual('* Hello.\n* Goodbye.', description) |
2696 | - |
2697 | - def test_wrapping(self): |
2698 | - # Ensure the description is correctly wrapped. |
2699 | - width = get_terminal_width() |
2700 | - elements = itertools.cycle('Lorem ipsum dolor sit amet.') |
2701 | - original = ''.join(itertools.islice(elements, width + 1)) |
2702 | - description = get_step_description(self.get_step(original)) |
2703 | - lines = description.splitlines() |
2704 | - self.assertEqual(2, len(lines)) |
2705 | - first_line, second_line = lines |
2706 | - self.assertStartsWith('* ', first_line) |
2707 | - self.assertStartsWith(' ', second_line) |
2708 | - |
2709 | - |
2710 | -class GetTerminalSizeTest(unittest.TestCase): |
2711 | +class GetTerminalWidthTest(unittest.TestCase): |
2712 | |
2713 | def test_is_integer(self): |
2714 | # Ensure the returned value is an integer number. |
2715 | self.assertIsInstance(get_terminal_width(), int) |
2716 | |
2717 | def test_positive_values(self): |
2718 | - # Ensure the returned value is greater than 0. |
2719 | - self.assertGreater(get_terminal_width(), 0) |
2720 | + # Ensure the returned value is greater than 0 in any case. |
2721 | + with environ(COLUMNS='0'): |
2722 | + self.assertGreater(get_terminal_width(), 0) |
2723 | + |
2724 | + def test_invalid_columns(self): |
2725 | + # Ensure a number is returned even if $COLUMNS contains something else. |
2726 | + with environ(COLUMNS='not_a_number'): |
2727 | + self.assertIsInstance(get_terminal_width(), int) |
2728 | |
2729 | |
2730 | class RenderToFileTest(unittest.TestCase): |
2731 | |
2732 | === modified file 'lpsetup/tests/utils.py' |
2733 | --- lpsetup/tests/utils.py 2012-07-30 13:37:25 +0000 |
2734 | +++ lpsetup/tests/utils.py 2012-08-13 12:44:19 +0000 |
2735 | @@ -6,11 +6,17 @@ |
2736 | |
2737 | __metaclass__ = type |
2738 | __all__ = [ |
2739 | + 'call_replaced_by', |
2740 | + 'capture', |
2741 | 'capture_error', |
2742 | 'capture_output', |
2743 | + 'create_test_branch', |
2744 | 'get_random_string', |
2745 | - 'StepsBasedSubCommandTestMixin', |
2746 | - 'SubCommandTestMixin', |
2747 | + 'ParserTestMixin', |
2748 | + 'RawInputReturning', |
2749 | + 'skip_if_no_lpuser', |
2750 | + 'StepsFactoryTestMixin', |
2751 | + 'SubCommandFactoryTestMixin', |
2752 | ] |
2753 | |
2754 | from contextlib import contextmanager |
2755 | @@ -30,12 +36,40 @@ |
2756 | run, |
2757 | ) |
2758 | |
2759 | -from lpsetup import argparser |
2760 | -from lpsetup.tests import examples |
2761 | +from lpsetup import ( |
2762 | + argparser, |
2763 | + cli, |
2764 | + ) |
2765 | from lpsetup.utils import call |
2766 | |
2767 | |
2768 | @contextmanager |
2769 | +def call_replaced_by(function): |
2770 | + """Temporarily mock *subprocess.call* with the given *function*.""" |
2771 | + original, subprocess.call = subprocess.call, function |
2772 | + try: |
2773 | + yield |
2774 | + finally: |
2775 | + subprocess.call = original |
2776 | + |
2777 | + |
2778 | +class CallReplacedByTest(unittest.TestCase): |
2779 | + """Tests for the *call_replaced_by* context manager.""" |
2780 | + |
2781 | + def test_call_is_replaced(self): |
2782 | + # Ensure *subprocess.call* is correctly replaced. |
2783 | + with call_replaced_by(lambda *args, **kwargs: 'mocked'): |
2784 | + self.assertEqual('mocked', subprocess.call(['ls'])) |
2785 | + |
2786 | + def test_call_is_restored(self): |
2787 | + # Ensure *subprocess.call* is correctly restored exiting from the cm. |
2788 | + original = subprocess.call |
2789 | + with call_replaced_by(lambda *args, **kwargs: 'mocked'): |
2790 | + pass |
2791 | + self.assertIs(original, subprocess.call) |
2792 | + |
2793 | + |
2794 | +@contextmanager |
2795 | def capture(attr): |
2796 | output = StringIO() |
2797 | backup = getattr(sys, attr) |
2798 | @@ -124,8 +158,15 @@ |
2799 | lpuser is None, 'You need to set up a Launchpad login to run this test.') |
2800 | |
2801 | |
2802 | +class ParserTestMixin(object): |
2803 | + |
2804 | + def setUp(self): |
2805 | + """Set up an argument parser.""" |
2806 | + self.parser = argparser.ArgumentParser() |
2807 | + |
2808 | + |
2809 | class RawInputReturning(object): |
2810 | - """Mocks the *raw_input* builtin function. |
2811 | + """Mocks the *raw_input* built-in function. |
2812 | |
2813 | This context manager takes one or more pre-defined answers and |
2814 | keeps track of mocked *raw_input* call count. |
2815 | @@ -143,93 +184,123 @@ |
2816 | def __exit__(self, exc_type, exc_val, exc_tb): |
2817 | self._builtin.raw_input = self.original |
2818 | |
2819 | - def input(self, question): |
2820 | + def input(self, prompt=''): |
2821 | self.call_count += 1 |
2822 | return self.answers.next() |
2823 | |
2824 | |
2825 | -@contextmanager |
2826 | -def root_not_needed(subcommand): |
2827 | - """Temporarily set to False the *needs_root* flag of *subcommand*.""" |
2828 | - original, subcommand.needs_root = subcommand.needs_root, False |
2829 | - try: |
2830 | - yield |
2831 | - finally: |
2832 | - subcommand.needs_root = original |
2833 | - |
2834 | - |
2835 | -class RootNotNeededTest(unittest.TestCase): |
2836 | - |
2837 | - def test_context_manager(self): |
2838 | - # Ensure the context manager temporarily sets to False *needs_root*. |
2839 | - subcommand = type( |
2840 | - 'SubCommand', (argparser.BaseSubCommand,), {'needs_root': True}) |
2841 | - with root_not_needed(subcommand): |
2842 | - self.assertFalse(subcommand.needs_root) |
2843 | - self.assertTrue(subcommand.needs_root) |
2844 | - |
2845 | - |
2846 | -class SubCommandTestMixin(object): |
2847 | - |
2848 | - sub_command_class = examples.SubCommand |
2849 | - sub_command_name = 'subcmd' |
2850 | - |
2851 | - def setUp(self): |
2852 | - """Set up an argument parser and instantiate *self.sub_command_class*. |
2853 | - |
2854 | - The name used to create the sub command instance is |
2855 | - *self.sub_command_name*. |
2856 | - """ |
2857 | - self.parser = argparser.ArgumentParser() |
2858 | - self.sub_command = self.parser.register_subcommand( |
2859 | - self.sub_command_name, self.sub_command_class) |
2860 | - |
2861 | - def parse(self, *args): |
2862 | - """Parse given *args* and return an initialized namespace object.""" |
2863 | - namespace = self.parser.parse_args((self.sub_command_name,) + args) |
2864 | - sub_command = self.sub_command |
2865 | - sub_command.init_namespace(namespace) |
2866 | - sub_command.prepare_namespace(self.parser, namespace) |
2867 | - return namespace |
2868 | - |
2869 | - def parse_and_call_main(self, *args): |
2870 | - """Create a namespace using the given *args* and invoke main.""" |
2871 | - namespace = self.parse(*args) |
2872 | - return namespace.main(namespace) |
2873 | - |
2874 | - def check_output(self, expected, output): |
2875 | - value = filter(None, output.getvalue().split('\n')) |
2876 | - self.assertSequenceEqual(expected, value) |
2877 | - |
2878 | - |
2879 | -class StepsBasedSubCommandTestMixin(SubCommandTestMixin): |
2880 | - """This mixin can be used to test sub commands steps and handlers. |
2881 | +class RawInputReturningTest(unittest.TestCase): |
2882 | + """Tests for the *RawInputReturning* context manager.""" |
2883 | + |
2884 | + def test_original_is_replaced(self): |
2885 | + # Ensure the original function is correctly replaced. |
2886 | + with RawInputReturning('mocked'): |
2887 | + self.assertEqual('mocked', raw_input()) |
2888 | + |
2889 | + def test_original_is_retrievable(self): |
2890 | + # Ensure the original function is still stored as an attribute of the |
2891 | + # context manager. |
2892 | + original = raw_input |
2893 | + with RawInputReturning('mocked') as cm: |
2894 | + self.assertIs(original, cm.original) |
2895 | + |
2896 | + def test_multiple_return_values(self): |
2897 | + # Each call returns one of the provided values in order. |
2898 | + with RawInputReturning(*range(2)): |
2899 | + self.assertEqual(0, raw_input()) |
2900 | + self.assertEqual(1, raw_input()) |
2901 | + |
2902 | + def test_call_count(self): |
2903 | + # Ensure the context manager keeps track of call count. |
2904 | + return_values = range(2) |
2905 | + with RawInputReturning(*return_values) as cm: |
2906 | + self.assertEqual(0, cm.call_count) |
2907 | + for i in return_values: |
2908 | + raw_input() |
2909 | + self.assertEqual(2, cm.call_count) |
2910 | + |
2911 | + |
2912 | +class StepsFactoryTestMixin(object): |
2913 | + |
2914 | + def make_step(self, name='step'): |
2915 | + """Return a step callable.""" |
2916 | + def step(*args): |
2917 | + return args |
2918 | + |
2919 | + step.__name__ = name |
2920 | + return step |
2921 | + |
2922 | + def make_named_step(self, step_name): |
2923 | + """Return a named step callable.""" |
2924 | + step = self.make_step() |
2925 | + step.step_name = step_name |
2926 | + return step |
2927 | + |
2928 | + def make_described_step(self, description): |
2929 | + """Return a step having a description.""" |
2930 | + step = self.make_step() |
2931 | + step.description = description |
2932 | + return step |
2933 | + |
2934 | + |
2935 | +class SubCommandFactoryTestMixin(object): |
2936 | + |
2937 | + def make_subcommand(self, **kwargs): |
2938 | + """Create and return a subcommand. |
2939 | + |
2940 | + Attributes can be specified using kwargs. |
2941 | + """ |
2942 | + return type('SubCommand', (argparser.BaseSubCommand,), kwargs)() |
2943 | + |
2944 | + def make_subcommand_returning(self, value, **kwargs): |
2945 | + """Create a subcommand having the run function returning *value*.""" |
2946 | + run = lambda self, namespace: value |
2947 | + return self.make_subcommand(run=run, **kwargs) |
2948 | + |
2949 | + def make_steps_based_subcommand(self, **kwargs): |
2950 | + """Create and return a steps based subcommand. |
2951 | + |
2952 | + Attributes can be specified using kwargs. |
2953 | + """ |
2954 | + return type( |
2955 | + 'StepsBasedSubCommand', |
2956 | + (argparser.StepsBasedSubCommand,), |
2957 | + kwargs)() |
2958 | + |
2959 | + |
2960 | +class StepsBasedSubCommandTestMixin(ParserTestMixin): |
2961 | + """This mixin can be used to test subcommands steps and handlers. |
2962 | |
2963 | Real TestCases subclassing this mixin must define: |
2964 | |
2965 | + - subcommand_class: the steps based subcommand class |
2966 | - expected_arguments: a sequence of command line arguments |
2967 | - used by the current tested sub command |
2968 | + used by the current tested subcommand |
2969 | - expected_handlers: a sequence of expected handler callables |
2970 | - expected_steps: a sequence of expected *(step_callable, arg_names)* |
2971 | - - needs_root: True if this sub command must be run as root |
2972 | + - needs_root: True if this subcommand must be run as root |
2973 | |
2974 | At this point steps and handlers are automatically tested, and the test |
2975 | - case also checks if root is required by the sub command. |
2976 | + case also checks if root is required by the subcommand. |
2977 | """ |
2978 | def setUp(self): |
2979 | """Set up a namespace using *self.expected_arguments*.""" |
2980 | super(StepsBasedSubCommandTestMixin, self).setUp() |
2981 | - self.namespace = self.parse(*self.expected_arguments) |
2982 | + self.subcommand = self.subcommand_class() |
2983 | + cli.prepare_parser(self.parser, self.subcommand) |
2984 | + self.namespace = self.parser.parse_args(self.expected_arguments) |
2985 | + argparser.init_namespace(self.namespace) |
2986 | |
2987 | def test_handlers(self): |
2988 | - # Ensure this sub command uses the expected handlers. |
2989 | - handlers = self.sub_command.get_handlers(self.namespace) |
2990 | + # Ensure this subcommand uses the expected handlers. |
2991 | + handlers = self.subcommand.get_handlers(self.namespace) |
2992 | self.assertSequenceEqual(self.expected_handlers, handlers) |
2993 | |
2994 | def test_steps(self): |
2995 | - # Ensure this sub command wants to run the expected steps. |
2996 | - steps = self.sub_command.get_steps(self.namespace) |
2997 | - real_steps = [[step, list(args)] for _, step, args in steps] |
2998 | + # Ensure this subcommand wants to run the expected steps. |
2999 | + self.subcommand.prepare_namespace(self.namespace) |
3000 | + steps = argparser.resolve_steps(self.subcommand.steps, self.namespace) |
3001 | + real_steps = [[step, list(args)] for step, args in steps] |
3002 | expected_steps = [] |
3003 | for step, arg_names in self.expected_steps: |
3004 | args = [getattr(self.namespace, name) for name in arg_names] |
3005 | @@ -237,6 +308,6 @@ |
3006 | self.assertListEqual(expected_steps, real_steps) |
3007 | |
3008 | def test_needs_root(self): |
3009 | - # The root user may or may not be required to run this sub command. |
3010 | - needs_root = self.sub_command.get_needs_root(self.namespace) |
3011 | + # The root user may or may not be required to run this subcommand. |
3012 | + needs_root = getattr(self.subcommand, 'root_required', False) |
3013 | self.assertEqual(self.needs_root, needs_root) |
3014 | |
3015 | === modified file 'lpsetup/utils.py' |
3016 | --- lpsetup/utils.py 2012-08-09 14:33:33 +0000 |
3017 | +++ lpsetup/utils.py 2012-08-13 12:44:19 +0000 |
3018 | @@ -13,7 +13,6 @@ |
3019 | 'get_lxc_gateway', |
3020 | 'get_network_interfaces', |
3021 | 'get_running_containers', |
3022 | - 'get_step_description', |
3023 | 'get_terminal_width', |
3024 | 'lxc_in_state', |
3025 | 'lxc_ip', |
3026 | @@ -39,7 +38,6 @@ |
3027 | import re |
3028 | import subprocess |
3029 | import shutil |
3030 | -import string |
3031 | import sys |
3032 | import textwrap |
3033 | import time |
3034 | @@ -216,43 +214,25 @@ |
3035 | visited[container] = 1 |
3036 | |
3037 | |
3038 | -def get_step_description(step, **kwargs): |
3039 | - """Retrieve and format step description from the given *step* callable. |
3040 | - |
3041 | - *kwargs*, if provided, will be used as context to format the description. |
3042 | - Formatting is done using the Python's builtin templating system |
3043 | - *string.Template* supporting $-based substitutions. |
3044 | - |
3045 | - If placeholders are missing from *kwargs* an error will be raised. |
3046 | - """ |
3047 | - description = step.description |
3048 | - if not description: |
3049 | - # This step has no description, nothing else to do. |
3050 | - return '' |
3051 | - if kwargs: |
3052 | - s = string.Template(description) |
3053 | - description = s.substitute(**kwargs) |
3054 | - # Remove multiple spaces from lines. |
3055 | - lines = [' '.join(line.split()) for line in description.splitlines()] |
3056 | - # Retrieve all the non empty lines. |
3057 | - lines = filter(None, lines) |
3058 | - # For each line, wrap the contents. Note that we can't wrap the text of |
3059 | - # the entire paragraph because we want to preserve existing new lines. |
3060 | - width = get_terminal_width() |
3061 | - return '\n'.join(textwrap.fill( |
3062 | - line, width=width, initial_indent='* ', subsequent_indent=' ') |
3063 | - for line in lines) |
3064 | - |
3065 | - |
3066 | def get_terminal_width(): |
3067 | - """Return the terminal width (number of columns).""" |
3068 | + """Return the terminal width (number of columns). |
3069 | + |
3070 | + By default, `stty` is used to retrieve the terminal width. |
3071 | + However, users can override the returned value exporting $COLUMNS |
3072 | + (usually not exported). |
3073 | + """ |
3074 | + default = 79 |
3075 | + # Try using the environment variable $COLUMNS. |
3076 | try: |
3077 | - width = run('stty', 'size').split()[1] |
3078 | - except subprocess.CalledProcessError: |
3079 | - # No input available for stty, falling back to $COLUMNS. |
3080 | - # If $COLUMNS is not exported, return a default value. |
3081 | - width = os.getenv('COLUMNS', 79) |
3082 | - return int(width) |
3083 | + width = int(os.getenv('COLUMNS')) |
3084 | + except (TypeError, ValueError): |
3085 | + # $COLUMNS not exported or not a number: try using `stty`. |
3086 | + try: |
3087 | + width = int(run('stty', 'size').split()[1]) |
3088 | + except subprocess.CalledProcessError: |
3089 | + # No input available for stty, return a default value. |
3090 | + return default |
3091 | + return width if width > 0 else default |
3092 | |
3093 | |
3094 | def lxc_in_state(state, lxc_name, timeout=30): |
* This change increases our LOC from 5948 to 6243 (excluding distribute_ setup.py) a 5% increase.
* s/dry execution/dry run execution/
* Please pick one of "subcommand" or "sub command" and standardize. I prefer "subcommand".
* Running within an Emacs shell (useful for Emacs users to run pdb) results in:
]0;sapa: /home/bac/ lpsetup/ francesco /home/bac/ lpsetup/ francesco> ./lp-setup update --dry-run exit(cli. main()) bac/lpsetup/ francesco/ lpsetup/ cli.py" , line 169, in main main(namespace) bac/lpsetup/ francesco/ lpsetup/ cli.py" , line 106, in main bac/lpsetup/ francesco/ lpsetup/ cli.py" , line 70, in run description = subcommand. get_description (namespace) bac/lpsetup/ francesco/ lpsetup/ argparser. py", line 319, in get_description description( i[0], **context) for i in steps] bac/lpsetup/ francesco/ lpsetup/ argparser. py", line 466, in get_step_ description bac/lpsetup/ francesco/ lpsetup/ argparser. py", line 466, in <genexpr> python2. 7/textwrap. py", line 358, in fill python2. 7/textwrap. py", line 330, in fill self.wrap( text)) python2. 7/textwrap. py", line 321, in wrap chunks( chunks) python2. 7/textwrap. py", line 250, in _wrap_chunks
Traceback (most recent call last):
File "./lp-setup", line 13, in <module>
sys.
File "/home/
return namespace.
File "/home/
return run(parser, subcommand, namespace)
File "/home/
command_
File "/home/
descriptions = [get_step_
File "/home/
for line in lines)
File "/home/
for line in lines)
File "/usr/lib/
return w.fill(text)
File "/usr/lib/
return "\n".join(
File "/usr/lib/
return self._wrap_
File "/usr/lib/
raise ValueError("invalid width %r (must be > 0)" % self.width)
ValueError: invalid width 0 (must be > 0)
* Why do needs_root and get_handlers take a namespace? It is not used. YAGNI?
* I'm glad to see some methods on BaseSubCommand that never should have been methods (those which never reference 'self') have been moved to functions.
The approach you've taken here seems quite sane. You made the obvious choices but didn't go to the point where de-factoring (I made that up!) would cause you to have to violate DRY principles.
Honestly, though, as an experiment I find it to be too complicated to be compelling. A 3,000 line diff is not something that can be easily digested and so I find it hard to find the lessons in the work. I'm glad you did it, and I hope you learned along the way, but this branch is not one that we as a group can look at as a guiding example. You've done nothing wrong but were super ambitious.
As I haven't been able to give this branch the thorough review I'd like, I'm not going to vote on its approval. Please fix the items I've mentioned but you'll need to get Gary or Benji to do a proper review.