eproxus / meck

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

Wait for a number of calls feature (#81) #99

Closed horkhe closed 11 years ago

horkhe commented 11 years ago

Provides a family of wait functions that allow you to block test flow until a particular number of matching function calls occurred.

wait(Mod, OptFunc, OptArgsSpec, Timeout)
wait(Times, Mod, OptFunc, OptArgsSpec, Timeout)
wait(Times, Mod, OptFunc, OptArgsSpec, OptCallerPid, Timeout)
horkhe commented 11 years ago

@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.

chernser commented 11 years ago

@horkhe @eproxus It would be grate to have this feature in main repo - I'm waiting this for my socket_io_erl project. Would I expect it in recent future? Thanks in advance!

eproxus commented 11 years ago

@horkhe Could you rebase this on the latest develop, please?

horkhe commented 11 years ago

Sure, give me a minute...

On Sun, Mar 31, 2013 at 3:07 PM, Adam Lindberg notifications@github.comwrote:

@horkhe https://github.com/horkhe Could you rebase this on the latest develop, please?

— Reply to this email directly or view it on GitHubhttps://github.com/eproxus/meck/pull/99#issuecomment-15689944 .

horkhe commented 11 years ago

@eproxus it is rebased.

horkhe commented 11 years ago

I changed wait to raise error:timeout rather then return {error, timeout}, for that makes unit tests using it a little bit less verbose. Same thing as you suggested in the capture feature pull request.

horkhe commented 11 years ago

Have you had a chance to look at it?

horkhe commented 11 years ago

@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 wait is a blocking call anyway.

lucaspiller commented 11 years ago

What is happening to this, would love to see it merged in :)

horkhe commented 11 years ago

Basically it is waiting for the code review to be completed.

horkhe commented 11 years ago

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.

horkhe commented 11 years ago

@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.

eproxus commented 11 years ago

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.

horkhe commented 11 years ago

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.

ghost commented 11 years ago

Is it possible for this to be merged?

klastochkin commented 11 years ago

@eproxus we really need this features, could you please consider merging it?

edgurgel commented 11 years ago

The last tag is one year old :(

Please, give some :heart: to meck :smile:

horkhe commented 11 years ago

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.

horkhe commented 11 years ago

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 :).

eproxus commented 11 years ago

I'm thinking about making a 0.8 tag (see #108) right now, and include this feature directly in the next following release. Thoughts?

horkhe commented 11 years ago

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.

edgurgel commented 11 years ago

@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.

eproxus commented 11 years ago

That's my thinking. There is no stopping us from doing 0.8 now and 0.9 the day after.

ghost commented 11 years ago

That sounds like a great idea @eproxus!

horkhe commented 11 years ago

Maxim Vladimirsky отправил вам приглашение

Твиттер позволяет вам быть в курсе всего происходящего вокруг, а также оставаться на связи с организациями и людьми, которые вам интересны.

Принять приглашение

https://twitter.com/i/9e59e9f7-9948-428e-b742-2e1aa7f1ae15


Это сообщение было отправлено Твиттером от имени его пользователей, которые пригласили вас в Твиттер, указав ваш адрес электронной почты. Отменить подписку: https://twitter.com/i/o?t=1&iid=89895c5d-a569-413d-942c-12eadc4a8d9a&uid=0&c=IuVwbCB3dgE4wkJkq0uQPrq4VEygSWfi2QbJnGZD17rIhh9RJ8pzSoEUoU%2FRPg8RLhvnm54MTIS6g%2BYhfO7TpdNLjfxSJePYqXmTL1JJ8M0pNBgObdvDVw%3D%3D&nid=9+26

Нужна помощь? https://support.twitter.com