Testing method usage in another method with mockito

Question

I'm writing simple app which retrieves contacts from one text file, messages from another text file and then send message via gateway using retrieved data. I want to carry out tests for this app using Mockito. I created class ContactDetailRetriever which contains two methods: retrieveContactDetails() and mapOfContact().

public class ContactDetailRetriever implements AutoCloseable {
private LinkedHashMap<String, String> contacts = new LinkedHashMap<>();
private ArrayList<String> arrOfContacts = new ArrayList<>();
private final String FILE_NAME;

public ContactDetailRetriever() {
    this.FILE_NAME = "contacts.txt";
}

public ContactDetailRetriever(String fileName) {
    this.FILE_NAME = fileName;
}

public ArrayList<String> retrieveContactDetails() {
    try(BufferedReader reader = new BufferedReader(new FileReader(new File(FILE_NAME)))) {
        String line;
        while((line = reader.readLine()) != null) {
            arrOfContacts.add(line);
        }
     }
    catch (IOException e) {
        System.out.println(e.getMessage());
        e.printStackTrace();
    }
    return arrOfContacts;
}

public LinkedHashMap<String, String> mapOfContact() {
    //Here I want to use method
    //retrieveContactDetails();
    String key = "";
    String value = "";
    int lineNo;
    for (lineNo = 0; lineNo < arrOfContacts.size(); lineNo++) {
        if(lineNo % 2 == 0) {
            key = arrOfContacts.get(lineNo);
        }
        if(lineNo % 2 == 1) {
            value = arrOfContacts.get(lineNo);
        }
        contacts.put(key, value);
    }
    return contacts;
}

What I wanted to do is to use retrieveContactDetails() inside mapOfContact() which works just as I wanted but I have a problem with testing usage of retrieveContactDetails(). What I ended up with is refactoring the code in class MessageSender which uses indirectly mapOfContact() so retrieveContactDetails() is not used inside mapOfContact() anymore.

public class MessageSender implements SendingTool {

private LinkedHashMap<String, String> contactsMap;
private FileInjector fileInjector;

public MessageSender() {
    this.fileInjector = new FileInjector();
}

public MessageSender(FileInjector fileInjector) {
    this.fileInjector = fileInjector;
}

public LinkedHashMap<String, String> retrieveContactsMap(String fileName) {
    try(ContactDetailRetriever contactDetailRetriever = fileInjector.buildContactDetailRetriever(fileName)) {
        //Two assignments instead of one
        contactArr = contactDetailRetriever.retrieveContactDetails();
        contactsMap = contactDetailRetriever.mapOfContact();
    }

    return contactsMap;
}

It still works OK but in my opinion it's not elegant way. Also it means that I edit my code for tests which as far as I'm concerned should not be the case. What is the best way to test if retrieveContactDetails() is used inside of mapOfContact()?

One more thing which may be relevant is that ContactDetailRetriever dependencies are injected to MessageSender by another class:

public class FileInjector {
public ContactDetailRetriever buildContactDetailRetriever(String fileName) {
    return new ContactDetailRetriever(fileName);
}

public ContactDetailRetriever buildContactDetailRetriever() {
    return new ContactDetailRetriever();
}

}

Answer 1

Changing your logic to make your code more testable is no bad thing, in fact it should be encouraged. You'll find that refactoring for testability, if done correctly will regularly make your code cleaner and simpler. In your case, refactoring is what you want to be doing but I don't think the bit you changed would be where you should be looking. The first thing I'd do in your code is remove the big chunk of logic that's "reinventing the wheel", all your retrieveContactDetails() method is doing is reading lines from a file, unless you're using a version of java prior to 1.7 just use the 1 liner Files.readAllLines(Paths.get(filename)). The beauty of doing this is because now you haven't written any of your own file reading logic, testing becomes a simpler prospect, this becomes the only line you need to mock out to make the rest of your logic perfectly testable.

If you're using a version of java older than 1.7 don't bother using mockito at all... Instead have a "TestContacts.txt" static test file with known test data as you need to test the logic you've written in retrieveContactDetails() anyway...

If you're using a more civilised version of java there are a few ways of mocking out that method, personally I'm not a fan of the "PowerMockito" approach to mock out the static method, I personally would prefer to stick an interface in front of that method.

If I was looking at testing your logic, this would probably be the "first pass" I'd do to make it testable:

public class ContactDetailRetriever {
    private static final String DEFAULT_FILE_NAME = "contacts.txt";
    private final String fileName;
    private final FileLinesReader fileLinesReader;

    public ContactDetailRetriever() {
        this(DEFAULT_FILE_NAME);
    }

    public ContactDetailRetriever(String fileName) {
        this(fileName, new DefaultFileLinesReader());
    }

    // Visible for testing
    ContactDetailRetriever(String fileName, FileLinesReader fileLinesReader) {
        this.fileName = fileName;
        this.fileLinesReader = fileLinesReader;
    }

    public List<String> retrieveContactDetails() {
        return fileLinesReader.readAllLines(fileName);
    }

    public Map<String, String> mapOfContact() {
        List<String> details = retrieveContactDetails();
        Map<String, String> result = new HashMap<>();
        for (int i = 0; i < details.size() - 1; i += 2) {
            result.put(details.get(i), details.get(i + 1));
        }
        return result;
    }

    public interface FileLinesReader {
        public List<String> readAllLines(String filename);
    }

    private static class DefaultFileLinesReader implements FileLinesReader {
        public List<String> readAllLines(String filename) {
            try {
                return Files.readAllLines(Paths.get(filename));
            } catch (IOException e) {
                throw new RuntimeException(e);
            }
        }
    }
}

And then your test logic becomes:

@RunWith(MockitoJUnitRunner.class)
public class ContactDetailRetrieverTest {

    private ContactDetailRetriever testSubject;

    @Mock
    private FileLinesReader fileLinesReader;

    @Test
    public void testMapOfContact() {
        when(fileLinesReader.readAllLines("contacts.txt"))
            .thenReturn(Arrays.asList("contactKey1", "contactValue1", "contactKeyA", "contactValueA"));
        Map<String, String> result = testSubject.mapOfContact();
        assertThat(result.size(), equalTo(2));
        assertThat(result.get("contactKey1"), equalTo("contactValue1"));
        assertThat(result.get("contactKeyA"), equalTo("contactValueA"));
    }

    @Before
    public void setup() throws Exception {
        testSubject = new ContactDetailRetriever("contacts.txt", fileLinesReader);
    }
}