fix(emqx_exhook): improve test coverage of the emqx_exhook_mgr

apps/emqx_exhook/src/emqx_exhook_mgr.erl

@@ -149,8 +149,8 @@ pre_config_update(_, {add, Conf}, OldConf) ->
 
 pre_config_update(_, {update, Name, Conf}, OldConf) ->
     case replace_conf(Name, fun(_) -> Conf end, OldConf) of
-                    not_found -> {error, not_found};
+        not_found -> {error, not_found};
-                    NewConf -> {ok, NewConf}
+        NewConf -> {ok, NewConf}
     end;
 
 pre_config_update(_, {delete, ToDelete}, OldConf) ->
@@ -433,7 +433,7 @@ ensure_reload_timer(none, Waiting, Stopped, TimerRef) ->
 ensure_reload_timer({Name, #{auto_reconnect := Intv}, Iter},
                     Waiting,
                     Stopped,
-                    TimerRef) ->
+                    TimerRef) when is_integer(Intv) ->
     Next = maps:next(Iter),
     case maps:is_key(Name, TimerRef) of
         true ->

apps/emqx_exhook/test/emqx_exhook_SUITE.erl

@@ -28,7 +28,19 @@ exhook {
   servers = [
     { name = default,
       url = \"http://127.0.0.1:9000\"
-    }]
+    },
+    { name = enable,
+      enable = false,
+      url = \"http://127.0.0.1:9000\"
+    },
+    { name = error,
+      url = \"http://127.0.0.1:9001\"
+    },
+    { name = not_reconnect,
+      auto_reconnect = false,
+      url = \"http://127.0.0.1:9001\"
+    }
+  ]
 }
 ">>).
 
@@ -104,6 +116,53 @@ t_access_failed_if_no_server_running(_) ->
                  emqx_exhook_handler:on_message_publish(Message)),
     emqx_exhook_mgr:enable(<<"default">>).
 
+t_lookup(_) ->
+    Result = emqx_exhook_mgr:lookup(<<"default">>),
+    ?assertMatch(#{name := <<"default">>, status := _}, Result),
+    not_found = emqx_exhook_mgr:lookup(<<"not_found">>).
+
+t_list(_) ->
+    [H | _] = emqx_exhook_mgr:list(),
+    ?assertMatch(#{name := _,
+                   status := _,
+                   hooks := _}, H).
+
+t_unexpected(_) ->
+    ok = gen_server:cast(emqx_exhook_mgr, unexpected),
+    unexpected = erlang:send(erlang:whereis(emqx_exhook_mgr), unexpected),
+    Result = gen_server:call(emqx_exhook_mgr, unexpected),
+    ?assertEqual(Result, ok).
+
+t_timer(_) ->
+    Pid = erlang:whereis(emqx_exhook_mgr),
+    refresh_tick = erlang:send(Pid, refresh_tick),
+    _ = erlang:send(Pid, {timeout, undefined, {reload, <<"default">>}}),
+    _ = erlang:send(Pid, {timeout, undefined, {reload, <<"not_found">>}}),
+    _ = erlang:send(Pid, {timeout, undefined, {reload, <<"error">>}}),
+    ok.
+
+t_error_update_conf(_) ->
+    Path = [exhook, servers],
+    Name = <<"error_update">>,
+    ErrorCfg = #{<<"name">> => Name},
+    {error, _} = emqx_exhook_mgr:update_config(Path, {update, Name, ErrorCfg}),
+    {error, _} = emqx_exhook_mgr:update_config(Path, {move, Name, top, <<>>}),
+    {error, _} = emqx_exhook_mgr:update_config(Path, {enable, Name, true}),
+
+    ErrorAnd = #{<<"name">> => Name, <<"url">> => <<"http://127.0.0.1:9001">>},
+    {ok, _} = emqx_exhook_mgr:update_config(Path, {add, ErrorAnd}),
+
+    DisableAnd = #{<<"name">> => Name, <<"url">> => <<"http://127.0.0.1:9001">>, <<"enable">> => false},
+    {ok, _} = emqx_exhook_mgr:update_config(Path, {add, DisableAnd}),
+
+    {ok, _} = emqx_exhook_mgr:update_config(Path, {delete, <<"error">>}),
+    {ok, _} = emqx_exhook_mgr:update_config(Path, {delete, <<"delete_not_exists">>}),
+    ok.
+
+t_error_server_info(_) ->
+    not_found = emqx_exhook_mgr:server_info(<<"not_exists">>),
+    ok.
+
 %%--------------------------------------------------------------------
 %% Utils
 %%--------------------------------------------------------------------

apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl

@@ -37,7 +37,9 @@ exhook {
 ">>).
 
 all() ->
-    [t_list, t_get, t_add, t_move_1, t_move_2, t_delete, t_hooks, t_update].
+    [ t_list, t_get, t_add, t_move_top, t_move_bottom
+    , t_move_before, t_move_after, t_delete, t_hooks, t_update
+    ].
 
 init_per_suite(Config) ->
     application:load(emqx_conf),
@@ -131,7 +133,15 @@ t_add(Cfg) ->
 
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
 
-t_move_1(_) ->
+t_move_top(_) ->
+    Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
+                         auth_header_(),
+                         #{position => top, related => <<>>}),
+
+    ?assertMatch({ok, <<>>}, Result),
+    ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
+
+t_move_bottom(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
                          #{position => bottom, related => <<>>}),
@@ -139,7 +149,7 @@ t_move_1(_) ->
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()).
 
-t_move_2(_) ->
+t_move_before(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
                          #{position => before, related => <<"test1">>}),
@@ -147,6 +157,14 @@ t_move_2(_) ->
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
 
+t_move_after(_) ->
+    Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
+                         auth_header_(),
+                         #{position => 'after', related => <<"test1">>}),
+
+    ?assertMatch({ok, <<>>}, Result),
+    ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()).
+
 t_delete(_) ->
     Result = request_api(delete, api_path(["exhooks", "test1"]), "",
                          auth_header_()),