So it turns out that the policy tests fail, since we now depend on rabbit_misc:parse_arguments() to fill in defaults for us. But that's not at all unreasonable I think, and in fact rabbit_tests:control_action() does some rather odd things around options. Rather than rewrite everything, just introduce a new variant that is rather closer to what rabbitmqctl actually does, and use that for policy tests. Oh, and add some more tests, that's always good.

2 files changed, 72 insertions, 24 deletions

diff --git a/src/rabbit_control_main.erl b/src/rabbit_control_main.erl
index fb8752b8..38a07b6a 100644
--- a/src/rabbit_control_main.erl
+++ b/src/rabbit_control_main.erl
@@ -17,7 +17,8 @@
 -module(rabbit_control_main).
 -include("rabbit.hrl").
 
--export([start/0, stop/0, action/5, sync_queue/1, cancel_sync_queue/1]).
+-export([start/0, stop/0, parse_arguments/2, action/5,
+         sync_queue/1, cancel_sync_queue/1]).
 
 -define(RPC_TIMEOUT, infinity).
 -define(EXTERNAL_CHECK_INTERVAL, 1000).
@@ -131,19 +132,13 @@
 start() ->
     {ok, [[NodeStr|_]|_]} = init:get_argument(nodename),
     {Command, Opts, Args} =
-        case rabbit_misc:parse_arguments(?COMMANDS, ?GLOBAL_DEFS(NodeStr),
-                                         init:get_plain_arguments())
-        of
+        case parse_arguments(init:get_plain_arguments(), NodeStr) of
             {ok, Res}  -> Res;
             no_command -> print_error("could not recognise command", []),
                           usage()
         end,
-    Opts1 = [case K of
-                 ?NODE_OPT -> {?NODE_OPT, rabbit_nodes:make(V)};
-                 _         -> {K, V}
-             end || {K, V} <- Opts],
-    Quiet = proplists:get_bool(?QUIET_OPT, Opts1),
-    Node = proplists:get_value(?NODE_OPT, Opts1),
+    Quiet = proplists:get_bool(?QUIET_OPT, Opts),
+    Node = proplists:get_value(?NODE_OPT, Opts),
     Inform = case Quiet of
                  true  -> fun (_Format, _Args1) -> ok end;
                  false -> fun (Format, Args1) ->
@@ -234,6 +229,18 @@ usage() ->
     io:format("~s", [rabbit_ctl_usage:usage()]),
     rabbit_misc:quit(1).
 
+parse_arguments(Args, NodeStr) ->
+    case rabbit_misc:parse_arguments(?COMMANDS, ?GLOBAL_DEFS(NodeStr), Args) of
+        {ok, {Cmd, Opts0, Args}} ->
+            Opts = [case K of
+                        ?NODE_OPT -> {?NODE_OPT, rabbit_nodes:make(V)};
+                        _         -> {K, V}
+                    end || {K, V} <- Opts0],
+            {ok, {Cmd, Opts, Args}};
+        E ->
+            E
+    end.
+
 %%----------------------------------------------------------------------------
 
 action(stop, Node, Args, _Opts, Inform) ->
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index 21c54f3e..ccea636d 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -60,6 +60,7 @@ all_tests() ->
     passed = test_user_management(),
     passed = test_runtime_parameters(),
     passed = test_policy_validation(),
+    passed = test_policy_opts_validation(),
     passed = test_ha_policy_validation(),
     passed = test_server_status(),
     passed = test_amqp_connection_refusal(),
@@ -1083,29 +1084,57 @@ test_runtime_parameters() ->
 
 test_policy_validation() ->
     rabbit_runtime_parameters_test:register_policy_validator(),
-    SetPol =
-        fun (Key, Val) ->
-                control_action(
-                  set_policy,
-                  ["name", ".*", rabbit_misc:format("{\"~s\":~p}", [Key, Val])])
-        end,
+    SetPol = fun (Key, Val) ->
+                     control_action_opts(
+                       ["set_policy", "name", ".*",
+                        rabbit_misc:format("{\"~s\":~p}", [Key, Val])])
+             end,
 
-    ok                 = SetPol("testeven", []),
-    ok                 = SetPol("testeven", [1, 2]),
-    ok                 = SetPol("testeven", [1, 2, 3, 4]),
-    ok                 = SetPol("testpos",  [2, 5, 5678]),
+    ok    = SetPol("testeven", []),
+    ok    = SetPol("testeven", [1, 2]),
+    ok    = SetPol("testeven", [1, 2, 3, 4]),
+    ok    = SetPol("testpos",  [2, 5, 5678]),
 
-    {error_string, _}  = SetPol("testpos",  [-1, 0, 1]),
-    {error_string, _}  = SetPol("testeven", [ 1, 2, 3]),
+    error = SetPol("testpos",  [-1, 0, 1]),
+    error = SetPol("testeven", [ 1, 2, 3]),
 
     ok = control_action(clear_policy, ["name"]),
     rabbit_runtime_parameters_test:unregister_policy_validator(),
     passed.
 
+test_policy_opts_validation() ->
+    Set  = fun (Extra) -> control_action_opts(
+                            ["set_policy", "name", ".*", "{\"ha-mode\":\"all\"}"
+                             | Extra]) end,
+    OK   = fun (Extra) -> ok = Set(Extra) end,
+    Fail = fun (Extra) -> error = Set(Extra) end,
+
+    OK  ([]),
+
+    OK  (["--priority", "0"]),
+    OK  (["--priority", "3"]),
+    Fail(["--priority", "banana"]),
+    Fail(["--priority"]),
+
+    OK  (["--apply-to", "all"]),
+    OK  (["--apply-to", "queues"]),
+    Fail(["--apply-to", "bananas"]),
+    Fail(["--apply-to"]),
+
+    OK  (["--priority", "3",      "--apply-to", "queues"]),
+    Fail(["--priority", "banana", "--apply-to", "queues"]),
+    Fail(["--priority", "3",      "--apply-to", "bananas"]),
+
+    Fail(["--offline"]),
+
+    ok = control_action(clear_policy, ["name"]),
+    passed.
+
 test_ha_policy_validation() ->
-    Set  = fun (JSON) -> control_action(set_policy, ["name", ".*", JSON]) end,
+    Set  = fun (JSON) -> control_action_opts(
+                           ["set_policy", "name", ".*", JSON]) end,
     OK   = fun (JSON) -> ok = Set(JSON) end,
-    Fail = fun (JSON) -> {error_string, _} = Set(JSON) end,
+    Fail = fun (JSON) -> error = Set(JSON) end,
 
     OK  ("{\"ha-mode\":\"all\"}"),
     Fail("{\"ha-mode\":\"made_up\"}"),
@@ -1611,6 +1640,18 @@ control_action(Command, Node, Args, Opts) ->
             Other
     end.
 
+control_action_opts(Raw) ->
+    NodeStr = atom_to_list(node()),
+    case rabbit_control_main:parse_arguments(Raw, NodeStr) of
+        {ok, {Cmd, Opts, Args}} ->
+            case control_action(Cmd, node(), Args, Opts) of
+                ok -> ok;
+                _  -> error
+            end;
+        _ ->
+            error
+    end.
+
 info_action(Command, Args, CheckVHost) ->
     ok = control_action(Command, []),
     if CheckVHost -> ok = control_action(Command, [], ["-p", "/"]);