From: John Mulligan <jmulligan@redhat.com>
Date: Mon, 15 Aug 2022 18:47:47 +0000 (-0400)
Subject: pybind/mgr: fix incorrect construction of mgr CLI arguments
X-Git-Tag: v18.0.0~130^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d4aa1754f85df2b231738d031176bba90c93fbdf;p=ceph.git

pybind/mgr: fix incorrect construction of mgr CLI arguments

Instead of adding the extra_args correctly the code was "adding" the
"--format" option with the wrong name - using whatever the last argument
name in the actual arguments of the decorated function.

Updated tests to verify that the format option is added to the "ceph
args" string properly.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
---

diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py
index a4528228321..41e7c211b20 100644
--- a/src/pybind/mgr/mgr_module.py
+++ b/src/pybind/mgr/mgr_module.py
@@ -404,7 +404,7 @@ class CLICommand(object):
                 continue
             arg_spec[argname] = argtype
             args.append(CephArgtype.to_argdesc(
-                argtype, dict(name=arg), has_default=True, positional=False
+                argtype, dict(name=argname), has_default=True, positional=False
             ))
         return desc, arg_spec, first_default, ' '.join(args)
 
diff --git a/src/pybind/mgr/tests/test_object_format.py b/src/pybind/mgr/tests/test_object_format.py
index 43b5ef971a0..f708333a068 100644
--- a/src/pybind/mgr/tests/test_object_format.py
+++ b/src/pybind/mgr/tests/test_object_format.py
@@ -287,10 +287,11 @@ class DecoDemo:
 
 
 @pytest.mark.parametrize(
-    "prefix, args, response",
+    "prefix, can_format, args, response",
     [
         (
             "alpha one",
+            True,
             {"name": "moonbase"},
             (
                 0,
@@ -301,6 +302,7 @@ class DecoDemo:
         # ---
         (
             "alpha one",
+            True,
             {"name": "moonbase2", "format": "yaml"},
             (
                 0,
@@ -311,6 +313,7 @@ class DecoDemo:
         # ---
         (
             "alpha one",
+            True,
             {"name": "moonbase2", "format": "chocolate"},
             (
                 -22,
@@ -321,6 +324,7 @@ class DecoDemo:
         # ---
         (
             "beta two",
+            True,
             {"name": "blocker"},
             (
                 0,
@@ -331,6 +335,7 @@ class DecoDemo:
         # ---
         (
             "beta two",
+            True,
             {"name": "test", "format": "yaml"},
             (
                 0,
@@ -341,6 +346,7 @@ class DecoDemo:
         # ---
         (
             "beta two",
+            True,
             {"name": "test", "format": "plain"},
             (
                 -22,
@@ -351,6 +357,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {},
             (
                 0,
@@ -361,6 +368,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {"size": 1, "format": "json"},
             (
                 0,
@@ -371,6 +379,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {"size": 1, "format": "plain"},
             (
                 0,
@@ -381,6 +390,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {"size": 2, "format": "plain"},
             (
                 0,
@@ -391,6 +401,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {"size": 2, "format": "xml"},
             (
                 0,
@@ -401,6 +412,7 @@ class DecoDemo:
         # ---
         (
             "gamma three",
+            True,
             {"size": 2, "format": "toml"},
             (
                 -22,
@@ -411,6 +423,7 @@ class DecoDemo:
         # ---
         (
             "z_err",
+            False,
             {"name": "foobar"},
             (
                 0,
@@ -421,6 +434,7 @@ class DecoDemo:
         # ---
         (
             "z_err",
+            False,
             {"name": "zamboni"},
             (
                 -22,
@@ -431,6 +445,7 @@ class DecoDemo:
         # ---
         (
             "empty one",
+            False,
             {"name": "zucchini"},
             (
                 0,
@@ -441,6 +456,7 @@ class DecoDemo:
         # ---
         (
             "empty one",
+            False,
             {"name": "pow"},
             (
                 -5,
@@ -450,9 +466,14 @@ class DecoDemo:
         ),
     ],
 )
-def test_cli_with_decorators(prefix, args, response):
+def test_cli_with_decorators(prefix, can_format, args, response):
     dd = DecoDemo()
-    assert CLICommand.COMMANDS[prefix].call(dd, args, None) == response
+    cmd = CLICommand.COMMANDS[prefix]
+    assert cmd.call(dd, args, None) == response
+    # slighly hacky way to check that the CLI "knows" about a --format option
+    # checking the extra_args feature of the Decorators that provide them (Responder)
+    if can_format:
+        assert 'name=format,' in cmd.args
 
 
 def test_error_response():