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 `meck:result/4-5' that returns the result value of a particular function #163

Merged
merged 1 commit into from
Dec 19, 2016

Conversation

amutake
Copy link
Contributor

@amutake amutake commented Jun 29, 2016

Hi! Thank you for the nice library.

I would like a function that returns the result value of a specified function, like follows.

-module(test).

-compile(export_all).

foo() -> a.

bar(a) -> b.
bar(_) -> c.
> Fun = fun() -> A = test:foo(), test:bar(A) end.
> Fun().
> meck:result(last, test, foo, 0). %=> a

I implemented meck:result(Occur, Mod, Func, OptArgsSpec) and meck:result(Occur, Mod, Func, OptArgsSpec, OptCallerPid). What do you think about this?

@eproxus
Copy link
Owner

eproxus commented Jun 30, 2016

Hi! Thanks for a very clean pull request!

What do you imagine the use case of this functionality to be? If you have had a need for this is production, would you mind detailing how you've used it? The reason I'm asking is I'd like to avoid polluting the top level Meck API space if possible. 😄

(I think the work makes a lot of sense anyway, so one possibility is to add it to the meck_history module and expose a public API from there instead.)

@amutake
Copy link
Contributor Author

amutake commented Jul 19, 2016

I'm sorry for my late and thank you for your feedback!

Usecase

1. testcase for functions related to state

One of my usecases of result is to test gen_server's handle_cast, handle_call, and handle_info (i.e., functions passed state as an argument).

Our production code has a process monitoring server like the following:

-module(meck_result_usecase_server).
-behaviour(gen_server).

%% exported apis
-export([start_link/0, monitor/1]).

%% gen_server callbacks
-export([init/1, handle_cast/2, handle_call/3, handle_info/2, terminate/2, code_change/3]).

-record(state, {monitoring_pids = maps:new() :: maps:map(pid(), reference()), ...(complex record)}).

%% -------------
%% Exported APIs
%% -------------

-spec start_link() ->
     {ok, pid()}.
start_link() ->

    gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).

-spec monitor(pid()) -> ok.
monitor(Pid) ->
    gen_server:cast(?MODULE, {monitor, Pid}).

%% --------------------
%% gen_server callbacks
%% --------------------

init([]) ->
    {ok, #state{}}.

handle_cast({monitor, Pid}, State = #state{monitoring_pids = Map}) ->
    Ref = erlang:monitor(process, Pid),
    {noreply, State#state{monitoring_pids = Map#{Pid => Ref}}};
handle_cast(_Req, State) ->
    {noreply, State}.


handle_call(_Req, _From, State) ->
    {noreply, State}.

handle_info({'DOWN', Ref, process, Pid, _Reason}, State = #state{monitoring_pids = Map}) ->
    case maps:find(Pid, Map) of
        {ok, Ref} ->
            {noreply, State#state{monitoring_pids = maps:remove(Pid, Map)}};
        _ ->
            {stop, not_monitored, State}
    end;
handle_info(_Info, State) ->
    {noreply, State}.

terminate(_Reason, _State) ->
    ok.

code_change(_OldVsn, State, _Extra) ->
    {ok, State}.

and eunit:

-module(meck_result_usecase_server_tests).

-include_lib("eunit/include/eunit.hrl").

%% eunit
meck_result_usecase_server_test_() ->
    {setup,
     fun() ->
             meck:new(meck_result_usecase_server, [unlink, passthrough]),
             {ok, Pid} = meck_result_usecase_server:start_link(),
             Pid
     end,
     fun(Pid) ->
             process_flag(trap_exit, true),
             exit(Pid, shutdown),
             meck:unload(),
             ok
     end,
     fun(_) ->
             fun() ->
                     Pid = spawn(timer, sleep, [infinity]),
                     ok = meck_result_usecase_server:monitor(Pid),
                     meck:wait(meck_result_usecase_server, handle_cast, 2, 3000),
                     exit(Pid, kill),
                     meck:wait(meck_result_usecase_server, handle_info, 2, 3000),
                     {Type, _} = meck:result(last, meck_result_usecase_server, handle_info, 2),
                     ?assertEqual(noreply, Type) %% check that `Type' is `noreply', not `stop'
             end
     end}.

I want to check that handle_info returns noreply, not stop.
One way is directly calling of handle_info, but I don't want to make state (which is the 2nd argument of handle_info, and a very complex record) by using tuple.

So I want to use a function like result.

2. for debugging

To debug test cases, I often want to confirm the result of a particular function using ?debugVal.

It is OK to achieve this purpose by meck:history/1, but using meck:history/1 with ?debugVal is not useable a bit because: (1) annoying function information not concerned, (2) ?debugVal often omit the result of meck:history/1.

toplevel or meck_history

I'm no problem if this function becomes available in toplevel or meck_history, but in my opinion, it should be a member of toplevel APIs like capture/5-6 because result/4-5 (return the result of a particular function) are duals of capture/5-6 (return the argument of a particular function) and capture/5-6 are members of toplevel APIs.

@eproxus
Copy link
Owner

eproxus commented Nov 21, 2016

Sorry for the long silence 🙇

If you move the functions into the history module, I'd be happy accepting the patch. For now, I don't want to add more functions to the main Meck API.

@amutake
Copy link
Contributor Author

amutake commented Dec 6, 2016

OK!

I have removed the functions from meck module instead of moving the functions into meck_history because meck:result/4-5 is almost same as meck_history:result/5.

Is it OK?

@eproxus
Copy link
Owner

eproxus commented Dec 19, 2016

Yes, that looks good. One final request: Can you squash everything into one commit detailing only the change to meck_history? Looks cleaner that way...

I can also do it while merging, but I'm not sure you'll retain your authorship that way 🙂

`meck_history:result/5' returns the result value of a particular function
@amutake
Copy link
Contributor Author

amutake commented Dec 19, 2016

I've squashed!

@eproxus eproxus merged commit e5bdc72 into eproxus:master Dec 19, 2016
@eproxus
Copy link
Owner

eproxus commented Dec 19, 2016

Great work, thanks again!

@amutake
Copy link
Contributor Author

amutake commented Dec 19, 2016

Thank you!

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

2 participants