views:

24

answers:

1

Hi,

I have the following JUnit test:

@Test
public final void testDivisors() {
    testDivisorsAux(1, new int[] { 1 });
    testDivisorsAux(6, new int[] { 1, 2, 3, 6 });
    testDivisorsAux(0, new int[] { });
    ...
}

private final void testDivisorsAux(final int number, final int[] expected) {
    List<Integer> divisors = Util.divisors(number);
    assertSame(divisors.size(), expected.length);
    for (int i : expected) {
        assertTrue(divisors.contains(i));
    }
}

The method that I'm testing is quite simple, it just receives a number and returns a List with its divisors. I don't want to repeat the test code many times, so I've created an auxiliar method, testDivisorsAux.

Everything works fine, I'm just wondering... is this a bad practice? Should I write the test in a different way? Maybe keep all the code within the "@Test method"?

PMD is telling me that JUnit tests should include assert() or fail() (for the first method), and JUnit 4 tests that execute tests should use the @Test annotation (for the second one). I know that PMD is using just a regular expression (well, actually it's XPath) to determine which rules I'm breaking... so I'm inclined to think that it's simply a "false positive" warning. But anyway, I would like to know if I'm doing something wrong. (Appart from writing tests 4 times longer than the method being tested :)

While I was searching for questions similar to this one, I've found something called parametrized tests... but it seems it's something oriented to much bigger scenarios, isn't it?

+2  A: 

Personally I think it is good practise to do this. There is a small danger of going too far and building masses of code to do your test in which case you should rethink the design of the stuff you're testing. You don't really want to have to write tests to test your tests but the example you give seems fine and it means your test cases are a lot simpler.

I have not used PMD and personally I would try to configure it to ingore these warnings. If they worry you you might be able to change your code to get rid of them. My guess is that the 2nd warning was because your helper method starts with the word test. In junit3 all methods that start with test are tests. Why not rename the method testDivisorsAux to be assertDivisors - maybe having a method that starts with assert will help with the 1st warning also.

Adam Butler
Thanks a lot for your answer, @Adam. I was not really worried by the PMD warnings, but anyway good trick to avoid them! :) Not only a good trick, I think the names you propose are also more descriptive than mine...
AJPerez