To mock or not to mock


I am just hacking along, all TDD style of course, and the one problem I always come across again is to realize when I should mock and when not. My current idea is to have two blocks of tests, one with mocked data, the other non-mocking. Let me lead you to why I came up with that. Input and discussion welcome, I am curious about ideas and possible improvements.

The task

I have a set of methods and want to find out if they are implemented. See the code below, sorted in logical order.

  // The big summary method, that spits out the final result.
  function forMethods(methods) {
    return {
      numTotal: methods.length,
      numImplemented: getNumberOfImplementedMethods(methods)
    };
  }
  function getNumberOfImplementedMethods(methods) {
    // some simple map+reduce using `isMethodImplemented()`
  }
  function isMethodImplemented(method) {
    var details = getMethodDetails(method);
    return details.exists && !details.isDummy;
  }
  function getMethodDetails(method) {
    return {
      exists: typeof method == 'function',
      isDummy: ''+method == ''+function(){}
    };
  }

And the tests for forMethods() as the user expects it to work, without mocking (or spying).

describe('forMethods (no mocking)', function() {
  it('should return {numTotal:1, numImplemented:1}', function() {
    var actual = summary.forMethods([function() {return 1}]);
    var expected = {numTotal: 1, numImplemented:1};
    expect(actual).toEqual(expected);
  });
  it('should return {numTotal:0, numImplemented:0}', function() {
    var actual = summary.forMethods([]);
    var expected = {numTotal: 0, numImplemented:0};
    expect(actual).toEqual(expected);
  });
});

This works fine as long as I don’t change any of the underlying functions or a requirement that is implemented in some function other than the forMethods() function that I have tests for. For example, I define a dummyFunction myself var dummyFunction = function(){ console.warn('not implemented') } to allow also function(){} to be a valid method, which was not the case yet. I change the first test to

  it('should return {numTotal:1, numImplemented:1}', function() {
    // Before there was `function(){ return 1 }` in the line below.
    var actual = summary.forMethods([function(){}]);
    var expected = {numTotal: 1, numImplemented:1};
    expect(actual).toEqual(expected);
  });

and add a new test

  it('should return {numTotal:1, numImplemented:0} for a dummy function', function() {
    var actual = summary.forMethods([summary.dummyFunction]);
    var expected = {numTotal: 1, numImplemented:0};
    expect(actual).toEqual(expected);
  });

Due to the new requirement I get two failing tests.

forMethods (no mocking)
  it should return {numTotal:1, numImplemented:1}
  Error: Failed at test/summary-spec.js:40:20
  Expected
      { numTotal : 1, numImplemented : 0 }
  to equal
      { numTotal : 1, numImplemented : 1 }.
    at [object Object].<anonymous> (test/summary-spec.js:40:20)

forMethods (no mocking)
  it should return {numTotal:1, numImplemented:0} for a dummy function
  Error: Failed at test/summary-spec.js:52:20
  Expected
      { numTotal : 1, numImplemented : 1 }
  to equal
      { numTotal : 1, numImplemented : 0 }.
    at [object Object].<anonymous> (test/summary-spec.js:52:20)

But I don’t see exactly where my code fails due to this new requirement. The tests tell me forMethods() fails, but there is a chain of four functions below. So let’s write a test for getNumberOfImplementedMethods() used by forMethods().

describe('getNumberOfImplementedMethods', function() {

  it('should return 1', function() {
    var actual = summary.getNumberOfImplementedMethods([function() {}]);
    expect(actual).toBe(1);
  });

  it('should return 0', function() {
    var actual = summary.getNumberOfImplementedMethods([]);
    expect(actual).toBe(0);
  });

  it('should return 0 for dummyFunctions only', function() {
    var actual = summary.getNumberOfImplementedMethods([summary.dummyFunction, summary.dummyFunction]);
    expect(actual).toBe(0);
  });

});

The last one of those tests fails too

getNumberOfImplementedMethods
  it should return 0 for dummyFunctions only
  Error: Failed at test/summary-spec.js:81:20
  Expected
      2
  to be
      0.
    at [object Object].<anonymous> (test/summary-spec.js:81:20)

Now I actually get more and more failing tests. Ouch, that hurts and I loose the overview quickly. In my mind I am kinda “yeah I know this one fails, it will fall into place once I am getting there”. Doesn’t feel good and if I get interrupted at this point I might come back and be very unhappy about so many failing tests. Anyway, let’s go one level deeper, and write tests for isMethodImplemented(), more failing tests. Writing tests for the last function and fixing the code accordingly will solve our dilemma here:

describe('getMethodDetails', function() {

  it('should return `isDummy=false`', function() {
    var actual = summary.getMethodDetails(function(){});
    expect(actual.isDummy).toBe(false);
  });

  it('should return `isDummy=true`', function() {
    var actual = summary.getMethodDetails(summary.dummyFunction);
    expect(actual.isDummy).toBe(true);
  });

});

Both of the tests fail.

getMethodDetails
  it should return `isDummy=false`
  Error: Failed at test/summary-spec.js:61:28
  Expected
      true
  to be
      false.
    at [object Object].<anonymous> (test/summary-spec.js:61:28)

getMethodDetails (no mocking)
  it should return `isDummy=true`
  Error: Failed at test/summary-spec.js:66:28
  Expected
      false
  to be
      true.
    at [object Object].<anonymous> (test/summary-spec.js:66:28)

Finished in 0.014 seconds

Of course they fail, since I didn’t fix the actual code that compares the dummyFunction, so I fix the implementation of getMethodDetails() to compare the method to this.dummyFunction and not function(){} anymore

getMethodDetails: function(method) {
  return {
    exists: typeof method == 'function',
    isDummy: ''+method == ''+this.dummyFunction
  };
},

If I run the tests now all of them are passing. Fine.

Started
..............

Finished in 0.005 seconds
14 tests, 14 assertions, 0 failures, 14 passes

That was a little painful. I had to go all the way down the rabbit hole to find the actual problem. If I imagine this in a bigger architecture I know why I rather start writing tests early, also with the danger of them getting thrown away, because the code becomes obsolete, gets rewritten etc. That’s fine.

Let’s try another approach. I write some mock tests and see if that gets me any further quicker.

describe('forMethods (all mocked)', function() {

  beforeEach(function() {
    spyOn(summary, 'getNumberOfImplementedMethods');
  });

  it('should call `summary.getNumberOfImplementedMethods` properly', function() {
    var funcs = [function() {}];
    summary.forMethods(funcs);
    expect(summary.getNumberOfImplementedMethods).toHaveBeenCalledWith(funcs);
  });

  it('should have return value of `summary.getNumberOfImplementedMethods` in `numImplemnted`', function() {
    summary.getNumberOfImplementedMethods.andReturn('myValue');
    var methodsSummary = summary.forMethods([]);
    expect(methodsSummary.numImplemented).toBe('myValue');
  });

  it('should set `numTotal` to 0 for no methods', function() {
    var methodsSummary = summary.forMethods([]);
    expect(methodsSummary.numTotal).toBe(0);
  });

  it('should set `numTotal` to 3 for 3 methods', function() {
    // The values don't matter, since we mock all underlying functions
    // that's why we can also pass in integers, we just test the amount here.
    var methodsSummary = summary.forMethods([1,2,3]);
    expect(methodsSummary.numTotal).toBe(3);
  });

});

Notice, that those tests do also test forMethods() but none of them fails. So it seems I have no problem on that level. Now I can write simple tests for each function. And I will get down to the tests I wrote above for getMethodDetails() and at this point I will discover the actual failure. So mocking leaves me with a way better feeling since I don’t just add another failing test in the hope of getting to the actual failure at some point.
Let me discuss in the following why I think it makes sense to mix both approaches why I think that there is not only one way to do it. The mix has to be right.

Not Mocking

How I started by not mocking anything, was a logical start. I have the user’s view, and in our case the API user just wants to get the result if a set of methods is implemented. Over time four methods came along, that forMethods() used. At some point when the hierarchy becomes deeper, esp. when functions get reused in multiple parts you start writing tests for the functions on the level below. Those tests are definitely a good thing, since you gain confidence in the underlying parts.
The problem I faced a couple of times was if I change a requirement that is handled by the fourth function down the hierarchy I get failing tests for all four functions. That means you need to know where to look first, either you know your code well or you have sorted the tests to run in a certain order, so that the failing functions lowest in the hierarchy show up first and you can work top down. But even knowing how to put them in the right order requires knowledge of the entire architecture and dependencies. And that will fail when your team grows because not everyone knows how to sort the tests right. And maintaining such an order will become tedious task and is prone to errors.
But non-mocking tests are an important part of testing, they allow me to stay on the same level where the API is used and I don’t loose the real use cases out of sight, but still they might be misleading.

Mocking

I experienced that starting out with mock tests is not the way to go either. I would nail down too many dependencies too soon. Therefore I stay with unmocked tests for the most part in the beginning, but once the architecture shapes I start mocking away stuff and therefore decoupling the different levels. In the above case I can easily find errors on each of the four levels by having mock tests. Yes, a mock test is describing the implementation closer, but if the requirements change the implementation might change too and then it is ok to change the according tests too. The great thing is that I can focus on one level of implementation and I am confident that the levels below are working so by mocking I am just assuring that each of the working pieces is just wired up correctly.
The mock tests do also have the advantage of running much faster since underlying functionality is not invoked that often. Asynchronous behavior can also be avoided nicely this way.
So it’s still a learning path for me and finding the right balance turns out to be the important thing.

Conclusion

I decided to have three sets of tests, which are always grouped into their own describe() each:

describe('xxx (all mocked)', function(){
  // all tests that mock their dependencies
});
describe('xxx (no mocking)', function(){
  // all tests that may use the entire stack of underlying functions
});
describe('xxx', function(){
  // mostly the low level functions that can not be mocked anyways
});

Now I would need a way to run all the “mocked” tests only to see just those failures. If I don’t touch jasmine I guess I will have to continue identifying the failing mock tests by searching for the suffix.
The unmocked tests will fall into place later too, if that is not the case I have an edge case that I didn’t write a mock test for yet, so I will need to do that.
One question that still keeps going around in my head is: are those unmocked tests actually integration tests, or as bad as them?

I am looking forward to input. How do you do it? What can I improve? If someone learned something here I am happy. And please share your experiences too, I want to learn too.
Find the source code on github.


About Wolfram Kriesing

Wolfram Kriesing has more than fourteen years professional experience in IT. The early involvement in web technologies provides him with deep knowledge and experience for designing and implementing stable and scalable architectures.