no need for orig_rargs, can use commandline_arguments (this avoids al…

…so redefining parse_args, which caused double paring of environment args)

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,15 +505,15 @@ 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
             return 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:

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,15 +659,15 @@ 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],
                            go_args=['--quiet'],
                            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: