Merge pull request #11683 from zmstone/0925-test-refactor-emqx-rpc-test

test: refactor emqx_rpc unit tests

.github/workflows/run_emqx_app_tests.yaml

@@ -55,7 +55,7 @@ jobs:
         cd apps/emqx
         ./rebar3 xref
         ./rebar3 dialyzer
-        ./rebar3 eunit -v
+        ./rebar3 eunit -v --name 'eunit@127.0.0.1'
         ./rebar3 as standalone_test ct --name 'test@127.0.0.1' -v --readable=true
         ./rebar3 proper -d test/props
     - uses: actions/upload-artifact@v3

Makefile

@@ -75,7 +75,7 @@ mix-deps-get: $(ELIXIR_COMMON_DEPS)
 
 .PHONY: eunit
 eunit: $(REBAR) merge-config
-	@ENABLE_COVER_COMPILE=1 $(REBAR) eunit -v -c --cover_export_name $(CT_COVER_EXPORT_PREFIX)-eunit
+	@ENABLE_COVER_COMPILE=1 $(REBAR) eunit  --name eunit@127.0.0.1 -v -c --cover_export_name $(CT_COVER_EXPORT_PREFIX)-eunit
 
 .PHONY: proper
 proper: $(REBAR)

apps/emqx/src/emqx_rpc.erl

@@ -43,6 +43,10 @@
     erpc_multicall/1
 ]).
 
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+-endif.
+
 -compile(
     {inline, [
         rpc_node/1,
@@ -75,15 +79,15 @@
 
 -spec call(node(), module(), atom(), list()) -> call_result().
 call(Node, Mod, Fun, Args) ->
-    filter_result(gen_rpc:call(rpc_node(Node), Mod, Fun, Args)).
+    maybe_badrpc(gen_rpc:call(rpc_node(Node), Mod, Fun, Args)).
 
 -spec call(term(), node(), module(), atom(), list()) -> call_result().
 call(Key, Node, Mod, Fun, Args) ->
-    filter_result(gen_rpc:call(rpc_node({Key, Node}), Mod, Fun, Args)).
+    maybe_badrpc(gen_rpc:call(rpc_node({Key, Node}), Mod, Fun, Args)).
 
 -spec call(term(), node(), module(), atom(), list(), timeout()) -> call_result().
 call(Key, Node, Mod, Fun, Args, Timeout) ->
-    filter_result(gen_rpc:call(rpc_node({Key, Node}), Mod, Fun, Args, Timeout)).
+    maybe_badrpc(gen_rpc:call(rpc_node({Key, Node}), Mod, Fun, Args, Timeout)).
 
 -spec multicall([node()], module(), atom(), list()) -> multicall_result().
 multicall(Nodes, Mod, Fun, Args) ->
@@ -127,18 +131,15 @@ rpc_nodes([], Acc) ->
 rpc_nodes([Node | Nodes], Acc) ->
     rpc_nodes(Nodes, [rpc_node(Node) | Acc]).
 
-filter_result({Error, Reason}) when
+maybe_badrpc({Error, Reason}) when Error =:= badrpc; Error =:= badtcp ->
-    Error =:= badrpc; Error =:= badtcp
-->
     {badrpc, Reason};
-filter_result(Delivery) ->
+maybe_badrpc(Delivery) ->
     Delivery.
 
 max_client_num() ->
     emqx:get_config([rpc, tcp_client_num], ?DefaultClientNum).
 
 -spec unwrap_erpc(emqx_rpc:erpc(A) | [emqx_rpc:erpc(A)]) -> A | {error, _Err} | list().
-
 unwrap_erpc(Res) when is_list(Res) ->
     [unwrap_erpc(R) || R <- Res];
 unwrap_erpc({ok, A}) ->
@@ -151,3 +152,73 @@ unwrap_erpc({exit, Err}) ->
     {error, Err};
 unwrap_erpc({error, {erpc, Err}}) ->
     {error, Err}.
+
+-ifdef(TEST).
+
+badrpc_call_test_() ->
+    application:ensure_all_started(gen_rpc),
+    Node = node(),
+    [
+        {"throw", fun() ->
+            ?assertEqual(foo, call(Node, erlang, throw, [foo]))
+        end},
+        {"error", fun() ->
+            ?assertMatch({badrpc, {'EXIT', {foo, _}}}, call(Node, erlang, error, [foo]))
+        end},
+        {"exit", fun() ->
+            ?assertEqual({badrpc, {'EXIT', foo}}, call(Node, erlang, exit, [foo]))
+        end},
+        {"timeout", fun() ->
+            ?assertEqual({badrpc, timeout}, call(key, Node, timer, sleep, [1000], 100))
+        end},
+        {"noconnection", fun() ->
+            %% mute crash report from gen_rpc
+            logger:set_primary_config(level, critical),
+            try
+                ?assertEqual(
+                    {badrpc, nxdomain}, call(key, 'no@such.node', foo, bar, [])
+                )
+            after
+                logger:set_primary_config(level, notice)
+            end
+        end}
+    ].
+
+multicall_test() ->
+    application:ensure_all_started(gen_rpc),
+    logger:set_primary_config(level, critical),
+    BadNode = 'no@such.node',
+    ThisNode = node(),
+    Nodes = [ThisNode, BadNode],
+    Call4 = fun(M, F, A) -> multicall(Nodes, M, F, A) end,
+    Call5 = fun(Key, M, F, A) -> multicall(Key, Nodes, M, F, A) end,
+    try
+        ?assertMatch({[foo], [{BadNode, _}]}, Call4(erlang, throw, [foo])),
+        ?assertMatch({[], [{ThisNode, _}, {BadNode, _}]}, Call4(erlang, error, [foo])),
+        ?assertMatch({[], [{ThisNode, _}, {BadNode, _}]}, Call4(erlang, exit, [foo])),
+        ?assertMatch({[], [{ThisNode, _}, {BadNode, _}]}, Call5(key, foo, bar, []))
+    after
+        logger:set_primary_config(level, notice)
+    end.
+
+unwrap_erpc_test_() ->
+    Nodes = [node()],
+    MultiC = fun(M, F, A) -> unwrap_erpc(erpc:multicall(Nodes, M, F, A, 100)) end,
+    [
+        {"throw", fun() ->
+            ?assertEqual([{error, foo}], MultiC(erlang, throw, [foo]))
+        end},
+        {"error", fun() ->
+            ?assertEqual([{error, foo}], MultiC(erlang, error, [foo]))
+        end},
+        {"exit", fun() ->
+            ?assertEqual([{error, {exception, foo}}], MultiC(erlang, exit, [foo]))
+        end},
+        {"noconnection", fun() ->
+            ?assertEqual(
+                [{error, noconnection}], unwrap_erpc(erpc:multicall(['no@such.node'], foo, bar, []))
+            )
+        end}
+    ].
+
+-endif.

apps/emqx/test/props/prop_emqx_rpc.erl

@@ -1,195 +0,0 @@
-%%--------------------------------------------------------------------
-%% Copyright (c) 2020-2023 EMQ Technologies Co., Ltd. All Rights Reserved.
-%%
-%% Licensed under the Apache License, Version 2.0 (the "License");
-%% you may not use this file except in compliance with the License.
-%% You may obtain a copy of the License at
-%%
-%%     http://www.apache.org/licenses/LICENSE-2.0
-%%
-%% Unless required by applicable law or agreed to in writing, software
-%% distributed under the License is distributed on an "AS IS" BASIS,
-%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-%% See the License for the specific language governing permissions and
-%% limitations under the License.
-%%--------------------------------------------------------------------
-
--module(prop_emqx_rpc).
-
--include_lib("proper/include/proper.hrl").
--include_lib("eunit/include/eunit.hrl").
-
--define(NODENAME, 'test@127.0.0.1').
-
--define(ALL(Vars, Types, Exprs),
-    ?SETUP(
-        fun() ->
-            State = do_setup(),
-            fun() -> do_teardown(State) end
-        end,
-        ?FORALL(Vars, Types, Exprs)
-    )
-).
-
-%%--------------------------------------------------------------------
-%% Properties
-%%--------------------------------------------------------------------
-
-prop_node() ->
-    ?ALL(
-        Node0,
-        nodename(),
-        begin
-            Node = punch(Node0),
-            ?assert(emqx_rpc:cast(Node, erlang, system_time, [])),
-            case emqx_rpc:call(Node, erlang, system_time, []) of
-                {badrpc, _Reason} -> true;
-                Delivery when is_integer(Delivery) -> true;
-                _Other -> false
-            end
-        end
-    ).
-
-prop_node_with_key() ->
-    ?ALL(
-        {Node0, Key},
-        nodename_with_key(),
-        begin
-            Node = punch(Node0),
-            ?assert(emqx_rpc:cast(Key, Node, erlang, system_time, [])),
-            case emqx_rpc:call(Key, Node, erlang, system_time, []) of
-                {badrpc, _Reason} -> true;
-                Delivery when is_integer(Delivery) -> true;
-                _Other -> false
-            end
-        end
-    ).
-
-prop_nodes() ->
-    ?ALL(
-        Nodes0,
-        nodesname(),
-        begin
-            Nodes = punch(Nodes0),
-            case emqx_rpc:multicall(Nodes, erlang, system_time, []) of
-                {RealResults, RealBadNodes} when
-                    is_list(RealResults);
-                    is_list(RealBadNodes)
-                ->
-                    true;
-                _Other ->
-                    false
-            end
-        end
-    ).
-
-prop_nodes_with_key() ->
-    ?ALL(
-        {Nodes0, Key},
-        nodesname_with_key(),
-        begin
-            Nodes = punch(Nodes0),
-            case emqx_rpc:multicall(Key, Nodes, erlang, system_time, []) of
-                {RealResults, RealBadNodes} when
-                    is_list(RealResults);
-                    is_list(RealBadNodes)
-                ->
-                    true;
-                _Other ->
-                    false
-            end
-        end
-    ).
-
-%%--------------------------------------------------------------------
-%%  Helper
-%%--------------------------------------------------------------------
-
-do_setup() ->
-    ensure_distributed_nodename(),
-    ok = logger:set_primary_config(#{level => warning}),
-    {ok, _Apps} = application:ensure_all_started(gen_rpc),
-    ok = application:set_env(gen_rpc, call_receive_timeout, 100),
-    ok = meck:new(gen_rpc, [passthrough, no_history]),
-    ok = meck:expect(
-        gen_rpc,
-        multicall,
-        fun(Nodes, Mod, Fun, Args) ->
-            gen_rpc:multicall(Nodes, Mod, Fun, Args, 100)
-        end
-    ).
-
-do_teardown(_) ->
-    ok = net_kernel:stop(),
-    ok = application:stop(gen_rpc),
-    ok = meck:unload(gen_rpc),
-    %% wait for tcp close
-    timer:sleep(2500).
-
-ensure_distributed_nodename() ->
-    case net_kernel:start([?NODENAME]) of
-        {ok, _} ->
-            ok;
-        {error, {already_started, _}} ->
-            net_kernel:stop(),
-            net_kernel:start([?NODENAME]);
-        {error, {{shutdown, {_, _, {'EXIT', nodistribution}}}, _}} ->
-            %% start epmd first
-            spawn_link(fun() -> os:cmd("epmd") end),
-            timer:sleep(100),
-            net_kernel:start([?NODENAME])
-    end.
-
-%%--------------------------------------------------------------------
-%% Generator
-%%--------------------------------------------------------------------
-
-nodename() ->
-    ?LET(
-        {NodePrefix, HostName},
-        {node_prefix(), hostname()},
-        begin
-            Node = NodePrefix ++ "@" ++ HostName,
-            list_to_atom(Node)
-        end
-    ).
-
-nodename_with_key() ->
-    ?LET(
-        {NodePrefix, HostName, Key},
-        {node_prefix(), hostname(), choose(0, 10)},
-        begin
-            Node = NodePrefix ++ "@" ++ HostName,
-            {list_to_atom(Node), Key}
-        end
-    ).
-
-nodesname() ->
-    oneof([list(nodename()), [node()]]).
-
-nodesname_with_key() ->
-    oneof([{list(nodename()), choose(0, 10)}, {[node()], 1}]).
-
-node_prefix() ->
-    oneof(["emqxct", text_like()]).
-
-text_like() ->
-    ?SUCHTHAT(Text, list(range($a, $z)), (length(Text) =< 100 andalso length(Text) > 0)).
-
-hostname() ->
-    oneof(["127.0.0.1", "localhost"]).
-
-%%--------------------------------------------------------------------
-%% Utils
-%%--------------------------------------------------------------------
-
-%% After running the props, the `node()` () is only able to return an
-%% incorrect node name - `nonode@nohost`, But we want a distributed nodename
-%% So, just translate the `nonode@nohost` to ?NODENAME
-punch(Nodes) when is_list(Nodes) ->
-    lists:map(fun punch/1, Nodes);
-punch('nonode@nohost') ->
-    %% Equal to ?NODENAME
-    node();
-punch(GoodBoy) ->
-    GoodBoy.

apps/emqx_bridge_kafka/test/emqx_bridge_kafka_tests.erl

@@ -139,6 +139,7 @@ kafka_consumer_test() ->
     ok.
 
 message_key_dispatch_validations_test() ->
+    Name = myproducer,
     Conf0 = kafka_producer_new_hocon(),
     Conf1 =
         Conf0 ++
@@ -155,7 +156,7 @@ message_key_dispatch_validations_test() ->
                     <<"message">> := #{<<"key">> := <<>>}
                 }
         },
-        emqx_utils_maps:deep_get([<<"bridges">>, <<"kafka">>, <<"myproducer">>], Conf)
+        emqx_utils_maps:deep_get([<<"bridges">>, <<"kafka">>, atom_to_binary(Name)], Conf)
     ),
     ?assertThrow(
         {_, [
@@ -166,8 +167,6 @@ message_key_dispatch_validations_test() ->
         ]},
         check(Conf)
     ),
-    %% ensure atoms exist
-    _ = [myproducer],
     ?assertThrow(
         {_, [
             #{

apps/emqx_bridge_pulsar/test/emqx_bridge_pulsar_tests.erl

@@ -11,6 +11,7 @@
 %%===========================================================================
 
 pulsar_producer_validations_test() ->
+    Name = my_producer,
     Conf0 = pulsar_producer_hocon(),
     Conf1 =
         Conf0 ++
@@ -24,7 +25,7 @@ pulsar_producer_validations_test() ->
             <<"strategy">> := <<"key_dispatch">>,
             <<"message">> := #{<<"key">> := <<>>}
         },
-        emqx_utils_maps:deep_get([<<"bridges">>, <<"pulsar_producer">>, <<"my_producer">>], Conf)
+        emqx_utils_maps:deep_get([<<"bridges">>, <<"pulsar_producer">>, atom_to_binary(Name)], Conf)
     ),
     ?assertThrow(
         {_, [
@@ -35,8 +36,6 @@ pulsar_producer_validations_test() ->
         ]},
         check(Conf)
     ),
-    %% ensure atoms exist
-    _ = [my_producer],
     ?assertThrow(
         {_, [
             #{