eproxus / meck

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

Feature/verify #68

Closed horkhe closed 11 years ago

horkhe commented 12 years ago

I would like to suggest extending the meck API with a family of verify functions. They are inspired by Mockito Java mocking framework and its verify capabilities in particular.

The idea behind verify that it checks a condition and either succeeds of raises a runtime error. It does not need to be wrapped in any ?assert(...) function, it is assertion itself. Therefore it makes tests a little bit shorter.

If the specified condition is not meant then a nice readable error message is displayed. Consider:

meck:verify({atleast, 3}, mymod, test, ['', 2, '_']).

{"Unexpected number of calls", {pattern,{mymod,test,['',2,'']}}, {expected,{at_least,3}}, {actual,2}}

As you probably noticed verify effectively covers functionality provided by called and num_calls and adds more that is not yet present (like at_least and at_most predicates)

If you approve such API extension then I will add another function to this family that will receive a list of calls specs along with times to verify that a particular call order has been adhered.

eproxus commented 12 years ago

I've been thinking about such an API for some time as well. I had as idea to integrate Meck with Hamcrest: https://github.com/hyperthunk/hamcrest-erlang

The API you've provided so far is interesting, but it's very oriented around the number of calls only (correct me if I'm wrong). I'd like a more general approach. I've put up a wiki page with some ideas.

horkhe commented 12 years ago

My primary goal was to have a unified verify method to substitute for called and num_calls. That acts similar to verify in Mockito. This commit is just the first iteration of this interface. If you accepted it I would continued to improve it to allow checking that calls were made in a particular order. consider:

verify([{{atleast, 3}, mod1, test, ['', 2, '_']}, {at_leastonce, mod1, test2, ['', '_']}, {never, mod2, test3, []}, {{atmost, 2}, mod1, test, ['', 3, 4]}]).

Latter we could allow using hamcrest expressions inside of the parameter list, just as you said.

Anyway, we are using your library in our company, and I would like to thank you for the great work you have done.

eproxus commented 12 years ago

Yeah, a unified verify function is the most optimal I think.

Makes sense to allow for Hamcrest-like expressions (maybe we don't have to depend on Hamcrest at all).

I'd like for us to have in the end:

Keeping the verify function is fine. Personally I think the different arguments can be a bit limiting and would rather see a more dynamic approach. Any chance we can refine the API to be more general?

horkhe commented 12 years ago

Could you please clarify what particular changes to verify you suggest by an example? E.g. how the following calls should look:

verify(never, mod, test1, [1, '_', 3]).

verify({atleast, 3}, mod, test2, [1, '', '_']).

eproxus commented 12 years ago

How about this?

verify(Mod, has(no(calls_to(test1, [1, '_', 3]))))
verify(Mod, has(at_least(3, calls_to(test1, [1, '', '']))))
horkhe commented 12 years ago

Hi Adam, the interface you suggest is definitely more readable but the at the same time it is more verbose. With this interface users would need to write has and calls_to in every verify. That might be fine, it all depends on what you think is more important in the interface of your library: readability or terseness.

My suggestion of the verify method was backed by a desire to make the Meck interface:

So your suggestion is not what I was looking for. As you can see I was trying to push (in a good sense :)) the interface towards Mockito. Which is the most popular mocking framework in the Java world nowadays. Since you are the farther of this wonderful library it is up to you to decide in what direction the API of the library should evolve. But I am not passionate enough to implement something that I won't be using. Though in the original pull request I would put MFA arguments of the verify function in a tuple to emphasise logical grouping of these arguments.

horkhe commented 12 years ago

I have been think about that for some time. And now it seems to me that a little bit more verbosity will pay off on account of better clarity. I will try to implement that more functional verify syntax.

eproxus commented 12 years ago

We could also design a more dynamic "nested tuple"-API which could be turned into the function call-interface like the one I proposed above. This is probably how it would work internally anyway, the functions just being shortcuts to the actual format.

horkhe commented 12 years ago

@eproxus So let's move definitions of all matching functions at_least, never, times, etc. along with syntactic sugar functions from feature/args-filter: val, seq, etc. to another module. That later module we will recommend to import then both expectation and verification definitions will be shorter in the user's unit tests.

If you support this motion then how do we name that module? meck_ss that stands for Meck Syntactic Sugar?

eproxus commented 12 years ago

Something which would make more sense, like meck_assert:at_least or meck_assert:never? Thevalandseqcould be in another module called for examplemeck_gen. That would give usmeck_gen:valandmeck_gen:seq` etc.

horkhe commented 12 years ago

@eproxus Consider this, if we have them in two different modules then users would need to have two -import directives instead of just one. Yes it is just one extra line to write but I would prefer to limit this number as possible. Do you think I am aggravating?

eproxus commented 12 years ago

My idea was to use a similar method to EUnit where you just include "meck.hrl" which will import any needed functions, maybe even selected functions from the meck itself.

horkhe commented 12 years ago

That is even better. So let's keep everything in one place (meck.erl) just to make it simple for users. The fewer module names they need to remember the better.

eproxus commented 12 years ago

Well, I'd also like to split Meck into several smaller modules since the main module is becoming a bit too long. I liked the idea of separating the verification from the actual mocking into a separate module for this reason. Possibly all functions could exist as thin wrappers in the main module still.