Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended expect syntax and more #73

Merged
merged 9 commits into from Sep 10, 2012
Merged

Extended expect syntax and more #73

merged 9 commits into from Sep 10, 2012

Conversation

horkhe
Copy link
Contributor

@horkhe horkhe commented Jul 31, 2012

With this patch I would like to suggest the following changes:

  1. Previously sequences of return values could be defined through special sequence and loop functions. This patch introduces an alternative syntax that allows defining return specifications right in the expect function. Consider:

    meck:expect(Mod, f, 1, meck:val(a)), % equivalent to meck:expect(Mod, f, 1, a)
    ?assertEqual(a, Mod:f(1001)),
    ?assertEqual(a, Mod:f(1001)),

    meck:expect(Mod, f, 1, meck:seq([a, b])),
    ?assertEqual(a, Mod:f(1001)),
    ?assertEqual(b, Mod:f(1001)),
    ?assertEqual(b, Mod:f(1001)),

    meck:expect(Mod, loop, 1, meck:loop([a, b])),
    ?assertEqual(a, Mod:loop(1001)),
    ?assertEqual(b, Mod:loop(1001)),
    ?assertEqual(a, Mod:loop(1001)),

  2. Previously expectations can be defined either as func-name/arity or as func-name/anon-func. This patch introduces the third way which is defining an expectations through a list of arg pattern/values just like defining an ordiry function but shorter. Consider:

    meck:expect(Mod, f, [{[1, 1], a},
    {[1, ''], b},
    {['
    ', '_'], c}]),
    ?assertEqual(a, Mod:f(1, 1)),
    ?assertEqual(b, Mod:f(1, 2)),
    ?assertEqual(c, Mod:f(2, 2)).

  3. Combining return specifications and argument patterns you can create quite sophisticated expectations. Consider:

    meck:expect(Mod, f, [{[1, 1], meck:seq([a, b, c])},
    {[1, ''], meck:loop([d, e])},
    {['
    ', '_'], meck:val(f)}]),
    ?assertEqual(d, Mod:f(1, 2)),
    ?assertEqual(f, Mod:f(2, 2)),
    ?assertEqual(e, Mod:f(1, 2)),
    ?assertEqual(a, Mod:f(1, 1)),
    ?assertEqual(d, Mod:f(1, 2)),
    ?assertEqual(b, Mod:f(1, 1)),
    ?assertEqual(c, Mod:f(1, 1)),
    ?assertEqual(f, Mod:f(2, 2)),
    ?assertEqual(c, Mod:f(1, 1)),
    ?assertEqual(e, Mod:f(1, 2)),
    ?assertEqual(c, Mod:f(1, 1)).

  4. Besides internal data structure redesign made in the scope of this patch allows easier introduction of hamcrest patterns into the expectation definition.

Adam, please consider this changes and provide your comments.

@horkhe
Copy link
Contributor Author

horkhe commented Aug 2, 2012

I did rebase and squashed changes to one commit for your convenience.

@eproxus
Copy link
Owner

eproxus commented Aug 3, 2012

I like this, a lot!

{Answer, Expects}
catch
_:Error ->
throw({"Undefined expectation",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should simulate a function_clause error instead of throwing our own custom exception here. Creating an expectation that only accepts [1001] as arguments should be considered to be faking a function equivalent to f(1001) -> ....

Internal data representatin needs to be changed to allow support for
hamcrest argument matchers and argument dependent sequences and loops.

Extend expect syntax to allow Arg patterns

The syntax of expect function was extended to allow defining
expectition through a list of Arg patterns like ['_', 1, blah]
and define results as any of sequence, loop, or value.

Fix merge mistake
Exports one per line;
Corrected when clauses;
Code kept to 80 chars per line;
One liners formatted to one line.
Deprecate obsolete functions to encourage usage of the
new API, that is expect/3,4 + seq/1,loop/1,/val/1.
Match specification compilation was moved to the calling
process to be make matching more time efficient.
@horkhe
Copy link
Contributor Author

horkhe commented Aug 6, 2012

@eproxus, I removed rebar.config and .gitignore changes from the original commit and that whipped out almost all your comments. Fortunately I managed to address all of them anyway. Please take a look.

@horkhe
Copy link
Contributor Author

horkhe commented Aug 22, 2012

@eproxus have you had a chance to take a look at the changes?

@eproxus
Copy link
Owner

eproxus commented Aug 28, 2012

Sorry for the late reply. I'm planning to pull this in ASAP, I'll keep you posted.

@horkhe
Copy link
Contributor Author

horkhe commented Sep 2, 2012

@eproxus just want to let you know that I've got another pull request coming that is based on this one. It will introduce 'honest' mocks that is mocks that allow you to create expectations for exported functions only. That will for example make your unit test fail if a module, mocked in your unit test, was updated to add a new parameter to a function that you have expectations for, hereby detecting the fact that you forgot to update your module and unit test earlier.

@travisbot
Copy link

This pull request passes (merged 12c2dea into beb8d14).

%% function. Values of `ret_spec' are constructed by {@link seq/1},
%% {@link loop/1}, and {@link val/1} functions. They are used to specify
%% return values in {@link expect/3} and {@link expect/4} functions.
-type ret_spec() :: term().
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the opaque directive: -opaque my_opaq_type() :: Type. (see http://www.erlang.org/doc/reference_manual/typespec.html#id75447)

@eproxus
Copy link
Owner

eproxus commented Sep 3, 2012

Also, you've managed to make creation of expect functions faster. 👍

Before:

1> meck_performance_test:run(10000).
                Min     Max     Med     Avg
expect/3        1246    3435    1370    1409
expect/3+args   1939    7537    2253    2547
expect/4        2291    5497    2446    2474
expect/4+args   2797    6436    2950    3002

                Min     Max     Med     Avg
normal          8       682     9       10
normal_args     8       1652    9       10
shortcut        0       21      1       1
shortcut_args   0       36      1       1
shortcut_opaque 8       1545    9       11

                Min     Max     Med     Avg
called          19      135     20      21

After:

1> meck_performance_test:run(10000).
                Min     Max     Med     Avg
expect/3        1195    3732    1275    1289
expect/3+args   1825    5115    1980    2012
expect/4        2019    5271    2142    2187
expect/4+args   2385    6972    2539    2607

                Min     Max     Med     Avg
normal          9       648     10      10
normal_args     9       1591    10      11
shortcut        9       276     9       10
shortcut_args   9       304     10      10
shortcut_opaque 9       4777    10      12

                Min     Max     Med     Avg
called          19      75      20      22

Note however that the "shortcut" versions of the expects (e.g. meck:expect(M, F, 1, ok)) are slower (they were previously compiled into the mock module itself (there are a few lines of coverage missing around 689 and 710). It's not terribly important right now, and I also realized that with the ArgsPattern structure there will be even more opportunities for code generation.

@travisbot
Copy link

This pull request passes (merged 92b8ae3 into beb8d14).

@horkhe
Copy link
Contributor Author

horkhe commented Sep 3, 2012

Glad to hear about the performance though it was not intentional. I addressed the most of your comments as good as I could manage at such late hour :)

@travisbot
Copy link

This pull request passes (merged fa0eb68 into beb8d14).

@horkhe
Copy link
Contributor Author

horkhe commented Sep 4, 2012

Seems like this is it :)

@travisbot
Copy link

This pull request passes (merged de63a05a into beb8d14).

@horkhe
Copy link
Contributor Author

horkhe commented Sep 7, 2012

@eproxus I do not want to sound pushy but is it possible to speed up code inspection a little? I have got 3 more pull requests coming.

eproxus added a commit that referenced this pull request Sep 10, 2012
Extended expect syntax and more
@eproxus eproxus merged commit 4275a06 into eproxus:develop Sep 10, 2012
@eproxus
Copy link
Owner

eproxus commented Sep 10, 2012

Sorry for the incredibly long lag on my responses. I've now merged this into develop so we can let it brew for a while. Nice work! :-)

@horkhe
Copy link
Contributor Author

horkhe commented Sep 10, 2012

Thanks! ...to be continued :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants