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

Should we support mocking modules without behaviour? #9

Closed
brandonjoyce opened this issue Sep 29, 2017 · 17 comments
Closed

Should we support mocking modules without behaviour? #9

brandonjoyce opened this issue Sep 29, 2017 · 17 comments

Comments

@brandonjoyce
Copy link
Contributor

It's possible to verify function name and arity without behaviour callbacks. I built support for this in double, but I'm curious if there are good reasons not to support this. Does it break the "explicit contract" mantra?

Related discussion: http://teamon.eu/2017/different-approach-to-elixir-mocks-doubles/#comment-3542001464

@josevalim
Copy link
Member

Yeah, in my opinion it breaks the explicit contract mantra which comes with all the issues mentioned in the article. A mock exists to replace external dependencies in your code and how are you going to replace it if you don't even have a contract?

If we want to have this functionality, I would put it under a very explicit and long name, since it is not a pattern I think we should promote. Something like:

defmock_temporarily_without_explicit_contract MyMock, for: MyModule

@brandonjoyce
Copy link
Contributor Author

Cool. I'm ok with closing this for now. Maybe if demand for it becomes obvious, we can come back?

@josevalim
Copy link
Member

Sounds great. I am fine if we keep this open though because I am sure other will ask about it too.

@idlehands
Copy link

For what it is worth, the requirement of an explicit contract is the reason that I'm bringing this package into our team's codebase. There are plenty of other tools out there that allow us to get around it. I'm glad the goal is to keep this requiring the behaviour.

@alex88
Copy link

alex88 commented Oct 19, 2017

What would be the "correct" solution to mock external libraries that doesn't have behaviorus? Create an internal wrapper that delegates function calls to the external library and use that as behaviour?

UPDATE: Right now for example I just did a wrapper like this for ExTwitter

defmodule DniCustomizer.TwitterClient do
  @callback configure(Keyword.t) :: :ok
  @callback configure(:global | :process, Keyword.t) :: :ok
  @callback update(String.t) :: ExTwitter.Model.Tweet.t
  @callback update_with_media(String.t, Keyword.t) :: ExTwitter.Model.Tweet.t

  defdelegate configure(oauth), to: ExTwitter
  defdelegate configure(scope, oauth), to: ExTwitter
  defdelegate update(status), to: ExTwitter
  defdelegate update_with_media(status, media_content), to: ExTwitter
end

declaring only the functions I use

@josevalim
Copy link
Member

@alex88 what the library defines is irrelevant to your application. Your application needs to decide on what it depends on. If it is the same API as the TwitterClient project, then good for you. :)

JoanZapata pushed a commit to JoanZapata/mox that referenced this issue Oct 25, 2017
JoanZapata pushed a commit to JoanZapata/mox that referenced this issue Oct 25, 2017
JoanZapata pushed a commit to JoanZapata/mox that referenced this issue Nov 7, 2017
@dcaixinha
Copy link

Hey @josevalim! First of all, thank you for the great work with this library! ❤️ I've read your blog post on this subject and I've been trying to follow it when writing my applications. In the modules of our own application, we can define the behaviors and then use Mox when creating mocks for those modules. However, I don't understand the approach when using external libraries.

For instance, I use the ExStatsD library. Currently, I have a ExStatsDMock module, which I inject into the application through the test.exs file. I'd like to port these tests to start using Mox. Since its first principle is "No ad-hoc mocks", what do you think should be the approach here? Create a wrapper module for ExStatsD (similar to what @alex88 did above)?

Thank you for your input 🙌

@josevalim
Copy link
Member

Yes, you define what API your application expects in terms of stats and that is your behaviour.

@dcaixinha
Copy link

@josevalim after some months of experience using this library, I'd like to raise a new question on this issue: should Mox also accept function specs (i.e. @spec) as well? I really like the idea of having a contract between the live implementation and the mock, but in certain cases I feel that a behaviour doesn't make sense for the mocks that I want to create.

If I want to have a mock for a certain module in my app (because it does some heavy computations, for instance) and want to use Mox for it, I'd need to create a Behaviour and then make that module adopt that behaviour, or just put some @callback entries on that module. To me a Behaviour makes sense whenever there's a common contract that different implementations then abide to. What would be the downside of Mox accepting @spec entries?

Moreover, there are some modules from the standard library that provide @spec entries, which would ease the creation of mocks for those cases. For instance, the File.open/2 provides it, and it'd facilitate having mocks for the interaction with the file system!

What are your thoughts on this?

@josevalim
Copy link
Member

Specs and functions are equivalent, so the same thought process that applies to functions should also apply to specs. Mox wants you to define meaningful contracts, instead of mechanical ones. You talk about File.open/2, but if you call File.open/2, you likely need to call something else to read from the opened file. So is File.open/2 actually part of the contract you want to define? Maybe you would a contract that is more meaningful to your application instead? As said earlier, what the library (or Elixir) defines is irrelevant to your application, your application needs to decide on what it depends on.

@dcaixinha
Copy link

Yeah, callbacks and specs aren't the same, but for the purpose of ensuring that the mock and the real implementation don't drift away, they both fulfill it, right? In what sense do you call this a "mechanical contract"? I agree that a contract through a Behaviour is more explicit, and I usually apply it when dealing with 3rd parties. However, it seems overkill to do it for the standard lib of a language. How would you actually employ this? Have a module like MyApp.File which then does defdelegate to the actual library call?

What do you think of the possibility of the File module (and all modules of the standard lib) to specify @callbacks for their public functions?

@whatyouhide
Copy link
Collaborator

Adding @callbacks to all the functions in the standard library would be a huge change. But more than that, what do library authors do if we do that? Do they add callbacks for all the functions that their library exposes? This quickly turns into every function ever defined in Elixir having a callback.

I think one important thing to consider is this: do you want to mock your interaction with the file system, even if you can actually use the file system in your tests? If you do, then having a clear interface on it is a benefit since you are considering it an external part of your system and setting a boundary on it in my opinion.

Another thing to consider: if you mock the File module, there's a really high chance that you're testing the implementation of your code instead of the behaviour of your code. If you want to abstract away a part of your implementation so that it's, say, swappable, then you should probably define a contract for that abstraction, and only expose the parts of the File module that you want to use (with an interface that is more appropriate for your application).

I might have reiterated something that José already said, but maybe putting it a different way can help see it in a clearer way :).

@josevalim
Copy link
Member

josevalim commented Sep 21, 2018

@dcaixinha to reinforce @whatyouhide arguments, you don't want to depend on the mechanical contracts based on the File and Enum APIs. You most likely want to define a contract that depends on your domain. Let's take a quick example, consuming the twitter API. The API you are going to mock is not HTTPoison.get! because that's not your domain. You are going to mock something like MyApp.TwitterClient.get_tweets which internally calls HTTPoison.get! (or whatever else).

mtarnovan added a commit to steady-media/pipedrive that referenced this issue Feb 26, 2019
@dcaixinha
Copy link

dcaixinha commented Mar 20, 2019

Hey guys, sorry for resurrecting this issue after so much time, but I have some further thoughts to debate with you. First of all, thank you for your feedback 🙌 I agree with most of your points (e.g. the importance of defining explicit contracts, avoid leaking implementation details), but there are some points I'm not sure I agree.

I think one important thing to consider is this: do you want to mock your interaction with the file system, even if you can actually use the file system in your tests?

This is a highly subjective topic and indeed I could just use the file system in the tests. Elixir has made me reconsider in several ways what a pure unit test is (for instance, we might not want to create mocks for every collaborator, especially if they're just running pure functions), there are still some cases I think mocks should be used, particularly when testing for side effects. My example above was not thoroughly explained, but imagine that you're doing a File.write somewhere in the subject under test. To me, this is a side effect produced by this code and the way to test it (at the unit level) is to focus on the "messages", particularly that the right function was called with the right arguments. For a higher-level test I'd remove all the mocks and let it use the real thing (file system, DB, etc.).

You most likely want to define a contract that depends on your domain. Let's take a quick example, consuming the twitter API. The API you are going to mock is not HTTPoison.get! because that's not your domain.

I completely agree with that example, but how would you translate it to a testing using File (or any other module from the standard library)? IMHO it feels overkill to define a "contract module" for every dependency from the standard lib. What do you guys think? Maybe this is just a cultural issue, but I feel that doing that for external dependencies makes sense, while in the case of the standard lib (that ships with the language) it doesn't.

@josevalim from your example, we would create a mock for MyApp.TwitterClient.get_tweets instead of HTTPoison.get!. But then how would you test MyApp.TwitterClient.get_tweets (given it'll contain the call to HTTPoison.get!)? Or woudn't test this module at all?

@josevalim
Copy link
Member

@josevalim from your example, we would create a mock for MyApp.TwitterClient.get_tweets instead of HTTPoison.get!. But then how would you test MyApp.TwitterClient.get_tweets (given it'll contain the call to HTTPoison.get!)? Or woudn't test this module at all?

You test it directly and in isolation. Usually behind some flag such as @tag :external so you have fine grained control of when to run it.

@yordis
Copy link

yordis commented Aug 13, 2022

Hey peeps,

I am wondering how you deal with having to create a module that defines a behavior with only one callback just because of Mox?

My concern is that I end up with many tiny modules doing that all over the place because my dependency is technically a function. Still, I have some limitations around it and/or want to take advantage of Mox library rather than bringing another strategy.

So I am wondering how you are dealing with mocking in your codebase and for such sitation.

@josevalim
Copy link
Member

From your description, it may be that:

  1. You are over-mocking. The whole point of this library is to enforce explicit contracts and mostly for external dependencies of your system (the ones you do not control)

  2. If you have single functions, then you may not need a mock. Passing functions as arguments may work just fine

Other than that, If you have only one callback, then you have to define a behaviour, because it needs to be a explicit contract of your code. We discuss all of this in: https://dashbit.co/blog/mocks-and-explicit-contracts

I will close this for now, because I think those discussions are likely better moved to ElixirForum from now on. :)

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

No branches or pull requests

7 participants