Merge pull request #7248 from zhongwencool/improve-ct-coverage

test: Improve test coverage

apps/emqx/src/emqx_release.erl

@@ -41,7 +41,7 @@ description() ->
 
 %% @doc Return EMQX edition info.
 %% Read info from persistent_term at runtime.
-%% Or meck this function to run tests for another eidtion.
+%% Or meck this function to run tests for another edition.
 -spec edition() -> ce | ee | edge.
 -ifdef(EMQX_RELEASE_EDITION).
 edition() -> ?EMQX_RELEASE_EDITION.

apps/emqx_conf/src/emqx_cluster_rpc_handler.erl

@@ -38,6 +38,7 @@ start_link(MaxHistory, CleanupMs) ->
 %%%===================================================================
 
 init([State]) ->
+    erlang:process_flag(trap_exit, true),
     {ok, ensure_timer(State)}.
 
 handle_call(Req, _From, State) ->

apps/emqx_conf/src/emqx_conf.erl

@@ -26,6 +26,7 @@
 -export([remove/2, remove/3]).
 -export([reset/2, reset/3]).
 -export([dump_schema/1, dump_schema/2]).
+-export([schema_module/0]).
 
 %% for rpc
 -export([get_node_and_config/1]).
@@ -145,6 +146,14 @@ dump_schema(Dir, SchemaModule) ->
     ok = gen_hot_conf_schema(HotConfigSchemaFile),
     ok.
 
+%% @doc return the root schema module.
+-spec schema_module() -> module().
+schema_module() ->
+    case os:getenv("SCHEMA_MOD") of
+        false -> emqx_conf_schema;
+        Value -> list_to_existing_atom(Value)
+    end.
+
 %%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------
@@ -247,14 +256,15 @@ typename_to_spec("percent()", _Mod) -> #{type => percent};
 typename_to_spec("file()", _Mod) -> #{type => string};
 typename_to_spec("ip_port()", _Mod) -> #{type => ip_port};
 typename_to_spec("url()", _Mod) -> #{type => url};
-typename_to_spec("bytesize()", _Mod) -> #{type => byteSize};
+typename_to_spec("bytesize()", _Mod) -> #{type => 'byteSize'};
-typename_to_spec("wordsize()", _Mod) -> #{type => byteSize};
+typename_to_spec("wordsize()", _Mod) -> #{type => 'byteSize'};
 typename_to_spec("qos()", _Mod) -> #{type => enum, symbols => [0, 1, 2]};
 typename_to_spec("comma_separated_list()", _Mod) -> #{type => comma_separated_string};
 typename_to_spec("comma_separated_atoms()", _Mod) -> #{type => comma_separated_string};
 typename_to_spec("pool_type()", _Mod) -> #{type => enum, symbols => [random, hash]};
 typename_to_spec("log_level()", _Mod) ->
-    #{type => enum, symbols => [debug, info, notice, warning, error, critical, alert, emergency, all]};
+    #{type => enum, symbols => [debug, info, notice, warning, error,
+        critical, alert, emergency, all]};
 typename_to_spec("rate()", _Mod) -> #{type => string};
 typename_to_spec("capacity()", _Mod) -> #{type => string};
 typename_to_spec("burst_rate()", _Mod) -> #{type => string};

apps/emqx_conf/src/emqx_conf_app.erl

@@ -35,15 +35,9 @@ stop(_State) ->
 init_conf() ->
     {ok, TnxId} = copy_override_conf_from_core_node(),
     emqx_app:set_init_tnx_id(TnxId),
-    emqx_config:init_load(schema_module()),
+    emqx_config:init_load(emqx_conf:schema_module()),
     emqx_app:set_init_config_load_done().
 
-schema_module() ->
-    case os:getenv("SCHEMA_MOD") of
-        false -> emqx_conf_schema;
-        Value -> list_to_existing_atom(Value)
-    end.
-
 copy_override_conf_from_core_node() ->
     case nodes() of
         [] -> %% The first core nodes is self.
@@ -67,37 +61,38 @@ copy_override_conf_from_core_node() ->
                         nodes => Nodes, failed => Failed, not_ready => NotReady}),
                     {error, "core node not ready"};
                 _ ->
-                    SortFun = fun({ok, #{wall_clock := W1}}, {ok, #{wall_clock := W2}}) -> W1 > W2 end,
+                    SortFun = fun({ok, #{wall_clock := W1}},
+                        {ok, #{wall_clock := W2}}) -> W1 > W2 end,
                     [{ok, Info} | _] = lists:sort(SortFun, Ready),
                     #{node := Node, conf := RawOverrideConf, tnx_id := TnxId} = Info,
-                    ?SLOG(debug, #{msg => "copy_overide_conf_from_core_node_success", node => Node}),
+                    Msg = #{msg => "copy_overide_conf_from_core_node_success", node => Node},
-                    ok = emqx_config:save_to_override_conf(RawOverrideConf, #{override_to => cluster}),
+                    ?SLOG(debug, Msg),
+                    ok = emqx_config:save_to_override_conf(RawOverrideConf,
+                        #{override_to => cluster}),
                     {ok, TnxId}
             end
     end.
 
 get_override_config_file() ->
     Node = node(),
+    Role = mria_rlog:role(),
     case emqx_app:get_init_config_load_done() of
         false -> {error, #{node => Node, msg => "init_conf_load_not_done"}};
-        true ->
+        true when Role =:= core ->
-            case mria_rlog:role() of
+            case erlang:whereis(emqx_config_handler) of
-                core ->
+                undefined -> {error, #{node => Node, msg => "emqx_config_handler_not_ready"}};
-                    case erlang:whereis(emqx_config_handler) of
+                _ ->
-                        undefined -> {error, #{node => Node, msg => "emqx_config_handler_not_ready"}};
+                    Fun = fun() ->
-                        _ ->
+                        TnxId = emqx_cluster_rpc:get_node_tnx_id(Node),
-                            Fun = fun() ->
+                        WallClock = erlang:statistics(wall_clock),
-                                TnxId = emqx_cluster_rpc:get_node_tnx_id(Node),
+                        Conf = emqx_config_handler:get_raw_cluster_override_conf(),
-                                WallClock = erlang:statistics(wall_clock),
+                        #{wall_clock => WallClock, conf => Conf, tnx_id => TnxId, node => Node}
-                                Conf = emqx_config_handler:get_raw_cluster_override_conf(),
+                          end,
-                                #{wall_clock => WallClock, conf => Conf, tnx_id => TnxId, node => Node}
+                    case mria:ro_transaction(?CLUSTER_RPC_SHARD, Fun) of
-                                  end,
+                        {atomic, Res} -> {ok, Res};
-                            case mria:ro_transaction(?CLUSTER_RPC_SHARD, Fun) of
+                        {aborted, Reason} -> {error, #{node => Node, msg => Reason}}
-                                {atomic, Res} -> {ok, Res};
+                    end
-                                {aborted, Reason} -> {error, #{node => Node, msg => Reason}}
+            end;
-                            end
+        true when Role =:= replicant ->
-                    end;
+            {ignore, #{node => Node}}
-                replicant ->
-                    {ignore, #{node => Node}}
-            end
     end.

apps/emqx_conf/test/emqx_cluster_rpc_SUITE.erl

@@ -225,6 +225,16 @@ t_fast_forward_commit(_Config) ->
         tnx_ids(List2)),
     ok.
 
+t_handler_unexpected_msg(_Config) ->
+    Handler = emqx_cluster_rpc_handler,
+    OldPid = erlang:whereis(Handler),
+    ok = gen_server:cast(Handler, unexpected_cast_msg),
+    ignore = gen_server:call(Handler, unexpected_cast_msg),
+    erlang:send(Handler, unexpected_info_msg),
+    NewPid = erlang:whereis(Handler),
+    ?assertEqual(OldPid, NewPid),
+    ok.
+
 tnx_ids(Status) ->
     lists:sort(lists:map(fun(#{tnx_id := TnxId, node := Node}) ->
         {Node, TnxId} end, Status)).
@@ -234,6 +244,7 @@ start() ->
     {ok, Pid2} = emqx_cluster_rpc:start_link({node(), ?NODE2}, ?NODE2, 500),
     {ok, Pid3} = emqx_cluster_rpc:start_link({node(), ?NODE3}, ?NODE3, 500),
     {ok, Pid4} = emqx_cluster_rpc_handler:start_link(100, 500),
+    true = erlang:register(emqx_cluster_rpc_handler, Pid4),
     {ok, [Pid1, Pid2, Pid3, Pid4]}.
 
 stop() ->
@@ -243,7 +254,8 @@ stop() ->
              P ->
                  erlang:unlink(P),
                  erlang:exit(P, kill)
-         end end || N <- [?NODE1, ?NODE2, ?NODE3]].
+         end end || N <- [?NODE1, ?NODE2, ?NODE3]],
+    gen_server:stop(emqx_cluster_rpc_handler, normal, 5000).
 
 receive_msg(0, _Msg) -> ok;
 receive_msg(Count, Msg) when Count > 0 ->

apps/emqx_dashboard/src/emqx_dashboard_swagger.erl

@@ -297,6 +297,18 @@ trans_required(Spec, true, _) -> Spec#{required => true};
 trans_required(Spec, _, path) -> Spec#{required => true};
 trans_required(Spec, _, _) -> Spec.
 
+trans_desc(Init, Hocon, Func, Name) ->
+    Spec0 =  trans_desc(Init, Hocon),
+    case Func =:= fun hocon_schema_to_spec/2 of
+        true -> Spec0;
+        false ->
+            Spec1 = Spec0#{label => Name},
+            case Spec1 of
+                #{description := _} -> Spec1;
+                _ -> Spec1#{description => <<"TODO(Rquired description): ", Name/binary>>}
+            end
+    end.
+
 trans_desc(Spec, Hocon) ->
     case hocon_schema:field_schema(Hocon, desc) of
         undefined -> Spec;
@@ -333,7 +345,8 @@ response(Status, ?R_REF(_Mod, _Name) = RRef, {Acc, RefsAcc, Module, Options}) ->
     SchemaToSpec = schema_converter(Options),
     {Spec, Refs} = SchemaToSpec(RRef, Module),
     Content = content(Spec),
-    {Acc#{integer_to_binary(Status) => #{<<"content">> => Content}}, Refs ++ RefsAcc, Module, Options};
+    {Acc#{integer_to_binary(Status) =>
+    #{<<"content">> => Content}}, Refs ++ RefsAcc, Module, Options};
 response(Status, Schema, {Acc, RefsAcc, Module, Options}) ->
     case hoconsc:is_schema(Schema) of
         true ->
@@ -447,7 +460,8 @@ typename_to_spec("duration_ms()", _Mod) -> #{type => string, example => <<"32s">
 typename_to_spec("percent()", _Mod) -> #{type => number, example => <<"12%">>};
 typename_to_spec("file()", _Mod) -> #{type => string, example => <<"/path/to/file">>};
 typename_to_spec("ip_port()", _Mod) -> #{type => string, example => <<"127.0.0.1:80">>};
-typename_to_spec("ip_ports()", _Mod) -> #{type => string, example => <<"127.0.0.1:80, 127.0.0.2:80">>};
+typename_to_spec("ip_ports()", _Mod) ->
+    #{type => string, example => <<"127.0.0.1:80, 127.0.0.2:80">>};
 typename_to_spec("url()", _Mod) -> #{type => string, example => <<"http://127.0.0.1">>};
 typename_to_spec("connect_timeout()", Mod) -> typename_to_spec("timeout()", Mod);
 typename_to_spec("timeout()", _Mod) -> #{<<"oneOf">> => [#{type => string, example => infinity},
@@ -551,8 +565,8 @@ parse_object(PropList = [_ | _], Module, Options) when is_list(PropList) ->
                 true ->
                     HoconType = hocon_schema:field_schema(Hocon, type),
                     Init0 = init_prop([default | ?DEFAULT_FIELDS], #{}, Hocon),
-                    Init = trans_desc(Init0, Hocon),
                     SchemaToSpec = schema_converter(Options),
+                    Init = trans_desc(Init0, Hocon, SchemaToSpec, NameBin),
                     {Prop, Refs1} = SchemaToSpec(HoconType, Module),
                     NewRequiredAcc =
                         case is_required(Hocon) of

apps/emqx_management/src/emqx_mgmt_api_configs.erl

@@ -136,22 +136,19 @@ find_schema(Path) ->
     Configs = config_list(),
     lists:keyfind(list_to_binary(Root), 1, Configs).
 
-%% we load all configs from emqx_conf_schema, some of them are defined as local ref
+%% we load all configs from emqx_*_schema, some of them are defined as local ref
-%% we need redirect to emqx_conf_schema.
+%% we need redirect to emqx_*_schema.
-%% such as hoconsc:ref("node") to hoconsc:ref(emqx_conf_schema, "node")
+%% such as hoconsc:ref("node") to hoconsc:ref(emqx_*_schema, "node")
-fields(Field) -> emqx_conf_schema:fields(Field).
+fields(Field) ->
+    Mod = emqx_conf:schema_module(),
+    apply(Mod, fields, [Field]).
 
 %%%==============================================================================================
 %% HTTP API Callbacks
 config(get, _Params, Req) ->
     Path = conf_path(Req),
-    case emqx_map_lib:deep_find(Path, get_full_config()) of
+    {ok, Conf} = emqx_map_lib:deep_find(Path, get_full_config()),
-        {ok, Conf} ->
+    {200, Conf};
-            {200, Conf};
-        {not_found, _, _} ->
-            {404, #{code => 'NOT_FOUND',
-                message => <<(list_to_binary(Path))/binary, " not found">>}}
-    end;
 
 config(put, #{body := Body}, Req) ->
     Path = conf_path(Req),
@@ -195,7 +192,9 @@ conf_path_reset(Req) ->
     string:lexemes(Path, "/ ").
 
 get_full_config() ->
-        emqx_config:fill_defaults(emqx:get_raw_config([])).
+        emqx_config:fill_defaults(
+            maps:without(?EXCLUDES,
+                emqx:get_raw_config([]))).
 
 conf_path_from_querystr(Req) ->
     case proplists:get_value(<<"conf_path">>, cowboy_req:parse_qs(Req)) of
@@ -204,7 +203,8 @@ conf_path_from_querystr(Req) ->
     end.
 
 config_list() ->
-    Roots = hocon_schema:roots(emqx_conf_schema),
+    Mod = emqx_conf:schema_module(),
+    Roots = hocon_schema:roots(Mod),
     lists:foldl(fun(Key, Acc) -> lists:keydelete(Key, 1, Acc) end, Roots, ?EXCLUDES).
 
 conf_path(Req) ->

apps/emqx_management/src/emqx_mgmt_api_trace.erl

@@ -67,7 +67,10 @@ schema("/trace") ->
             description => "Create new trace",
             'requestBody' => delete([status, log_size], fields(trace)),
             responses => #{
-                200 => hoconsc:ref(trace)
+                200 => hoconsc:ref(trace),
+                400 => emqx_dashboard_swagger:error_codes(['ALREADY_EXISTS',
+                    'DUPLICATE_CONDITION', 'INVALID_PARAMS'],
+                    <<"trace name already exists">>)
             }
         },
         delete => #{
@@ -274,7 +277,7 @@ trace(post, #{body := Param}) ->
             }};
         {error, Reason} ->
             {400, #{
-                code => 'INCORRECT_PARAMS',
+                code => 'INVALID_PARAMS',
                 message => ?TO_BIN(Reason)
             }}
     end;

apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl

@@ -0,0 +1,101 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2022 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(emqx_mgmt_api_configs_SUITE).
+
+-compile(export_all).
+-compile(nowarn_export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all() ->
+    emqx_common_test_helpers:all(?MODULE).
+
+init_per_suite(Config) ->
+    emqx_mgmt_api_test_util:init_suite(),
+    Config.
+
+end_per_suite(_) ->
+    emqx_mgmt_api_test_util:end_suite().
+
+t_get(_Config) ->
+    {ok, Configs} = get_configs(),
+    maps:map(fun(Name, Value) ->
+        {ok, Config} = get_config(Name),
+        ?assertEqual(Value, Config)
+    end, maps:remove(<<"license">>, Configs)),
+    ok.
+
+t_update(_Config) ->
+    %% update ok
+    {ok, SysMon} = get_config(<<"sysmon">>),
+    #{<<"vm">> := #{<<"busy_port">> := BusyPort}} = SysMon,
+    NewSysMon = emqx_map_lib:deep_put([<<"vm">>, <<"busy_port">>], SysMon, not BusyPort),
+    {ok, #{}} = update_config(<<"sysmon">>, NewSysMon),
+    {ok, SysMon1} = get_config(<<"sysmon">>),
+    #{<<"vm">> := #{<<"busy_port">> := BusyPort1}} = SysMon1,
+    ?assertEqual(BusyPort, not BusyPort1),
+
+    %% update failed
+    ErrorSysMon = emqx_map_lib:deep_put([<<"vm">>, <<"busy_port">>], SysMon, "123"),
+    ?assertMatch({error, {"HTTP/1.1", 400, _}},
+        update_config(<<"sysmon">>, ErrorSysMon)),
+    {ok, SysMon2} = get_config(<<"sysmon">>),
+    ?assertEqual(SysMon1, SysMon2),
+
+    %% reset specific config
+    ok = reset_config(<<"sysmon">>, "conf_path=vm.busy_port"),
+    {ok, SysMon3} = get_config(<<"sysmon">>),
+    ?assertMatch(#{<<"vm">> := #{<<"busy_port">> := true}}, SysMon3),
+
+    %% reset no_default_value config
+    NewSysMon1 = emqx_map_lib:deep_put([<<"vm">>, <<"busy_port">>], SysMon, false),
+    {ok, #{}} = update_config(<<"sysmon">>, NewSysMon1),
+    ?assertMatch({error, {"HTTP/1.1", 400, _}}, reset_config(<<"sysmon">>, "")),
+    {ok, SysMon4} = get_config(<<"sysmon">>),
+    ?assertMatch(#{<<"vm">> := #{<<"busy_port">> := false}}, SysMon4),
+    ok.
+
+get_config(Name) ->
+    Path = emqx_mgmt_api_test_util:api_path(["configs", Name]),
+    case emqx_mgmt_api_test_util:request_api(get, Path) of
+        {ok, Res} ->
+            {ok, emqx_json:decode(Res, [return_maps])};
+        Error -> Error
+    end.
+
+get_configs() ->
+    Path = emqx_mgmt_api_test_util:api_path(["configs"]),
+    case emqx_mgmt_api_test_util:request_api(get, Path) of
+        {ok, Res} -> {ok, emqx_json:decode(Res, [return_maps])};
+        Error -> Error
+    end.
+
+update_config(Name, Change) ->
+    AuthHeader = emqx_mgmt_api_test_util:auth_header_(),
+    UpdatePath = emqx_mgmt_api_test_util:api_path(["configs", Name]),
+    case emqx_mgmt_api_test_util:request_api(put, UpdatePath, "", AuthHeader, Change) of
+        {ok, Update} -> {ok, emqx_json:decode(Update, [return_maps])};
+        Error -> Error
+    end.
+
+reset_config(Name, Key) ->
+    AuthHeader = emqx_mgmt_api_test_util:auth_header_(),
+    Path = binary_to_list(iolist_to_binary(
+        emqx_mgmt_api_test_util:api_path(["configs_reset", Name]))),
+    case emqx_mgmt_api_test_util:request_api(post, Path, Key, AuthHeader, []) of
+        {ok, []} -> ok;
+        Error -> Error
+    end.

apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl

@@ -80,6 +80,8 @@ t_http_test(_Config) ->
     ?assertEqual(#{<<"enable">> => false,
         <<"name">> => <<"test-name">>}, json(Update)),
 
+    ?assertMatch({error, {"HTTP/1.1", 404, _}, _},
+        request_api(put, api_path("trace/test-name-not-found/stop"), Header, #{})),
     {ok, List1} = request_api(get, api_path("trace"), Header),
     [Data1] = json(List1),
     Node = atom_to_binary(node()),
@@ -115,6 +117,39 @@ t_http_test(_Config) ->
     unload(),
     ok.
 
+t_create_failed(_Config) ->
+    load(),
+    Header = auth_header_(),
+    Trace = [{<<"type">>, <<"topic">>}, {<<"topic">>, <<"/x/y/z">>}],
+
+    BadName1 = {<<"name">>, <<"test/bad">>},
+    ?assertMatch({error, {"HTTP/1.1", 400, _}, _},
+        request_api(post, api_path("trace"), Header, [BadName1 | Trace])),
+    BadName2 = {<<"name">>, list_to_binary(lists:duplicate(257, "t"))},
+    ?assertMatch({error, {"HTTP/1.1", 400, _}, _},
+        request_api(post, api_path("trace"), Header, [BadName2 | Trace])),
+
+    %% already_exist
+    GoodName = {<<"name">>, <<"test-name-0">>},
+    {ok, Create} = request_api(post, api_path("trace"), Header, [GoodName | Trace]),
+    ?assertMatch(#{<<"name">> := <<"test-name-0">>}, json(Create)),
+    ?assertMatch({error, {"HTTP/1.1", 400, _}, _},
+        request_api(post, api_path("trace"), Header, [GoodName | Trace])),
+
+    %% MAX Limited
+    lists:map(fun(Seq) ->
+        Name0 = list_to_binary("name" ++ integer_to_list(Seq)),
+        Trace0 = [{name, Name0}, {type, topic},
+            {topic, list_to_binary("/x/y/" ++ integer_to_list(Seq))}],
+        {ok, _} = emqx_trace:create(Trace0)
+              end, lists:seq(1, 30 - ets:info(emqx_trace, size))),
+    GoodName1 = {<<"name">>, <<"test-name-1">>},
+    ?assertMatch({error, {"HTTP/1.1", 400, _}, _},
+        request_api(post, api_path("trace"), Header, [GoodName1 | Trace])),
+    unload(),
+    emqx_trace:clear(),
+    ok.
+
 t_download_log(_Config) ->
     ClientId = <<"client-test-download">>,
     Now = erlang:system_time(second),
@@ -128,7 +163,8 @@ t_download_log(_Config) ->
     Header = auth_header_(),
     {ok, Binary} = request_api(get, api_path("trace/test_client_id/download"), Header),
     {ok, [_Comment,
-        #zip_file{name = ZipName, info = #file_info{size = Size, type = regular, access = read_write}}]}
+        #zip_file{name = ZipName,
+            info = #file_info{size = Size, type = regular, access = read_write}}]}
         = zip:table(Binary),
     ?assert(Size > 0),
     ZipNamePrefix = lists:flatten(io_lib:format("~s-trace_~s", [node(), Name])),