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