From a28d1efd39ac8880866cd38714b2dd8f198c3582 Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 21 Jan 2022 16:12:21 +0800 Subject: [PATCH] fix(emqx_exhook): improve test coverage of the emqx_exhook_mgr --- apps/emqx_exhook/src/emqx_exhook_mgr.erl | 6 +- apps/emqx_exhook/test/emqx_exhook_SUITE.erl | 61 ++++++++++++++++++- .../test/emqx_exhook_api_SUITE.erl | 24 +++++++- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook_mgr.erl b/apps/emqx_exhook/src/emqx_exhook_mgr.erl index 300858807..e9db95abc 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mgr.erl +++ b/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}; - NewConf -> {ok, NewConf} + not_found -> {error, not_found}; + 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 -> diff --git a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl index 002900f1e..87bdc1641 100644 --- a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl +++ b/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 %%-------------------------------------------------------------------- diff --git a/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl index 2b0a42e27..cc83c33b3 100644 --- a/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl +++ b/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_()),