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.

Sign up for GitHub

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

Jump to bottom

Handle Dialyzer warnings #81

Merged
merged 7 commits into from
Mar 5, 2024
Merged

Handle Dialyzer warnings #81

merged 7 commits into from
Mar 5, 2024

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Feb 28, 2024

Run dialyzer on the test profile during make dialyzer, and in CI runs.
This PR fixes all dialyzer warnings found in the rebar3 test profile.

@bjosv bjosv requested a review from zuiderkwast February 28, 2024 13:14
zuiderkwast
zuiderkwast reviewed Feb 28, 2024
View reviewed changes
test/eredis_sentinel_SUITE.erl Outdated Show resolved Hide resolved
test/eredis_test_utils.erl Outdated Show resolved Hide resolved
include/eredis.hrl Show resolved Hide resolved
@zuiderkwast
Copy link
Collaborator

zuiderkwast commented Feb 28, 2024 •
edited
Loading

A multibulk can contain a null value, like '$-1\r\n'
which is an 'undefined' in the return_value().

What about integers and other types? INCR returns an integer IIRC. In CLUSTER SLOTS we have integers inside array (multibulk).

@bjosv
Check eredis:stop(C) with pattern matching in tests
95f77a3 
Fixes Dialyzer warning:
The variable X__V can never match since previous clauses completely covered the type 'ok'
@bjosv
Copy link
Collaborator Author

bjosv commented Mar 1, 2024

What about integers and other types? INCR returns an integer IIRC. In CLUSTER SLOTS we have integers inside array (multibulk).

I believe integers should be covered by the second binary() in:
-type return_value() :: undefined | binary() | [binary() | nonempty_list() | undefined].

as in the test

multibulk_multitype_test() ->
    %% [1, 2, 3, 4, "foobar"]
    B = <<"*5\r\n:1\r\n:2\r\n:3\r\n:4\r\n$6\r\nfoobar\r\n">>,
    ?assertEqual({ok, [<<"1">>, <<"2">>, <<"3">>, <<"4">>, <<"foobar">>], #pstate{}}, parse(init(), B)).

@zuiderkwast
Copy link
Collaborator

So integers are returned as binary. Sad but true.

@bjosv bjosv changed the title Hande Dialyzer warnings Handle Dialyzer warnings Mar 1, 2024
bjosv added 6 commits March 3, 2024 12:34
@bjosv
Correcting specs in eredis_test_utils
5175a7d
@bjosv
Remove a redundant testcase in eredis_parser_tests
b7db938 
The testcase parse_bulk_test is covering the same scenario as bulk_test,
but was wrong (an underscore in ?_assertEqual)
@bjosv
Update spec for return_value()
f396c99 
A multibulk can contain a null value, like '$-1\r\n'
which is an 'undefined' in the return_value().
@bjosv
Correcting spec for eredis_parser:parse/2
d46b179 
The function will return {error, unknown_response}
when an unknown type is found while parsing.
@bjosv
Correcting spec for eredis:q and eredis:qp
fc08c4f 
A gen_tcp:send / ssl:send can return errors like
  {'error','enotconn'}
  {'error', {'tls_alert', {'certificate_expired', _}}}
which propagates to the user.
@bjosv
Run dialyzer on the test profile during make dialyzer
c9949b4 
This makes sure we run dialyzer on testcode during development and in CI.
@bjosv bjosv merged commit b683b65 into Nordix:master Mar 5, 2024
12 checks passed
@bjosv bjosv deleted the dialyzer branch March 5, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zuiderkwast zuiderkwast zuiderkwast approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bjosv @zuiderkwast