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

Add introspection of which modules are currently mocked #210

Merged
merged 1 commit into from
May 13, 2020
Merged

Add introspection of which modules are currently mocked #210

merged 1 commit into from
May 13, 2020

Conversation

fcristovao
Copy link

There's already an meck:expects/[1,2], allowing introspection into
what kind of expectations are set.
This allows one to also check which modules is meck handling at any
point.

@seriyps
Copy link

seriyps commented May 13, 2020

Feels like it would benefit from sharing part of implementation with unload_if_mocked or unload/0

@fcristovao
Copy link
Author

I thought the same, but I wanted to prevent doing 2 passes over a (sub)list.
If that would be ok, I think that unload could just call mocked() instead of registered().

@eproxus
Copy link
Owner

eproxus commented May 13, 2020

❤️

If you are up for it, I would suggest some refactoring that allows us to re-use code in all scenarios:

  1. Implement an for_each_mock/1 function that looks approximately like your existing loop:
    for_each_mock(Fun) ->
        lists:foldl(fun(M, Acc) ->
            case lists:suffix("_meck", atom_to_list(M)) of
                true ->
                    case Fun(M) of
                        {ok, Elem} -> [Elem|Acc];
                        skip       -> Acc
                    end;
                false ->
                    ok
            end
        end, [], erlang:registered()).
  2. Implement mocked/0 like so:
    mocked() -> for_each_mock(fun(M) -> {ok, M} end).
  3. Implement unload/0 like so:
    unload() ->
        for_each_mock(fun(M) ->
            try
                unload(M),
                {ok, M}
            catch
                error:{not_mocked, M} -> skip
            end
        end).

This should keep the existing behaviour and then we can delete the old unload_if_mocked function. This version also only makes one pass over the names list. Thoughts?

@fcristovao
Copy link
Author

I really like the idea!
However, It honestly feels like this is more like a "filterfoldl".
That way, unload and mocked would just be regular foldl funs, albeit only applied to modules we know are mocked.

I'll do the changes tonight and submit it :)

@seriyps
Copy link

seriyps commented May 13, 2020

Or fold_mocks(fun(MockName, Acc) -> <...>, NewAcc end, InitialAcc) - this one should be more flexible since fun is responsible for accumulator

@eproxus
Copy link
Owner

eproxus commented May 13, 2020

fold_mocks(fun F/2, InitialAcc) looks nice! 👍

There's already an `meck:expects/[1,2]`, allowing introspection into
what kind of expectations are set.
This allows one to also check which modules is meck handling at any
point.

Introduced a `foldl_mocks` internal function to reuse the same logic
for both `unload/0` and `mocked/0`
@fcristovao
Copy link
Author

I've updated the PR.
I've left the way to find out which mocks are mocked as the way I had. The given suggestion (with the lists:reverse), would still require a split somehow to find the original name. This way, in one operation we get the original name of the mocked modules.
And all used functions are available in OTP 18 onwards, as requested.

@eproxus
Copy link
Owner

eproxus commented May 13, 2020

@fcristovao Sorry, I did a later edit to lists:suffix("_meck", atom_to_list(M)) which I think is the cleanest, most compatible solution. Mind updating it?

@eproxus
Copy link
Owner

eproxus commented May 13, 2020

Never mind, missed that you need the original module name, not the mock name! 🤦

@eproxus
Copy link
Owner

eproxus commented May 13, 2020

However, I think you don't need your old expression since all modules are guaranteed to have the _meck suffix when the fun is called. You can just created the original name from the module name (might I suggest a new function in meck_util for this?).

@fcristovao
Copy link
Author

fcristovao commented May 13, 2020

@eproxus Is your suggestion that the "foldl" functions should get the name of the modules in <original>_meck form instead of <original>? I can do that, but I don't really see the benefit of that part (as both functions would then still need to get the original module name anyway)?
The foldl_mocks function still needs to filter, as its source is the registered() call instead.

@eproxus eproxus merged commit 1ed7933 into eproxus:master May 13, 2020
@eproxus
Copy link
Owner

eproxus commented May 13, 2020

Thanks, great work! I really like the test as well!

Fun :: fun((Elem :: module(), AccIn) -> AccOut),
AccIn :: term(),
AccOut :: term().
foldl_mocks(Fun, Acc0) when is_function(Fun, 2) ->
Copy link

Choose a reason for hiding this comment

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

I would say l in foldl is not needed because the order is not defined :) See, for example, maps:fold / sets:fold

Copy link
Owner

Choose a reason for hiding this comment

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

Good point. I'll remove it on master.

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

Successfully merging this pull request may close these issues.

None yet

3 participants