Writing Unit-Testable Code

I once believed we could write code for implementing features in whatever way, and should be able to write unit tests for the code as well. But that is not quite true. We do need to think about how to test our code when designing our methods. I learned the lesson when I was trying to write a unit test for a simple Java class. The original Java class was like this:

import com.dummy.MetadataCore;
…
import javax.annotation.Nonnull;
…
public class SummaryCollector implements DataCollector {
    private final Supplier<String> metadataPathSupplier;

    public SummaryCollector(@Nonnull final Supplier<String> metadataPathSupplier) {
        this.metadataPathSupplier = metadataPathSupplier;
    }

    @Override
    public void collect(final Map<Pair<Integer, String>,   
            DataSet.Builder> dataSet) {
        final String metadataDir = metadataPathSupplier.get();                    
        final MetadataCore metadataCore =
            MetadataCore.readFromDisk(metadataDir);
            // iterate over the data from the disk, process the data, and add the data into dataSet.
            …
    }
}        

Later on, an implementation issue was found. So I decided to add a unit test for the collect method in a separate file. I found that I had to use Java PowerMock to mock the class MetadataCore and its static method readFromDisk(metadataDir). That means I have to introduce another mocking framework to the codebase, which is not ideal. The test code would be something like the following:

…
import org.junit.Assert;
import org.junit.Test;
…
import org.junit.runner.RunWith;
import org.mockito.BDDMockito;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;

import org.powermock.modules.junit4.PowerMockRunner;

@RunWith(PowerMockRunner.class)
@PrepareForTest(MetadataCore.class)
public class SummaryCollectorTest {
    private final MetadataCore metadataCore = new MetadataCore(
        …
    );

    @Test
    public void test() {
        final String artifactDir = "/tmp";
        PowerMockito.mockStatic(MetadataCore.class);
        BDDMockito.given(MetadataCore.readFromDisk(artifactDir))
            .willReturn(MetadataCore);
        …
    }
}        

Alternatively, I modified the Java class by adding an argument:

public class SummaryCollector implements DataCollector {
    private final Supplier<String> metadataPathSupplier;
    private final MetadataReader metadataReader;

    public SummaryCollector(@Nonnull final Supplier<String> metadataPathSupplier,
        @Nonnull final MetadataReader metadataReader) {
            this.metadataPathSupplier = metadataPathSupplier;
            this.metadataReader = metadataReader;
        }	

    @Override
    public void collect(final Map<Pair<Integer, String>,
                DataSet.Builder> dataSet) {
    final String metadataDir = metadataPathSupplier.get();
    final MetadataCore metadataCore =
        metadataReader.readFromDisk(metadataDir);
        // iterate over the data from the disk, process the data, and add the data into dataSet.
        …
    }
}        

And I modified the code that created the instance for SummaryCollector

new SummaryCollector(() -> {...}, MetadataCore.createReader());        

And then the test code would be something like the following:

…
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
…
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
public class SummaryCollectorTest {
    @Mock
    private MetadataReader metadataReader;

    private final MetadataCore metadataCore = new MetadataCore(
        …
    );

    @Test
    public void test() {
        final String artifactDir = "/tmp";
        when(metadataReader.readFromDisk(artifactDir))
            .thenReturn(metadataCore);
        Map<Pair<Integer, String>, DataSet.Builder> dataSet = 
                new HashMap<>();
        SummaryCollector summaryCollector = new SummaryCollector(
            () -> artifactDir,
            metadataReader);
        summaryCollector.collect(dataSet);
        Assert.assertEquals(6, dataSet.size());
        …
    }
}        

This implementation has a good side effect of removing the MetadataCore dependency from SummaryCollector and using MetadataReader directly instead.

Conclusion

Think about how to write unit tests for your code when writing your code.

Thoughts?