cohttp-eio: migrate eio tests to mdx based tests#948
Conversation
d8b93c3 to
61837dd
Compare
|
@mseri The PR is ready for review now. It is mostly tests changes so please advise if the PR needs a Changelog. |
61837dd to
38b3743
Compare
|
/cc @mseri I have rebased the PR against the latest master. Ready for merging once ci are all green. |
ec78a8d to
38b3743
Compare
|
It is still not merged because I need a bit of time to actually review it, even though it is just moving tests around |
cohttp-eio/tests/client.md
Outdated
| host | ||
| "/") | ||
| |> Client.read_fixed | ||
| |> ignore);; |
There was a problem hiding this comment.
Why are you ignoring the result? This isn't testing anything except that the client read from the socket!
(this comment applies to the other tests in this file too)
There was a problem hiding this comment.
Why are you ignoring the result? This isn't testing anything except that the client read from the socket!
The tests in client.md mostly tests the Cohttp_eio.Client response headers and that we can successfully receive response without any exceptions/errors. However, yes it does seem wasteful to ignore the read_fixed results. I have modified the tests to check for valid response body where possible.
38b3743 to
ac6aab4
Compare
|
Can you rebase on master and for e push? If tests are green I will merge |
ac6aab4 to
bac72a4
Compare
|
Thanks. |
This PR updates cohttp-eio tests to use mdx based tests primarily using
eio.mockpackage. This allows tests to be repeatable especially with regards to networks and system time facilities.