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

Feature/honest mocks #75

Merged
merged 4 commits into from
Oct 11, 2012
Merged

Feature/honest mocks #75

merged 4 commits into from
Oct 11, 2012

Conversation

horkhe
Copy link
Contributor

@horkhe horkhe commented Sep 11, 2012

Meck is so powerful that it allows mocking non-existent functions and even non-existent modules. But as every coin it comes with two sides. And the bad thing here is that all sorts of problem caused by refactoring/removing/renaming actions are not detected by meck featured unit tests. Namely:

  • a mocked module was removed/renamed;
  • an expected function was removed/renamed;
  • arguments were removed/added to an expected function;

To have such cases detected at the earliest possibility honest mocks were introduced. They expose the following properties:

  • they are created with honest option;
  • an attempt to create a mock by meck:new(mod, [honest]) for a module that does not exist fails with undefined_module error;
  • an attempt to expect a function that is not exported from the honestly mocked module fails with cannot_mock_fake error.

I personally think that all mocks created by meck should be honest by default, but I did not want to break the backwards compatibility.

@eproxus
Copy link
Owner

eproxus commented Sep 15, 2012

I also like the idea of using honest by default. My guess is that 95-99 percent of the mocking done with Meck is with preexisting modules. Perhaps (if we make it default) another option could be added to make them "non-honest"? Like meck:new(mod, [unknown])?

@horkhe
Copy link
Contributor Author

horkhe commented Sep 15, 2012

@eproxus so I will make mocks "honest" by default, and introduce an option to allow mocking of non-existing functions/modules. Though I find "unknown" a little bit confusing. What about "non_strict"?

@eproxus
Copy link
Owner

eproxus commented Sep 16, 2012

non_strict is much better.

true ->
ok;
_ ->
{error, {cannot_mock_fake, {M, F, A}}}
Copy link
Owner

Choose a reason for hiding this comment

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

I find cannot_mock_fake a bit vague. Cannot come up with anything better, ideas I had were only_existing_functions_allowed, strict_mode, function_not_in_original or only_original_functions. They're all a bit long though. Perhaps undefined_function to go with undefined_module?

@horkhe
Copy link
Contributor Author

horkhe commented Sep 16, 2012

Yep, undefined_function is good.

@horkhe
Copy link
Contributor Author

horkhe commented Sep 22, 2012

@eproxus I made mocks honest by default and changed the exception for the undefined function case as we discussed.

@horkhe
Copy link
Contributor Author

horkhe commented Sep 28, 2012

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

@horkhe
Copy link
Contributor Author

horkhe commented Oct 5, 2012

@eproxus hey man, how are you? I hope you did not fall off the planet :-)

@eproxus
Copy link
Owner

eproxus commented Oct 6, 2012

@horkhe Haha, no. I just moved to Germany so I've been a bit busy. :-) I'll do a last sweep and then we'll merge.

init([Mod, Options, Exports])
catch
error:undef ->
{stop, module_undefined}
Copy link
Owner

Choose a reason for hiding this comment

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

To better match undefined_function, make this undefined_module instead.

@horkhe
Copy link
Contributor Author

horkhe commented Oct 8, 2012

@eproxus Moved to Germany, that is cool. I suppose that was caused by a generous job offer, so my congratulations!

Back to business. The code is rebased and adjusted.

@eproxus
Copy link
Owner

eproxus commented Oct 11, 2012

@horkhe You, sir, are correct. :-)

eproxus added a commit that referenced this pull request Oct 11, 2012
By default, only allow mocking of existing modules and functions
@eproxus eproxus merged commit b1ae9e9 into eproxus:develop Oct 11, 2012
@horkhe
Copy link
Contributor Author

horkhe commented Oct 12, 2012

@eproxus I am genuinely curious if you new job is Erlang related?

@eproxus
Copy link
Owner

eproxus commented Oct 12, 2012

Yes, it is. I've started working at Wooga in Berlin. If you want a referral, let me know. :-)

@horkhe
Copy link
Contributor Author

horkhe commented Oct 15, 2012

Thanks a lot for the suggestion! :-)

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