Add OTP-24 to CI #273
Add OTP-24 to CI #273
Conversation
seriyps
force-pushed
the
otp-24
branch
2 times, most recently
from
7efce4c to
a974899
Compare
.github/workflows/ci.yml
Outdated
| - name: Cover | ||
| run: make cover | ||
| if: "${{ matrix.otp_vsn != '24.0' }}" |
There was a problem hiding this comment.
I didn't manage to find a better way to use ranch20 profile for OTP-24 and test profile for al lother versions. Any ideas?
There was a problem hiding this comment.
I could probably be more explicit and add rebar profile to the matrix, so we run all the tests with both ranch versions, but then exclude some unsupported combinations (like ranch 1.7 + OTP-24 ranch 2.0 + OTP<21.3). Maybe no need to run dialyzer on all of the though...
smth like
matrix:
otp_vsn: [20.3, 21.3, 22.3, 23.3, 24.0]
profile: [test, ranch20]
exclude:
- otp_vsn: 24.0
profile: test
- otp_vsn: 20.3
profile: ranch20
...
steps:
- name: Test
run: make test REBAR_PROFILE=${{matrix.profile}}
| @@ -709,7 +710,7 @@ handle_request({<<"STARTTLS">>, <<>>}, #state{socket = Socket, module = Module, | |||
| case ranch_ssl:handshake(Socket, [{packet, line}, {mode, list}, {ssl_imp, new} | TlsOpts2], 5000) of %XXX: see smtp_socket:?SSL_LISTEN_OPTIONS | |||
| {ok, NewSocket} -> | |||
| ?log(debug, "SSL negotiation sucessful~n"), | |||
| ranch_ssl:setopts(NewSocket, [{packet, line}]), | |||
| ranch_ssl:setopts(NewSocket, [{packet, line}, binary]), | |||
There was a problem hiding this comment.
for some reason, without this change ssl socket started to send lines as strings instead of binaries on OTP-24
There was a problem hiding this comment.
That is weird, sounds like a bug in OTP.
There was a problem hiding this comment.
Yes I traced this one down here: erlang/otp#4585
There was a problem hiding this comment.
Ah, so we fixed it for SMTP client, but forgot about the server 😄
But good to know it's not some mystery but actual known OTP issue.
.github/workflows/ci.yml
Outdated
| runs-on: ${{matrix.os}} | ||
|
|
||
| strategy: | ||
| matrix: | ||
| otp_vsn: [20.3, 21.3, 22.3, 23.1] | ||
| otp_vsn: [20.3, 21.3, 22.3, 23.3, 24.0] |
There was a problem hiding this comment.
I think we can drop OTP 20, as rebar3 is only supporting the last three OTP releases.
For this reason we have also removed OTP 20 from the testing matrix in Zotonic.
There was a problem hiding this comment.
rebar3 version I'm bundling in this PR 3.15.2 works fine for all versions from 20 to 24. But maybe I'm just lucky. I was jsut thinking that #267 would have to drop OTP-20 (because logger was introduced in OTP-21), so, it can be done there?
.github/workflows/ci.yml
Outdated
| os: [ubuntu-latest] | ||
| profile: [test, ranch20] | ||
| exclude: | ||
| - otp_vsn: 24.0 # ranch 1.x does not work on OTP24+ |
There was a problem hiding this comment.
We are using ranch 1.8 with OTP 24.0.5.
I suspect any problems with 1.x and OTP 24 were resolved in 1.8
There was a problem hiding this comment.
Ah, quite possible 1.8 might work. We just don't have rebar.config profile that enforces 1.8. Do you suggest I should add ranch18 profile and add it to the matrix as well? Or it's fine as it is now?
|
After this is merged we can rebase #267 |
* add `binary` SSL setopts * use ranch 2.0 for OTP-24 tests
|
ok, so I removed OTP-20 and also added one more test profile that includes ranch 1.8 |
|
Looks good! |
Rebar3 is updated to
(downloaded it from https://github.com/erlang/rebar3/releases/tag/3.15.2)
Have to make some changes in gen_smtp_server_session.erl because on some OTP versions it started to ignore
binaryoption after TLS connection upgrade