Help with testing basic String array whose enclosing method returns boolean.

[James Dekker]

Hello there,

I am trying to write a unit test for a helper class that was already pre-written (excuse me that development was not test first development).

Here is the particular helper class's code:

public class MyHelper {
	public static boolean isValidCode(String code) {
		boolean isValid = (code.equalsIgnoreCase("CU") || code.equalsIgnoreCase("CR") || code.equalsIgnoreCase("CE")
			|| code.equalsIgnoreCase("CG") || code.equalsIgnoreCase("CP") || code.equalsIgnoreCase("CPF")
			|| code.equalsIgnoreCase("CSL") || code.equalsIgnoreCase("CSLF") || code.equalsIgnoreCase("CA")
			|| code.equalsIgnoreCase("CAP") || code.equalsIgnoreCase("PCAH") || code.equalsIgnoreCase("PCPS")
			|| code.equalsIgnoreCase("PCSR") || code.equalsIgnoreCase("PCPP"));
		return isValid;
	}
}

Am trying to write a JUnit test case for this method, here's what I've come up with so far:

public class MyHelper extends TestCase {
	public void testIsValidCode() throws Exception {
		String [] validCodeGroup = {"CU", "CR", "CE", "CG", "CP", "CPF", "CSL", "CSLF", "CA", "CAP", "PCAH", "PCPS", "PCSR", "PCPP"};
		for (int i=0; i<validCodeGroup.length; i++) {
			assertTrue("Code doesn't belong in group", validCodeGroup[i], // What do I put here for actual[i]?);
		}
	}
}

Any help is greatly appreciated...>

[David Newton]

What's this "actual[i]" you're talking about?

You should be checking to see if the method returns true for every code group you send it--what else could you possibly do?

[James Dekker]

David,

What's this "actual[i]" you're talking about?

I think this is the heart of my question...

Just used this as pseudo code.

Don't most test cases use this convention:

assertEquals(expected, actual)

You should be checking to see if the method returns true for every code group you send it...

Can you elaborate on this?

Given the code belonging to the helper class does anyone know of a good way to unit test this?

Many thanks for responding!

[David Newton]

James Dekker wrote:Don't most test cases use this convention:

assertEquals(expected, actual)

No, they use whatever they need to test. You *could* use assertEquals(isValidCode(group), true), but it's silly, sort of like return (foo == true) ? true : false.

Can you elaborate on this?

Not much to elaborate, but:

public void testIsValidCode() throws Exception {
    String [] groups = { "CU", "CR", "CE", "CG", "CP", "CPF", "CSL", "CSLF", "CA", "CAP", "PCAH", "PCPS", "PCSR", "PCPP" };
    for (String group : groups) {
        assertTrue(isValidCode(group));
    }
}

Personally, I'd implement the code under test as a map, rather than a ginormous if statement. I'd also be pretty cautious about this--this is one of those kinds of methods where if there's suddenly another valid group, you won't necessarily know to test it. Whereas if the valid groups are in a map, you have a list of the valid groups you can use to test. Of course, then you're just testing the JRE's implementation of Map.contains(), which seems equally silly.

[Ernest Friedman-Hill]

HI James,

"assertTrue()" doesn't have an "actual" argument -- it accepts a boolean, and the test passes if the boolean is true; so your assertTrue() call would just look like:

assertTrue("Code doesn't belong in group", MyHelper.isValidCode(validCodeGroup[i]);

There are a few other things you would test: first, test that a bunch of non-valid codes return false -- i.e., define another array "invalidCodeGroup" and then try

assertFalse("Code doesn't belong in group", MyHelper.isValidCode(invalidCodeGroup[i]);

I'd also test lower-case valid codes, to make sure the method returns "true" for them too.

Finally, you'll probably want to test what happens if you pass in an empty string (should return false) and what happens if you pass in null (should return false, I'd say, but the actual method will throw a NullPointerException, which I think should be fixed.)

[David Newton]

Ernest Friedman-Hill wrote:the actual method will throw a NullPointerException, which I think should be fixed.)

I'm not totally sure I agree on that--it depends on how the method is to be used.

If it's expected to always receive a non-null then an NPE may be appropriate, as it would indicate a programming error.