From 5c0b8c2b0229992671e076e74c1256a880381d62 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 16 Sep 2014 15:50:04 -0700 Subject: testsuite: Added unit tests for new option parsing --- doc/development/option_parsing.txt | 15 +- src/lib/Bcfg2/Options/Actions.py | 12 +- src/lib/Bcfg2/Options/OptionGroups.py | 61 ++- src/lib/Bcfg2/Options/Options.py | 128 ++++-- src/lib/Bcfg2/Options/Parser.py | 94 +++-- src/lib/Bcfg2/Options/Subcommands.py | 153 +++---- src/lib/Bcfg2/Options/Types.py | 10 +- src/lib/Bcfg2/Reporting/Reports.py | 3 +- src/lib/Bcfg2/Server/Admin.py | 14 +- src/lib/Bcfg2/Server/Info.py | 25 +- src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py | 4 +- .../Testlib/TestClient/TestTools/Test_init.py | 19 +- testsuite/Testsrc/Testlib/TestOptions/One.py | 6 + .../Testsrc/Testlib/TestOptions/TestComponents.py | 210 +++++++++ .../Testsrc/Testlib/TestOptions/TestConfigFiles.py | 48 +++ .../Testlib/TestOptions/TestOptionGroups.py | 145 +++++++ .../Testsrc/Testlib/TestOptions/TestOptions.py | 469 +++++++++++++++++++++ .../Testsrc/Testlib/TestOptions/TestSubcommands.py | 141 +++++++ testsuite/Testsrc/Testlib/TestOptions/TestTypes.py | 111 +++++ .../Testsrc/Testlib/TestOptions/TestWildcards.py | 47 +++ testsuite/Testsrc/Testlib/TestOptions/Two.py | 6 + testsuite/Testsrc/Testlib/TestOptions/__init__.py | 85 ++++ .../Testlib/TestServer/TestPlugin/Testbase.py | 1 + .../Testlib/TestServer/TestPlugin/Testhelpers.py | 1 + testsuite/Testsrc/Testlib/TestUtils.py | 4 - testsuite/Testsrc/__init__.py | 0 26 files changed, 1593 insertions(+), 219 deletions(-) create mode 100644 testsuite/Testsrc/Testlib/TestOptions/One.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestComponents.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestOptions.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestTypes.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/TestWildcards.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/Two.py create mode 100644 testsuite/Testsrc/Testlib/TestOptions/__init__.py create mode 100644 testsuite/Testsrc/__init__.py diff --git a/doc/development/option_parsing.txt b/doc/development/option_parsing.txt index 091f43cdd..642b9a36c 100644 --- a/doc/development/option_parsing.txt +++ b/doc/development/option_parsing.txt @@ -174,28 +174,31 @@ The normal implementation pattern is this: #. Define all of your subcommands as children of :class:`Bcfg2.Options.Subcommand`. -#. Define a :class:`Bcfg2.Options.CommandRegistry` object that will be +#. Create a :class:`Bcfg2.Options.CommandRegistry` object that will be used to register all of the commands. Registering a command collect its options and adds it as a :class:`Bcfg2.Options.Subparser` option group to the main option parser. #. Register your commands with - :func:`Bcfg2.Options.register_commands`, parse options, and run. + :func:`Bcfg2.Options.register_commands`. +#. Add options from the + :attr:`Bcfg2.Options.CommandRegistry.command_options` + attribute to the option parser. +#. Parse options, and run. :mod:`Bcfg2.Server.Admin` provides a fairly simple implementation, -where the CLI class is itself the command registry: +where the CLI class subclasses the command registry: .. code-block:: python class CLI(Bcfg2.Options.CommandRegistry): def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, - globals().values(), - parent=AdminCmd) + self.register_commands(globals().values(), parent=AdminCmd) parser = Bcfg2.Options.get_parser( description="Manage a running Bcfg2 server", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() In this case, commands are collected from amongst all global variables diff --git a/src/lib/Bcfg2/Options/Actions.py b/src/lib/Bcfg2/Options/Actions.py index 8b941f2bb..2916cc194 100644 --- a/src/lib/Bcfg2/Options/Actions.py +++ b/src/lib/Bcfg2/Options/Actions.py @@ -2,7 +2,8 @@ import sys import argparse -from Bcfg2.Options.Parser import get_parser +from Bcfg2.Options.Parser import get_parser, OptionParserException +from Bcfg2.Options.Options import _debug __all__ = ["ConfigFileAction", "ComponentAction", "PluginsAction"] @@ -101,7 +102,7 @@ class ComponentAction(FinalizableAction): fail_silently = False def __init__(self, *args, **kwargs): - if self.mapping: + if self.mapping and not self.islist: if 'choices' not in kwargs: kwargs['choices'] = self.mapping.keys() FinalizableAction.__init__(self, *args, **kwargs) @@ -143,7 +144,7 @@ class ComponentAction(FinalizableAction): if cls: get_parser().add_component(cls) elif not self.fail_silently: - print("Could not load component %s" % name) + raise OptionParserException("Could not load component %s" % name) return cls def __call__(self, parser, namespace, values, option_string=None): @@ -168,7 +169,10 @@ class ConfigFileAction(FinalizableAction): ``bcfg2-lint.conf``). """ def __call__(self, parser, namespace, values, option_string=None): - parser.add_config_file(self.dest, values, reparse=False) + if values: + parser.add_config_file(self.dest, values, reparse=False) + else: + _debug("No config file passed for %s" % self) FinalizableAction.__call__(self, parser, namespace, values, option_string=option_string) diff --git a/src/lib/Bcfg2/Options/OptionGroups.py b/src/lib/Bcfg2/Options/OptionGroups.py index 465358fab..426ee5dc0 100644 --- a/src/lib/Bcfg2/Options/OptionGroups.py +++ b/src/lib/Bcfg2/Options/OptionGroups.py @@ -10,7 +10,7 @@ __all__ = ["OptionGroup", "ExclusiveOptionGroup", "Subparser", "WildcardSectionGroup"] -class OptionContainer(list): +class _OptionContainer(list): """ Parent class of all option groups """ def list_options(self): @@ -29,7 +29,7 @@ class OptionContainer(list): opt.add_to_parser(parser) -class OptionGroup(OptionContainer): +class OptionGroup(_OptionContainer): """ Generic option group that is used only to organize options. This uses :meth:`argparse.ArgumentParser.add_argument_group` behind the scenes. """ @@ -43,16 +43,16 @@ class OptionGroup(OptionContainer): :param description: A longer description of the option group :param description: string """ - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) self.title = kwargs.pop('title') self.description = kwargs.pop('description', None) def add_to_parser(self, parser): group = parser.add_argument_group(self.title, self.description) - OptionContainer.add_to_parser(self, group) + _OptionContainer.add_to_parser(self, group) -class ExclusiveOptionGroup(OptionContainer): +class ExclusiveOptionGroup(_OptionContainer): """ Option group that ensures that only one argument in the group is present. This uses :meth:`argparse.ArgumentParser.add_mutually_exclusive_group` @@ -66,15 +66,15 @@ class ExclusiveOptionGroup(OptionContainer): specified. :type required: boolean """ - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) self.required = kwargs.pop('required', False) def add_to_parser(self, parser): group = parser.add_mutually_exclusive_group(required=self.required) - OptionContainer.add_to_parser(self, group) + _OptionContainer.add_to_parser(self, group) -class Subparser(OptionContainer): +class Subparser(_OptionContainer): """ Option group that adds options in it to a subparser. This uses a lot of functionality tied to `argparse Sub-commands `_. @@ -99,7 +99,7 @@ class Subparser(OptionContainer): """ self.name = kwargs.pop('name') self.help = kwargs.pop('help', None) - OptionContainer.__init__(self, items) + _OptionContainer.__init__(self, items) def __repr__(self): return "%s %s(%s)" % (self.__class__.__name__, @@ -111,11 +111,11 @@ class Subparser(OptionContainer): self._subparsers[parser] = parser.add_subparsers(dest='subcommand') subparser = self._subparsers[parser].add_parser(self.name, help=self.help) - OptionContainer.add_to_parser(self, subparser) + _OptionContainer.add_to_parser(self, subparser) -class WildcardSectionGroup(OptionContainer, Option): - """ WildcardSectionGroups contain options that may exist in +class WildcardSectionGroup(_OptionContainer, Option): + """WildcardSectionGroups contain options that may exist in several different sections of the config that match a glob. It works by creating options on the fly to match the sections described in the glob. For example, consider: @@ -134,7 +134,7 @@ class WildcardSectionGroup(OptionContainer, Option): .. code-block:: python >>> Bcfg2.Options.setup - Namespace(myplugin_bar_description='Bar description', myplugin_bar_number=2, myplugin_foo_description='Foo description', myplugin_foo_number=1, myplugin_sections=['myplugin:foo', 'myplugin:bar']) + Namespace(myplugin_bar_description='Bar description', myplugin_myplugin_bar_number=2, myplugin_myplugin_foo_description='Foo description', myplugin_myplugin_foo_number=1, myplugin_sections=['myplugin:foo', 'myplugin:bar']) All options must have the same section glob. @@ -146,10 +146,10 @@ class WildcardSectionGroup(OptionContainer, Option): ```` is the original `dest `_ of the option. ``
`` is the section that it's found in. - ```` is automatically generated from the section glob by - replacing all consecutive characters disallowed in Python variable - names into underscores. (This can be overridden with the - constructor.) + ```` is automatically generated from the section glob. + (This can be overridden with the constructor.) Both ``
`` + and ```` have had all consecutive characters disallowed in + Python variable names replaced with underscores. This group stores an additional option, the sections themselves, in an option given by ``sections``. @@ -171,17 +171,17 @@ class WildcardSectionGroup(OptionContainer, Option): that match the glob. :param dest: string """ - OptionContainer.__init__(self, []) + _OptionContainer.__init__(self, []) self._section_glob = items[0].cf[0] # get a default destination self._prefix = kwargs.get("prefix", self._dest_re.sub('_', self._section_glob)) Option.__init__(self, dest=kwargs.get('dest', self._prefix + "sections")) - self._options = items + self.option_templates = items def list_options(self): - return [self] + OptionContainer.list_options(self) + return [self] + _OptionContainer.list_options(self) def from_config(self, cfp): sections = [] @@ -189,10 +189,12 @@ class WildcardSectionGroup(OptionContainer, Option): if fnmatch.fnmatch(section, self._section_glob): sections.append(section) newopts = [] - for opt_tmpl in self._options: + for opt_tmpl in self.option_templates: option = copy.deepcopy(opt_tmpl) option.cf = (section, option.cf[1]) - option.dest = self._prefix + section + "_" + option.dest + option.dest = "%s%s_%s" % (self._prefix, + self._dest_re.sub('_', section), + option.dest) newopts.append(option) self.extend(newopts) for parser in self.parsers: @@ -201,4 +203,17 @@ class WildcardSectionGroup(OptionContainer, Option): def add_to_parser(self, parser): Option.add_to_parser(self, parser) - OptionContainer.add_to_parser(self, parser) + _OptionContainer.add_to_parser(self, parser) + + def __eq__(self, other): + return (_OptionContainer.__eq__(self, other) and + self.option_templates == getattr(other, "option_templates", + None)) + + def __repr__(self): + if len(self) == 0: + return "%s(%s)" % (self.__class__.__name__, + ", ".join(".".join(o.cf) + for o in self.option_templates)) + else: + return _OptionContainer.__repr__(self) diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 3874f810d..4efd76929 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -1,11 +1,15 @@ -""" The base :class:`Bcfg2.Options.Option` object represents an -option. Unlike options in :mod:`argparse`, an Option object does not -need to be associated with an option parser; it exists on its own.""" +"""Base :class:`Bcfg2.Options.Option` object to represent an option. -import os +Unlike options in :mod:`argparse`, an Option object does not need to +be associated with an option parser; it exists on its own. +""" + +import argparse import copy import fnmatch -import argparse +import os +import sys + from Bcfg2.Options import Types from Bcfg2.Compat import ConfigParser @@ -19,8 +23,9 @@ def _debug(msg): they're options, after all -- so option parsing verbosity is enabled by changing this to True. The verbosity here is primarily of use to developers. """ - if os.environ.get('BCFG2_OPTIONS_DEBUG', '0') == '1': - print(msg) + if os.environ.get('BCFG2_OPTIONS_DEBUG', '0').lower() in ["true", "yes", + "on", "1"]: + sys.stderr.write("%s\n" % msg) #: A dict that records a mapping of argparse action name (e.g., @@ -133,10 +138,11 @@ class Option(object): self._dest = None if 'dest' in self._kwargs: self._dest = self._kwargs.pop('dest') - elif self.cf is not None: - self._dest = self.cf[1] elif self.env is not None: self._dest = self.env + elif self.cf is not None: + self._dest = self.cf[1] + self._dest = self._dest.lower().replace("-", "_") kwargs = copy.copy(self._kwargs) kwargs.pop("action", None) self.actions[None] = action_cls(self._dest, self._dest, **kwargs) @@ -175,6 +181,17 @@ class Option(object): _debug("Finalizing %s" % self) action.finalize(parser, namespace) + @property + def _type_func(self): + """get a function for converting a value to the option type. + + this always returns a callable, even when ``type`` is None. + """ + if self.type: + return self.type + else: + return lambda x: x + def from_config(self, cfp): """ Get the value of this option from the given :class:`ConfigParser.ConfigParser`. If it is not found in the @@ -201,21 +218,33 @@ class Option(object): self.cf[1]) if o not in exclude]) else: - rv = dict() + rv = {} else: - if self.type: - rtype = self.type - else: - rtype = lambda x: x try: - rv = rtype(cfp.getboolean(*self.cf)) - except ValueError: - rv = rtype(cfp.get(*self.cf)) + rv = self._type_func(self.get_config_value(cfp)) except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): rv = None _debug("Setting %s from config file(s): %s" % (self, rv)) return rv + def get_config_value(self, cfp): + """fetch a value from the config file. + + This is passed the config parser. Its result is passed to the + type function for this option. It can be overridden to, e.g., + handle boolean options. + """ + return cfp.get(*self.cf) + + def get_environ_value(self, value): + """fetch a value from the environment. + + This is passed the raw value from the environment variable, + and its result is passed to the type function for this + option. It can be overridden to, e.g., handle boolean options. + """ + return value + def default_from_config(self, cfp): """ Set the default value of this option from the config file or from the environment. @@ -224,7 +253,8 @@ class Option(object): :type cfp: ConfigParser.ConfigParser """ if self.env and self.env in os.environ: - self.default = os.environ[self.env] + self.default = self._type_func( + self.get_environ_value(os.environ[self.env])) _debug("Setting the default of %s from environment: %s" % (self, self.default)) else: @@ -257,6 +287,13 @@ class Option(object): for action in self.actions.values(): action.dest = value + def early_parsing_hook(self, early_opts): # pylint: disable=C0111 + """Hook called at the end of early option parsing. + + This can be used to save option values for macro fixup. + """ + pass + #: The namespace destination of this option (see `dest #: `_) dest = property(_get_dest, _set_dest) @@ -289,8 +326,8 @@ class PathOption(Option): :meth:`Bcfg2.Options.Types.path` to transform the argument into a canonical path. - The type of a path option can also be overridden to return an - option file-like object. For example: + The type of a path option can also be overridden to return a + file-like object. For example: .. code-block:: python @@ -301,27 +338,52 @@ class PathOption(Option): """ def __init__(self, *args, **kwargs): - kwargs.setdefault('type', Types.path) + self._original_type = kwargs.pop('type', lambda x: x) + kwargs['type'] = self._type kwargs.setdefault('metavar', '') Option.__init__(self, *args, **kwargs) + self.repository = None + + def early_parsing_hook(self, early_opts): + self.repository = early_opts.repository + + def _type(self, value): + """Type function that fixes up macros.""" + if self.repository is None: + rv = value + else: + rv = value.replace("", self.repository) + return self._original_type(Types.path(rv)) class _BooleanOptionAction(argparse.Action): - """ BooleanOptionAction sets a boolean value in the following ways: + """BooleanOptionAction sets a boolean value. + - if None is passed, store the default - if the option_string is not None, then the option was passed on the command line, thus store the opposite of the default (this is the argparse store_true and store_false behavior) - if a boolean value is passed, use that + Makes a copy of the initial default, because otherwise the default + can be changed by config file settings or environment + variables. For instance, if a boolean option that defaults to True + was set to False in the config file, specifying the option on the + CLI would then set it back to True. + Defined here instead of :mod:`Bcfg2.Options.Actions` because otherwise - there is a circular import Options -> Actions -> Parser -> Options """ + there is a circular import Options -> Actions -> Parser -> Options. + """ + + def __init__(self, *args, **kwargs): + argparse.Action.__init__(self, *args, **kwargs) + self.original = self.default def __call__(self, parser, namespace, values, option_string=None): if values is None: setattr(namespace, self.dest, self.default) elif option_string is not None: - setattr(namespace, self.dest, not self.default) + setattr(namespace, self.dest, not self.original) else: setattr(namespace, self.dest, bool(values)) @@ -340,9 +402,25 @@ class BooleanOption(Option): kwargs.setdefault('action', _BooleanOptionAction) kwargs.setdefault('nargs', 0) kwargs.setdefault('default', False) - Option.__init__(self, *args, **kwargs) + def get_environ_value(self, value): + if value.lower() in ["false", "no", "off", "0"]: + return False + elif value.lower() in ["true", "yes", "on", "1"]: + return True + else: + raise ValueError("Invalid boolean value %s" % value) + + def get_config_value(self, cfp): + """fetch a value from the config file. + + This is passed the config parser. Its result is passed to the + type function for this option. It can be overridden to, e.g., + handle boolean options. + """ + return cfp.getboolean(*self.cf) + class PositionalArgument(Option): """ Shortcut for positional arguments. """ diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index 677a69e4c..ec470f8d3 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -1,13 +1,15 @@ -""" The option parser """ +"""The option parser.""" +import argparse import os import sys -import argparse + from Bcfg2.version import __version__ from Bcfg2.Compat import ConfigParser from Bcfg2.Options import Option, PathOption, BooleanOption, _debug -__all__ = ["setup", "OptionParserException", "Parser", "get_parser"] +__all__ = ["setup", "OptionParserException", "Parser", "get_parser", + "new_parser"] #: The repository option. This is specified here (and imported into @@ -108,13 +110,28 @@ class Parser(argparse.ArgumentParser): for component in components: self.add_component(component) + def _check_duplicate_cf(self, option): + """Check for a duplicate config file option.""" + def add_options(self, options): """ Add an explicit list of options to the parser. When possible, prefer :func:`Bcfg2.Options.Parser.add_component` to add a whole component instead.""" + _debug("Adding options: %s" % options) self.parsed = False for option in options: if option not in self.option_list: + # check for duplicates + if (hasattr(option, "env") and option.env and + option.env in [o.env for o in self.option_list]): + raise OptionParserException( + "Duplicate environment variable option: %s" % + option.env) + if (hasattr(option, "cf") and option.cf and + option.cf in [o.cf for o in self.option_list]): + raise OptionParserException( + "Duplicate config file option: %s" % (option.cf,)) + self.option_list.extend(option.list_options()) option.add_to_parser(self) @@ -169,8 +186,8 @@ class Parser(argparse.ArgumentParser): _debug("Resetting namespace") for attr in dir(self.namespace): if (not attr.startswith("_") and - attr not in ['uri', 'version', 'name'] and - attr not in self._config_files): + attr not in ['uri', 'version', 'name'] and + attr not in self._config_files): _debug("Deleting %s" % attr) delattr(self.namespace, attr) @@ -210,7 +227,7 @@ class Parser(argparse.ArgumentParser): """ _debug("Parsing options") if argv is None: - argv = sys.argv[1:] + argv = sys.argv[1:] # pragma: nocover if self.parsed and self.argv == argv: _debug("Returning already parsed namespace") return self.namespace @@ -231,8 +248,8 @@ class Parser(argparse.ArgumentParser): # phase 2: re-parse command line for early options; currently, # that's database options - _debug("Option parsing phase 2: Parse early options") if not self._early: + _debug("Option parsing phase 2: Parse early options") early_opts = argparse.Namespace() early_parser = Parser(add_help=False, namespace=early_opts, early=True) @@ -250,6 +267,10 @@ class Parser(argparse.ArgumentParser): if hasattr(component, "component_parsed_hook"): _debug("Calling component_parsed_hook on %s" % component) getattr(component, "component_parsed_hook")(early_opts) + for option in self.option_list: + option.early_parsing_hook(early_opts) + else: + _debug("Skipping parsing phase 2 in early mode") # phase 3: re-parse command line, loading additional # components, until all components have been loaded. On each @@ -275,29 +296,24 @@ class Parser(argparse.ArgumentParser): # namespace and _parse_config_options will never look at them again. self._set_defaults_from_config() self._parse_config_options() + remaining = [] while not self.parsed: self.parsed = True self._set_defaults_from_config() - self.parse_known_args(args=self.argv, namespace=self.namespace) + _debug("Parsing known arguments") + try: + _, remaining = self.parse_known_args(args=self.argv, + namespace=self.namespace) + except OptionParserException: + self.error(sys.exc_info()[1]) self._parse_config_options() self._finalize() + if len(remaining) and not self._early: + self.error("Unknown options: %s" % " ".join(remaining)) - # phase 4: fix up macros - _debug("Option parsing phase 4: Fix up macros") - repo = getattr(self.namespace, "repository", repository.default) - for attr in dir(self.namespace): - value = getattr(self.namespace, attr) - if (not attr.startswith("_") and - hasattr(value, "replace") and - "" in value): - setattr(self.namespace, attr, - value.replace("", repo, 1)) - _debug("Fixing up macros in %s: %s -> %s" % - (attr, value, getattr(self.namespace, attr))) - - # phase 5: call post-parsing hooks - _debug("Option parsing phase 5: Call hooks") + # phase 4: call post-parsing hooks if not self._early: + _debug("Option parsing phase 4: Call hooks") for component in self.components: if hasattr(component, "options_parsed_hook"): _debug("Calling post-parsing hook on %s" % component) @@ -311,23 +327,23 @@ class Parser(argparse.ArgumentParser): _parser = Parser() # pylint: disable=C0103 +def new_parser(): + """Create a new :class:`Bcfg2.Options.Parser` object. + + The new object can be retrieved with + :func:`Bcfg2.Options.get_parser`. This is useful for unit + testing. + """ + global _parser + _parser = Parser() + + def get_parser(description=None, components=None, namespace=None): - """ Get an existing :class:`Bcfg2.Options.Parser` object. (One is - created at the module level when :mod:`Bcfg2.Options` is - imported.) If no arguments are given, then the existing parser is - simply fetched. - - If arguments are given, then one of two things happens: - - * If this is the first ``get_parser`` call with arguments, then - the values given are set accordingly in the parser, and it is - returned. - * If this is not the first such call, then - :class:`Bcfg2.Options.OptionParserException` is raised. - - That is, a ``get_parser`` call with options is considered to - initialize the parser that already exists, and that can only - happen once. + """Get an existing :class:`Bcfg2.Options.Parser` object. + + A Parser is created at the module level when :mod:`Bcfg2.Options` + is imported. If any arguments are given, then the existing parser + is modified before being returned. :param description: Set the parser description :type description: string diff --git a/src/lib/Bcfg2/Options/Subcommands.py b/src/lib/Bcfg2/Options/Subcommands.py index 660bd5077..8972bde00 100644 --- a/src/lib/Bcfg2/Options/Subcommands.py +++ b/src/lib/Bcfg2/Options/Subcommands.py @@ -7,12 +7,13 @@ import sys import copy import shlex import logging + from Bcfg2.Compat import StringIO -from Bcfg2.Options import PositionalArgument +from Bcfg2.Options import PositionalArgument, _debug from Bcfg2.Options.OptionGroups import Subparser from Bcfg2.Options.Parser import Parser, setup as master_setup -__all__ = ["Subcommand", "HelpCommand", "CommandRegistry", "register_commands"] +__all__ = ["Subcommand", "CommandRegistry"] class Subcommand(object): @@ -96,8 +97,8 @@ class Subcommand(object): sio = StringIO() self.parser.print_usage(file=sio) usage = self._ws_re.sub(' ', sio.getvalue()).strip()[7:] - doc = self._ws_re.sub(' ', getattr(self, "__doc__")).strip() - if doc is None: + doc = self._ws_re.sub(' ', getattr(self, "__doc__") or "").strip() + if not doc: self._usage = usage else: self._usage = "%s - %s" % (usage, doc) @@ -119,120 +120,130 @@ class Subcommand(object): command from the interactive shell. :type setup: argparse.Namespace """ - raise NotImplementedError + raise NotImplementedError # pragma: nocover def shutdown(self): - """ Perform any necessary shtudown tasks for this command This + """ Perform any necessary shutdown tasks for this command This is called to when the program exits (*not* when this command is finished executing). """ - pass + pass # pragma: nocover -class HelpCommand(Subcommand): - """ Get help on a specific subcommand. This must be subclassed to - create the actual help command by overriding - :func:`Bcfg2.Options.HelpCommand.command_registry` and giving the - command access to a :class:`Bcfg2.Options.CommandRegistry`. """ +class Help(Subcommand): + """List subcommands and usage, or get help on a specific subcommand.""" options = [PositionalArgument("command", nargs='?')] # the interactive shell has its own help interactive = False - def command_registry(self): - """ Return a :class:`Bcfg2.Options.CommandRegistry` class. - All commands registered with the class will be included in the - help message. """ - raise NotImplementedError + def __init__(self, registry): + Subcommand.__init__(self) + self._registry = registry def run(self, setup): - commands = self.command_registry() + commands = self._registry.commands if setup.command: try: commands[setup.command].parser.print_help() return 0 except KeyError: print("No such command: %s" % setup.command) + return 1 for command in sorted(commands.keys()): print(commands[command].usage()) class CommandRegistry(object): - """ A ``CommandRegistry`` is used to register subcommands and - provides a single interface to run them. It's also used by - :class:`Bcfg2.Options.HelpCommand` to produce help messages for - all available commands. """ + """A ``CommandRegistry`` is used to register subcommands and provides + a single interface to run them. It's also used by + :class:`Bcfg2.Options.Subcommands.Help` to produce help messages + for all available commands. + """ + + def __init__(self): + #: A dict of registered commands. Keys are the class names, + #: lowercased (i.e., the command names), and values are instances + #: of the command objects. + self.commands = dict() - #: A dict of registered commands. Keys are the class names, - #: lowercased (i.e., the command names), and values are instances - #: of the command objects. - commands = dict() + #: A list of options that should be added to the option parser + #: in order to handle registered subcommands. + self.subcommand_options = [] - options = [] + #: the help command + self.help = Help(self) + self.register_command(self.help) def runcommand(self): """ Run the single command named in ``Bcfg2.Options.setup.subcommand``, which is where :class:`Bcfg2.Options.Subparser` groups store the subcommand. """ + _debug("Running subcommand %s" % master_setup.subcommand) try: return self.commands[master_setup.subcommand].run(master_setup) finally: self.shutdown() def shutdown(self): - """ Perform shutdown tasks. This calls the ``shutdown`` - method of all registered subcommands. """ + """Perform shutdown tasks. + + This calls the ``shutdown`` method of the subcommand that was + run. + """ + _debug("Shutting down subcommand %s" % master_setup.subcommand) self.commands[master_setup.subcommand].shutdown() - @classmethod - def register_command(cls, cmdcls): + def register_command(self, cls_or_obj): """ Register a single command. - :param cmdcls: The command class to register - :type cmdcls: type + :param cls_or_obj: The command class or object to register + :type cls_or_obj: type or Subcommand :returns: An instance of ``cmdcls`` """ - cmd_obj = cmdcls() + if isinstance(cls_or_obj, type): + cmdcls = cls_or_obj + cmd_obj = cmdcls() + else: + cmd_obj = cls_or_obj + cmdcls = cmd_obj.__class__ name = cmdcls.__name__.lower() - cls.commands[name] = cmd_obj + self.commands[name] = cmd_obj # py2.5 can't mix *magic and non-magical keyword args, thus # the **dict(...) - cls.options.append( + self.subcommand_options.append( Subparser(*cmdcls.options, **dict(name=name, help=cmdcls.__doc__))) - if issubclass(cls, cmd.Cmd) and cmdcls.interactive: - setattr(cls, "do_%s" % name, cmd_obj) - setattr(cls, "help_%s" % name, cmd_obj.parser.print_help) + if issubclass(self.__class__, cmd.Cmd) and cmdcls.interactive: + setattr(self, "do_%s" % name, cmd_obj) + setattr(self, "help_%s" % name, cmd_obj.parser.print_help) return cmd_obj - -def register_commands(registry, candidates, parent=Subcommand): - """ Register all subcommands in ``candidates`` against the - :class:`Bcfg2.Options.CommandRegistry` subclass given in - ``registry``. A command is registered if and only if: - - * It is a subclass of the given ``parent`` (by default, - :class:`Bcfg2.Options.Subcommand`); - * It is not the parent class itself; and - * Its name does not start with an underscore. - - :param registry: The :class:`Bcfg2.Options.CommandRegistry` - subclass against which commands will be - registered. - :type registry: Bcfg2.Options.CommandRegistry - :param candidates: A list of objects that will be considered for - registration. Only objects that meet the - criteria listed above will be registered. - :type candidates: list - :param parent: Specify a parent class other than - :class:`Bcfg2.Options.Subcommand` that all - registered commands must subclass. - :type parent: type - """ - for attr in candidates: - try: - if (issubclass(attr, parent) and - attr != parent and - not attr.__name__.startswith("_")): - registry.register_command(attr) - except TypeError: - pass + def register_commands(self, candidates, parent=Subcommand): + """ Register all subcommands in ``candidates`` against the + :class:`Bcfg2.Options.CommandRegistry` subclass given in + ``registry``. A command is registered if and only if: + + * It is a subclass of the given ``parent`` (by default, + :class:`Bcfg2.Options.Subcommand`); + * It is not the parent class itself; and + * Its name does not start with an underscore. + + :param registry: The :class:`Bcfg2.Options.CommandRegistry` + subclass against which commands will be + registered. + :type registry: Bcfg2.Options.CommandRegistry + :param candidates: A list of objects that will be considered for + registration. Only objects that meet the + criteria listed above will be registered. + :type candidates: list + :param parent: Specify a parent class other than + :class:`Bcfg2.Options.Subcommand` that all + registered commands must subclass. + :type parent: type + """ + for attr in candidates: + if (isinstance(attr, type) and + issubclass(attr, parent) and + attr != parent and + not attr.__name__.startswith("_")): + self.register_command(attr) diff --git a/src/lib/Bcfg2/Options/Types.py b/src/lib/Bcfg2/Options/Types.py index d11e54fba..1c04fede3 100644 --- a/src/lib/Bcfg2/Options/Types.py +++ b/src/lib/Bcfg2/Options/Types.py @@ -38,9 +38,11 @@ def comma_dict(value): for item in items: if '=' in item: key, value = item.split(r'=', 1) - try: - result[key] = bool(value) - except ValueError: + if value in ["true", "yes", "on"]: + result[key] = True + elif value in ["false", "no", "off"]: + result[key] = False + else: try: result[key] = int(value) except ValueError: @@ -107,8 +109,6 @@ def size(value): """ Given a number of bytes in a human-readable format (e.g., '512m', '2g'), get the absolute number of bytes as an integer. """ - if value == -1: - return value mat = _bytes_re.match(value) if not mat: raise ValueError("Not a valid size", value) diff --git a/src/lib/Bcfg2/Reporting/Reports.py b/src/lib/Bcfg2/Reporting/Reports.py index 219d74584..3b9c83433 100755 --- a/src/lib/Bcfg2/Reporting/Reports.py +++ b/src/lib/Bcfg2/Reporting/Reports.py @@ -267,10 +267,11 @@ class CLI(Bcfg2.Options.CommandRegistry): def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values()) + self.register_commands(globals().values()) parser = Bcfg2.Options.get_parser( description="Query the Bcfg2 reporting subsystem", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() def run(self): diff --git a/src/lib/Bcfg2/Server/Admin.py b/src/lib/Bcfg2/Server/Admin.py index 0807fb2b0..b6dded175 100644 --- a/src/lib/Bcfg2/Server/Admin.py +++ b/src/lib/Bcfg2/Server/Admin.py @@ -439,15 +439,6 @@ class Compare(AdminCmd): print("") -class Help(AdminCmd, Bcfg2.Options.HelpCommand): - """ Get help on a specific subcommand """ - def command_registry(self): - return CLI.commands - - def run(self, setup): - Bcfg2.Options.HelpCommand.run(self, setup) - - class Init(AdminCmd): """Interactively initialize a new repository.""" @@ -1194,13 +1185,14 @@ class Xcmd(_ProxyAdminCmd): class CLI(Bcfg2.Options.CommandRegistry): """ CLI class for bcfg2-admin """ + def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values(), - parent=AdminCmd) + self.register_commands(globals().values(), parent=AdminCmd) parser = Bcfg2.Options.get_parser( description="Manage a running Bcfg2 server", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() def run(self): diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index a5136f01d..0baa29a9f 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -123,15 +123,6 @@ class InfoCmd(Bcfg2.Options.Subcommand): # pylint: disable=W0223 list(self.core.metadata.groups.keys())) -class Help(InfoCmd, Bcfg2.Options.HelpCommand): - """ Get help on a specific subcommand """ - def command_registry(self): - return self.core.commands - - def run(self, setup): - Bcfg2.Options.HelpCommand.run(self, setup) - - class Debug(InfoCmd): """ Shell out to a Python interpreter """ interpreters, default_interpreter = load_interpreters() @@ -805,15 +796,12 @@ if HAS_PROFILE: display_trace(prof) -class InfoCore(cmd.Cmd, - Bcfg2.Server.Core.Core, - Bcfg2.Options.CommandRegistry): +class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core): """Main class for bcfg2-info.""" def __init__(self): cmd.Cmd.__init__(self) Bcfg2.Server.Core.Core.__init__(self) - Bcfg2.Options.CommandRegistry.__init__(self) self.prompt = 'bcfg2-info> ' def get_locals(self): @@ -853,16 +841,17 @@ class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core.shutdown(self) -class CLI(object): +class CLI(Bcfg2.Options.CommandRegistry): """ The bcfg2-info CLI """ options = [Bcfg2.Options.BooleanOption("-p", "--profile", help="Profile")] def __init__(self): - Bcfg2.Options.register_commands(InfoCore, globals().values(), - parent=InfoCmd) + Bcfg2.Options.CommandRegistry.__init__(self) + self.register_commands(globals().values(), parent=InfoCmd) parser = Bcfg2.Options.get_parser( description="Inspect a running Bcfg2 server", components=[self, InfoCore]) + parser.add_options(self.subcommand_options) parser.parse() if Bcfg2.Options.setup.profile and HAS_PROFILE: @@ -874,11 +863,11 @@ class CLI(object): print("Profiling functionality not available.") self.core = InfoCore() - for command in self.core.commands.values(): + for command in self.commands.values(): command.core = self.core def run(self): """ Run bcfg2-info """ if Bcfg2.Options.setup.subcommand != 'help': self.core.run() - return self.core.runcommand() + return self.runcommand() diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py b/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py index 48304d26e..f26d6ba18 100644 --- a/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py +++ b/src/lib/Bcfg2/Server/Plugins/Packages/YumHelper.py @@ -383,10 +383,10 @@ class CLI(Bcfg2.Options.CommandRegistry): def __init__(self): Bcfg2.Options.CommandRegistry.__init__(self) - Bcfg2.Options.register_commands(self.__class__, globals().values(), - parent=HelperSubcommand) + self.register_commands(globals().values(), parent=HelperSubcommand) parser = Bcfg2.Options.get_parser("Bcfg2 yum helper", components=[self]) + parser.add_options(self.subcommand_options) parser.parse() self.logger = logging.getLogger(parser.prog) diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py index 0e9e3a141..b9de703ff 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py @@ -17,18 +17,17 @@ while path != "/": path = os.path.dirname(path) from common import * -# try to find true -if os.path.exists("/bin/true"): - TRUE = "/bin/true" -elif os.path.exists("/usr/bin/true"): - TRUE = "/usr/bin/true" -else: - TRUE = None - class TestTool(Bcfg2TestCase): test_obj = Tool + if os.path.exists("/bin/true"): + true = "/bin/true" + elif os.path.exists("/usr/bin/true"): + true = "/usr/bin/true" + else: + true = None + def setUp(self): set_setup_default('command_timeout') set_setup_default('interactive', False) @@ -77,11 +76,11 @@ class TestTool(Bcfg2TestCase): ["/test"] + [e.get("name") for e in important]) t.getSupportedEntries.assert_called_with() - @skipIf(TRUE is None, "/bin/true or equivalent not found") + @skipIf(true is None, "/bin/true or equivalent not found") def test__check_execs(self): t = self.get_obj() if t.__execs__ == []: - t.__execs__.append(TRUE) + t.__execs__.append(self.true) @patch("os.stat") def inner(mock_stat): diff --git a/testsuite/Testsrc/Testlib/TestOptions/One.py b/testsuite/Testsrc/Testlib/TestOptions/One.py new file mode 100644 index 000000000..dac7f4558 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/One.py @@ -0,0 +1,6 @@ +"""Test module for component loading.""" + + +class One(object): + """Test class for component loading.""" + pass diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py b/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py new file mode 100644 index 000000000..d637d34c5 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py @@ -0,0 +1,210 @@ +"""test component loading.""" + +import argparse +import os + +from Bcfg2.Options import Option, BooleanOption, ComponentAction, get_parser, \ + new_parser, Types, ConfigFileAction + +from testsuite.Testsrc.Testlib.TestOptions import make_config, One, Two, \ + OptionTestCase + + +# create a bunch of fake components for testing component loading options + +class ChildOne(object): + """fake component for testing component loading.""" + options = [Option("--child-one")] + + +class ChildTwo(object): + """fake component for testing component loading.""" + options = [Option("--child-two")] + + +class ChildComponentAction(ComponentAction): + """child component loader action.""" + islist = False + mapping = {"one": ChildOne, + "two": ChildTwo} + + +class ComponentOne(object): + """fake component for testing component loading.""" + options = [BooleanOption("--one")] + + +class ComponentTwo(object): + """fake component for testing component loading.""" + options = [Option("--child", default="one", action=ChildComponentAction)] + + +class ComponentThree(object): + """fake component for testing component loading.""" + options = [BooleanOption("--three")] + + +class ConfigFileComponent(object): + """fake component for testing component loading.""" + options = [Option("--config2", action=ConfigFileAction), + Option(cf=("config", "test"), dest="config2_test", + default="bar")] + + +class ParentComponentAction(ComponentAction): + """parent component loader action.""" + mapping = {"one": ComponentOne, + "two": ComponentTwo, + "three": ComponentThree, + "config": ConfigFileComponent} + + +class TestComponentOptions(OptionTestCase): + """test cases for component loading.""" + + def setUp(self): + self.options = [ + Option("--parent", type=Types.comma_list, + default=["one", "two"], action=ParentComponentAction)] + + self.result = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.result, + description="component testing parser") + + @make_config() + def test_loading_components(self, config_file): + """load a single component during option parsing.""" + self.parser.parse(["-C", config_file, "--parent", "one"]) + self.assertEqual(self.result.parent, [ComponentOne]) + + @make_config() + def test_component_option(self, config_file): + """use options from a component loaded during option parsing.""" + self.parser.parse(["--one", "-C", config_file, "--parent", "one"]) + self.assertEqual(self.result.parent, [ComponentOne]) + self.assertTrue(self.result.one) + + @make_config() + def test_multi_component_load(self, config_file): + """load multiple components during option parsing.""" + self.parser.parse(["-C", config_file, "--parent", "one,three"]) + self.assertEqual(self.result.parent, [ComponentOne, ComponentThree]) + + @make_config() + def test_multi_component_options(self, config_file): + """use options from multiple components during option parsing.""" + self.parser.parse(["-C", config_file, "--three", + "--parent", "one,three", "--one"]) + self.assertEqual(self.result.parent, [ComponentOne, ComponentThree]) + self.assertTrue(self.result.one) + self.assertTrue(self.result.three) + + @make_config() + def test_component_default_not_loaded(self, config_file): + """options from default but unused components not available.""" + self.assertRaises( + SystemExit, + self.parser.parse, + ["-C", config_file, "--child", "one", "--parent", "one"]) + + @make_config() + def test_tiered_components(self, config_file): + """load child component.""" + self.parser.parse(["-C", config_file, "--parent", "two", + "--child", "one"]) + self.assertEqual(self.result.parent, [ComponentTwo]) + self.assertEqual(self.result.child, ChildOne) + + @make_config() + def test_options_tiered_components(self, config_file): + """use options from child component.""" + self.parser.parse(["--child-one", "foo", "-C", config_file, "--parent", + "two", "--child", "one"]) + self.assertEqual(self.result.parent, [ComponentTwo]) + self.assertEqual(self.result.child, ChildOne) + self.assertEqual(self.result.child_one, "foo") + + @make_config() + def test_bogus_component(self, config_file): + """error out with bad component name.""" + self.assertRaises(SystemExit, + self.parser.parse, + ["-C", config_file, "--parent", "blargle"]) + + @make_config() + @make_config({"config": {"test": "foo"}}) + def test_config_component(self, config1, config2): + """load component with alternative config file.""" + self.parser.parse(["-C", config1, "--config2", config2, + "--parent", "config"]) + self.assertEqual(self.result.config2, config2) + self.assertEqual(self.result.config2_test, "foo") + + @make_config() + def test_config_component_no_file(self, config_file): + """load component with missing alternative config file.""" + self.parser.parse(["-C", config_file, "--parent", "config"]) + self.assertEqual(self.result.config2, None) + + +class ImportComponentAction(ComponentAction): + """action that imports real classes for testing.""" + islist = False + bases = ["testsuite.Testsrc.Testlib.TestOptions"] + + +class ImportModuleAction(ImportComponentAction): + """action that only imports modules for testing.""" + module = True + + +class TestImportComponentOptions(OptionTestCase): + """test cases for component loading.""" + + def setUp(self): + self.options = [Option("--cls", action=ImportComponentAction), + Option("--module", action=ImportModuleAction)] + + self.result = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.result) + + @make_config() + def test_import_component(self, config_file): + """load class components by importing.""" + self.parser.parse(["-C", config_file, "--cls", "One"]) + self.assertEqual(self.result.cls, One.One) + + @make_config() + def test_import_module(self, config_file): + """load module components by importing.""" + self.parser.parse(["-C", config_file, "--module", "One"]) + self.assertEqual(self.result.module, One) + + @make_config() + def test_import_full_path(self, config_file): + """load components by importing the full path.""" + self.parser.parse(["-C", config_file, "--cls", "os.path"]) + self.assertEqual(self.result.cls, os.path) + + @make_config() + def test_import_bogus_class(self, config_file): + """fail to load class component that cannot be imported.""" + self.assertRaises(SystemExit, + self.parser.parse, + ["-C", config_file, "--cls", "Three"]) + + @make_config() + def test_import_bogus_module(self, config_file): + """fail to load module component that cannot be imported.""" + self.assertRaises(SystemExit, + self.parser.parse, + ["-C", config_file, "--module", "Three"]) + + @make_config() + def test_import_bogus_path(self, config_file): + """fail to load component that cannot be imported by full path.""" + self.assertRaises(SystemExit, + self.parser.parse, + ["-C", config_file, "--cls", "Bcfg2.No.Such.Thing"]) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py b/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py new file mode 100644 index 000000000..aee2ff666 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py @@ -0,0 +1,48 @@ +"""test reading multiple config files.""" + +import argparse + +from Bcfg2.Options import Option, PathOption, ConfigFileAction, get_parser, \ + new_parser + +from testsuite.Testsrc.Testlib.TestOptions import make_config, OptionTestCase + + +class TestConfigFiles(OptionTestCase): + def setUp(self): + self.options = [ + PathOption(cf=("test", "config2"), action=ConfigFileAction), + PathOption(cf=("test", "config3"), action=ConfigFileAction), + Option(cf=("test", "foo")), + Option(cf=("test", "bar")), + Option(cf=("test", "baz"))] + self.results = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.results) + + @make_config({"test": {"baz": "baz"}}) + def test_config_files(self, config3): + """read multiple config files.""" + # Because make_config() generates temporary files for the + # configuration, we have to work backwards here. first we + # generate config3, then we generate config2 (which includes a + # reference to config3), then we finally generate the main + # config file, which contains a reference to config2. oh how + # I wish we could use context managers here... + + @make_config({"test": {"bar": "bar", "config3": config3}}) + def inner1(config2): + @make_config({"test": {"foo": "foo", "config2": config2}}) + def inner2(config): + self.parser.parse(["-C", config]) + self.assertEqual(self.results.foo, "foo") + self.assertEqual(self.results.bar, "bar") + self.assertEqual(self.results.baz, "baz") + + inner2() + + inner1() + + def test_no_config_file(self): + """fail to read config file.""" + self.assertRaises(SystemExit, self.parser.parse, []) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py new file mode 100644 index 000000000..de1abbb1b --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py @@ -0,0 +1,145 @@ +"""test reading multiple config files.""" + +import argparse + +from Bcfg2.Options import Option, BooleanOption, Parser, OptionGroup, \ + ExclusiveOptionGroup, WildcardSectionGroup, new_parser, get_parser + +from testsuite.common import Bcfg2TestCase +from testsuite.Testsrc.Testlib.TestOptions import make_config, OptionTestCase + + +class TestOptionGroups(Bcfg2TestCase): + def setUp(self): + self.options = None + + def _test_options(self, options): + """test helper.""" + result = argparse.Namespace() + parser = Parser(components=[self], namespace=result) + parser.parse(options) + return result + + def test_option_group(self): + """basic option group functionality.""" + self.options = [OptionGroup(BooleanOption("--foo"), + BooleanOption("--bar"), + BooleanOption("--baz"), + title="group")] + result = self._test_options(["--foo", "--bar"]) + self.assertTrue(result.foo) + self.assertTrue(result.bar) + self.assertFalse(result.baz) + + def test_exclusive_option_group(self): + """parse options from exclusive option group.""" + self.options = [ + ExclusiveOptionGroup(BooleanOption("--foo"), + BooleanOption("--bar"), + BooleanOption("--baz"))] + result = self._test_options(["--foo"]) + self.assertTrue(result.foo) + self.assertFalse(result.bar) + self.assertFalse(result.baz) + + self.assertRaises(SystemExit, + self._test_options, ["--foo", "--bar"]) + + def test_required_exclusive_option_group(self): + """parse options from required exclusive option group.""" + self.options = [ + ExclusiveOptionGroup(BooleanOption("--foo"), + BooleanOption("--bar"), + BooleanOption("--baz"), + required=True)] + result = self._test_options(["--foo"]) + self.assertTrue(result.foo) + self.assertFalse(result.bar) + self.assertFalse(result.baz) + + self.assertRaises(SystemExit, self._test_options, []) + + def test_option_group(self): + """nest option groups.""" + self.options = [ + OptionGroup( + BooleanOption("--foo"), + BooleanOption("--bar"), + OptionGroup( + BooleanOption("--baz"), + BooleanOption("--quux"), + ExclusiveOptionGroup( + BooleanOption("--test1"), + BooleanOption("--test2")), + title="inner"), + title="outer")] + result = self._test_options(["--foo", "--baz", "--test1"]) + self.assertTrue(result.foo) + self.assertFalse(result.bar) + self.assertTrue(result.baz) + self.assertFalse(result.quux) + self.assertTrue(result.test1) + self.assertFalse(result.test2) + + self.assertRaises(SystemExit, + self._test_options, ["--test1", "--test2"]) + + +class TestWildcardSectionGroups(OptionTestCase): + config = { + "four:one": { + "foo": "foo one", + "bar": "bar one", + "baz": "baz one" + }, + "four:two": { + "foo": "foo two", + "bar": "bar two" + }, + "five:one": { + "foo": "foo one", + "bar": "bar one" + }, + "five:two": { + "foo": "foo two", + "bar": "bar two" + }, + "five:three": { + "foo": "foo three", + "bar": "bar three" + } + } + + def setUp(self): + self.options = [ + WildcardSectionGroup( + Option(cf=("four:*", "foo")), + Option(cf=("four:*", "bar"))), + WildcardSectionGroup( + Option(cf=("five:*", "foo")), + Option(cf=("five:*", "bar")), + prefix="", + dest="sections")] + self.results = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.results) + + @make_config(config) + def test_wildcard_section_groups(self, config_file): + """parse options from wildcard section groups.""" + self.parser.parse(["-C", config_file]) + self.assertEqual(self.results.four_four_one_foo, "foo one") + self.assertEqual(self.results.four_four_one_bar, "bar one") + self.assertEqual(self.results.four_four_two_foo, "foo two") + self.assertEqual(self.results.four_four_two_bar, "bar two") + self.assertItemsEqual(self.results.four_sections, + ["four:one", "four:two"]) + + self.assertEqual(self.results.five_one_foo, "foo one") + self.assertEqual(self.results.five_one_bar, "bar one") + self.assertEqual(self.results.five_two_foo, "foo two") + self.assertEqual(self.results.five_two_bar, "bar two") + self.assertEqual(self.results.five_three_foo, "foo three") + self.assertEqual(self.results.five_three_bar, "bar three") + self.assertItemsEqual(self.results.sections, + ["five:one", "five:two", "five:three"]) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py new file mode 100644 index 000000000..b76cd6d3a --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py @@ -0,0 +1,469 @@ +"""basic option parsing tests.""" + +import argparse +import os +import tempfile + +import mock + +from Bcfg2.Compat import ConfigParser +from Bcfg2.Options import Option, PathOption, BooleanOption, Parser, \ + PositionalArgument, OptionParserException, Common, new_parser, get_parser +from testsuite.Testsrc.Testlib.TestOptions import OptionTestCase, \ + make_config, clean_environment + + +class TestBasicOptions(OptionTestCase): + """test basic option parsing.""" + def setUp(self): + # parsing options can modify the Option objects themselves. + # that's probably bad -- and it's definitely bad if we ever + # want to do real on-the-fly config changes -- but it's easier + # to leave it as is and set the options on each test. + self.options = [ + BooleanOption("--test-true-boolean", env="TEST_TRUE_BOOLEAN", + cf=("test", "true_boolean"), default=True), + BooleanOption("--test-false-boolean", env="TEST_FALSE_BOOLEAN", + cf=("test", "false_boolean"), default=False), + BooleanOption(cf=("test", "true_config_boolean"), + default=True), + BooleanOption(cf=("test", "false_config_boolean"), + default=False), + Option("--test-option", env="TEST_OPTION", cf=("test", "option"), + default="foo"), + PathOption("--test-path-option", env="TEST_PATH_OPTION", + cf=("test", "path"), default="/test")] + + @clean_environment + def _test_options(self, options=None, env=None, config=None): + """helper to test a set of options. + + returns the namespace from parsing the given CLI options with + the given config and environment. + """ + if config is not None: + config = {"test": config} + if options is None: + options = [] + + @make_config(config) + def inner(config_file): + """do the actual tests, since py2.4 lacks context managers.""" + result = argparse.Namespace() + parser = Parser(components=[self], namespace=result) + parser.parse(argv=["-C", config_file] + options) + return result + + if env is not None: + for name, value in env.items(): + os.environ[name] = value + + return inner() + + def test_expand_path(self): + """expand ~ in path option.""" + options = self._test_options(options=["--test-path-option", + "~/test"]) + self.assertEqual(options.test_path_option, + os.path.expanduser("~/test")) + + def test_canonicalize_path(self): + """get absolute path from path option.""" + options = self._test_options(options=["--test-path-option", + "./test"]) + self.assertEqual(options.test_path_option, + os.path.abspath("./test")) + + def test_default_bool(self): + """use the default value of boolean options.""" + options = self._test_options() + self.assertTrue(options.test_true_boolean) + self.assertFalse(options.test_false_boolean) + self.assertTrue(options.true_config_boolean) + self.assertFalse(options.false_config_boolean) + + def test_default(self): + """use the default value of an option.""" + options = self._test_options() + self.assertEqual(options.test_option, "foo") + + def test_default_path(self): + """use the default value of a path option.""" + options = self._test_options() + self.assertEqual(options.test_path_option, "/test") + + def test_invalid_boolean(self): + """set boolean to invalid values.""" + self.assertRaises(ValueError, + self._test_options, + config={"true_boolean": "you betcha"}) + self.assertRaises(ValueError, + self._test_options, + env={"TEST_TRUE_BOOLEAN": "hell no"}) + + def test_set_boolean_in_config(self): + """set boolean options in config files.""" + set_to_defaults = {"true_boolean": "1", + "false_boolean": "0", + "true_config_boolean": "yes", + "false_config_boolean": "no"} + options = self._test_options(config=set_to_defaults) + self.assertTrue(options.test_true_boolean) + self.assertFalse(options.test_false_boolean) + self.assertTrue(options.true_config_boolean) + self.assertFalse(options.false_config_boolean) + + set_to_other = {"true_boolean": "false", + "false_boolean": "true", + "true_config_boolean": "off", + "false_config_boolean": "on"} + options = self._test_options(config=set_to_other) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + self.assertFalse(options.true_config_boolean) + self.assertTrue(options.false_config_boolean) + + def test_set_in_config(self): + """set options in config files.""" + options = self._test_options(config={"option": "foo"}) + self.assertEqual(options.test_option, "foo") + + options = self._test_options(config={"option": "bar"}) + self.assertEqual(options.test_option, "bar") + + def test_set_path_in_config(self): + """set path options in config files.""" + options = self._test_options(config={"path": "/test"}) + self.assertEqual(options.test_path_option, "/test") + + options = self._test_options(config={"path": "/foo"}) + self.assertEqual(options.test_path_option, "/foo") + + def test_set_boolean_in_env(self): + """set boolean options in environment.""" + set_to_defaults = {"TEST_TRUE_BOOLEAN": "1", + "TEST_FALSE_BOOLEAN": "0"} + options = self._test_options(env=set_to_defaults) + self.assertTrue(options.test_true_boolean) + self.assertFalse(options.test_false_boolean) + + set_to_other = {"TEST_TRUE_BOOLEAN": "false", + "TEST_FALSE_BOOLEAN": "true"} + options = self._test_options(env=set_to_other) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + + def test_set_in_env(self): + """set options in environment.""" + options = self._test_options(env={"TEST_OPTION": "foo"}) + self.assertEqual(options.test_option, "foo") + + options = self._test_options(env={"TEST_OPTION": "bar"}) + self.assertEqual(options.test_option, "bar") + + def test_set_path_in_env(self): + """set path options in environment.""" + options = self._test_options(env={"TEST_PATH_OPTION": "/test"}) + self.assertEqual(options.test_path_option, "/test") + + options = self._test_options(env={"TEST_PATH_OPTION": "/foo"}) + self.assertEqual(options.test_path_option, "/foo") + + def test_set_boolean_in_cli(self): + """set boolean options in CLI options.""" + # passing the option yields the reverse of the default, no + # matter the default + options = self._test_options(options=["--test-true-boolean", + "--test-false-boolean"]) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + + def test_set_in_cli(self): + """set options in CLI options.""" + options = self._test_options(options=["--test-option", "foo"]) + self.assertEqual(options.test_option, "foo") + + options = self._test_options(options=["--test-option", "bar"]) + self.assertEqual(options.test_option, "bar") + + def test_set_path_in_cli(self): + """set path options in CLI options.""" + options = self._test_options(options=["--test-path-option", "/test"]) + self.assertEqual(options.test_path_option, "/test") + + options = self._test_options(options=["--test-path-option", "/foo"]) + self.assertEqual(options.test_path_option, "/foo") + + def test_env_overrides_config_bool(self): + """setting boolean option in the environment overrides config file.""" + config = {"true_boolean": "false", + "false_boolean": "true"} + env = {"TEST_TRUE_BOOLEAN": "yes", + "TEST_FALSE_BOOLEAN": "no"} + options = self._test_options(config=config, env=env) + self.assertTrue(options.test_true_boolean) + self.assertFalse(options.test_false_boolean) + + def test_env_overrides_config(self): + """setting option in the environment overrides config file.""" + options = self._test_options(config={"option": "bar"}, + env={"TEST_OPTION": "baz"}) + self.assertEqual(options.test_option, "baz") + + def test_env_overrides_config_path(self): + """setting path option in the environment overrides config file.""" + options = self._test_options(config={"path": "/foo"}, + env={"TEST_PATH_OPTION": "/bar"}) + self.assertEqual(options.test_path_option, "/bar") + + def test_cli_overrides_config_bool(self): + """setting boolean option in the CLI overrides config file.""" + config = {"true_boolean": "on", + "false_boolean": "off"} + options = ["--test-true-boolean", "--test-false-boolean"] + options = self._test_options(options=options, config=config) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + + def test_cli_overrides_config(self): + """setting option in the CLI overrides config file.""" + options = self._test_options(options=["--test-option", "baz"], + config={"option": "bar"}) + self.assertEqual(options.test_option, "baz") + + def test_cli_overrides_config_path(self): + """setting path option in the CLI overrides config file.""" + options = self._test_options(options=["--test-path-option", "/bar"], + config={"path": "/foo"}) + self.assertEqual(options.test_path_option, "/bar") + + def test_cli_overrides_env_bool(self): + """setting boolean option in the CLI overrides environment.""" + env = {"TEST_TRUE_BOOLEAN": "0", + "TEST_FALSE_BOOLEAN": "1"} + options = ["--test-true-boolean", "--test-false-boolean"] + options = self._test_options(options=options, env=env) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + + def test_cli_overrides_env(self): + """setting option in the CLI overrides environment.""" + options = self._test_options(options=["--test-option", "baz"], + env={"TEST_OPTION": "bar"}) + self.assertEqual(options.test_option, "baz") + + def test_cli_overrides_env_path(self): + """setting path option in the CLI overrides environment.""" + options = self._test_options(options=["--test-path-option", "/bar"], + env={"TEST_PATH_OPTION": "/foo"}) + self.assertEqual(options.test_path_option, "/bar") + + def test_cli_overrides_all_bool(self): + """setting boolean option in the CLI overrides everything else.""" + config = {"true_boolean": "no", + "false_boolean": "yes"} + env = {"TEST_TRUE_BOOLEAN": "0", + "TEST_FALSE_BOOLEAN": "1"} + options = ["--test-true-boolean", "--test-false-boolean"] + options = self._test_options(options=options, env=env) + self.assertFalse(options.test_true_boolean) + self.assertTrue(options.test_false_boolean) + + def test_cli_overrides_all(self): + """setting option in the CLI overrides everything else.""" + options = self._test_options(options=["--test-option", "baz"], + env={"TEST_OPTION": "bar"}, + config={"test": "quux"}) + self.assertEqual(options.test_option, "baz") + + def test_cli_overrides_all_path(self): + """setting path option in the CLI overrides everything else.""" + options = self._test_options(options=["--test-path-option", "/bar"], + env={"TEST_PATH_OPTION": "/foo"}, + config={"path": "/baz"}) + self.assertEqual(options.test_path_option, "/bar") + + @make_config() + def _test_dest(self, *args, **kwargs): + """helper to test that ``dest`` is set properly.""" + args = list(args) + expected = args.pop(0) + config_file = args.pop() + + sentinel = object() + kwargs["default"] = sentinel + + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([Option(*args, **kwargs)]) + parser.parse(["-C", config_file]) + + self.assertTrue(hasattr(result, expected)) + self.assertEqual(getattr(result, expected), sentinel) + + def test_explicit_dest(self): + """set the ``dest`` of an option explicitly.""" + self._test_dest("bar", dest="bar") + + def test_dest_from_env_var(self): + """set the ``dest`` of an option from the env var name.""" + self._test_dest("foo", env="FOO") + + def test_dest_from_cf(self): + """set the ``dest`` of an option from the config option.""" + self._test_dest("foo_bar", cf=("test", "foo-bar")) + + def test_dest_from_cli(self): + """set the ``dest`` of an option from the CLI option.""" + self._test_dest("test_foo", "--test-foo") + + def test_dest_from_all(self): + """set the ``dest`` of an option from the best of multiple sources.""" + self._test_dest("foo_baz", cf=("test", "foo-bar"), env="FOO_BAZ") + self._test_dest("xyzzy", + "--xyzzy", cf=("test", "foo-bar"), env="FOO_BAZ") + self._test_dest("quux", + "--xyzzy", cf=("test", "foo-bar"), env="FOO_BAZ", + dest="quux") + + @make_config() + def test_positional_args(self, config_file): + """get values from positional arguments.""" + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([PositionalArgument("single")]) + parser.parse(["-C", config_file, "single"]) + self.assertEqual(result.single, "single") + + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([PositionalArgument("one"), + PositionalArgument("two")]) + parser.parse(["-C", config_file, "one", "two"]) + self.assertEqual(result.one, "one") + self.assertEqual(result.two, "two") + + def test_duplicate_cli_option(self): + """add duplicate CLI option.""" + parser = Parser(components=[self]) + self.assertRaises( + argparse.ArgumentError, + parser.add_options, + [Option("--test-option")]) + + def test_duplicate_env_option(self): + """add duplicate environment option.""" + parser = Parser(components=[self]) + self.assertRaises( + OptionParserException, + parser.add_options, + [Option(env="TEST_OPTION")]) + + def test_duplicate_cf_option(self): + """add duplicate config file option.""" + parser = Parser(components=[self]) + self.assertRaises( + OptionParserException, + parser.add_options, + [Option(cf=("test", "option"))]) + + @make_config() + def test_repository_macro(self, config_file): + """fix up macros.""" + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([PathOption("--test1"), + PathOption("--test2"), + Common.repository]) + parser.parse(["-C", config_file, "-Q", "/foo/bar", + "--test1", "/test1", + "--test2", ""]) + self.assertEqual(result.repository, "/foo/bar") + self.assertEqual(result.test1, "/foo/bar/test1") + self.assertEqual(result.test2, "/foo/bar/foo/bar") + + @make_config() + def test_file_like_path_option(self, config_file): + """get file-like object from PathOption.""" + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([PathOption("--test", type=argparse.FileType('r'))]) + + fd, name = tempfile.mkstemp() + fh = os.fdopen(fd, "w") + fh.write("test") + fh.close() + + parser.parse(["-C", config_file, "--test", name]) + self.assertEqual(result.test.name, name) + self.assertEqual(result.test.read(), "test") + + @clean_environment + @make_config() + def test_unknown_options(self, config_file): + """error on unknown options.""" + parser = Parser(components=[self]) + self.assertRaises(SystemExit, + parser.parse, + ["-C", config_file, "--not-a-real-option"]) + + @clean_environment + @make_config() + def test_reparse(self, config_file): + """reparse options.""" + result = argparse.Namespace() + parser = Parser(components=[self], namespace=result) + parser.parse(["-C", config_file]) + self.assertFalse(result.test_false_boolean) + + parser.parse(["-C", config_file]) + self.assertFalse(result.test_false_boolean) + + parser.reparse() + self.assertFalse(result.test_false_boolean) + + parser.reparse(["-C", config_file, "--test-false-boolean"]) + self.assertTrue(result.test_false_boolean) + + cfp = ConfigParser.ConfigParser() + cfp.add_section("test") + cfp.set("test", "false_boolean", "on") + parser.parse(["-C", config_file]) + cfp.write(open(config_file, "w")) + self.assertTrue(result.test_false_boolean) + + +class TestParsingHooks(OptionTestCase): + """test option parsing hooks.""" + def setUp(self): + self.options_parsed_hook = mock.Mock() + self.options = [BooleanOption("--test", default=False)] + self.results = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.results) + + @make_config() + def test_parsing_hooks(self, config_file): + """option parsing hooks are called.""" + self.parser.parse(["-C", config_file]) + self.options_parsed_hook.assert_called_with() + + +class TestEarlyParsingHooks(OptionTestCase): + """test early option parsing hooks.""" + parse_first = True + + def setUp(self): + self.component_parsed_hook = mock.Mock() + self.options = [BooleanOption("--early-test", default=False)] + self.results = argparse.Namespace() + new_parser() + self.parser = get_parser(components=[self], namespace=self.results) + + @make_config() + def test_parsing_hooks(self, config_file): + """early option parsing hooks are called.""" + self.parser.parse(["-C", config_file, "--early-test"]) + self.assertEqual(self.component_parsed_hook.call_count, 1) + early_opts = self.component_parsed_hook.call_args[0][0] + self.assertTrue(early_opts.early_test) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py b/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py new file mode 100644 index 000000000..35da909cb --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py @@ -0,0 +1,141 @@ +"""test subcommand option parsing.""" + +import argparse +import sys + +from Bcfg2.Compat import StringIO +from Bcfg2.Options import Option, get_parser, new_parser, Subcommand, \ + Subparser, CommandRegistry +import Bcfg2.Options.Subcommands + +from testsuite.Testsrc.Testlib.TestOptions import make_config, OptionTestCase + + +class MockSubcommand(Subcommand): + """fake subcommand that just records the options it was called with.""" + run_options = None + + def run(self, setup): + self.__class__.run_options = setup + + +class One(MockSubcommand): + """fake subcommand for testing.""" + options = [Option("--test-one")] + + +class Two(MockSubcommand): + """fake subcommand for testing.""" + options = [Option("--test-two")] + + +def local_subclass(cls): + """get a subclass of ``cls`` that adds no functionality. + + This can be used to subclass the various test classes above so + that their options don't get modified by option parsing. + """ + return type("Local%s" % cls.__name__, (cls,), {}) + + +class TestSubcommands(OptionTestCase): + """tests for subcommands and subparsers.""" + + def setUp(self): + self.registry = CommandRegistry() + + self.one = local_subclass(One) + self.two = local_subclass(Two) + + self.registry.register_command(self.one) + self.registry.register_command(self.two) + + self.result = argparse.Namespace() + Bcfg2.Options.Subcommands.master_setup = self.result + + new_parser() + self.parser = get_parser(namespace=self.result, + components=[self]) + self.parser.add_options(self.registry.subcommand_options) + + def test_register_commands(self): + """register subcommands.""" + registry = CommandRegistry() + registry.register_commands(globals().values(), + parent=MockSubcommand) + self.assertItemsEqual(registry.commands.keys(), + ["one", "two", "help"]) + self.assertIsInstance(registry.commands['one'], One) + self.assertIsInstance(registry.commands['two'], Two) + + @make_config() + def test_get_subcommand(self, config_file): + """parse simple subcommands.""" + self.parser.parse(["-C", config_file, "localone"]) + self.assertEqual(self.result.subcommand, "localone") + + def test_subcommand_usage(self): + """sane usage message from subcommands.""" + self.assertEqual( + One().usage(), + "one [--test-one TEST_ONE] - fake subcommand for testing.") + + # subclasses do not inherit the docstring from the parent, so + # this tests a command subclass without a docstring, even + # though that should never happen due to the pylint tests. + self.assertEqual(self.one().usage().strip(), + "localone [--test-one TEST_ONE]") + + @make_config() + def test_help(self, config_file): + """sane help message from subcommand registry.""" + self.parser.parse(["-C", config_file, "help"]) + old_stdout = sys.stdout + sys.stdout = StringIO() + self.assertIn(self.registry.runcommand(), [0, None]) + help_message = sys.stdout.getvalue().splitlines() + sys.stdout = old_stdout + + # the help message will look like: + # + # localhelp [] + # localone [--test-one TEST_ONE] + # localtwo [--test-two TEST_TWO] + commands = [] + command_help = { + "help": self.registry.help.usage(), + "localone": self.one().usage(), + "localtwo": self.two().usage()} + for line in help_message: + command = line.split()[0] + commands.append(command) + if command not in command_help: + self.fail("Got help for unknown command %s: %s" % + (command, line)) + self.assertEqual(line, command_help[command]) + self.assertItemsEqual(commands, command_help.keys()) + + @make_config() + def test_subcommand_help(self, config_file): + """get help message on a single command.""" + self.parser.parse(["-C", config_file, "help", "localone"]) + old_stdout = sys.stdout + sys.stdout = StringIO() + self.assertIn(self.registry.runcommand(), [0, None]) + help_message = sys.stdout.getvalue().splitlines() + sys.stdout = old_stdout + + self.assertEqual(help_message[0].strip(), + "usage: %s" % self.one().usage().strip()) + + @make_config() + def test_nonexistent_subcommand_help(self, config_file): + """get help message on a nonexistent command.""" + self.parser.parse(["-C", config_file, "help", "blargle"]) + old_stdout = sys.stdout + sys.stdout = StringIO() + self.assertNotEqual(self.registry.runcommand(), 0) + help_message = sys.stdout.getvalue().splitlines() + sys.stdout = old_stdout + + self.assertIn("No such command", help_message[0]) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestTypes.py b/testsuite/Testsrc/Testlib/TestOptions/TestTypes.py new file mode 100644 index 000000000..404d67fdc --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestTypes.py @@ -0,0 +1,111 @@ +"""test builtin option types.""" + +import argparse + +from mock import patch + +from Bcfg2.Options import Option, Types, Parser +from testsuite.common import Bcfg2TestCase + + +class TestOptionTypes(Bcfg2TestCase): + """test builtin option types.""" + def setUp(self): + self.options = None + + def _test_options(self, options): + """helper to test option types. + + this expects that self.options is set to a single option named + test. The value of that option is returned. + """ + result = argparse.Namespace() + parser = Parser(components=[self], namespace=result) + parser.parse(options) + return result.test + + def test_comma_list(self): + """parse comma-list values.""" + self.options = [Option("--test", type=Types.comma_list)] + + expected = ["one", "two", "three"] + self.assertItemsEqual(self._test_options(["--test", "one,two,three"]), + expected) + self.assertItemsEqual(self._test_options(["--test", + "one, two, three"]), + expected) + self.assertItemsEqual(self._test_options(["--test", + "one , two ,three"]), + expected) + self.assertItemsEqual(self._test_options(["--test", "one two, three"]), + ["one two", "three"]) + + def test_colon_list(self): + """parse colon-list values.""" + self.options = [Option("--test", type=Types.colon_list)] + self.assertItemsEqual(self._test_options(["--test", "one:two three"]), + ["one", "two three"]) + + def test_comma_dict(self): + """parse comma-dict values.""" + self.options = [Option("--test", type=Types.comma_dict)] + expected = { + "one": True, + "two": 2, + "three": "three", + "four": False} + self.assertDictEqual( + self._test_options(["--test", + "one=yes, two=2 , three=three,four=no"]), + expected) + + self.assertDictEqual( + self._test_options(["--test", "one,two=2,three=three,four=off"]), + expected) + + def test_anchored_regex_list(self): + """parse regex lists.""" + self.options = [Option("--test", type=Types.anchored_regex_list)] + self.assertItemsEqual( + [r.pattern for r in self._test_options(["--test", r'\d+ \s*'])], + [r'^\d+$', r'^\s*$']) + self.assertRaises(SystemExit, + self._test_options, ["--test", '(]']) + + def test_octal(self): + """parse octal options.""" + self.options = [Option("--test", type=Types.octal)] + self.assertEqual(self._test_options(["--test", "0777"]), 511) + self.assertEqual(self._test_options(["--test", "133114255"]), 23894189) + + @patch("pwd.getpwnam") + def test_username(self, mock_getpwnam): + """parse username options.""" + self.options = [Option("--test", type=Types.username)] + mock_getpwnam.return_value = ("test", '********', 1001, 1001, + "Test user", "/home/test", "/bin/bash") + self.assertEqual(self._test_options(["--test", "1001"]), 1001) + self.assertEqual(self._test_options(["--test", "test"]), 1001) + + @patch("grp.getgrnam") + def test_groupname(self, mock_getpwnam): + """parse group name options.""" + self.options = [Option("--test", type=Types.groupname)] + mock_getpwnam.return_value = ("test", '*', 1001, ["test"]) + self.assertEqual(self._test_options(["--test", "1001"]), 1001) + self.assertEqual(self._test_options(["--test", "test"]), 1001) + + def test_timeout(self): + """parse timeout options.""" + self.options = [Option("--test", type=Types.timeout)] + self.assertEqual(self._test_options(["--test", "1.0"]), 1.0) + self.assertEqual(self._test_options(["--test", "1"]), 1.0) + self.assertEqual(self._test_options(["--test", "0"]), None) + + def test_size(self): + """parse human-readable size options.""" + self.options = [Option("--test", type=Types.size)] + self.assertEqual(self._test_options(["--test", "5k"]), 5120) + self.assertEqual(self._test_options(["--test", "5"]), 5) + self.assertRaises(SystemExit, + self._test_options, ["--test", "g5m"]) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestWildcards.py b/testsuite/Testsrc/Testlib/TestOptions/TestWildcards.py new file mode 100644 index 000000000..da196a912 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/TestWildcards.py @@ -0,0 +1,47 @@ +"""test wildcard options.""" + +import argparse + +from Bcfg2.Options import Option, Parser +from testsuite.Testsrc.Testlib.TestOptions import OptionTestCase, make_config + + +class TestWildcardOptions(OptionTestCase): + """test parsing wildcard options.""" + config = { + "foo": { + "test1": "test1", + "test2": "test2", + "thing1": "thing1", + "thing2": "thing2", + "foo": "foo" + } + } + + def setUp(self): + # parsing options can modify the Option objects themselves. + # that's probably bad -- and it's definitely bad if we ever + # want to do real on-the-fly config changes -- but it's easier + # to leave it as is and set the options on each test. + self.options = [ + Option(cf=("foo", "*"), dest="all"), + Option(cf=("foo", "test*"), dest="test"), + Option(cf=("foo", "bogus*"), dest="unmatched"), + Option(cf=("bar", "*"), dest="no_section"), + Option(cf=("foo", "foo"))] + + @make_config(config) + def test_wildcard_options(self, config_file): + """parse wildcard options.""" + result = argparse.Namespace() + parser = Parser(components=[self], namespace=result) + parser.parse(argv=["-C", config_file]) + + self.assertDictEqual(result.all, {"test1": "test1", + "test2": "test2", + "thing1": "thing1", + "thing2": "thing2"}) + self.assertDictEqual(result.test, {"test1": "test1", + "test2": "test2"}) + self.assertDictEqual(result.unmatched, {}) + self.assertDictEqual(result.no_section, {}) diff --git a/testsuite/Testsrc/Testlib/TestOptions/Two.py b/testsuite/Testsrc/Testlib/TestOptions/Two.py new file mode 100644 index 000000000..189e0817f --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/Two.py @@ -0,0 +1,6 @@ +"""Test module for component loading.""" + + +class Two(object): + """Test class for component loading.""" + pass diff --git a/testsuite/Testsrc/Testlib/TestOptions/__init__.py b/testsuite/Testsrc/Testlib/TestOptions/__init__.py new file mode 100644 index 000000000..b051f65e5 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestOptions/__init__.py @@ -0,0 +1,85 @@ +"""helper functions for option testing.""" + +import os +import tempfile + +from Bcfg2.Compat import wraps, ConfigParser +from Bcfg2.Options import Parser +from testsuite.common import Bcfg2TestCase + + +class make_config(object): # pylint: disable=invalid-name + """decorator to create a temporary config file from a dict. + + The filename of the temporary config file is added as the last + positional argument to the function call. + """ + def __init__(self, config_data=None): + self.config_data = config_data or {} + + def __call__(self, func): + @wraps(func) + def inner(*args, **kwargs): + """decorated function.""" + cfp = ConfigParser.ConfigParser() + for section, options in self.config_data.items(): + cfp.add_section(section) + for key, val in options.items(): + cfp.set(section, key, val) + fd, name = tempfile.mkstemp() + config_file = os.fdopen(fd, 'w') + cfp.write(config_file) + config_file.close() + + args = list(args) + [name] + rv = func(*args, **kwargs) + os.unlink(name) + return rv + + return inner + + +def clean_environment(func): + """decorator that unsets any environment variables used by options. + + The list of options is taken from the first argument, which is + presumed to be ``self``. The variables are restored at the end of + the function. + """ + @wraps(func) + def inner(self, *args, **kwargs): + """decorated function.""" + envvars = {} + for opt in self.options: + if opt.env is not None: + envvars[opt.env] = os.environ.get(opt.env) + if opt.env in os.environ: + del os.environ[opt.env] + rv = func(self, *args, **kwargs) + for name, val in envvars.items(): + if val is None and name in os.environ: + del os.environ[name] + elif val is not None: + os.environ[name] = val + return rv + + return inner + + +class OptionTestCase(Bcfg2TestCase): + """test case that doesn't mock out config file reading.""" + + @classmethod + def setUpClass(cls): + # ensure that the option parser actually reads config files + Parser.unit_test = False + + @classmethod + def tearDownClass(cls): + Parser.unit_test = True + + + +# TODO: +# * subcommands +# * common options diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py index f135a0197..290a7c092 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py @@ -22,6 +22,7 @@ class TestPlugin(TestDebuggable): def setUp(self): TestDebuggable.setUp(self) set_setup_default("filemonitor", MagicMock()) + set_setup_default("repository", datastore) def get_obj(self, core=None): if core is None: diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py index 37beaa26c..5a82100d0 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py @@ -55,6 +55,7 @@ class TestDatabaseBacked(TestPlugin): def setUp(self): TestPlugin.setUp(self) set_setup_default("%s_db" % self.test_obj.__name__.lower(), False) + set_setup_default("db_engine", None) @skipUnless(HAS_DJANGO, "Django not found") def test__use_db(self): diff --git a/testsuite/Testsrc/Testlib/TestUtils.py b/testsuite/Testsrc/Testlib/TestUtils.py index 349d6cd40..4bed67248 100644 --- a/testsuite/Testsrc/Testlib/TestUtils.py +++ b/testsuite/Testsrc/Testlib/TestUtils.py @@ -1,9 +1,5 @@ import os import sys -import copy -import lxml.etree -import subprocess -from mock import Mock, MagicMock, patch from Bcfg2.Utils import * # add all parent testsuite directories to sys.path to allow (most) diff --git a/testsuite/Testsrc/__init__.py b/testsuite/Testsrc/__init__.py new file mode 100644 index 000000000..e69de29bb -- cgit v1.2.3-1-g7c22 From 7a763f1ca474203e07379fe2e71606b01c5b62fb Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 14 Oct 2014 09:40:24 -0500 Subject: testsuite: skip nested exclusive option group test on py2.6 --- src/lib/Bcfg2/Options/OptionGroups.py | 4 ++-- .../Testsrc/Testlib/TestOptions/TestOptionGroups.py | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/lib/Bcfg2/Options/OptionGroups.py b/src/lib/Bcfg2/Options/OptionGroups.py index 426ee5dc0..49340ab36 100644 --- a/src/lib/Bcfg2/Options/OptionGroups.py +++ b/src/lib/Bcfg2/Options/OptionGroups.py @@ -70,8 +70,8 @@ class ExclusiveOptionGroup(_OptionContainer): self.required = kwargs.pop('required', False) def add_to_parser(self, parser): - group = parser.add_mutually_exclusive_group(required=self.required) - _OptionContainer.add_to_parser(self, group) + _OptionContainer.add_to_parser( + self, parser.add_mutually_exclusive_group(required=self.required)) class Subparser(_OptionContainer): diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py index de1abbb1b..7611d6202 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptionGroups.py @@ -1,11 +1,12 @@ """test reading multiple config files.""" import argparse +import sys from Bcfg2.Options import Option, BooleanOption, Parser, OptionGroup, \ ExclusiveOptionGroup, WildcardSectionGroup, new_parser, get_parser -from testsuite.common import Bcfg2TestCase +from testsuite.common import Bcfg2TestCase, skipUnless from testsuite.Testsrc.Testlib.TestOptions import make_config, OptionTestCase @@ -59,8 +60,10 @@ class TestOptionGroups(Bcfg2TestCase): self.assertRaises(SystemExit, self._test_options, []) - def test_option_group(self): - """nest option groups.""" + +class TestNestedOptionGroups(TestOptionGroups): + def setUp(self): + TestOptionGroups.setUp(self) self.options = [ OptionGroup( BooleanOption("--foo"), @@ -73,6 +76,9 @@ class TestOptionGroups(Bcfg2TestCase): BooleanOption("--test2")), title="inner"), title="outer")] + + def test_option_group(self): + """nest option groups.""" result = self._test_options(["--foo", "--baz", "--test1"]) self.assertTrue(result.foo) self.assertFalse(result.bar) @@ -81,6 +87,10 @@ class TestOptionGroups(Bcfg2TestCase): self.assertTrue(result.test1) self.assertFalse(result.test2) + @skipUnless(sys.version_info >= (2, 7), + "Nested exclusive option groups do not work in Python 2.6") + def test_nested_exclusive_option_groups(self): + """nest exclusive option groups.""" self.assertRaises(SystemExit, self._test_options, ["--test1", "--test2"]) -- cgit v1.2.3-1-g7c22 From a73d70b7fa79b988c97e8c258bc2b6c29224bf01 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 15 Oct 2014 12:41:31 -0700 Subject: Test failure to parse config file when bcfg2.conf exists --- testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py | 4 +++- testsuite/Testsrc/Testlib/TestOptions/__init__.py | 6 ------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py b/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py index aee2ff666..78acadf1f 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestConfigFiles.py @@ -2,9 +2,10 @@ import argparse +import mock + from Bcfg2.Options import Option, PathOption, ConfigFileAction, get_parser, \ new_parser - from testsuite.Testsrc.Testlib.TestOptions import make_config, OptionTestCase @@ -43,6 +44,7 @@ class TestConfigFiles(OptionTestCase): inner1() + @mock.patch("os.path.exists", mock.Mock(return_value=False)) def test_no_config_file(self): """fail to read config file.""" self.assertRaises(SystemExit, self.parser.parse, []) diff --git a/testsuite/Testsrc/Testlib/TestOptions/__init__.py b/testsuite/Testsrc/Testlib/TestOptions/__init__.py index b051f65e5..00e250356 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/__init__.py +++ b/testsuite/Testsrc/Testlib/TestOptions/__init__.py @@ -77,9 +77,3 @@ class OptionTestCase(Bcfg2TestCase): @classmethod def tearDownClass(cls): Parser.unit_test = True - - - -# TODO: -# * subcommands -# * common options -- cgit v1.2.3-1-g7c22 From 9c08059110a62a926128c5f2ec0b726ef1083822 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 15 Oct 2014 12:46:51 -0700 Subject: testsuite: capture stderr by default This quiets down a lot of tests, especially for option parsing. --- testsuite/Testsrc/Testlib/TestOptions/__init__.py | 2 ++ testsuite/common.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/testsuite/Testsrc/Testlib/TestOptions/__init__.py b/testsuite/Testsrc/Testlib/TestOptions/__init__.py index 00e250356..688f4e54c 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/__init__.py +++ b/testsuite/Testsrc/Testlib/TestOptions/__init__.py @@ -73,7 +73,9 @@ class OptionTestCase(Bcfg2TestCase): def setUpClass(cls): # ensure that the option parser actually reads config files Parser.unit_test = False + Bcfg2TestCase.setUpClass() @classmethod def tearDownClass(cls): Parser.unit_test = True + Bcfg2TestCase.tearDownClass() diff --git a/testsuite/common.py b/testsuite/common.py index 5a08f8db5..49579d7ef 100644 --- a/testsuite/common.py +++ b/testsuite/common.py @@ -119,6 +119,19 @@ class Bcfg2TestCase(TestCase): :func:`assertXMLEqual`, a useful assertion method given all the XML used by Bcfg2. """ + capture_stderr = True + + @classmethod + def setUpClass(cls): + cls._stderr = sys.stderr + if cls.capture_stderr: + sys.stderr = sys.stdout + + @classmethod + def tearDownClass(cls): + if cls.capture_stderr: + sys.stderr = cls._stderr + def assertXMLEqual(self, el1, el2, msg=None): """ Test that the two XML trees given are equal. """ if msg is None: -- cgit v1.2.3-1-g7c22 From 474462ff24e8f29d715ee80ee0c07acb16eb1dab Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 16 Sep 2014 15:50:04 -0700 Subject: testsuite: Added unit tests for new option parsing --- testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py index b9de703ff..047905fc3 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py @@ -21,6 +21,7 @@ from common import * class TestTool(Bcfg2TestCase): test_obj = Tool + # try to find true if os.path.exists("/bin/true"): true = "/bin/true" elif os.path.exists("/usr/bin/true"): -- cgit v1.2.3-1-g7c22 From a3998969ec25acf20d5e42a1166e001288ca7b0e Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 07:58:01 -0500 Subject: fixed some places where plugin loading should fail silently --- src/lib/Bcfg2/Options/Actions.py | 8 ++++++-- src/lib/Bcfg2/Server/Plugins/Packages/__init__.py | 1 + src/lib/Bcfg2/Server/Plugins/Svn.py | 18 ++++++++++++------ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/lib/Bcfg2/Options/Actions.py b/src/lib/Bcfg2/Options/Actions.py index 2916cc194..854e6039d 100644 --- a/src/lib/Bcfg2/Options/Actions.py +++ b/src/lib/Bcfg2/Options/Actions.py @@ -113,9 +113,12 @@ class ComponentAction(FinalizableAction): try: return getattr(__import__(module, fromlist=[name]), name) except (AttributeError, ImportError): + msg = "Failed to load %s from %s: %s" % (name, module, + sys.exc_info()[1]) if not self.fail_silently: - print("Failed to load %s from %s: %s" % - (name, module, sys.exc_info()[1])) + print(msg) + else: + _debug(msg) return None def _load_component(self, name): @@ -181,3 +184,4 @@ class PluginsAction(ComponentAction): """ :class:`Bcfg2.Options.ComponentAction` subclass for loading Bcfg2 server plugins. """ bases = ['Bcfg2.Server.Plugins'] + fail_silently = True diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py index d11ac60fe..cb533f4f1 100644 --- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py +++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py @@ -33,6 +33,7 @@ class PackagesBackendAction(Bcfg2.Options.ComponentAction): """ ComponentAction to load Packages backends """ bases = ['Bcfg2.Server.Plugins.Packages'] module = True + fail_silently = True class OnDemandDict(MutableMapping): diff --git a/src/lib/Bcfg2/Server/Plugins/Svn.py b/src/lib/Bcfg2/Server/Plugins/Svn.py index b752650f0..2ca518e53 100644 --- a/src/lib/Bcfg2/Server/Plugins/Svn.py +++ b/src/lib/Bcfg2/Server/Plugins/Svn.py @@ -17,12 +17,6 @@ except ImportError: class Svn(Bcfg2.Server.Plugin.Version): """Svn is a version plugin for dealing with Bcfg2 repos.""" options = Bcfg2.Server.Plugin.Version.options + [ - Bcfg2.Options.Option( - cf=("svn", "conflict_resolution"), dest="svn_conflict_resolution", - type=lambda v: v.replace("-", "_"), - choices=dir(pysvn.wc_conflict_choice), # pylint: disable=E1101 - default=pysvn.wc_conflict_choice.postpone, # pylint: disable=E1101 - help="SVN conflict resolution method"), Bcfg2.Options.Option( cf=("svn", "user"), dest="svn_user", help="SVN username"), Bcfg2.Options.Option( @@ -31,6 +25,18 @@ class Svn(Bcfg2.Server.Plugin.Version): cf=("svn", "always_trust"), dest="svn_trust_ssl", help="Always trust SSL certs from SVN server")] + if HAS_SVN: + options.append( + Bcfg2.Options.Option( + cf=("svn", "conflict_resolution"), + dest="svn_conflict_resolution", + type=lambda v: v.replace("-", "_"), + # pylint: disable=E1101 + choices=dir(pysvn.wc_conflict_choice), + default=pysvn.wc_conflict_choice.postpone, + # pylint: enable=E1101 + help="SVN conflict resolution method")) + __author__ = 'bcfg-dev@mcs.anl.gov' __vcs_metadata_path__ = ".svn" if HAS_SVN: -- cgit v1.2.3-1-g7c22 From 1a54f2a549b86be5a976d2a43a7985c1265d916a Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 08:07:28 -0500 Subject: call shutdown on subcommand registries --- src/lib/Bcfg2/Server/Admin.py | 7 +++++-- src/lib/Bcfg2/Server/Info.py | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/lib/Bcfg2/Server/Admin.py b/src/lib/Bcfg2/Server/Admin.py index b6dded175..ef7741880 100644 --- a/src/lib/Bcfg2/Server/Admin.py +++ b/src/lib/Bcfg2/Server/Admin.py @@ -1197,5 +1197,8 @@ class CLI(Bcfg2.Options.CommandRegistry): def run(self): """ Run bcfg2-admin """ - self.commands[Bcfg2.Options.setup.subcommand].setup() - return self.runcommand() + try: + self.commands[Bcfg2.Options.setup.subcommand].setup() + return self.runcommand() + finally: + self.shutdown() diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index 0baa29a9f..da2312cc8 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -837,7 +837,6 @@ class InfoCore(cmd.Cmd, Bcfg2.Server.Core.Core): pass def shutdown(self): - Bcfg2.Options.CommandRegistry.shutdown(self) Bcfg2.Server.Core.Core.shutdown(self) @@ -868,6 +867,9 @@ class CLI(Bcfg2.Options.CommandRegistry): def run(self): """ Run bcfg2-info """ - if Bcfg2.Options.setup.subcommand != 'help': - self.core.run() - return self.runcommand() + try: + if Bcfg2.Options.setup.subcommand != 'help': + self.core.run() + return self.runcommand() + finally: + self.shutdown() -- cgit v1.2.3-1-g7c22 From 7a4dd4b3436cd85ee46acd07e85e7769f739b87f Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:02:51 -0500 Subject: testsuite: better debug capturing for options tests --- src/lib/Bcfg2/Options/Options.py | 8 +++- .../Testsrc/Testlib/TestOptions/TestSubcommands.py | 44 +++++++++++----------- testsuite/common.py | 6 +++ 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 4efd76929..bd1a72fc7 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -17,14 +17,18 @@ from Bcfg2.Compat import ConfigParser __all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", "_debug"] +unit_test = False + def _debug(msg): """ Option parsing happens before verbose/debug have been set -- they're options, after all -- so option parsing verbosity is enabled by changing this to True. The verbosity here is primarily of use to developers. """ - if os.environ.get('BCFG2_OPTIONS_DEBUG', '0').lower() in ["true", "yes", - "on", "1"]: + if unit_test: + print("DEBUG: %s" % msg) + elif os.environ.get('BCFG2_OPTIONS_DEBUG', '0').lower() in ["true", "yes", + "on", "1"]: sys.stderr.write("%s\n" % msg) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py b/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py index 35da909cb..65b4c19c0 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestSubcommands.py @@ -86,15 +86,21 @@ class TestSubcommands(OptionTestCase): self.assertEqual(self.one().usage().strip(), "localone [--test-one TEST_ONE]") - @make_config() - def test_help(self, config_file): - """sane help message from subcommand registry.""" - self.parser.parse(["-C", config_file, "help"]) + def _get_subcommand_output(self, args): + self.parser.parse(args) old_stdout = sys.stdout sys.stdout = StringIO() - self.assertIn(self.registry.runcommand(), [0, None]) - help_message = sys.stdout.getvalue().splitlines() + rv = self.registry.runcommand() + output = [l for l in sys.stdout.getvalue().splitlines() + if not l.startswith("DEBUG: ")] sys.stdout = old_stdout + return (rv, output) + + @make_config() + def test_help(self, config_file): + """sane help message from subcommand registry.""" + rv, output = self._get_subcommand_output(["-C", config_file, "help"]) + self.assertIn(rv, [0, None]) # the help message will look like: # @@ -106,7 +112,7 @@ class TestSubcommands(OptionTestCase): "help": self.registry.help.usage(), "localone": self.one().usage(), "localtwo": self.two().usage()} - for line in help_message: + for line in output: command = line.split()[0] commands.append(command) if command not in command_help: @@ -118,24 +124,16 @@ class TestSubcommands(OptionTestCase): @make_config() def test_subcommand_help(self, config_file): """get help message on a single command.""" - self.parser.parse(["-C", config_file, "help", "localone"]) - old_stdout = sys.stdout - sys.stdout = StringIO() - self.assertIn(self.registry.runcommand(), [0, None]) - help_message = sys.stdout.getvalue().splitlines() - sys.stdout = old_stdout - - self.assertEqual(help_message[0].strip(), + rv, output = self._get_subcommand_output( + ["-C", config_file, "help", "localone"]) + self.assertIn(rv, [0, None]) + self.assertEqual(output[0].strip(), "usage: %s" % self.one().usage().strip()) @make_config() def test_nonexistent_subcommand_help(self, config_file): """get help message on a nonexistent command.""" - self.parser.parse(["-C", config_file, "help", "blargle"]) - old_stdout = sys.stdout - sys.stdout = StringIO() - self.assertNotEqual(self.registry.runcommand(), 0) - help_message = sys.stdout.getvalue().splitlines() - sys.stdout = old_stdout - - self.assertIn("No such command", help_message[0]) + rv, output = self._get_subcommand_output( + ["-C", config_file, "help", "blargle"]) + self.assertNotEqual(rv, 0) + self.assertIn("No such command", output[0]) diff --git a/testsuite/common.py b/testsuite/common.py index 49579d7ef..a86e9c5d9 100644 --- a/testsuite/common.py +++ b/testsuite/common.py @@ -38,7 +38,13 @@ def set_setup_default(option, value=None): if not hasattr(Bcfg2.Options.setup, option): setattr(Bcfg2.Options.setup, option, value) +# these two variables do slightly different things for unit tests; the +# former skips config file reading, while the latter sends option +# debug logging to stdout so it can be captured. These are separate +# because we want to enable config file reading in order to test +# option parsing. Bcfg2.Options.Parser.unit_test = True +Bcfg2.Options.Options.unit_test = True try: import django.conf -- cgit v1.2.3-1-g7c22 From 0e3fc3def6e0b0b8e31bf564790fd58817c4d621 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:03:17 -0500 Subject: DBSettings: fix up in database name --- src/lib/Bcfg2/DBSettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Bcfg2/DBSettings.py b/src/lib/Bcfg2/DBSettings.py index 12dba7fba..a8f88d3d4 100644 --- a/src/lib/Bcfg2/DBSettings.py +++ b/src/lib/Bcfg2/DBSettings.py @@ -211,7 +211,7 @@ class _OptionContainer(object): Bcfg2.Options.Option( cf=('database', 'engine'), default='sqlite3', help='Database engine', dest='db_engine'), - Bcfg2.Options.Option( + Bcfg2.Options.PathOption( cf=('database', 'name'), default='/etc/bcfg2.sqlite', help="Database name", dest="db_name"), Bcfg2.Options.Option( -- cgit v1.2.3-1-g7c22 From 58caed11b409905641913f545f3a87280705f1a6 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 11:03:48 -0500 Subject: Options: ensure macros are always fixed up This fixes several cases in which macros would not be properly processed: options that are not added to the parser yet when early options are parsed; and config file options whose default value is used. --- src/lib/Bcfg2/Options/Options.py | 59 ++++++++++++----- src/lib/Bcfg2/Options/Parser.py | 77 +++++++++++++++------- .../Testsrc/Testlib/TestOptions/TestComponents.py | 25 ++++++- .../Testsrc/Testlib/TestOptions/TestOptions.py | 8 ++- testsuite/Testsrc/Testlib/TestOptions/__init__.py | 7 +- 5 files changed, 130 insertions(+), 46 deletions(-) diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index bd1a72fc7..36148c279 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -17,7 +17,7 @@ from Bcfg2.Compat import ConfigParser __all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", "_debug"] -unit_test = False +unit_test = False # pylint: disable=C0103 def _debug(msg): @@ -46,7 +46,7 @@ def _get_action_class(action_name): on. So we just instantiate a dummy parser, add a dummy argument, and determine the class that way. """ if (isinstance(action_name, type) and - issubclass(action_name, argparse.Action)): + issubclass(action_name, argparse.Action)): return action_name if action_name not in _action_map: action = argparse.ArgumentParser().add_argument(action_name, @@ -159,9 +159,10 @@ class Option(object): sources.append("%s.%s" % self.cf) if self.env: sources.append("$" + self.env) - spec = ["sources=%s" % sources, "default=%s" % self.default] - spec.append("%d parsers" % (len(self.parsers))) - return 'Option(%s: %s)' % (self.dest, ", ".join(spec)) + spec = ["sources=%s" % sources, "default=%s" % self.default, + "%d parsers" % len(self.parsers)] + return '%s(%s: %s)' % (self.__class__.__name__, + self.dest, ", ".join(spec)) def list_options(self): """ List options contained in this option. This exists to @@ -228,7 +229,7 @@ class Option(object): rv = self._type_func(self.get_config_value(cfp)) except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): rv = None - _debug("Setting %s from config file(s): %s" % (self, rv)) + _debug("Getting value of %s from config file(s): %s" % (self, rv)) return rv def get_config_value(self, cfp): @@ -326,12 +327,11 @@ class Option(object): class PathOption(Option): - """ Shortcut for options that expect a path argument. Uses - :meth:`Bcfg2.Options.Types.path` to transform the argument into a - canonical path. + """Shortcut for options that expect a path argument. - The type of a path option can also be overridden to return a - file-like object. For example: + Uses :meth:`Bcfg2.Options.Types.path` to transform the argument + into a canonical path. The type of a path option can also be + overridden to return a file-like object. For example: .. code-block:: python @@ -339,25 +339,50 @@ class PathOption(Option): Bcfg2.Options.PathOption( "--input", type=argparse.FileType('r'), help="The input file")] + + PathOptions also do translation of ```` macros on the + fly. It's done this way instead of just fixing up all values at + the end of parsing because macro expansion needs to be done before + path canonicalization and other stuff. """ + repository = None def __init__(self, *args, **kwargs): self._original_type = kwargs.pop('type', lambda x: x) kwargs['type'] = self._type kwargs.setdefault('metavar', '') Option.__init__(self, *args, **kwargs) - self.repository = None def early_parsing_hook(self, early_opts): - self.repository = early_opts.repository + if hasattr(early_opts, "repository"): + if self.__class__.repository is None: + _debug("Setting repository to %s for PathOptions" % + early_opts.repository) + self.__class__.repository = early_opts.repository + else: + _debug("Repository is already set for %s" % self.__class__) + + def _get_default(self): + """ Getter for the ``default`` property """ + if self.__class__.repository is None or self._default is None: + return self._default + else: + return self._default.replace("", + self.__class__.repository) + + default = property(_get_default, Option._set_default) def _type(self, value): """Type function that fixes up macros.""" - if self.repository is None: - rv = value + if self.__class__.repository is None: + _debug("Cannot fix up macros yet for %s" % self) + return value else: - rv = value.replace("", self.repository) - return self._original_type(Types.path(rv)) + rv = self._original_type(Types.path( + value.replace("", self.__class__.repository))) + _debug("Fixing up macros in %s: %s -> %s" % + (self, value, rv)) + return rv class _BooleanOptionAction(argparse.Action): diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index ec470f8d3..6715d90f9 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -168,6 +168,8 @@ class Parser(argparse.ArgumentParser): (opt, value)) action(self, self.namespace, value) else: + _debug("Setting config file-only option %s to %s" % + (opt, value)) setattr(self.namespace, opt.dest, value) def _finalize(self): @@ -191,6 +193,53 @@ class Parser(argparse.ArgumentParser): _debug("Deleting %s" % attr) delattr(self.namespace, attr) + def _parse_early_options(self): + """Parse early options. + + Early options are options that need to be parsed before other + options for some reason. These fall into two basic cases: + + 1. Database options, which need to be parsed so that Django + modules can be imported, since Django configuration is all + done at import-time; + 2. The repository (``-Q``) option, so that ```` + macros in other options can be resolved. + """ + _debug("Option parsing phase 2: Parse early options") + early_opts = argparse.Namespace() + early_parser = Parser(add_help=False, namespace=early_opts, + early=True) + + # add the repo option so we can resolve + # macros + early_parser.add_options([repository]) + + early_components = [] + for component in self.components: + if getattr(component, "parse_first", False): + early_components.append(component) + early_parser.add_component(component) + early_parser.parse(self.argv) + + _debug("Fixing up macros in early options") + for attr_name in dir(early_opts): + if not attr_name.startswith("_"): + attr = getattr(early_opts, attr_name) + if hasattr(attr, "replace"): + setattr(early_opts, attr_name, + attr.replace("", + early_opts.repository)) + + _debug("Early parsing complete, calling hooks") + for component in early_components: + if hasattr(component, "component_parsed_hook"): + _debug("Calling component_parsed_hook on %s" % component) + getattr(component, "component_parsed_hook")(early_opts) + _debug("Calling early parsing hooks; early options: %s" % + early_opts) + for option in self.option_list: + option.early_parsing_hook(early_opts) + def add_config_file(self, dest, cfile, reparse=True): """ Add a config file, which triggers a full reparse of all options. """ @@ -207,10 +256,11 @@ class Parser(argparse.ArgumentParser): def reparse(self, argv=None): """ Reparse options after they have already been parsed. - :param argv: The argument list to parse. By default, + :param argv: The argument list to parse. By default, :attr:`Bcfg2.Options.Parser.argv` is reused. (I.e., the argument list that was initially - parsed.) :type argv: list + parsed.) + :type argv: list """ _debug("Reparsing all options") self._reset_namespace() @@ -249,26 +299,7 @@ class Parser(argparse.ArgumentParser): # phase 2: re-parse command line for early options; currently, # that's database options if not self._early: - _debug("Option parsing phase 2: Parse early options") - early_opts = argparse.Namespace() - early_parser = Parser(add_help=False, namespace=early_opts, - early=True) - # add the repo option so we can resolve - # macros - early_parser.add_options([repository]) - early_components = [] - for component in self.components: - if getattr(component, "parse_first", False): - early_components.append(component) - early_parser.add_component(component) - early_parser.parse(self.argv) - _debug("Early parsing complete, calling hooks") - for component in early_components: - if hasattr(component, "component_parsed_hook"): - _debug("Calling component_parsed_hook on %s" % component) - getattr(component, "component_parsed_hook")(early_opts) - for option in self.option_list: - option.early_parsing_hook(early_opts) + self._parse_early_options() else: _debug("Skipping parsing phase 2 in early mode") @@ -299,13 +330,13 @@ class Parser(argparse.ArgumentParser): remaining = [] while not self.parsed: self.parsed = True - self._set_defaults_from_config() _debug("Parsing known arguments") try: _, remaining = self.parse_known_args(args=self.argv, namespace=self.namespace) except OptionParserException: self.error(sys.exc_info()[1]) + self._set_defaults_from_config() self._parse_config_options() self._finalize() if len(remaining) and not self._early: diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py b/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py index d637d34c5..61b87de2a 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestComponents.py @@ -3,8 +3,8 @@ import argparse import os -from Bcfg2.Options import Option, BooleanOption, ComponentAction, get_parser, \ - new_parser, Types, ConfigFileAction +from Bcfg2.Options import Option, BooleanOption, PathOption, ComponentAction, \ + get_parser, new_parser, Types, ConfigFileAction, Common from testsuite.Testsrc.Testlib.TestOptions import make_config, One, Two, \ OptionTestCase @@ -51,18 +51,27 @@ class ConfigFileComponent(object): default="bar")] +class PathComponent(object): + """fake component for testing macros in child components.""" + options = [PathOption(cf=("test", "test_path")), + PathOption(cf=("test", "test_path_default"), + default="/test/default")] + + class ParentComponentAction(ComponentAction): """parent component loader action.""" mapping = {"one": ComponentOne, "two": ComponentTwo, "three": ComponentThree, - "config": ConfigFileComponent} + "config": ConfigFileComponent, + "path": PathComponent} class TestComponentOptions(OptionTestCase): """test cases for component loading.""" def setUp(self): + OptionTestCase.setUp(self) self.options = [ Option("--parent", type=Types.comma_list, default=["one", "two"], action=ParentComponentAction)] @@ -147,6 +156,16 @@ class TestComponentOptions(OptionTestCase): self.parser.parse(["-C", config_file, "--parent", "config"]) self.assertEqual(self.result.config2, None) + @make_config({"test": {"test_path": "/test"}}) + def test_macros_in_component_options(self, config_file): + """fix up macros in component options.""" + self.parser.add_options([Common.repository]) + self.parser.parse(["-C", config_file, "-Q", "/foo/bar", + "--parent", "path"]) + self.assertEqual(self.result.test_path, "/foo/bar/test") + self.assertEqual(self.result.test_path_default, + "/foo/bar/test/default") + class ImportComponentAction(ComponentAction): """action that imports real classes for testing.""" diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py index b76cd6d3a..9f4a7873c 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py @@ -20,6 +20,7 @@ class TestBasicOptions(OptionTestCase): # that's probably bad -- and it's definitely bad if we ever # want to do real on-the-fly config changes -- but it's easier # to leave it as is and set the options on each test. + OptionTestCase.setUp(self) self.options = [ BooleanOption("--test-true-boolean", env="TEST_TRUE_BOOLEAN", cf=("test", "true_boolean"), default=True), @@ -367,13 +368,16 @@ class TestBasicOptions(OptionTestCase): parser.add_options, [Option(cf=("test", "option"))]) - @make_config() + @make_config({"test": {"test_path": "/test"}}) def test_repository_macro(self, config_file): """fix up macros.""" result = argparse.Namespace() parser = Parser(namespace=result) parser.add_options([PathOption("--test1"), PathOption("--test2"), + PathOption(cf=("test", "test_path")), + PathOption(cf=("test", "test_path_default"), + default="/test/default"), Common.repository]) parser.parse(["-C", config_file, "-Q", "/foo/bar", "--test1", "/test1", @@ -381,6 +385,8 @@ class TestBasicOptions(OptionTestCase): self.assertEqual(result.repository, "/foo/bar") self.assertEqual(result.test1, "/foo/bar/test1") self.assertEqual(result.test2, "/foo/bar/foo/bar") + self.assertEqual(result.test_path, "/foo/bar/test") + self.assertEqual(result.test_path_default, "/foo/bar/test/default") @make_config() def test_file_like_path_option(self, config_file): diff --git a/testsuite/Testsrc/Testlib/TestOptions/__init__.py b/testsuite/Testsrc/Testlib/TestOptions/__init__.py index 688f4e54c..ca2c41359 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/__init__.py +++ b/testsuite/Testsrc/Testlib/TestOptions/__init__.py @@ -4,7 +4,7 @@ import os import tempfile from Bcfg2.Compat import wraps, ConfigParser -from Bcfg2.Options import Parser +from Bcfg2.Options import Parser, PathOption from testsuite.common import Bcfg2TestCase @@ -75,7 +75,10 @@ class OptionTestCase(Bcfg2TestCase): Parser.unit_test = False Bcfg2TestCase.setUpClass() + def setUp(self): + Bcfg2TestCase.setUp(self) + PathOption.repository = None + @classmethod def tearDownClass(cls): Parser.unit_test = True - Bcfg2TestCase.tearDownClass() -- cgit v1.2.3-1-g7c22 From ee50f531ce6ba4a23d0b8c6e1ec81f09c0652874 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 12:53:54 -0500 Subject: testsuite: unlink temporary files This cleans up the temporary config files created by the option parsing unit tests. Courtesy Alexander Sulfrian. --- testsuite/Testsrc/Testlib/TestOptions/TestOptions.py | 9 ++++++--- testsuite/Testsrc/Testlib/TestOptions/__init__.py | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py index 9f4a7873c..94d30dd3a 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py @@ -400,9 +400,12 @@ class TestBasicOptions(OptionTestCase): fh.write("test") fh.close() - parser.parse(["-C", config_file, "--test", name]) - self.assertEqual(result.test.name, name) - self.assertEqual(result.test.read(), "test") + try: + parser.parse(["-C", config_file, "--test", name]) + self.assertEqual(result.test.name, name) + self.assertEqual(result.test.read(), "test") + finally: + os.unlink(name) @clean_environment @make_config() diff --git a/testsuite/Testsrc/Testlib/TestOptions/__init__.py b/testsuite/Testsrc/Testlib/TestOptions/__init__.py index ca2c41359..e92f95e94 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/__init__.py +++ b/testsuite/Testsrc/Testlib/TestOptions/__init__.py @@ -32,8 +32,10 @@ class make_config(object): # pylint: disable=invalid-name config_file.close() args = list(args) + [name] - rv = func(*args, **kwargs) - os.unlink(name) + try: + rv = func(*args, **kwargs) + finally: + os.unlink(name) return rv return inner -- cgit v1.2.3-1-g7c22 From bb21c79f38c155fb14289479840efd8abdf54689 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 22 Oct 2014 13:02:35 -0500 Subject: Options: fix path canonicalization and file-like objects This fixes canonicalizing PathOption values when the default value of a config file-only option is used. It also fixes PathOptions that get a file-like object instead of a filename string. --- src/lib/Bcfg2/Options/Options.py | 11 +++-------- testsuite/Testsrc/Testlib/TestOptions/TestOptions.py | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index 36148c279..f1a201f1e 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -364,25 +364,20 @@ class PathOption(Option): def _get_default(self): """ Getter for the ``default`` property """ - if self.__class__.repository is None or self._default is None: + if not hasattr(self._default, "replace"): return self._default else: - return self._default.replace("", - self.__class__.repository) + return self._type(self._default) default = property(_get_default, Option._set_default) def _type(self, value): """Type function that fixes up macros.""" if self.__class__.repository is None: - _debug("Cannot fix up macros yet for %s" % self) return value else: - rv = self._original_type(Types.path( + return self._original_type(Types.path( value.replace("", self.__class__.repository))) - _debug("Fixing up macros in %s: %s -> %s" % - (self, value, rv)) - return rv class _BooleanOptionAction(argparse.Action): diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py index 94d30dd3a..a3190f2ca 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py @@ -75,6 +75,20 @@ class TestBasicOptions(OptionTestCase): self.assertEqual(options.test_path_option, os.path.abspath("./test")) + @make_config() + def test_default_path_canonicalization(self, config_file): + """canonicalize default PathOption values.""" + testdir = os.path.expanduser("~/test") + result = argparse.Namespace() + parser = Parser(namespace=result) + parser.add_options([PathOption("--test1", default="~/test"), + PathOption(cf=("test", "test2"), + default="~/test"), + Common.repository]) + parser.parse(["-C", config_file]) + self.assertEqual(result.test1, testdir) + self.assertEqual(result.test2, testdir) + def test_default_bool(self): """use the default value of boolean options.""" options = self._test_options() -- cgit v1.2.3-1-g7c22 From 9e5d720a54b51b606a7a4b6f0a1efd5905e5d2a3 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 10:23:30 -0600 Subject: Options: gather as much data from config file first --- src/lib/Bcfg2/Options/Parser.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/lib/Bcfg2/Options/Parser.py b/src/lib/Bcfg2/Options/Parser.py index 6715d90f9..c846e8093 100644 --- a/src/lib/Bcfg2/Options/Parser.py +++ b/src/lib/Bcfg2/Options/Parser.py @@ -325,8 +325,16 @@ class Parser(argparse.ArgumentParser): # _parse_config_options is called, all config file options will get set # to their hardcoded defaults. This process defines the options in the # namespace and _parse_config_options will never look at them again. - self._set_defaults_from_config() - self._parse_config_options() + # + # we have to do the parsing in two loops: first, we squeeze as + # much data out of the config file as we can to ensure that + # all config file settings are read before we use any default + # values. then we can start looking at the command line. + while not self.parsed: + self.parsed = True + self._set_defaults_from_config() + self._parse_config_options() + self.parsed = False remaining = [] while not self.parsed: self.parsed = True -- cgit v1.2.3-1-g7c22 From ca78be11051dab7421a414fc3ae4104c82f1f9e7 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 10:32:10 -0600 Subject: Options: further command registry fixes This updates the documentation for some of the recent changes to subcommand handling, and ensures that the server core is shut down by bcfg2-info. --- doc/development/option_parsing.txt | 7 +++---- src/lib/Bcfg2/Server/Info.py | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/doc/development/option_parsing.txt b/doc/development/option_parsing.txt index 642b9a36c..e14031e1e 100644 --- a/doc/development/option_parsing.txt +++ b/doc/development/option_parsing.txt @@ -179,8 +179,9 @@ The normal implementation pattern is this: collect its options and adds it as a :class:`Bcfg2.Options.Subparser` option group to the main option parser. -#. Register your commands with - :func:`Bcfg2.Options.register_commands`. +#. Register your commands with the + :func:`Bcfg2.Options.CommandRegistry.register_commands` method of + your ``CommandRegistry`` object. #. Add options from the :attr:`Bcfg2.Options.CommandRegistry.command_options` attribute to the option parser. @@ -211,9 +212,7 @@ At a minimum, the :func:`Bcfg2.Options.Subcommand.run` method must be overridden, and a docstring written. .. autoclass:: Subcommand -.. autoclass:: HelpCommand .. autoclass:: CommandRegistry -.. autofunction:: register_commands Actions ------- diff --git a/src/lib/Bcfg2/Server/Info.py b/src/lib/Bcfg2/Server/Info.py index da2312cc8..6af561089 100644 --- a/src/lib/Bcfg2/Server/Info.py +++ b/src/lib/Bcfg2/Server/Info.py @@ -873,3 +873,7 @@ class CLI(Bcfg2.Options.CommandRegistry): return self.runcommand() finally: self.shutdown() + + def shutdown(self): + Bcfg2.Options.CommandRegistry.shutdown(self) + self.core.shutdown() -- cgit v1.2.3-1-g7c22 From 0e133c157755908d05c44c3a36b1dc0668e1d111 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Nov 2014 11:42:42 -0600 Subject: Options: Fixed non-path database name parsing The database name is sometimes a path (SQLite) and sometimes not (MySQL, PostgreSQL). This introduces a new Option type, RepositoryMacroOption, that expands macros without canonicalizing the path, so SQLite users can use in their settings but MySQL users' database name settings will not be destroyed by path canonicalization. The unfortunate downside is that SQLite users can't use ~ in their database name. --- src/lib/Bcfg2/DBSettings.py | 2 +- src/lib/Bcfg2/Options/Options.py | 63 ++++++++++++++-------- .../Testsrc/Testlib/TestOptions/TestOptions.py | 16 ++++-- 3 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/lib/Bcfg2/DBSettings.py b/src/lib/Bcfg2/DBSettings.py index a8f88d3d4..982a299c0 100644 --- a/src/lib/Bcfg2/DBSettings.py +++ b/src/lib/Bcfg2/DBSettings.py @@ -211,7 +211,7 @@ class _OptionContainer(object): Bcfg2.Options.Option( cf=('database', 'engine'), default='sqlite3', help='Database engine', dest='db_engine'), - Bcfg2.Options.PathOption( + Bcfg2.Options.RepositoryMacroOption( cf=('database', 'name'), default='/etc/bcfg2.sqlite', help="Database name", dest="db_name"), Bcfg2.Options.Option( diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py index f1a201f1e..752e01b4e 100644 --- a/src/lib/Bcfg2/Options/Options.py +++ b/src/lib/Bcfg2/Options/Options.py @@ -14,8 +14,8 @@ from Bcfg2.Options import Types from Bcfg2.Compat import ConfigParser -__all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument", - "_debug"] +__all__ = ["Option", "BooleanOption", "RepositoryMacroOption", "PathOption", + "PositionalArgument", "_debug"] unit_test = False # pylint: disable=C0103 @@ -326,24 +326,13 @@ class Option(object): (self, parser)) -class PathOption(Option): - """Shortcut for options that expect a path argument. - - Uses :meth:`Bcfg2.Options.Types.path` to transform the argument - into a canonical path. The type of a path option can also be - overridden to return a file-like object. For example: - - .. code-block:: python - - options = [ - Bcfg2.Options.PathOption( - "--input", type=argparse.FileType('r'), - help="The input file")] +class RepositoryMacroOption(Option): + """Option that does translation of ```` macros. - PathOptions also do translation of ```` macros on the - fly. It's done this way instead of just fixing up all values at - the end of parsing because macro expansion needs to be done before - path canonicalization and other stuff. + Macro translation is done on the fly instead of just fixing up all + values at the end of parsing because macro expansion needs to be + done before path canonicalization for + :class:`Bcfg2.Options.Options.PathOption`. """ repository = None @@ -356,8 +345,8 @@ class PathOption(Option): def early_parsing_hook(self, early_opts): if hasattr(early_opts, "repository"): if self.__class__.repository is None: - _debug("Setting repository to %s for PathOptions" % - early_opts.repository) + _debug("Setting repository to %s for %s" % + (early_opts.repository, self.__class__.__name__)) self.__class__.repository = early_opts.repository else: _debug("Repository is already set for %s" % self.__class__) @@ -371,15 +360,45 @@ class PathOption(Option): default = property(_get_default, Option._set_default) + def transform_value(self, value): + """transform the value after macro expansion. + + this can be overridden to further transform the value set by + the user *after* macros are expanded, but before the user's + ``type`` function is applied. principally exists for + PathOption to canonicalize the path. + """ + return value + def _type(self, value): """Type function that fixes up macros.""" if self.__class__.repository is None: return value else: - return self._original_type(Types.path( + return self._original_type(self.transform_value( value.replace("", self.__class__.repository))) +class PathOption(RepositoryMacroOption): + """Shortcut for options that expect a path argument. + + Uses :meth:`Bcfg2.Options.Types.path` to transform the argument + into a canonical path. The type of a path option can also be + overridden to return a file-like object. For example: + + .. code-block:: python + + options = [ + Bcfg2.Options.PathOption( + "--input", type=argparse.FileType('r'), + help="The input file")] + + PathOptions also do translation of ```` macros. + """ + def transform_value(self, value): + return Types.path(value) + + class _BooleanOptionAction(argparse.Action): """BooleanOptionAction sets a boolean value. diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py index a3190f2ca..a2dc8ffe2 100644 --- a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py +++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py @@ -7,8 +7,9 @@ import tempfile import mock from Bcfg2.Compat import ConfigParser -from Bcfg2.Options import Option, PathOption, BooleanOption, Parser, \ - PositionalArgument, OptionParserException, Common, new_parser, get_parser +from Bcfg2.Options import Option, PathOption, RepositoryMacroOption, \ + BooleanOption, Parser, PositionalArgument, OptionParserException, \ + Common, new_parser, get_parser from testsuite.Testsrc.Testlib.TestOptions import OptionTestCase, \ make_config, clean_environment @@ -382,16 +383,21 @@ class TestBasicOptions(OptionTestCase): parser.add_options, [Option(cf=("test", "option"))]) - @make_config({"test": {"test_path": "/test"}}) + @make_config({"test": {"test_path": "/test", + "test_macro": ""}}) def test_repository_macro(self, config_file): """fix up macros.""" result = argparse.Namespace() parser = Parser(namespace=result) parser.add_options([PathOption("--test1"), - PathOption("--test2"), + RepositoryMacroOption("--test2"), PathOption(cf=("test", "test_path")), PathOption(cf=("test", "test_path_default"), default="/test/default"), + RepositoryMacroOption(cf=("test", "test_macro")), + RepositoryMacroOption( + cf=("test", "test_macro_default"), + default=""), Common.repository]) parser.parse(["-C", config_file, "-Q", "/foo/bar", "--test1", "/test1", @@ -399,6 +405,8 @@ class TestBasicOptions(OptionTestCase): self.assertEqual(result.repository, "/foo/bar") self.assertEqual(result.test1, "/foo/bar/test1") self.assertEqual(result.test2, "/foo/bar/foo/bar") + self.assertEqual(result.test_macro, "/foo/bar") + self.assertEqual(result.test_macro_default, "/foo/bar") self.assertEqual(result.test_path, "/foo/bar/test") self.assertEqual(result.test_path_default, "/foo/bar/test/default") -- cgit v1.2.3-1-g7c22