Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Dialyzer warnings #81

Merged
merged 7 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ xref:
@rebar3 xref

dialyzer:
@rebar3 dialyzer
@rebar3 as test dialyzer

elvis:
@elvis rock
Expand Down
2 changes: 1 addition & 1 deletion include/eredis.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
-type options() :: [option()].
-type server_args() :: options(). % for backwards compatibility

-type return_value() :: undefined | binary() | [binary() | nonempty_list()].
-type return_value() :: undefined | binary() | [binary() | nonempty_list() | undefined].
bjosv marked this conversation as resolved.
Show resolved Hide resolved

-type pipeline() :: [iolist()].

Expand Down
10 changes: 4 additions & 6 deletions src/eredis.erl
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ stop(Client) ->
eredis_client:stop(Client).

-spec q(Client::client(), Command::[any()]) ->
{ok, return_value()} | {error, Reason::binary() | no_connection}.
{ok, return_value()} | {error, Reason::any() | no_connection}.
%% @doc: Executes the given command in the specified connection. The
%% command must be a valid Redis command and may contain arbitrary
%% data which will be converted to binaries. The returned values will
Expand All @@ -151,15 +151,14 @@ q(Client, Command) ->
call(Client, Command, ?TIMEOUT).

-spec q(Client::client(), Command::[any()], Timeout::timeout()) ->
{ok, return_value()} | {error, Reason::binary() | no_connection}.
{ok, return_value()} | {error, Reason::any() | no_connection}.
%% @doc Like q/2 with a custom timeout.
q(Client, Command, Timeout) ->
call(Client, Command, Timeout).


-spec qp(Client::client(), Pipeline::pipeline()) ->
[{ok, return_value()} | {error, Reason::binary()}] |
{error, no_connection}.
[{ok, return_value()} | {error, Reason::any() | no_connection}].
%% @doc: Executes the given pipeline (list of commands) in the
%% specified connection. The commands must be valid Redis commands and
%% may contain arbitrary data which will be converted to binaries. The
Expand All @@ -168,8 +167,7 @@ qp(Client, Pipeline) ->
pipeline(Client, Pipeline, ?TIMEOUT).

-spec qp(Client::client(), Pipeline::pipeline(), Timeout::timeout()) ->
[{ok, return_value()} | {error, Reason::binary()}] |
{error, no_connection}.
[{ok, return_value()} | {error, Reason::any() | no_connection}].
%% @doc Like qp/2 with a custom timeout.
qp(Client, Pipeline, Timeout) ->
pipeline(Client, Pipeline, Timeout).
Expand Down
1 change: 1 addition & 0 deletions src/eredis_parser.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ init() ->
{ok, return_value(), Rest::binary(), NewState::#pstate{}} |
{error, ErrString::binary(), NewState::#pstate{}} |
{error, ErrString::binary(), Rest::binary(), NewState::#pstate{}} |
{error, unknown_response} |
{continue, NewState::#pstate{}}.

%% @doc: Parses the (possibly partial) response from Redis. Returns
Expand Down
5 changes: 0 additions & 5 deletions test/eredis_parser_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,6 @@ bulk_nil_and_rest_test() ->
B = <<"$-1\r\n$3\r\nfoo\r\n">>,
?assertEqual({ok, undefined, <<"$3\r\nfoo\r\n">>, init()}, parse(init(), B)).

%% parse_bulk function tests
parse_bulk_test() ->
B = <<"$3\r\nbar\r\n">>,
?_assertEqual({ok, <<"bar">>, <<>>}, parse(init(), B)).

parse_bulk_too_much_data_in_continuation_test() ->
B1 = <<"$1\r\n">>,
B2 = <<"1\r\n$1\r\n2\r\n$1\r\n3\r\n">>,
Expand Down
10 changes: 5 additions & 5 deletions test/eredis_sentinel_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ t_connect_with_explicit_options(Config) when is_list(Config) ->
t_stop(Config) when is_list(Config) ->
process_flag(trap_exit, true),
C = c_sentinel(),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _, _} -> died
after 1000 -> still_alive end,
process_flag(trap_exit, false),
Expand Down Expand Up @@ -97,7 +97,7 @@ t_connection_failure_during_start_reconnect(Config) when is_list(Config) ->
IsDead = receive {'EXIT', C, _} -> died
after 400 -> still_alive end,
?assertEqual(still_alive, IsDead),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead2 = receive {'EXIT', _Pid, _Reason} -> died
after 1000 -> still_alive end,
process_flag(trap_exit, false),
Expand All @@ -115,7 +115,7 @@ t_reconnect_success_on_sentinel_process_exit(Config) when is_list(Config) ->
eredis:q(C1, ["CLIENT", "KILL", "TYPE", "NORMAL"]),
timer:sleep(100),
?assert(is_process_alive(whereis(mymaster))),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _Pid, _Reason} -> died
after 400 -> still_alive end,
process_flag(trap_exit, false),
Expand All @@ -137,7 +137,7 @@ t_reconnect_success_on_sentinel_connection_break(Config) when is_list(Config) ->
timer:sleep(100),
{links, LinkedPids3} = process_info(whereis(mymaster), links),
?assertMatch(2, length(LinkedPids3)),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _Pid, _Reason} -> died
after 400 -> still_alive end,
process_flag(trap_exit, false),
Expand All @@ -159,7 +159,7 @@ connect_eredis_sentinel(Options) ->
?assertMatch({ok, _}, Res),
{ok, C} = Res,
?assertMatch({ok, [<<"master">> | _]}, eredis:q(C, ["ROLE"])),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _Pid, _Reason} -> died
after 400 -> still_alive end,
process_flag(trap_exit, false),
Expand Down
30 changes: 15 additions & 15 deletions test/eredis_tcp_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ t_connect_local(Module, Config) when is_list(Config) ->
t_stop(Config) when is_list(Config) ->
process_flag(trap_exit, true),
C = c(),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _, _} -> died
after 1000 -> still_alive end,
process_flag(trap_exit, false),
Expand All @@ -173,15 +173,15 @@ t_get_set(Config) when is_list(Config) ->
?assertEqual({ok, undefined}, eredis:q(C, ["GET", foo])),
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", foo, bar])),
?assertEqual({ok, <<"bar">>}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_set_get_term(Config) when is_list(Config) ->
C = c(),
?assertMatch({ok, _}, eredis:q(C, ["DEL", term])),

?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", term, C])),
?assertEqual({ok, term_to_binary(C)}, eredis:q(C, ["GET", term])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_delete(Config) when is_list(Config) ->
C = c(),
Expand All @@ -190,7 +190,7 @@ t_delete(Config) when is_list(Config) ->
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", foo, bar])),
?assertEqual({ok, <<"1">>}, eredis:q(C, ["DEL", foo])),
?assertEqual({ok, undefined}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_mset_mget(Config) when is_list(Config) ->
C = c(),
Expand All @@ -204,7 +204,7 @@ t_mset_mget(Config) when is_list(Config) ->
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["MSET" | lists:flatten(KeyValuePairs)])),
?assertEqual({ok, ExpectedResult}, eredis:q(C, ["MGET" | Keys])),
?assertMatch({ok, _}, eredis:q(C, ["DEL" | Keys])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_exec(Config) when is_list(Config) ->
C = c(),
Expand All @@ -222,7 +222,7 @@ t_exec(Config) when is_list(Config) ->
?assertEqual({ok, ExpectedResult}, eredis:q(C, ["EXEC"])),

?assertMatch({ok, _}, eredis:q(C, ["DEL", "k1", "k2"])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_exec_nil(Config) when is_list(Config) ->
C1 = c(),
Expand All @@ -234,8 +234,8 @@ t_exec_nil(Config) when is_list(Config) ->
?assertEqual({ok, <<"QUEUED">>}, eredis:q(C1, ["GET", "x"])),
?assertEqual({ok, undefined}, eredis:q(C1, ["EXEC"])),
?assertMatch({ok, _}, eredis:q(C1, ["DEL", "x"])),
?assertMatch(ok, eredis:stop(C1)),
?assertMatch(ok, eredis:stop(C2)).
ok = eredis:stop(C1),
ok = eredis:stop(C2).

t_pipeline(Config) when is_list(Config) ->
C = c(),
Expand Down Expand Up @@ -264,7 +264,7 @@ t_pipeline(Config) when is_list(Config) ->
eredis:qp(C, P3, 5000)),

?assertMatch({ok, _}, eredis:q(C, ["DEL", a, b])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_pipeline_mixed(Config) when is_list(Config) ->
C = c(),
Expand All @@ -280,15 +280,15 @@ t_pipeline_mixed(Config) when is_list(Config) ->
end),
timer:sleep(10),
?assertMatch({ok, _}, eredis:q(C, ["DEL", c, d])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_q_noreply(Config) when is_list(Config) ->
C = c(),
?assertEqual(ok, eredis:q_noreply(C, ["GET", foo])),
?assertEqual(ok, eredis:q_noreply(C, ["SET", foo, bar])),
%% Even though q_noreply doesn't wait, it is sent before subsequent requests:
?assertEqual({ok, <<"bar">>}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_q_async(Config) when is_list(Config) ->
C = c(),
Expand All @@ -304,7 +304,7 @@ t_q_async(Config) when is_list(Config) ->
?assertEqual(Msg2, {ok, <<"bar">>}),
?assertMatch({ok, _}, eredis:q(C, ["DEL", foo]))
end,
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_undefined_database(Config) when is_list(Config) ->
t_undefined_database(eredis, Config).
Expand Down Expand Up @@ -411,13 +411,13 @@ t_unknown_client_call(Config) when is_list(Config) ->
C = c(),
Request = {test},
?assertEqual(unknown_request, gen_server:call(C, Request)),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_unknown_client_cast(Config) when is_list(Config) ->
C = c(),
Request = {test},
?assertEqual(ok, gen_server:cast(C, Request)),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_tcp_closed(Config) when is_list(Config) ->
{ok, C} = eredis:start_link([{reconnect_sleep, 1000}]),
Expand All @@ -426,7 +426,7 @@ t_tcp_closed(Config) when is_list(Config) ->
tcp_closed_rig(C),
timer:sleep(100), % Instant reconnect. No sleep before the first attempt.
?assertMatch({ok, _}, eredis:q(C, ["DEL", foo], 500)),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_connect_no_reconnect(Config) when is_list(Config) ->
t_connect_no_reconnect(eredis, Config).
Expand Down
5 changes: 3 additions & 2 deletions test/eredis_test_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ get_tcp_ports(Pid) ->

%% Start a server port and run Fun when a client connect.
-spec start_server(Fun :: fun((ClientSocket::inet:socket()) -> ok)) ->
{ok, inet:port_number()}.
{ok, pid(), inet:port_number()}.
start_server(Fun) ->
Pid = spawn_link(?MODULE, start_server, [self(), Fun]),
Port = receive_from(Pid, 5000),
Expand All @@ -35,7 +35,8 @@ start_server(Fun) ->
%% Stop server and close client socket as well if needed.
-spec stop_server(Pid :: pid()) -> ok.
stop_server(Pid) ->
Pid ! shutdown.
Pid ! shutdown,
ok.

%% Wait for a client to connect to server
-spec await_connect(Pid :: pid()) -> ok.
Expand Down
18 changes: 9 additions & 9 deletions test/eredis_tls_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ suite() -> [{timetrap, {minutes, 3}}].
t_tls_connect(Config) when is_list(Config) ->
C = c_tls(),
process_flag(trap_exit, true),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
IsDead = receive {'EXIT', _, _} -> died
after 1000 -> still_alive end,
process_flag(trap_exit, false),
Expand Down Expand Up @@ -77,15 +77,15 @@ t_tls_get_set(Config) when is_list(Config) ->
?assertEqual({ok, undefined}, eredis:q(C, ["GET", foo])),
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", foo, bar])),
?assertEqual({ok, <<"bar">>}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_tls_closed(Config) when is_list(Config) ->
C = c_tls(),
?assertMatch({ok, _}, eredis:q(C, ["DEL", foo], 5000)),
tls_closed_rig(C),
timer:sleep(1300), %% Wait for reconnection (1000ms)
?assertMatch({ok, _}, eredis:q(C, ["DEL", foo], 5000)),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_tls_connect_database(Config) when is_list(Config) ->
ExtraOptions = [{database, 2}],
Expand All @@ -95,14 +95,14 @@ t_tls_connect_database(Config) when is_list(Config) ->
?assertEqual({ok, undefined}, eredis:q(C, ["GET", foo])),
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", foo, bar])),
?assertEqual({ok, <<"bar">>}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

t_tls_1_2_cert_expired(Config) when is_list(Config) ->
ExtraOptions = [],
CertDir = "tls_expired_client_certs",
C = c_tls(ExtraOptions, CertDir, [{versions, ['tlsv1.2']}]),
?assertMatch({error, no_connection}, eredis:q(C, ["GET", foo])),
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).

-ifdef(OTP_RELEASE).
-if(?OTP_RELEASE >= 22).
Expand All @@ -119,7 +119,7 @@ t_tls_1_3_cert_expired(Config) when is_list(Config) ->
{error, {tls_alert, {certificate_expired, _}}} -> ok;
{error, no_connection} -> ok
end,
?assertMatch(ok, eredis:stop(C)).
ok = eredis:stop(C).
-endif.
-endif.

Expand Down Expand Up @@ -153,7 +153,7 @@ t_soon_expiring_cert(Config) when is_list(Config) ->
?assertEqual({ok, <<"OK">>}, eredis:q(C, ["SET", foo, bar3])),
?assertEqual({ok, <<"bar3">>}, eredis:q(C, ["GET", foo])),
ct:pal(user, "Stopping client"),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),

%% Reconnect, will give ok during connect+handshake
%% but trigger a ssl_error that makes the client try reconnect
Expand All @@ -165,7 +165,7 @@ t_soon_expiring_cert(Config) when is_list(Config) ->
timer:sleep(200),

?assertEqual({error, no_connection}, eredis:q(C2, ["SET", foo, bar4])),
?assertMatch(ok, eredis:stop(C2)).
ok = eredis:stop(C2).

%% Make sure a reconnect cleanup old sockets
%% i.e we have maximum 1 tcp/tls port open
Expand All @@ -177,7 +177,7 @@ t_reconnect(Config) when is_list(Config) ->
C = c_tls(ExtraOptions),
timer:sleep(1000), %% expect a couple of reconnect attempts
?assert(length(get_tcp_ports()) =< 1),
?assertMatch(ok, eredis:stop(C)),
ok = eredis:stop(C),
timer:sleep(200), %% Wait for reconnect process to terminate
?assertEqual(0, length(get_tcp_ports())).

Expand Down
Loading