Skip to content

Commit

Permalink
Dialyzer and eunit fixes
Browse files Browse the repository at this point in the history
redbug:start_distribution/1 was using an invalid call to net_kernel:start/1 passing a map (an invalid one, at that) instead of the required list.
net_kernel:start/1 has been deprecated since at least OTP 22, with net_kernel:start/2 (which *does* take a map) preferred.
Dialyzer rightly complained, although the code *appeared* to function somehow, despite net_kernel:start/? code explicitly raising an error on the passed configuration properties.
This redbug:start_distribution/1 change makes dialyzer happy, complies with the documented and implemented net_kernel:start/2 function, and appears to work properly on OTP 24 and 26.

The spec for redbug:start/2 was wildly inaccurate and has been corrected (mostly) - there's still room for tightening up the return types, but dialyzer is happy now.

The result pattern matching in redbug_dist_eunit:x_test_/0 has been revised to work on OTP <25 as well as 25+ through use of a macro, though it's unclear *why* the match was failing as the test code predates OTP 25.
Given the age of the OTP releases that need this macro, it's questionable whether further investigation is worth the effort.

Co-authored-by: Martin Sumner <[email protected]>
Co-authored-by: Ted Burghart <[email protected]>
  • Loading branch information
tburghart and martinsumner committed Oct 30, 2024
1 parent b07c943 commit a18a40f
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 18 deletions.
16 changes: 13 additions & 3 deletions src/redbug.erl
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,14 @@ handle_args([Trc | Rest], Config = #cnf{trc = Trcs}) ->
handle_args(Rest, Config#cnf{trc = Trcs++[trc(Trc)]}).

start_distribution(Cnf) ->
DistOptions = #{name => random_node_name(), name_domain => name_domain(Cnf), hidden => true},
{ok, _} = net_kernel:start(DistOptions),
NameDomain = name_domain(Cnf),
DistOptions = case list_to_integer(erlang:system_info(otp_release)) of
Rel when Rel >= 25 ->
#{name_domain => NameDomain, hidden => true};
_ ->
#{name_domain => NameDomain}
end,
{ok, _} = net_kernel:start(random_node_name(), DistOptions),
assert_cookie(Cnf).

random_node_name() ->
Expand Down Expand Up @@ -340,7 +346,11 @@ stop(Target) ->
start(RTPs) ->
start(RTPs, []).

-spec start(RTPs::list(), Opts::map()) -> {Procs::integer(), Functions::integer()}.
-type start_rtp() :: 'send' | 'receive' | nonempty_string().
-type start_rtps() :: start_rtp() | list(start_rtp()).
-type start_opts() :: #{atom() => term()} | list(atom() | {atom(), term()}).
-type start_result() :: atom() | tuple().
-spec start(RTPs::start_rtps(), Opts::start_opts()) -> start_result().

start('send', Props) -> start([send], Props);
start('receive', Props) -> start(['receive'], Props);
Expand Down
41 changes: 26 additions & 15 deletions test/redbug_dist_eunit.erl
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,37 @@
-include_lib("eunit/include/eunit.hrl").
-include_lib("kernel/include/file.hrl").

%% On OTP 25+ the assertions in the x_test_/0 fixture match on explicit
%% lists, whereas the result lists appear to be empty on older releases.
%% ToDo: It's not clear why the results differ, deeper investigation needed.
-if(?OTP_RELEASE >= 25).
% match the specified list pattern
-define(XT_TUPLE(Atom, List), {Atom, List}).
-else.
% match any list
-define(XT_TUPLE(Atom, List), {Atom, L} when is_list(L)).
-endif.

x_test_() ->
[?_assertMatch(
{noconnection,
[{call, {{erlang, nodes, []}, <<>>}, _, _}|_]},
?XT_TUPLE(noconnection,
[{call, {{erlang, nodes, []}, <<>>}, _, _}|_]),
runner(
mk_tracer("erlang:nodes/0", [{time, 3000}]),
mk_action(100, 100, "erlang:nodes(). "))),

?_assertMatch(
{timeout,
[{call, {{erlang, nodes, []}, <<>>}, _, _}|_]},
?XT_TUPLE(timeout,
[{call, {{erlang, nodes, []}, <<>>}, _, _}|_]),
runner(
mk_tracer("erlang:nodes/0", [{time, 300}]),
mk_action(100, 400, "erlang:nodes(). "))),

?_assertMatch(
{timeout,
[{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#{'_RECORD':=file_info}}},_,_},
{call,{{erlang,setelement,[1,{ok,#{'_RECORD':=file_info}},bla]},<<>>},_,_}]},
?XT_TUPLE(timeout,[
{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#{'_RECORD':=file_info}}},_,_},
{call,{{erlang,setelement,[1,{ok,#{'_RECORD':=file_info}},bla]},<<>>},_,_}]),
runner(
mk_tracer(
["erlang:setelement(_, {_, file#file_info{type=directory}}, _)",
Expand All @@ -36,9 +47,9 @@ x_test_() ->
mk_action(100, 400, "setelement(1, file:read_file_info(\"/\"), bla). "))),

?_assertMatch(
{timeout,
[{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#{'_RECORD':=file_info}}},_,_}]},
?XT_TUPLE(timeout, [
{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#{'_RECORD':=file_info}}},_,_}]),
runner(
mk_tracer(
["erlang:setelement(_, {_, file#file_info{type=regular}}, _)",
Expand All @@ -47,10 +58,10 @@ x_test_() ->
mk_action(100, 400, "setelement(1, file:read_file_info(\"/\"), bla). "))),

?_assertMatch(
{timeout,
[{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#file_info{}}},_,_},
{call,{{erlang,setelement,[1,{ok,#file_info{}},bla]},<<>>},_,_}]},
?XT_TUPLE(timeout, [
{call,{{file,read_file_info,["/"]},<<>>},_,_},
{retn,{{file,read_file_info,1},{ok,#file_info{}}},_,_},
{call,{{erlang,setelement,[1,{ok,#file_info{}},bla]},<<>>},_,_}]),
runner(
mk_tracer(
["erlang:setelement(_, {_, file#file_info{type=directory}}, _)",
Expand Down

0 comments on commit a18a40f

Please sign in to comment.