Is it okay to fake part of the class under test?

Question

Suppose I have a class (forgive the contrived example and the bad design of it):

class MyProfit
{
  public decimal GetNewYorkRevenue();
  public decimal GetNewYorkExpenses();
  public decimal GetNewYorkProfit();

  public decimal GetMiamiRevenue();
  public decimal GetMiamiExpenses();
  public decimal GetMiamiProfit();

  public bool BothCitiesProfitable();

}

(Note the GetxxxRevenue() and GetxxxExpenses() methods have dependencies which are stubbed out)

Now I'm unit testing BothCitiesProfitable() which depends on GetNewYorkProfit() and GetMiamiProfit(). Is it okay to stub GetNewYorkProfit() and GetMiamiProfit()?

It seems like if I don't then I'm simultaneously testing GetNewYorkProfit() and GetMiamiProfit() along with BothCitiesProfitable(). I'd have to make sure I setup the stubbing for GetxxxRevenue() and GetxxxExpenses() so that the GetxxxProfit() methods return the correct values.

So far I've only seen example of stubbing dependencies on external classes not internal methods.

And if it is okay, is there a particular pattern I should use to do this?

UPDATE

I'm concerned that we might be missing the core issue and that is probably the fault of my poor example. The fundamental question is: if a method in a class has a dependency on another publicly exposed method in that same class is it okay (or even recommended) to stub that other method out?

Maybe I am missing something, but I'm not sure splitting up the class always makes sense. Perhaps another minimally better example would be:

class Person
{
 public string FirstName()
 public string LastName()
 public string FullName()
}

where full name is defined as:

public string FullName()
{
  return FirstName() + " " + LastName();
}

Is it okay to stub FirstName() and LastName() when testing FullName()?

Answer 1

You should break up the class under question.

Each class should do some simple task. If you task is too complicated to test, then the task the class does is too big.

Ignoring the goofiness of this design:

class NewYork
{
    decimal GetRevenue();
    decimal GetExpenses();
    decimal GetProfit();
}


class Miami
{
    decimal GetRevenue();
    decimal GetExpenses();
    decimal GetProfit();
}

class MyProfit
{
     MyProfit(NewYork new_york, Miami miami);
     boolean bothProfitable();
}

UPDATE

The problem with stubbing methods in a class is that you are violating the encapsulation. Your test should be checking to see whether or not the external behaviour of the object matches the specifications. Whatever happens inside the object is none of its business.

The fact that FullName uses FirstName and LastName is an implementation detail. Nothing outside of the class should care that that is true. By mocking the public methods in order to test the object, you are making an assumption about that object is implemented.

At some point in the future, that assumption may cease to be correct. Perhaps all of the name logic will be relocated to a Name object which Person simply calls. Perhaps FullName will directly access member variables first_name and last_name rather then calling FirstName and LastName.

The second question is why you feel the need to do so. After all your person class could be tested something like:

Person person = new Person("John", "Doe");
Test.AssertEquals(person.FullName(), "John Doe");

You shouldn't feel the need stub anything for this example. If you do then you are stub-happy and well... stop it! There is no benefit to mocking the methods there because you've got control over what is in them anyways.

The only case where it would seem to make sense for the methods FullName uses to be mocked is if somehow FirstName() and LastName() were non-trivial operations. Maybe you are writing one of those random name generators, or FirstName and LastName query the database for answer, or something. But if that's what happening it suggest that object is doing something which doesn't belong in the Person class.

Putting it another way, mocking the methods is taking the object and breaking into two pieces. One piece is being mocked while the other piece is being tested. What you are doing is essentially an ad-hoc breaking up of the object. If that's the case, just break up the object already.

If your class is simple, you shouldn't feel the need to mock out pieces of it during a test. If your class is complex enough that you feel the need to mock, then you should break the class up into simpler pieces.

UPDATE AGAIN

The way I see it, an object has external and internal behavior. External behavior includes returns values calls into other objects etc. Obviously, anything in that category should be tested. (otherwise what would you test?) But internal behavior shouldn't really be tested.

Now the internal behavior is tested, because it is what results in the external behavior. But I don't write tests directly on the internal behavior, only indirectly through the external behavior.

If I want to test something, I figure it should be moved so that it becomes external behavior. That's why I think if you want to mock something, you should split the object so that the thing you want to mock is now in the external behavior of the objects in question.

But, what difference does it make? If FirstName() and LastName() are members of another object does it really change the issue of FullName()? If we decide that it is neccessary to mock FirstName and LastName does is actually help for them to be on another object?

I think if you use your mocking approach, then you create a seam in the object. You have functions like FirstName() and LastName() which directly communicate with an external data source. You also have FullName() which does not. But since they are all in the same class that is not apparent. Some pieces aren't supposed to directly access the data source and other are. Your code will be clearer if just break out those two groups.

EDIT

Let's take a step back and ask: why do we mock objects when we test?

1. Make the tests run consistently (avoid accessing things which change from run to run)
2. Avoid accessing expensive resources (don't hit third party services, etc.)
3. Simplify the system under test
4. Make it easier to test all possible scenarios (i.e. things like simulating failure, etc)
5. Avoid depending on the details of other pieces of code so that changes in those other pieces of code won't break this test.

Now, I think reasons 1-4 don't apply to this scenario. Mocking the external source when testing fullname takes care of all of those reasons for mocking. The only piece not handled that is the simplicity, but it seems the object is simple enough that's not a concern.

I think your concern is reason number 5. The concern is that at some point in future changing the implementation of FirstName and LastName will break the test. In the future FirstName and LastName may get the names from a different location or source. But FullName will probably always be FirstName() + " " + LastName(). That's why you want to test FullName by mocking FirstName and LastName.

What you have then, is some subset of the person object which is more likely to change then the others. The rest of the object uses this subset. That subset currently fetches its data using one source, but may fetch that data in a completely different way at a later date. But to me it sounds like that subset is a distinct object trying to get out.

It seems to me that if you mock the object's method you are splitting the object up. But you are doing so in a an ad-hoc manner. Your code does not make it clear that there are two distinct pieces inside your Person object. So simply split that object in your actual code, so that it is clear from reading your code what is going on. Pick the actual split of the object that makes sense and don't try to split the object up differently for each test.

I suspect you may object to splitting up your object, but why?

EDIT

I was wrong.

You should split objects rather then introducing ad-hoc splits by mocking individual methods. However, I was overly focused on the one method of splitting objects. However, OO provides multiple methods of splitting an object.

What I'd propose:

class PersonBase
{
      abstract sring FirstName();
      abstract string LastName();

      string FullName()
      {
            return FirstName() + " " + LastName();
      }
 }

 class Person extends PersonBase
 {
      string FirstName(); 
      string LastName();
 }

 class FakePerson extends PersonBase
 {
      void setFirstName(string);
      void setLastName(string);
      string getFirstName();
      string getLastName();
 }

Maybe that is what you were doing all along. But I don't think this method will have the problems I saw with mocking methods because we've clearly delineated which side each method is on. And by using inheritance, we avoid the awkwardness that would arise if we used an additional wrapper object.

This does introduce some complexity, and for only a couple of utility functions I'd probably just test them by mocking the underlying 3rd party source. Sure, they are at increased danger of breaking but its not worth rearranging. If you've got a complex enough object that you need to split it, then I think something like this is a good idea.

Answer 2

While I agree with Winston Ewert's answer, sometimes it's just not possible to break up a class, whether due to time constraints, the API already being used elsewhere, or what have you.

In a case like that, instead of mocking methods, I'd write my tests to cover the class incrementally. Test the getExpenses() and getRevenue() methods, then test the getProfit() methods, then test the shared methods. It's true that you'll have more than one test covering a particular method, but since you wrote passing tests to cover the methods individually, you can be sure that your outputs are reliable given a tested input.

Answer 3

To simplify your examples further, say you're testing C(), which depends on A() and B(), each of which have their own dependencies. IMO it comes down to what your test is trying to achieve.

If you're testing the behaviour of C() given known behaviours of A() and B() then it is probably easiest and best to stub out A() and B(). This would probably be called a unit test by purists.

If you're testing the behaviour of the entire system (from C()'s perspective), I would leave A() and B() as implemented and either stub out their dependencies (if it makes it possible to test) or set up a sandbox environment, such as a test database. I would call this an integration test.

Both approaches may be valid, so if it's designed so you can test it either way up-front it will probably be better in the long run.