From fe624d09941834d61757245bb2b6079df926e66b Mon Sep 17 00:00:00 2001 From: stdweird Date: Thu, 8 Oct 2015 23:27:57 +0200 Subject: [PATCH] no need for orig_rargs, can use commandline_arguments (this avoids also redefining parse_args, which caused double paring of environment args) --- lib/vsc/utils/generaloption.py | 20 +++-------------- test/generaloption.py | 39 +++++++++++++++++++++------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/lib/vsc/utils/generaloption.py b/lib/vsc/utils/generaloption.py index 093f490b..bfc53035 100644 --- a/lib/vsc/utils/generaloption.py +++ b/lib/vsc/utils/generaloption.py @@ -227,15 +227,14 @@ def process(self, opt, value, values, parser): """Handle option-as-value issues before actually processing option.""" if hasattr(parser, 'is_value_a_commandline_option'): - prefix = "%s=" % self._long_opts[0] if self._long_opts else self._short_opts[0] errmsg = parser.is_value_a_commandline_option(value) if errmsg is not None: + prefix = "%s=" % self._long_opts[0] if self._long_opts else self._short_opts[0] self.log.raiseException("%s. Use '%s%s' if the value is correct." % (errmsg, prefix, value), exception=OptionValueError) return Option.process(self, opt, value, values, parser) - def take_action(self, action, dest, opt, value, values, parser): """Extended take_action""" orig_action = action # keep copy @@ -488,15 +487,6 @@ def __init__(self, *args, **kwargs): self.environment_arguments = None self.commandline_arguments = None - self.orig_rargs = None # copy of the original arguments - - def parse_args(self, args=None, values=None): - """Keep a copy of the original arguments for callback / option processing""" - # seems like self._get_args returns a copy, but better safe then sorry here - self.orig_rargs = self._get_args(args)[:] - - # continue as usual - return OptionParser.parse_args(self, args=args, values=values) def is_value_a_commandline_option(self, value, index=None): """ @@ -515,7 +505,7 @@ def is_value_a_commandline_option(self, value, index=None): # --longopt=value, so no issues there either. try: - cmdline_index = self.orig_rargs.index(value) + cmdline_index = self.commandline_arguments.index(value) except ValueError: # There is no ambiguity if the value is not passed # as standalone argument via commandline @@ -523,7 +513,7 @@ def is_value_a_commandline_option(self, value, index=None): if index is None: # index of last parsed arg in orig_rargs via remainder of rargs - index = len(self.orig_rargs) - len(self.rargs) - 1 + index = len(self.commandline_arguments) - len(self.rargs) - 1 if index != cmdline_index: # This is not the value you are looking for @@ -793,10 +783,6 @@ def get_env_options_prefix(self): def get_env_options(self): """Retrieve options from the environment: prefix_longopt.upper()""" - if self.environment_arguments is not None: - # already done, let's not do this again - return self.environment_arguments - self.environment_arguments = [] if not self.process_env_options: diff --git a/test/generaloption.py b/test/generaloption.py index bcd4a159..44f02878 100644 --- a/test/generaloption.py +++ b/test/generaloption.py @@ -613,21 +613,29 @@ def test_get_options_by_property(self): def test_loglevel(self): """Test the loglevel default setting""" + def _loglevel(lvl, msg): + lvl_int = topt.log.getEffectiveLevel() + lvl_name = [k for k,v in logging._levelNames.items() if v == lvl_int][0] + self.assertEqual(lvl_int, + fancylogger.getLevelInt(lvl), + msg="%s (expected %s got %s)" % (msg, lvl, lvl_name)) + + topt = TestOption1(go_args=['--ext-optional=REALVALUE'], go_nosystemexit=True,) - self.assertEqual(topt.log.getEffectiveLevel(), fancylogger.getLevelInt(topt.DEFAULT_LOGLEVEL.upper())) + _loglevel(topt.DEFAULT_LOGLEVEL.upper(), 'Test default loglevel') topt = TestOption1(go_args=['--debug'], go_nosystemexit=True,) - self.assertEqual(topt.log.getEffectiveLevel(), logging.DEBUG) + _loglevel('DEBUG', '--debug gives DEBUG') topt = TestOption1(go_args=['--info'], go_nosystemexit=True,) - self.assertEqual(topt.log.getEffectiveLevel(), logging.INFO) + _loglevel('INFO', '--info gives INFO') topt = TestOption1(go_args=['--quiet'], go_nosystemexit=True,) - self.assertEqual(topt.log.getEffectiveLevel(), logging.WARNING) + _loglevel('WARNING', '--quiet gives WARNING') # last one wins topt = TestOption1(go_args=['--debug', '--info', '--quiet'], go_nosystemexit=True,) - self.assertEqual(topt.log.getEffectiveLevel(), logging.WARNING) + _loglevel('WARNING', 'last wins: --debug --info --quiet gives WARNING') CONFIGFILE1 = """ [base] @@ -642,7 +650,7 @@ def test_loglevel(self): go_nosystemexit=True, envvar_prefix=envvar ) - self.assertEqual(topt.log.getEffectiveLevel(), logging.DEBUG) + _loglevel('DEBUG', 'DEBUG set via configfile') # set via environment; environment wins over cfg file os.environ['%s_INFO' % envvar] = '1'; @@ -651,7 +659,7 @@ def test_loglevel(self): go_nosystemexit=True, envvar_prefix=envvar ) - self.assertEqual(topt.log.getEffectiveLevel(), logging.INFO) + _loglevel('INFO', 'env wins: debug in configfile and _INFO in env gives INFO') # commandline always wins topt = TestOption1(go_configfiles=[tmp1.name], @@ -659,7 +667,7 @@ def test_loglevel(self): go_nosystemexit=True, envvar_prefix=envvar ) - self.assertEqual(topt.log.getEffectiveLevel(), logging.WARNING) + _loglevel('WARNING', 'commandline wins: debug in configfile, _INFO in env and --quiet gives WARNING') # remove tmp1 del os.environ['%s_INFO' % envvar] @@ -748,12 +756,13 @@ def test_error_env_options(self): os.environ['GENERALOPTIONTEST_XYZ'] = '1' topt1 = TestOption1(go_args=['--level-level'], envvar_prefix='GENERALOPTIONTEST') - # no errors logged - self.assertEqual(self.count_logcache('error'), 0) + self.assertEqual(self.count_logcache('error'), 0, + msg='no errors logged, got %s' % self.count_logcache('error')) topt1 = TestOption1(go_args=['--level-level'], envvar_prefix='GENERALOPTIONTEST', error_env_options=True) - # one error should be logged - self.assertEqual(self.count_logcache('error'), 1) + print self.LOGCACHE['error'] + self.assertEqual(self.count_logcache('error'), 1, + msg='one error should be logged, got %s' % self.count_logcache('error')) # using a custom error method def raise_error(msg, *args): @@ -777,7 +786,7 @@ def test_nosuchoption(self): def test_is_value_a_commandline_option(self): """Test ExtOptionParser is_value_a_commandline_option method""" - topt = TestOption1() + topt = TestOption1(go_args=[]) tp = topt.parser fn = tp.is_value_a_commandline_option @@ -790,9 +799,9 @@ def test_is_value_a_commandline_option(self): # fake commandline args # this is purely to test the function, actual usage is in test_option_as_value - tp.orig_rargs = ['--base', '-something', 'base'] + tp.commandline_arguments = ['--base', '-something', 'base'] def t(fail, tpl): - for idx, value in enumerate(tp.orig_rargs): + for idx, value in enumerate(tp.commandline_arguments): msg = tpl % value res = fn(value, index=idx) if idx in fail: