Proper way to unit test MD5 Hashing of Zip Files from URL using JUnit 5?

Question

Am using Java 1.8 and JUnit 5 to test a program which I wrote which grabs external zip files from a public url and then converts them into MD5 based hashes.

This post / question serves as not only a code review (but also contains a design & implementation question) for the class that is under test and the test case, itself.

---

JUnit 5 dependencies declared in pom.xml:

<properties>
    <junit-jupiter.version>5.5.2</junit-jupiter.version>
</properties>

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-api</artifactId>
    <version>${junit-jupiter.version}</version>
    <scope>test</scope>
</dependency>

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-params</artifactId>
    <version>${junit-jupiter.version}</version>
    <scope>test</scope>
</dependency>

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-engine</artifactId>
    <version>${junit-jupiter.version}</version>
    <scope>test</scope>
</dependency>

---

FileHasher.java:

import java.io.InputStream;
import java.net.URL;
import java.security.DigestInputStream;
import java.security.MessageDigest;

public class FileHasher {

    public static String makeHashFromUrl(String fileUrl) {
        try {
            MessageDigest md = MessageDigest.getInstance("MD5");
            InputStream is = new URL(fileUrl).openStream();

            try {
                is = new DigestInputStream(is, md);

                // Up to 8K per read
                byte[] ignoredBuffer = new byte[8 * 1024];

                while (is.read(ignoredBuffer) > 0) { }
            } finally {
                is.close();
            }
            byte[] digest = md.digest();
            StringBuilder sb = new StringBuilder();

            for (int i = 0; i < digest.length; i++) {
                sb.append(Integer.toString((digest[i] & 0xff) + 0x100, 16).substring(1));
            }
            return sb.toString();

        } catch (Exception ex) {
            throw new RuntimeException(ex);
        }
    }
}

---

FileHasterTest.java (JUnit 5 test case):

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class FileHasterTest {

    private String bsrFileUrl = "https://raw.githubusercontent.com/mlampros/DataSets/master/BSR_bsds500.zip";
    private String fastTextFileUrl = "https://raw.githubusercontent.com/mlampros/DataSets/master/fastText_data.zip";

    @Test
    public void hashesAreTheSame() {
        String hashedBsrFile1 = FileHasher.makeHashFromUrl(bsrFileUrl);
        String hashedBsrFile2 = FileHasher.makeHashFromUrl(bsrFileUrl);
        assertThat(hashedBsrFile1).isEqualTo(hashedBsrFile2);
    }

    @Test
    public void hashesAreDifferent() {
        String hashedBsrFile = FileHasher.makeHashFromUrl(bsrFileUrl);
        String hashedFastTextFile = FileHasher.makeHashFromUrl(fastTextFileUrl);
        assertThat(hashedBsrFile).isNotEqualTo(hashedFastTextFile);
    }

    @Test
    public void hashIsNotNull() {
        String hashedBsrFile = FileHasher.makeHashFromUrl(bsrFileUrl);
        assertThat(hashedBsrFile).isNotNull();
    }

    @Test
    public void hashedFileThrowsRuntimeExceptionWhenUrlIsNullOrEmpty() {
        Assertions.assertThrows(RuntimeException.class, () -> {
            String hashedNull = FileHasher.makeHashFromUrl(null);
        });

        Assertions.assertThrows(RuntimeException.class, () -> {
            String hashedEmpty = FileHasher.makeHashFromUrl("");
        });
    }

}

---

Question(s):

Unit test specific:

1. Is this good test coverage (am I missing any edge cases)?

2. Is there a better way (e.g. am I missing something) to test my FileHasher class?

3. Is there a better way to test when external file URL is empty or null?

Implementation / Design specific:

4. Is throwing a RuntimeException seen as bad practice? If so, what's a better way to catch Exceptions in FileHasher?

Answer 1

Nice implementation, easy to understand and well tested. Few suggestions:

Close resources with try-with-resources

Java provides the try-with-resources statement to close resources automatically. Instead of:

try {
    is = new DigestInputStream(is, md);
    //...
} finally {
    is.close();
}

You can use:

try (DigestInputStream is = new DigestInputStream(is, md)) {
    //...
}

Exception handling

Wrapping an IOException into a RuntimeException is not good practice, but when a method throws too many different exceptions is not nice either. A trade off could be to create your own exception that wraps all other exceptions and provide the users enough information when an error occurs.

Regarding the input validation I suggest to launch an IllegalArgumentException:

public static String makeHashFromUrl(String fileUrl) {
    if(fileUrl == null) {
        throw new IllegalArgumentException("Input cannot be null");
    }
    //...
}

Unit test isolation

Two properties of the unit test:

• Anyone should be able to run it
• It should run quickly

If there is no internet connection your tests fail, which is in contrast with the first property. And if the internet connection is too slow the tests take to long to run, which invalidates the second property.

There are more than one approach to tests methods that download files:

• Spin up an HTTP server which serves your resources
• Change the method to accept a URL instead of a String and use a local file
• ...

I suggest the second approach because it's simpler but if you can't change the signature of your method you need to take other approaches. The method signature changes from:

public static String makeHashFromUrl(String fileUrl)

To:

public static String makeHashFromUrl(URL url)

And then test it with a local file:

@Test
public void myTest() {
    URL localUrl = ClassLoader.getSystemResource("my local zip file.zip");

    String hash = FileHasher.makeHashFromUrl(localUrl);

    // asserts
}

Minor changes

• Typo in class name FileHasterTest

Code refactored

public class FileHasher {
    public static String makeHashFromUrl(URL url) {
        if(url == null) {
            throw new IllegalArgumentException("Input url cannot be null");
        }
        
        MessageDigest md = createMessageDigest();

        try (DigestInputStream dis = new DigestInputStream(url.openStream(), md)) {
            // Up to 8K per read
            byte[] ignoredBuffer = new byte[8 * 1024];

            while (dis.read(ignoredBuffer) > 0) {
            }
        } catch (IOException e) {
            new RuntimeException(e);
        }
        
        return digestToString(md.digest());
    }
    
    private static MessageDigest createMessageDigest() {
        MessageDigest md = null;
        try {
            md = MessageDigest.getInstance("MD5");
        } catch (NoSuchAlgorithmException e) {
            new RuntimeException("No Providers for algorithm MD5",e);
        }
        return md;
    }
    
    private static String digestToString(byte[] digest) {
        StringBuilder sb = new StringBuilder();

        for (int i = 0; i < digest.length; i++) {
            sb.append(Integer.toString((digest[i] & 0xff) + 0x100, 16).substring(1));
        }
        return sb.toString();
    }
}

And the FileHasherTest:

public class FileHasherTest {
    private static final String testResourcesFilePath = "src/test/resources";
    private static final String bsrFileName = "BSR_bsds500.zip";
    private static final String fastTextFileName = "fastText_data.zip";

    @Test
    public void hashesAreTheSame() {
        URL bsrURL = ClassLoader.getSystemResource(bsrFileName);

        String hashedBsrFile1 = FileHasher.makeHashFromUrl(bsrURL);
        String hashedBsrFile2 = FileHasher.makeHashFromUrl(bsrURL);

        assertThat(hashedBsrFile1).isEqualTo(hashedBsrFile2);
    }

    @Test
    public void hashesAreDifferent() {
        URL bsrURL = ClassLoader.getSystemResource(bsrFileName);
        URL fastTextUrl = ClassLoader.getSystemResource(fastTextFileName);

        String hashedBsrFile = FileHasher.makeHashFromUrl(bsrURL);
        String hashedFastTextFile = FileHasher.makeHashFromUrl(fastTextUrl);

        assertThat(hashedBsrFile).isNotEqualTo(hashedFastTextFile);
    }

    @Test
    public void hashIsNotNull() {
        URL bsrURL = ClassLoader.getSystemResource(bsrFileName);

        String hashedBsrFile = FileHasher.makeHashFromUrl(bsrURL);

        assertThat(hashedBsrFile).isNotNull();
    }

    @Test
    public void hashedFileThrowsIllegalArgumentExceptionWhenUrlIsNull() {
        IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> {
            FileHasher.makeHashFromUrl(null);
        });

        assertThat(thrown.getMessage()).isEqualTo("Input url cannot be null");
    }
}