Appropriate method encapsulation for unit testing

Question

My class contains 14 private methods and 1 public method. The public method calls all the private method directly or indirectly via other private methods.

The public method also has a call to a DAO that queries the database. I wrote a unit test for the class. Since you can't write unit test for private methods, I changed all the private methods to default access and wrote unit test for them.

I was told that I shouldn't change the encapsulation just for the purpose of testing. But my public method has a call to the DAO and gets its data from the call. Even if I were to write a test for the public method, I'm assuming it would be really long and complicated.

How should I approach this problem. On one hand, I have to write a really complicated test for the public method which accesses a DAO and on the other hand, change the access level of the methods and write short, simple test methods for them. What should I do?

Any suggestions will be greatly appreciated

Answer 1

Purists will tell you that the private methods could be extracted to another helper class providing accessible methods, and they could be right.

But if it makes sense to keep these utility methods inside the class, if the class is not part of a public API and is not intended to be subclassed (it could be final, for example), I don't see any problem with making some of its private methods package-protected or protected. Especially if this non-private visibility is documented, for example with the Guava annotation @VisibleForTesting.

Answer 2

Seems like you have two problems here:

1. How to test private methods (assuming in Java):

   I would look at this question: How do I test a class that has private methods, fields or inner classes?

   I personally like Trumpi's response:

   The best way to test a private method is via another public method. If this cannot be done, then one of the following conditions is true:

   1. The private method is dead code
   2. There is a design smell near the class that you are testing
   3. The method that you are trying to test should not be private

2. How to break the dependency of the DAO

   You could try to use Dependency Injection to get rid of your dependency on the DAO. Then you can mock out the DAO and inject it into your test case. The benefit is it truly becomes a unit test and not an integration test.

Answer 3

If it's complicated, it's probably because your class have more than one responsability. Normally, when you have private methods that do different things, is that you could have different classes with public methods that do that for you. Your class will become more easy to read, to test, and you will separate responsability. 14 private methods normally indicates this kind of thing :P

For example, you could have something like

public class LeFooService {
    private final OtherServiceForConversion barService;
    private final FooDao fooDao;

    public LeeFooService(FooDao dao, OtherServiceForConversion barService) {
        this.barService = barService;
        this.fooDao = dao;
    }

    public void createAsFoo(Bar bar) throws ConversionException {
        Foo foo = convert(bar);

        fooDao.create(foo);
    }

    private Foo convert(Bar bar) {
        // lots of conversion stuff, services calling D:
    }
}

for testing correctly, you will have to test if conversion was done correctly. Because it's private, you will have to capture the foo sent to FooDao and see if all fields were set correctly. You can use argThat to capture what's sent to fooDao to test the conversion then. Your test would look something like

....
@Test
public void shouldHaveConvertedFooCorrectly() {
     // given
     Bar bar = mock(Bar.class);

     // when
     fooService.createAsFoo(bar);

     // then
     verify(fooDao).create(argThat(fooIsConvertedCorrectly());
}

private ArgumentMatcher<Foo> fooIsConvertedCorrectly() {
     return new ArgumentMatcher<Foo>() { /*test stuff*/ };
}
....

But, if you separated the conversion to another class, like this:

public class LeFooService {
    private final BarToFooConverter bar2FooConverter;
    private final FooDao fooDao;

    public LeeFooService(FooDao dao, BarToFooConverter bar2FooConverter) {
        this.bar2FooConverter = bar2FooConverter;
        this.fooDao = dao;
    }

    public void createAsFoo(Bar bar) throws ConversionException {
        Foo foo = bar2FooConverter.convert(bar);

        fooDao.create(foo);
    }
}

you will be able to test what's really important to LeeFooService: The flow of the calls. The tests of the conversion from Foo to Bar will be the responsability of the unit tests from BarToFooConverter. An example test of LeeFooService would be

@RunWith(MockitoJUnitRunner.class)
public class LeFooServiceTest {
     @Mock
     private FooDao fooDao;
     @Mock
     private BarToFooConverter converter;
     @InjectMocks
     private LeeFooService service;

     @Test(expected = ConversionException.class)
     public void shouldForwardConversionException() {
         // given 
         given(converter.convert(Mockito.any(Bar.class))
            .willThrown(ConversionException.class);

         // when
         service.createAsFoo(mock(Bar.class));

         // then should have thrown exception
     }

     @Test
     public void shouldCreateConvertedFooAtDatabase() {
         // given 
         Foo convertedFoo = mock(Foo.class);
         given(converter.convert(Mockito.any(Bar.class))
            .willReturn(convertedFoo); 

         // when
         service.createAsFoo(mock(Bar.class));

         // then
         verify(fooDao).create(convertedFoo);
     }
}

Hope that helped somehow :)

Some links that might be useful:

SOLID

BDD Mockito

Answer 4

As a parent would tell their child: DON'T EXPOSE YOUR PRIVATES!

You don't need to expose your private methods to test them. You can get 100 PERCENT test coverage of your class, including those private methods, without exposing them.

The rub is that some people think the 'unit' in unit testing is the function, when it's really the class.

For example: I have a class with 1 public method: bool CheckIfPalindrome(string wordToCheck).

Internally, I have private methods to validate the length of the wordToCheck, if it's null, if it's empty, bla bla bla.

But as the tester, I don't need to know or care about how I the developer organized (or will organize) the internal code. I'm testing the implementation of the interface.

'Given the word is "Mike", When CheckIfPalindronme is called, it should return false'

'Given the word is "Mom", When CheckIfPalindronme is called, it should return true'

'Given the word is "", When CheckIfPalindronme is called, it should return false'

'Given the word is null, When CheckIfPalindronme is called, it should throw an error'

If I cover all of the possible inputs and expected outputs, I will be testing your private functions.

This is the basis of TDD / BDD, without this, TDD wouldn't be possible because we would have to wait and see how did you decide to organize your code before we can write our test.

TDD / BDD says write your tests before you even write your code (and it works great btw! It identifies flaws in requirements / design very quickly)

Answer 5

A class containing one public method and 14 private methods, is close to impossible to test. Without having seen it, I would be willing to bet, that it is very un-SOLID. Like JB Nizet says; me and my purist colleaagues would extract most or all private methods to helper classes. The reasons are:

• Easy to test
• Easy to refactor
• Easy to read
• Easy to reuse

The reason not to extract: * A lot!! of classes * It takes time to extract * At times, performance issues hinder the pretty-ness of "proper" design.

IMHO extraction of logic should always be considered in case of:

• Private methods
• Big classes (I usually start to think of it when the vertical scrollbar appears in my editor winow )
• Loops within loops

The keyword here is Single Responsibility (SRP).