search

Azolo / websockex

An Elixir Websocket Client
MIT License
520 stars 100 forks source link

chore: fix testing in OTP 24 #104

Closed qhwa closed 2 years ago

qhwa commented 2 years ago

Hi there!

We use it in production and It works just fine. Thanks for the library!

Lately, I wanted to add some proxy support for it but found the test has been broken on OTP 24. So here's my first PR to fix the test in OTP 24.

In order to work on OTP 24, we need to upgrade Ranch which is required by Cowboy. So I upgraded Cowboy to the latest stable version (2.9) and changed some test modules accordingly.

Please take a review and let me know what I need to append. Thank you in advance! :beers:

fixes https://github.com/Azolo/websockex/issues/97

qhwa commented 2 years ago

One test case may have been broken: https://github.com/qhwa/websockex/blob/chore/for-otp-24/test/websockex_test.exs#L1275-L1291

It doesn't pass when run alone as: mix test test/websockex_test.exs:1290

However, I'm not sure this is still the expected behavior.

Azolo commented 2 years ago

This is great.

One test case may have been broken: https://github.com/qhwa/websockex/blob/chore/for-otp-24/test/websockex_test.exs#L1275-L1291

The expected behavior is that when a WebSockex client is in the connecting state but not yet established a connection then receives a shutdown signal from a parent (like a supervisor), the result is that the terminate callback does not execute.

qhwa commented 2 years ago

The expected behavior is that when a WebSockex client is in the connecting state but not yet established a connection then receives a shutdown signal from a parent (like a supervisor), the result is that the terminate callback does not execute.

Thanks for the explanation. I'll fix the test case and push back later.

qhwa commented 2 years ago

There're some compilation warnings. I'll fix them and push again.

qhwa commented 2 years ago

Sorry I've been busy recently. Let me update the progress. The warnings have been fixed. The PR is almost done except there's one test case failing sometimes:

mix test test/websockex_test.exs:226

I'm working on it, trying to figure out what is the cause. During the debugging, I understand more about the library. So I feel very close to fixing it.

Azolo commented 2 years ago

No worries, if have any questions feel free to ask.

qhwa commented 2 years ago

No worries, if have any questions feel free to ask.

Thanks. It has been fixed and is ready for review. :smile:

Azolo commented 2 years ago

Sorry, I've been busy/sick for the last week. Give me a couple days and I'll get this looked at.

Thanks for your work and contribution!

qhwa commented 2 years ago

Hi @Azolo, I hope you're better now. Don't worry and take your time. Nothing is more important than your health and happiness. Thanks to you for sharing it.

qhwa commented 2 years ago

Sorry, I am currently with a family relocating to a new continent so was a bit busy. I will check your feedback and get back to you soon.

Azolo commented 2 years ago

AppVeyor remains broken, so I don't care enough anymore. I just removed it.

Thanks for your work! This was a huge help!