Merge "Ambiguous option error should not appear if Arg is SUPPRESS"

cinderclient/shell.py

@@ -67,6 +67,29 @@ class CinderClientArgumentParser(argparse.ArgumentParser):
                       'mainp': progparts[0],
                       'subp': progparts[2]})
 
+    def _get_option_tuples(self, option_string):
+        """Avoid ambiguity in argument abbreviation.
+
+        The idea of this method is to override the default behaviour to
+        avoid ambiguity in the abbreviation feature of argparse.
+        In the case that the ambiguity is generated by 2 or more parameters
+        and only one is visible in the help and the others are with
+        help=argparse.SUPPRESS, the ambiguity is solved by taking the visible
+        one.
+        The use case is for parameters that are left hidden for backward
+        compatibility.
+        """
+
+        result = super(CinderClientArgumentParser, self)._get_option_tuples(
+            option_string)
+
+        if len(result) > 1:
+            aux = [x for x in result if x[0].help != argparse.SUPPRESS]
+            if len(aux) == 1:
+                result = aux
+
+        return result
+
 
 class OpenStackCinderShell(object):
 

cinderclient/tests/test_shell.py

@@ -11,6 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import argparse
 import re
 import sys
 
@@ -19,7 +20,7 @@ from six import moves
 from testtools import matchers
 
 from cinderclient import exceptions
-import cinderclient.shell
+from cinderclient import shell
 from cinderclient.tests import utils
 
 
@@ -43,7 +44,7 @@ class ShellTest(utils.TestCase):
         orig = sys.stdout
         try:
             sys.stdout = moves.StringIO()
-            _shell = cinderclient.shell.OpenStackCinderShell()
+            _shell = shell.OpenStackCinderShell()
             _shell.main(argstr.split())
         except SystemExit:
             exc_type, exc_value, exc_traceback = sys.exc_info()
@@ -78,3 +79,45 @@ class ShellTest(utils.TestCase):
         for r in required:
             self.assertThat(help_text,
                             matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE))
+
+
+class CinderClientArgumentParserTest(utils.TestCase):
+    def test_ambiguity_solved_for_one_visible_argument(self):
+        parser = shell.CinderClientArgumentParser(add_help=False)
+        parser.add_argument('--test-parameter',
+                            dest='visible_param',
+                            action='store_true')
+        parser.add_argument('--test_parameter',
+                            dest='hidden_param',
+                            action='store_true',
+                            help=argparse.SUPPRESS)
+
+        opts = parser.parse_args(['--test'])
+
+        #visible argument must be set
+        self.assertTrue(opts.visible_param)
+        self.assertFalse(opts.hidden_param)
+
+    def test_raise_ambiguity_error_two_visible_argument(self):
+        parser = shell.CinderClientArgumentParser(add_help=False)
+        parser.add_argument('--test-parameter',
+                            dest="visible_param1",
+                            action='store_true')
+        parser.add_argument('--test_parameter',
+                            dest="visible_param2",
+                            action='store_true')
+
+        self.assertRaises(SystemExit, parser.parse_args, ['--test'])
+
+    def test_raise_ambiguity_error_two_hidden_argument(self):
+        parser = shell.CinderClientArgumentParser(add_help=False)
+        parser.add_argument('--test-parameter',
+                            dest="hidden_param1",
+                            action='store_true',
+                            help=argparse.SUPPRESS)
+        parser.add_argument('--test_parameter',
+                            dest="hidden_param2",
+                            action='store_true',
+                            help=argparse.SUPPRESS)
+
+        self.assertRaises(SystemExit, parser.parse_args, ['--test'])