Should I mock local method invocation inside a tested method?

Question

My question about the concept of unit testing:

class A {
   public void m1() {
      // code
      m2();
      //code
   }

   public void m2() {
      //some code
   }

}

According to best practices, how should I test the m1 method? Is the unit the class or is the unit the method?

My view - I should test m2 separately and I shouldn't test m1 and m2 integration.

To use my view is hard enough - I should use sophisticated frameworks for testing and use very modern things.

According to my sense of unit testing, tests should be simple! If your code is good, you can test it without sophisticated things. But invoking m2() inside m1() is normal code.

Please clarify my misunderstanding.

update:

mocking example(pseudocode):

//prepare
A testClass = mock(A.class);
when(testClass.m2()).doNothing();
when(testClass.m1()).callRealMethod();
//execute:
testClass.m1();
//verify
check testClass state after method m1 invocation.

This is how I test a mocked class. Is this normal?

Answer 1

First, when you unit-test, test all public methods. In your example m1 and m2 are public, so you'd have tests for both.

There are several reasons that you might want to stub or mock m2:

1. If, when you test m1, you encounter any problems because m1 calls m2, stub or mock m2. Some problems you might encounter:

   • m2 might call external services
   • m2 might just be slow
   • it might be difficult to call m1 with parameters that satisfy m2 (your m2 has no parameters, but I'm speaking generally)

2. Sometimes, when you test a method that calls another method and also test the other method, you find that there is duplication between the tests of the two methods -- some of the tests of the calling method are really testing the called method. Deal with that by stubbing or mocking the called method out of the calling method, testing the calling method just enough to prove that the called method is called, and thoroughly testing the called method.

3. If you do TDD, you might write m1 before you write m2. You would then stub or mock m2 so that your tests of m1 would pass, and then go on to test and write m2.

But if you don't have any reason to stub or mock m2, don't. It is common and reasonable for a method to call other methods in ways that don't require stubbing or mocking. The called methods might be short and simple, or the calling method might just be broken up into a bunch of subsidiary methods for readability. That's even true if the called method is in another class (because it's used by more than one calling method); if it is fully tested by tests of methods that call it, and if there is no duplication between tests, there is no need to stub or mock.

The example above of mocking m2 without running m1 is a perfectly normal thing to want to do and it gets the job done, but it's ugly since Mockito takes over all of a class's methods. You can do it more nicely with a Mockito spy, discussed here: What is the difference between mocking and spying when using Mockito?.

A a = spy(new A());
doNothing().when(spy).m2();
a.m1();
# check state of a

Answer 2

You should still test m1().

You could use mocks of method m2(),

Answer 3

The origin a the term unit test is from "unit of work". A distinct unit of work is often, but not always a single method. In practice unit test often refers to testing a single class (compare SRP) and contrasts integration and acceptance tests. The JUnit framework comes from unit testing, but nowadays is used for all kinds and layers of tests.

As a basic rule of thumb you should test all public methods. If you want, include negative tests, which feed your methods with invalid input. Usually they reveal the code's weaknesses. If two public methods overlap - as in your case - I am not aware of a hard use-all-the-time principle. You could

• A) Use Mockito or similar to mock m2() during access from m1().
• B) Test m1() and m2() the ususal way.

A) gives you a test more sensitive to m1 code but is more costly. Choose this if m1 code is important as a single unit of work. B) is the faster result and in my opinion is the better option as in case of an m1 error you will see immediately if there is an error in m2 as well. Maybe leave a small nice comment for other programmers that explains the dependence.

Answer 4

Consider for a moment that m1 was not dependent on m2.

Now consider that m1 didn't invoke m2, but rather duplicated the code of m2 within itself.

(In either of the above scenarios, you would test both m1 and m2, right?)

Now consider what would happen if you refactored m1 to remove duplicate code, by having it invoke m2. Since this is a true refactoring (i.e., it does not change the behavior of the method), your existing tests should continue to pass.

My point is that the dependence of m1 on m2 is a private implementation detail. You generally want to hide implementation details from your tests, in order to keep them from becoming brittle. Consequently, you should write your tests as if they had no idea about this dependency.

Edit: Adding some code to demonstrate.

Imagine we had written the following code and tests (apologies for any syntax errors; I don't have a compiler handy):

interface Foo {
    public void doStuff(int value);
}

interface Bar {
    public void doOtherStuff();
}

class MyClass {
    private Foo foo;
    private Bar bar;

    public MyClass(Foo foo, Bar bar) {
        this.foo = foo;
        this.bar = bar;
    }

    public void m1() {
        foo.doStuff(42);
        foo.doOtherStuff();
    }

    public void m2() {
        foo.doStuff(42);
    }
}

@Test
public void m1ShouldDoALotOfStuff() throws Exception {
    Foo foo = PowerMockito.mock(Foo.class);
    Bar bar = PowerMockito.mock(Bar.class);
    MyClass sut = new MyClass(foo, bar);

    sut.m1();

    verify(foo).doStuff(42);
    verify(bar).doOtherStuff();
}

@Test
public void m2ShouldJustDoStuff() throws Exception {
    Foo foo = PowerMockito.mock(Foo.class);
    Bar bar = PowerMockito.mock(Bar.class);
    MyClass sut = new MyClass(foo, bar);

    sut.m2();

    verify(foo).doStuff(42);
}

In the above code we have green tests for both m1 and m2. Now we notice that there is some duplicated code in m1 and m2: foo.doStuff(42);. So we refactor m1 to get rid of the duplication:

    public void m1() {
        m2();
        foo.doOtherStuff();
    }

Our tests are still green after making this change, because we haven't changed the behavior of m1; we've only changed the details of how it carries out that behavior. In order to be robust, our tests should test what we expect the SUT to do without testing how it does it.