How to properly unit test a class that gets file from an FTP server

Question

In fact, my question is divided into two parts.

1. How do I isolate my test from outside but still make sure that the functionality works?
2. How can I mock the apache commons FtpClient class using Mockito? When I mock it, I get null at:

InputStream inputStream = ftpClient.retrieveFileStream(ftpParameters.getSourceFileName());

Here's the test class:

public class SimpleFtpFileImporterTest {
   FtpParameters ftpParams =new FtpParameters();
   SimpleFtpFileImporter fileImporter=new SimpleFtpFileImporter();
   FTPClient ftpMock= mock(FTPClient.class);



@Before
public void startup(){
    this.ftpParams.setServer("10.0.206.126");
    this.ftpParams.setPort(21);
    this.ftpParams.setUserName("mikola");
    this.ftpParams.setPassword("password");
    this.ftpParams.setSourceFileName("readme.txt");
}


@Test
public void returnNullWhenFileCouldNotBeFetchedCompletely() throws IOException{
    when(ftpMock.completePendingCommand()).thenReturn(false);
    fileImporter=new SimpleFtpFileImporter(ftpMock);
    byte[] bytes= fileImporter.downloadFile(ftpParams);
    assertNull(bytes);
}

}

And here's the system under test:

public class SimpleFtpFileImporter implements IFileImporter {

private FTPClient ftpClient;

static Logger logger = Logger.getLogger(SimpleFtpFileImporter.class);

static {
    PropertyConfigurator.configure("config/log4j.properties");
}

/**
 * Creates a SimpleFtpFileImporter class instance passing an FtpClient.
 * This constructor helps create unit tests by passing any kind of FTPClient, eg. an http ftp client.
 *
 * @param ftpClient An FTPClient object
 */
public SimpleFtpFileImporter(FTPClient ftpClient) {
    this.ftpClient = ftpClient;
}

public SimpleFtpFileImporter() {

}
/**
 * Gets the file specified from the specified FTP server
 *
 * @param ftpParameters An FtpParametrs object that bears the needed information
 * @return File in byte array if successful, otherwise null
 */
public byte[] downloadFile(FtpParameters ftpParameters) {
    if (this.ftpClient == null)
        this.ftpClient = new FTPClient();
    if (!ftpParameters.isProperlyPopulated()) {
        logger.warn("Not all FTP parameters have been set. Execution will halt.");
        throw new FtpParametersNotSetException("Ftp parameters not properly set.");
    }
    try {
        ftpClient.connect(ftpParameters.getServer());
        ftpClient.login(ftpParameters.getUserName(), ftpParameters.getPassword());
        ftpClient.enterLocalPassiveMode();
        ftpClient.setFileType(FTP.BINARY_FILE_TYPE);
        logger.info("FTP connection succesfully established. Preparing to retrieve file:"+ftpParameters.getSourceFileName());

        InputStream inputStream = ftpClient.retrieveFileStream(ftpParameters.getSourceFileName());
        if (inputStream != null) {
            byte[] bytes = IOUtils.toByteArray(inputStream);
            boolean success = ftpClient.completePendingCommand();
            logger.info("File received");
            inputStream.close();
            if (success) {
                return bytes;
            } else{
                logger.warn("File fetching process could not be through. Returning null.");
                return null;
            }
        }else{
            logger.warn("Wrong file name specified. File name:"+ftpParameters.getSourceFileName());
            throw new RuntimeException("Wrong file name specified");
        }


    } catch (IOException ex) {
        logger.error("Problem while trying to get file from remote FTP. Message: " + ex.getMessage() + " \n\r" + ex);
    }

    return null;
}

}

It seems mocked FtpClient object is not suitable for making real thing, although I supply all the needed parameters (host, port, userName and password)

Answer 1

To answer that part that can be answered so far - you have to learn how to write testable code; a good starting point would be these videos.

There is already a misunderstanding on your side: It seems mocked FtpClient object is not suitable for making real thing.

Exactly. A mock is a mock, an empty test stub. It doesn't do anything real. That is the whole point of using mocks. They are empty shells that do nothing but provide the behavior that you specify for them. What I mean: the object created by Mockito is not a true FtpClient. It is just a mock that "looks" like an FtpClient. It doesn't have any connection to the "real" FtpClient" class. In other words: yes, you can call the methods on it that FtpClient has, but they are all empty. They do nothing (well, they do what you specify them to do their).

The point is: you use mocks to decouple you completely from an external implementation. By giving a mock to your code under test, you simply can ignore everything that the "real" class is doing.

Looking at your code and the "actual" problem:

InputStream inputStream = ftpClient.retrieveFileStream(

In order to make that work, you have to configure your mock in order to return something useful when retrieveFileStream() is called!

What I mean is: you already do:

when(ftpMock.completePendingCommand()).thenReturn(false);

to tell Mockito: when completePendingCommand() is called, then return false. You need to do that for every method that your code under test is calling!

Beyond that; many problems with your code, like:

public SimpleFtpFileImporter() {
}

Should not be empty; instead, you should be doing something like:

public SimpleFtpFileImporter() {
 this(new FtpClient());
}

Simple answer there: fields should be final by default (unless you have good reasons to not make them final):

private final FTPClient ftpClient;

and the compiler would have told you that you forgot to init that field! In your case there is absolutely no reason to keep that field not init'ed upon construction. That just makes the whole class more complicated and harder to test.