Monday, June 27, 2016

Unit Test Factory Methods: Return an Interface or a Mock?

Had a great question today regarding what to return when using factory methods with unit tests:
Should a unit test factory method return the interface or a mock of the interface?
The best way to look at this is to look at an example. Let's say that we have a class ("DataParser") that take an "ILogger" as a constructor parameter. This class would look like this:


Now in our tests, we want to test the "ParseData" method behavior. But we need an "ILogger" to satisfy the constructor parameter. In some situations we just need the "ILogger" for construction (technically, behaving as a stub), and for other situations, we need to assert against the "ILogger" (behaving as a mock).

So if we have a factory method, should it return "ILogger" or "Mock<ILogger>"? Here's what the options would look like:


Note: this code is using Moq for mocking. This mocking framework has met most of my needs over the years.

Disclaimer
Before going any further, I want to say that my "recommendation" is just my opinion -- this is the way that I prefer to do this. Roy Osherove (who wrote the excellent The Art of Unit Testing) says that we need to make a clear distinction in our code between a stub and a mock. (Also, he's not a fan of Moq as a mocking framework because it does not make that distinction.)

What I've found is that in the tests that I've written, that distinction is unneeded. So, I've chosen a direction that (hopefully) minimizes confusion and enhances readability of the code.

Picking One
One option would be to keep both of these methods and use "GetLogger" when we need a stub and "GetMockLogger" when we need a mock. I would rather minimize the number of factory methods that I have unless they are really needed, so I'm going to pick one.

Note: it's perfectly valid to keep both of these; however, I might recommend having the "GetLogger" method call the "GetMockLogger" method so that we are only creating mocks in one place.

So, let's look at both options, and then I'll give you my preference.

Option 1: Returning the Mock<ILogger>
Let's start by looking at a factory method that returns the mock object:


A test that needs a stub would look like this:


Notice that when we create the "DataParser" object on the second line, we need to use the "Object" property of our mock. This gets the actual "ILogger" object out of our mock so that we can use it as a parameter for our constructor.

I don't really like this. It requires too much knowledge of the mocking framework inside the test. If the test does not care about the mock object, I don't want it to be "polluted" with that knowledge.

Option 2: Returning ILogger
So, what if we have our factory return "ILogger"?


We still create the mock object (because we may still need the in-memory behavior of a mock), but we return the "ILogger" itself from the factory.

Then our test would look like this:


I like this much better. This test does not care about whether we have a mock or a fake (meaning, a real class that implements the interfaces and is used for testing purposes). And I think it lends to the readability: we don't have extra stuff that stops us from reading the code.

But What if We Need a Mock?
There are times that we really need a mock object. For example, in this case, we want to verify that the "ILogger.Log()" method is getting called the right number of times by our "ParseData" method. So what does that code look like?

Returning Mock<ILogger>
If we use the method that returns the mock, then our test could look like this:


In this case, we actually need the mock, because we are calling the "Verify" method on it. In Moq, the "Verify" method lets us check to see if a method was called with certain parameters or called a certain number of times (among other things). In this case, we are checking that the "Log" method is called only one time (that's the "Times.Once()" parameter).

We don't care about the parameters, which is why we have the "It.IsAny<string>()" methods in there. I won't get into the details of those right now. We'll save those for a longer article about mocking.

This code is pretty clean. So, let's look at the other option.

Returning ILogger
We can still use the method that returns "ILogger" here. That's because of one of the features of Moq. Here's what that same test looks like:


Notice that we get the "ILogger" from the factory and then pass it to the "DataParser" constructor. But since we want to use the "Verify" method, and that requires a "Mock<ILogger>" to work, we need to do a little extra work.

The last line of the "Arrange" section uses the "Mock.Get()" method. This is a static method in Moq that allows us to take an interface that has been mocked and get the original mock object back. We can see in this case, we end up with a "Mock<ILogger>", which is just what we need.

The rest of the test, including the call to "Verify" is the same as above.

The Danger
This code seems a little bit dangerous. What if "ILogger" is a fake object (instead of a mock)? Well, this code will throw an exception. "Mock.Get()" will only work with objects that were created with Moq.

This means that if we change the "GetLogger" method to return a fake instead of a mock, all of the tests that need a mock will blow up.

Am I worried about this? Not really. If I need a mock object that I'm running assertions against, then I'm probably extra aware of what's going on in the factory methods.

My Preference
So, my preference is to have a single factory method that returns an "ILogger":


Then my tests look like this:



This gives me the most readable code for the first test (where I just need a stub and don't care about asserting against the "ILogger"). This also makes writing these tests really easy. I don't have to worry about the internals of my mocking framework.

The second test is a bit more dangerous, but because I'm asserting against a mock object, I'm going to be paying closer attention to this anyway. And if I'm writing new tests that assert against the mock object, I'm going to have to know something about the mocking framework. So it makes sense to need that additional knowledge here instead of in the "simple" test.

As I noted above, this is just my preference based on my experience. I've written several different kinds of unit tests (and lots of them), and this "feels" the best to me.

Wrap Up
Your mileage may vary. It is perfectly valid to have *both* factory methods, but I prefer to keep factory methods to a minimum. And as I noted before, the distinction between stubs and mocks is more of a technical one in my world -- having the clear separation between them has not been important.
When you're new to unit testing, you'll come across lots of opinion and even more "best practices". 
Try out the ones that look interesting. If they fit into your environment, then keep them. If they don't fit, then feel free to discard them. This is part of the process of learning what works best for you. And that may be different than what works best for me.

Happy Coding!

2 comments:

  1. I can't speak for Roy but as someone who did a few projects with him I think he no longer cares about "Stub" or "Mock" - rather he prefers to use "Fake objects" (and so do I).
    Another point to make clear is that when using Moq if you return ILogger and need to set a behavior on that "Fake" object you cannot. In this case you must return Mock instead. Some like this kind of behavior some don't but it will affect how you structure your factory methods.
    Lastly IMHO there are better ways to initialize objects in your test namely autoMocking (AutoFaking?) container: http://blog.drorhelper.com/2013/12/on-object-creation-and-unit-tests.html

    Check it out and let me know what you think.

    Happy coding...

    ReplyDelete
    Replies
    1. Hi Dror, Great article -- the first sentence is what I'm constantly thinking about. I've been using Option #2 with factory methods (obviously). In the example here, I don't care about the behavior of the ILogger, but when I do, I set that behavior in the factory method rather than in the test itself (hopefully the dependencies aren't affecting the behavior of the tests too much -- but that sounds like another article).

      You're the second person to recommend AutoFixture, so it's definitely something that I need to look into (and I've generally liked the approaches that Mark Seemann uses). I'll admit that my initial reaction is a bit divided. But I'll reserve judgment until I have a chance to work with it.

      Thank you for taking the time to pass along your experience and recommendation. This is how we get better as a community.
      -Jeremy

      Delete