Wait for a number of calls feature (#81) #99
Conversation
|
@eproxus I would really appreciate if you could review at least the interface of this feature at your earliest convenience. We are starting to use it extensively in our in-house project and it would be very frustrating if the interface changes after the old one got spread throughout the code. |
|
@horkhe Could you rebase this on the latest |
|
Sure, give me a minute... On Sun, Mar 31, 2013 at 3:07 PM, Adam Lindberg notifications@github.comwrote:
|
|
@eproxus it is rebased. |
|
I changed |
|
Have you had a chance to look at it? |
|
@eproxus I simplified the solution. Got rid of one file. Please note that at one moment it is possible to wait for only one function call, which is ok since |
|
What is happening to this, would love to see it merged in :) |
|
Basically it is waiting for the code review to be completed. |
| ok. | ||
| wait(Mod, Times, OptFunc, ArgsMatcher, OptCallerPid, Timeout) | ||
| when erlang:is_integer(Times) andalso Times > 0 andalso | ||
| erlang:is_integer(Timeout) andalso Timeout >= 0 -> |
There was a problem hiding this comment.
The guards should be moved to the interface function in meck where the first call is made. erlang:is_integer/1 can be only is_integer/1. In fact, the call to is_integer/1 can be eliminated completely by calling erlang:start_timer(round(Timeout), ...) instead.
|
Sorry guys, I have been terribly busy lately and had no time to do anything else but work. Will try to get some time during the coming weekend to address all the comments. |
- Move guards to `meck:wait`; - Allow `0` call number in `meck:wait` - Allow waiting from different processes on different or the same call patterns; - Instead of starting a timer do garbage collection of stale wait trackers.
|
@eproxus I addressed all the comments that you suggested above. Sorry for such a long response time. You can say that I am in your shoes now, that is not working with meck on regular basis as I used to do. In fact I have not written a line of Erlang code for the past couple of months, which is sad. Anyways please review. |
| Timeout::non_neg_integer()) -> | ||
| ok. | ||
| wait(Mod, Times, OptFunc, ArgsMatcher, OptCallerPid, Timeout) -> | ||
| EffectiveTimeout = case Timeout of |
There was a problem hiding this comment.
This case statement is not needed since there is a guard on meck:wait/6.
There was a problem hiding this comment.
No It is needed because it is not to guard from wrong input, but to make sure that the remote process has chance to respond in case when 0 timeout it specified but the specified number of calls already happened. In that case wait should complete successfully. Previously the caller waited for response from meck_proc but now caller has its own timeout and that is why 0 timeout requires its own case. If you do not get what I am saying that is probably because I am typing an answer having a couple of bears in me :)
|
Hmm, maybe you won't like it, but I have a proposal for a different implementation. :-) I think there is bit too much specific code in this pull request, when probably less code could be spent on a more generic solution. The idea I had was to implement a simple event system in meck (function called, etc.) and then implement the wait function as a client that sets an event filter, waits for X number of events and then returns. |
|
Honestly man, I do not see a use case for a generic messaging system in a simple mocking library as meck. If you do not mind I would rather suggest accepting existing specific solution since there are users that obviously need that functionality. You can always change the implementation later if you want. |
|
Is it possible for this to be merged? |
|
@eproxus we really need this features, could you please consider merging it? |
|
The last tag is one year old :( Please, give some ❤️ to meck 😄 |
|
Hey Folks, the thing is that after all code inspection comments were addressed the @eproxus suggested a new way to implement this feature, which in my opinion is too much for a mocking library. I neither have time no desire to do that. So if somebody can step in, please, be my guest. |
|
Everybody, if you would like to see all this or other features that you are missing in meck/master, you got to be proactive: comment on pull requests, features, bugs or create new ones. Your silence makes owners think that you are content with what meck provides to you. Which is totally ok if that is indeed the case, but if it is not... then you need to say that out loud :). |
|
I'm thinking about making a 0.8 tag (see #108) right now, and include this feature directly in the next following release. Thoughts? |
|
As far as I understand people express interest in this particular feature, so closing 0.8 without it won't help them. Please consider accepting this implementation and switching to the more generic solution later on. After all we finalized the API of this function. And therefore changing implementation can be done at any time without affecting anybody. |
|
@eproxus, looks ok if we think that the last release was 1 year ago. Maybe adding this feature is too much on a single release. |
|
That's my thinking. There is no stopping us from doing 0.8 now and 0.9 the day after. |
|
That sounds like a great idea @eproxus! |
|
Maxim Vladimirsky отправил вам приглашение Твиттер позволяет вам быть в курсе всего происходящего вокруг, а также оставаться на связи с организациями и людьми, которые вам интересны. https://twitter.com/i/9e59e9f7-9948-428e-b742-2e1aa7f1ae15 Это сообщение было отправлено Твиттером от имени его пользователей, которые пригласили вас в Твиттер, указав ваш адрес электронной почты. Нужна помощь? |
Provides a family of wait functions that allow you to block test flow until a particular number of matching function calls occurred.