ceph.in: print all matched commands if arg missing

so if only a single command matches, the "Invalid command:" error
message is
printed, otherwise, the top 10 matched candidate commands are printed.

also, validate_command() now always returns `{}` if the command is not
valid. because we just the retval of it using
```py
if validate_command(...):
  # ...
```
and we do not differentiate `None` from `{}` here. and it'd be more
consistent and easier from the implementation perspective this way.

Fixes: http://tracker.ceph.com/issues/22344
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit ab076edf87a67e2cc5f8b0108c015a5b9736b771)

diff --git a/src/pybind/ceph_argparse.py b/src/pybind/ceph_argparse.py
index 6582931f0f35e2b2f1b5dbbffe2baf3a27134310..cbc2adad70e097f019e6483955bbc0a76e1966cc 100644 (file)
--- a/src/pybind/ceph_argparse.py
+++ b/src/pybind/ceph_argparse.py
@@ -53,6 +53,13 @@ class ArgumentFormat(ArgumentError):
     pass
 
 
+class ArgumentMissing(ArgumentError):
+    """
+    Argument value missing in a command
+    """
+    pass
+
+
 class ArgumentValid(ArgumentError):
     """
     Argument value is otherwise invalid (doesn't match choices, for instance)
@@ -944,7 +951,7 @@ def validate(args, signature, flags=0, partial=False):
                         return d
                     # special-case the "0 expected 1" case
                     if desc.numseen == 0 and desc.n == 1:
-                        raise ArgumentNumber(
+                        raise ArgumentMissing(
                             'missing required parameter {0}'.format(desc)
                         )
                     raise ArgumentNumber(
@@ -1042,6 +1049,7 @@ def validate_command(sigdict, args, verbose=False):
             print("bestcmds_sorted: ", file=sys.stderr)
             pprint.PrettyPrinter(stream=sys.stderr).pprint(bestcmds_sorted)
 
+        e = None
         # for everything in bestcmds, look for a true match
         for cmdsig in bestcmds_sorted:
             for cmd in cmdsig.values():
@@ -1054,6 +1062,10 @@ def validate_command(sigdict, args, verbose=False):
                     # ignore prefix mismatches; we just haven't found
                     # the right command yet
                     pass
+                except ArgumentMissing as e:
+                    if len(bestcmds) == 1:
+                        found = cmd
+                    break
                 except ArgumentTooFew:
                     # It looked like this matched the beginning, but it
                     # didn't have enough args supplied.  If we're out of
@@ -1066,23 +1078,25 @@ def validate_command(sigdict, args, verbose=False):
                     # Solid mismatch on an arg (type, range, etc.)
                     # Stop now, because we have the right command but
                     # some other input is invalid
-                    print("Invalid command: ", e, file=sys.stderr)
-                    print(concise_sig(sig), ': ', cmd['help'], file=sys.stderr)
-                    return {}
-            if found:
+                    found = cmd
+                    break
+            if found or e:
                 break
 
-        if not found:
+        if found:
+            if not valid_dict:
+                print("Invalid command:", e, file=sys.stderr)
+                print(concise_sig(sig), ': ', cmd['help'], file=sys.stderr)
+        else:
             bestcmds = bestcmds[:10]
             print('no valid command found; {0} closest matches:'.format(len(bestcmds)), file=sys.stderr)
             for cmdsig in bestcmds:
                 for (cmdtag, cmd) in cmdsig.items():
                     print(concise_sig(cmd['sig']), file=sys.stderr)
-            return None
-
         return valid_dict
 
 
+
 def find_cmd_target(childargs):
     """
     Using a minimal validation, figure out whether the command

diff --git a/src/test/pybind/test_ceph_argparse.py b/src/test/pybind/test_ceph_argparse.py
index bc9f2d929fc9f5e77232edc604b2b01f3f32934a..b0f7f74b2c7a2fdebdbab2663a274218caecac13 100755 (executable)
--- a/src/test/pybind/test_ceph_argparse.py
+++ b/src/test/pybind/test_ceph_argparse.py
@@ -22,7 +22,9 @@ from ceph_argparse import validate_command, parse_json_funcsigs
 
 import os
 import re
+import sys
 import json
+from StringIO import StringIO
 
 def get_command_descriptions(what):
     CEPH_BIN = os.environ['CEPH_BIN']
@@ -45,8 +47,7 @@ class TestArgparse:
 
     def assert_valid_command(self, args):
         result = validate_command(sigdict, args)
-        assert_not_equal(result,None)
-        assert_not_equal(result,{})
+        assert_not_in(result, [{}, None])
 
     def check_1_natural_arg(self, prefix, command):
         self.assert_valid_command([prefix, command, '1'])
@@ -88,6 +89,20 @@ class TestArgparse:
                                                     command,
                                                     'toomany']))
 
+       def capture_output(self, args, stdout=None, stderr=None):
+        if stdout:
+            stdout = StringIO()
+            sys.stdout = stdout
+        if stderr:
+            stderr = StringIO()
+            sys.stderr = stderr
+        ret = validate_command(sigdict, args)
+        if stdout:
+            stdout = stdout.getvalue().strip()
+        if stderr:
+            stderr = stderr.getvalue().strip()
+        return ret, stdout, stderr
+
 
 class TestBasic:
 
@@ -95,13 +110,13 @@ class TestBasic:
         # ArgumentPrefix("no match for {0}".format(s)) is not able to convert
         # unicode str parameter into str. and validate_command() should not
         # choke on it.
-        assert_is_none(validate_command(sigdict, [u'章鱼和鱿鱼']))
-        assert_is_none(validate_command(sigdict, [u'–w']))
+        assert_equal({}, validate_command(sigdict, [u'章鱼和鱿鱼']))
+        assert_equal({}, validate_command(sigdict, [u'–w']))
         # actually we always pass unicode strings to validate_command() in "ceph"
         # CLI, but we also use bytestrings in our tests, so make sure it does not
         # break.
-        assert_is_none(validate_command(sigdict, ['章鱼和鱿鱼']))
-        assert_is_none(validate_command(sigdict, ['–w']))
+        assert_equal({}, validate_command(sigdict, ['章鱼和鱿鱼']))
+        assert_equal({}, validate_command(sigdict, ['–w']))
 
 
 class TestPG(TestArgparse):
@@ -180,6 +195,17 @@ class TestPG(TestArgparse):
         assert_equal({}, validate_command(sigdict, ['pg', 'debug',
                                                     'invalid']))
 
+    def test_pg_missing_args_output(self):
+        ret, _, stderr = self.capture_output(['pg'], stderr=True)
+        assert_equal({}, ret)
+        assert_regexp_matches(stderr, re.compile('no valid command found.* closest matches'))
+
+    def test_pg_wrong_arg_output(self):
+        ret, _, stderr = self.capture_output(['pg', 'map', 'bad-pgid'],
+                                             stderr=True)
+        assert_equal({}, ret)
+        assert_in("Invalid command", stderr)
+
 
 class TestAuth(TestArgparse):
 
@@ -872,7 +898,7 @@ class TestOSD(TestArgparse):
                                                         'pause', 'toomany']))
 
     def test_cluster_snap(self):
-        assert_equal(None, validate_command(sigdict, ['osd', 'cluster_snap']))
+        assert_equal({}, validate_command(sigdict, ['osd', 'cluster_snap']))
 
     def test_down(self):
         self.check_1_or_more_string_args('osd', 'down')