JAVA best practice unit testing with JUnit

Question

I am currently using the following method and unit test for the method. I think the test could/should be broken down into more tests but I'm not sure how many tests to write for this or what the more important parts are especially considering the method deals with establishing a Connection, using sql queries, etc... All help is appreciated.

JAVA Method:

public static ArrayList<HashMap<String, String>> executeSelect(
        Connection conn, Statement stmt, Query query) {

    ResultSet rs = null;
    ArrayList<HashMap<String, String>> serviceRequests = new ArrayList<HashMap<String, String>>();

    try {
        long queryStart = System.nanoTime();
        rs = stmt.executeQuery(query.getQuery());
        long queryEnd = System.nanoTime();
        long queryDuration = queryEnd-queryStart;
        queryTime = String.valueOf(queryDuration);

        while (rs.next()) {

            HashMap<String, String> serviceRequestData = new HashMap<>();

            if (QueryUtil.hasColumn(rs, "ID")) {
                String id = rs.getString("ID");
                serviceRequestData.put("ID", id);
            }
            else{
                serviceRequestData.put("ID", " ");
            }
            if (QueryUtil.hasColumn(rs, "FN_Contact")) {
                String firstName = rs.getString("FN_Contact");
                serviceRequestData.put("FN_Contact", firstName);
            }
            else{
                serviceRequestData.put("FN_Contact", " ");
            }
            if (QueryUtil.hasColumn(rs, "LN_Contact")) {
                String lastName = rs.getString("LN_Contact");
                serviceRequestData.put("LN_Contact", lastName);
            }
            else{
                serviceRequestData.put("LN_Contact", " ");
            }
            if (QueryUtil.hasColumn(rs, "Notes")) {
                String notes = rs.getString("Notes");
                serviceRequestData.put("Notes", notes);
            }
            else{
                serviceRequestData.put("Notes", " ");
            }
            if (QueryUtil.hasColumn(rs, "Email")) {
                String email = rs.getString("Email");
                serviceRequestData.put("Email", email);
            }
            else{
                serviceRequestData.put("Email", " ");

            }

            serviceRequests.add(serviceRequestData);

        }
    } catch (SQLException e) {
        e.printStackTrace();
        sqlException = true;
    }
    return serviceRequests;
}

JUnit Test:

@Test
public void testFirstName() {
    ArrayList<HashMap<String, String>> testMap = new ArrayList<HashMap<String,String>>();
    Connection conn = null;
    Statement stmt = null;
    try {
        Class.forName("com.mysql.jdbc.Driver");

        String connectionUrl = "jdbc:mysql://localhost:3306/gc_image";
        String connectionUser = "root";
        String connectionPassword = "GCImage";
        conn = DriverManager.getConnection(connectionUrl, connectionUser,
                connectionPassword);
        conn.
        stmt = conn.createStatement();
        Query testQuery = new Query();
        testQuery
                .setQuery("select * from service_request where FN_contact = 'Trevor'");
        testMap = QueryController.executeSelect(conn, stmt, testQuery);

        assertEquals("Janke", testMap.get(0).get("LN_Contact"));
        assertEquals("Hello World", testMap.get(0).get("Notes"));
        assertEquals("janke@gmail.com", testMap.get(0).get("Email"));
        assertEquals("ID", testMap.get(0).get("7"));

    } catch (ClassNotFoundException e) {
        e.printStackTrace();
    } catch (SQLException e) {
        e.printStackTrace();
    } finally {
        try {
            stmt.close();
            conn.close();
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

}

Answer 1

Yes you are right, your test case should be broken into many unit tests. I will apply some of the unit tests best practices provided by the source at the bottom of this answer.

Your unit test should be descriptive

when I read your unit test I have to spend too much effort to understand what are you trying to validate. Use a good convention naming, don't be afraid of using long method names ( example : executeSelect_Should_Return_Empty_List_When_No_Results() ) .

Test one thing at a time

Same as the previous point, Try to test one and only one thing at time, this thing could be :

• Do I have results
• What if there is no results ?
• What happen if cannot get a connection
• What happens if I have a SQL Error ( Test exceptions ) .

Write isolated unit tests

Your unit test seems to depend on a real database. your test should not depend on external resources such as a Database ( even a local database), the file system, REST or SOAP Services, etc. Instead of that you can "Mock" your database. In addition, external resources may slow down your test(s).

Avoid logic in unit tests

Logic makes your test less readable and can introduce bugs into your tests. Tests are the last place you want to have bugs. it is true you dont have structures like "if, for, switch, etc" but that try...catch is not necessary there, this is something you should brake into multiple test cases. Prefer to add exceptions to method test signature. and as stated before, have other tests to validate the exceptions (ie: assertThrows ) .

Use constants instead of "Magic Values"

Prefer to use constants instead of string values, try to express the intention of your test with descriptive constant names( ie: EXPECTED_FIRST_NAME)

Source : Unit tests best practices

Answer 2

You should start off by identifying what part of this test's setup is not truly part of the test method; i.e., what is boilerplate that could be extracted into @Before and @After methods. That will probably require you to pull a few local variables into class variables. This makes each @Test method less verbose, and allows you to focus on the functionality under test.

You should next either remove all of the catch blocks, or fail the test with something likefail(exception.getMessage()) if your code or test code raises an unintended exception. If you remove them, it's easy enough to change the unit test's method signature to throws Exception to allow it to generically fail if an exception is thrown. As it is now, you could completely fail to connect to the database in your setup and the Junit test would still go green!

Ideally, you'd have a unit test that covers each if...else block. That would give you 10 tests. In those, the focus (the Assert) would be to confirm the presence of the found/not found value in serviceRequests. You should also test your exception handling, so you'd want a test that forces SQLException to be caught.

I would finally suggest that, to reduce overhead, you consider using a mocking framework, such as Mockito to stub out the database I/O completely. This SO question has some good examples of mocking out the database.

I noticed a few quick things in executeSelect() that could be fixed:

• Remove the Connection an argument -- it's unused, as the Statement is already instantiated.
• Consider throwing the SQLException rather than returning a List -- it gives a false advertisement that a DB read has been performed succesfully.
• Prefer interfaces and abstract classes over concrete classes: change your method signature and serviceRequests from ArrayList<HashMap<String, String>> to List<Map<String, String>>. Among many resources, Josh Bloch's Effective Java is a good reference for this subject.

You could simplify this method even more -- which would make testing even more straightforward -- by moving the following code into another method:

    long queryStart = System.nanoTime();
    rs = stmt.executeQuery(query.getQuery());
    long queryEnd = System.nanoTime();
    long queryDuration = queryEnd-queryStart;
    queryTime = String.valueOf(queryDuration);

and then passing only ResultSet into executeSelect() ...or whatever it would be renamed to :).