eproxus / meck

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

Make it possible to mock meck and use meck:passthrough/1 #233

Closed pergu closed 2 years ago

pergu commented 3 years ago

Before this change calling a mocked function from inside an expectation fun, and then calling meck:passthrough/1 raises an exception. This happens because the current function state is invalidated by the mocked function.

This is a follow on change to https://github.com/eproxus/meck/pull/232 solving the problem in a more general way.

eproxus commented 3 years ago

The fix in the first commit I get, but what is the point of the second commit? šŸ˜„

pergu commented 3 years ago

It's to make it possible to mock meck, if you try to do that before the fix and calling meck:passthrough/1 from an expectation fun you end up having the wrong context (meck thinks that you are in meck:passthrough, when actually you are in some other meck function). e.g. for the test case:

meck:new(meck, [passthrough]), :: all functions in meck will be implemented by meck_meck_original

meck:expect(meck, expect, fun([M, F, E]) :: meck:passthrough([M, F, E]) end), meck:expect/3 will be implemented as an explicit passthrough

meck:new(meck_test_module) :: Creates a mock for meck_test_module

meck:expect(meck_test_module, b, fun() -> ok end) :: meck:expect/3 is a mock so it gets evaled. This sets put(?CURRENT_CALL, [{meck, expect} | undefined]). During eval meck:passthrough/1 is called. This is mocked meaning it gets evaled which sets put(?CURRENT_CALL, [{meck, passthrough}, {meck, expect} | undefined]) now the important change here is that when control gets passed back to the passthrough module we set put(?CURRENT_CALL, [{meck, expect} | undefined]) which means that when meck_meck_original:passthrough/1 gets called it can correctly see that the current function is {meck, expect}, before it sets put(?CURRENT_CALL, undefined) when handing off to meck_meck_original:expect/3 and then the state is restored as the call stack unwinds.

Before this change meck_meck_original:passthrough/1 would have thought that the current call was {meck,passthrough} so it would have tried to call meck_meck_original:passthrough/3 which does not exist.

pergu commented 3 years ago

Sorry for the confusion I named this PR incorrectly.

eproxus commented 3 years ago

I mean, why would you want to do that?

pergu commented 3 years ago

Let's say that you have a testsuite which uses meck extensively, and you want to globally change how some of these mocks work, what they report and so on, then mocking meck is one way to do it.

eproxus commented 3 years ago

šŸ˜°

Iā€™m not sure I want to support that by adding more complexity, to be honest. It is more cleanly achievable with other means (happy to discuss ideas for that).

pergu commented 3 years ago

I think that's fair, that's why I made two different PRs because I think https://github.com/eproxus/meck/pull/232 is more of a genuine bug fix.

eproxus commented 3 years ago

@pergu Understand. First I thought this PR was supposed to replace #232.

pergu commented 3 years ago

šŸ˜°

Iā€™m not sure I want to support that by adding more complexity, to be honest. It is more cleanly achievable with other means (happy to discuss ideas for that).

So what are other ways to achieve this? I.e. to mock meck and use passthrough?

eproxus commented 3 years ago

Still not sure why you would want to mock Meck itself.

If you want to change how mocks behave (globally, because all Erlang modules are global), you can just use the normal expect functions etc. If you want more dynamic behavior, you can use the loop and sequence function for simpler stuff. For more advanced behavior, make a dynamic expectation fun that runs different code based on some run-time decision (talking to another process, fetching stuff from an ETS table etc.).

pergu commented 3 years ago

The answer in our case is that we want to deal with the cases when meck is not composable, for example when calling meck:unload/0, in principle we could fix this by changing all the places our testsuite where we call meck:unload/0 and change it to only unload the mocked modules, but that is non-trivial amounts of work when all we want is for meck:unload/0 to not unload some modules which we have mocked for the entire testsuite.

pergu commented 3 years ago

There are still other workarounds, so happy to close this if you don't think it makes sense.

eproxus commented 3 years ago

@pergu Not sure I follow completely, but if I understand you correctly you have a different set of mocked modules per test suite and you want to only unload those?

The two simple alternatives are:

meck:unload(UnloadList),
% or
meck:unload(meck:mocked() -- KeepList),

I would assume you have UnloadList or KeepList from your test harness (since you need one of those to implement your existing functionality).

pergu commented 3 years ago

The scenario is more like this: I have several hundred testsuites which may use meck in some way now, I want to mock something for all those testsuites. I'm not so keen on changing all of these testsuites to just unload what they use or add code to them which has to do with this specific way of running the testsuites. They are effectively a black box, but I do want to mock specific calls which are made when the testsuites run.

Den ons 15 sep. 2021 kl 10:50 skrev Adam Lindberg @.***

:

@pergu https://github.com/pergu Not sure I follow completely, but if I understand you correctly you have a different set of mocked modules per test suite and you want to only unload those?

The two simple alternatives are:

meck:unload(UnloadList),% ormeck:unload(meck:mocked() -- KeepList),

I would assume you have UnloadList or KeepList from your test harness (since you need one of those to implement your existing functionality).

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eproxus/meck/pull/233#issuecomment-919870803, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJAFNUZIKCKJ7ZU3SGJCDUCBT7HANCNFSM5CUPMDBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

eproxus commented 2 years ago

@pergu Got it. I think then that my response at this time will be to not merge the PR since I don't want the maintenance burden of extra complexity for an extremely niche issue which could be solved with refactoring (amount of work notwithstanding).

Hope you understand. šŸ™‚