From 477c9c4119df5fd45c1129651922d238dccad8c9 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 --- src/lib/Bcfg2/Options/Parser.py | 94 ++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 39 deletions(-) (limited to 'src/lib/Bcfg2/Options/Parser.py') 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 -- cgit v1.2.3-1-g7c22 From 4ec92cb9e7d1eb2f90d36e5255ee8814ca0a8513 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/Parser.py | 77 +++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 23 deletions(-) (limited to 'src/lib/Bcfg2/Options/Parser.py') 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: -- cgit v1.2.3-1-g7c22 From 75225b07acbda8dab8215a626088ba19739f73cb 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(-) (limited to 'src/lib/Bcfg2/Options/Parser.py') 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