search

eproxus / meck

A mocking library for Erlang
http://eproxus.github.io/meck
Apache License 2.0
811 stars 231 forks source link

Ensure gen_server calls don't timeout #211

Closed fcristovao closed 4 years ago

fcristovao commented 4 years ago

The likelihood of hitting this problem is very low. However, as described here http://erlang.org/pipermail/erlang-questions/2010-August/052636.html the only real usage should be with infinity nowadays. The code was also not handling the case where it timed out.

eproxus commented 4 years ago

Hi! I'm not convinced this is the best setting for Meck. If it takes more than 5 seconds to create or set expectations, something is wrong with Meck or the test setup. I would rather have Meck timeout internally than for the test to timeout later... thoughts?

fcristovao commented 4 years ago

According to the linked email, that should really be the only option, given that errors are returned if one did happen. Another thing is in which state would one be if there was a timeout? Since no response was gotten, one wouldn't know if the expectation had been set or not? Recovering, to then know the actual state would imply more gen_server calls, which then would compound the problem. Another option would be to somehow trigger a full meck clean up in such a case? Potentially restarting supervision trees? I don't think these are better options though.

eproxus commented 4 years ago

The problems described are only relevant if the test code does a lot of defensive programming.

In the default case what will happen is that the call to set the expectation will crash (after 5000 milliseconds, let's say). Then the test process will crash (and the Meck process as well, since it is linked by default to the process creating the mock). So, in a huge test with a configured timeout of 2 hours (just to give a really bad edge-case example) it would crash after 5 seconds when Meck is not working correctly. The error message and stack trace would correctly point out which call failed and why.

If we change this to infinity, the test would crash after two hours with a timeout error and the developer would have to figure which line of code actually froze with lots of debugging.

I'm not convinced. 😄

fcristovao commented 4 years ago

I understand. Closing the PR :)

eproxus commented 4 years ago

🙇