CrowdHailer / Ace

HTTP web server and client, supports http1 and http2
https://hex.pm/packages/ace
MIT License
305 stars 26 forks source link

Update build matrix #136

Closed nietaki closed 4 years ago

nietaki commented 5 years ago

I wanted to remove the problem where the dialyzer was failing for a reason unrelated to the project in one of the CI builds - a change similar to this one has worked in Raxx.

This one has some problems though

Erlang Elixir result
20.3 1.7.4 multiple errors around HTTP2 Client
21.3 1.7.4 :0: Unknown function 'Elixir.Enum':count/1 - type errors in dialyzer, same as with Raxx
21.3 1.8.1 tests pass, lib/ace/http2/connection.ex:37: Function init/1 has no local return dialyzer error
22.0 1.8.1 everything passes

I don't think I'm going to try looking into the test errors.

As for the dialyzer, I think most of the time it's more trouble than it's worth. I'd probably exclude it from most of the test matrix and maybe leave it for the latest version combination in general

CrowdHailer commented 5 years ago

I'm happy to test on the latest only. It would be nice to keep test one or two back but I still think it's more trouble than it's worth where dialyzer is involved. Maybe this is a particularly bad period. there was some changes around internal beam structures around 21 or 22.

nietaki commented 5 years ago

I'd just make it happen, but there's some actual test failures on 20.3/1.7.4, which removing the dialyzer wouldn't fix:

15:50:13.631 [warn]  ERROR: :protocol_error, "Unexpected frame"
  4) test send synchronously with a body in response (Ace.HTTP2.ClientTest)
     test/ace/http2/client_test.exs:65
     ** (exit) exited in: GenServer.call(#PID<0.13757.0>, {:new_stream, #PID<0.13756.0>}, 5000)
         ** (EXIT) normal
     code: {:ok, response} = Client.send_sync(client, request)
     stacktrace:
       (elixir) lib/gen_server.ex:924: GenServer.call/3
       (ace) lib/ace/http2/client.ex:161: Ace.HTTP2.Client.send_sync/2
       test/ace/http2/client_test.exs:67: (test)
nietaki commented 5 years ago

I updated the travis setup, have a look at the current results and see what you want to do with it: https://travis-ci.org/CrowdHailer/Ace/builds/580133218

CrowdHailer commented 5 years ago

Cheers, I'll take a closer look on the weekend.

CrowdHailer commented 4 years ago

Good solution for running dialyzer on only the latest. I think it's probably a sensible I'm going to see what works and what doesn't on the very latest versions of Elixir OTP. Probably the next release (0.19.0) will support only elixir 1.9 and 1.10. Last two seems sensible and probably all I have time for right now.

CrowdHailer commented 4 years ago

Merging this to master as it is useful for other PR's will have to ensure builds are green before next release