HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Unit tests for an argument-checking class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
argumentcheckingtestsforclassunit

Problem

I am wondering if my current unit tests could be written better, I am most interested in whether it is possible to make the code look more concise, as currently it a boiler-plated mess.

I have full access to Java 8, if that would help by making it more concise.

The Arguments class itself:

```
public final class Arguments {
private Arguments() {
throw new UnsupportedOperationException();
}

public static int requirePositive(final int value) throws IllegalArgumentException {
return requirePositive(value, "value");
}

public static int requirePositive(final int value, final String name) throws IllegalArgumentException {
Objects.requireNonNull(name);
if (value = 0) {
throw new IllegalArgumentException("the " + name + " must be negative: " + value);
}
return value;
}

public static int requirePositiveOrZero(final int value) throws IllegalArgumentException {
return requirePositiveOrZero(value, "value");
}

public static int requirePositiveOrZero(final int value, final String name) throws IllegalArgumentException {
Objects.requireNonNull(name);
if (value 0) {
throw new IllegalArgumentException("the " + name + " must be negative or zero: " + value);
}
return value;
}

public static int requireInRange(final int value, final int lowInclusive, final int highExclusive) throws IllegalArgumentException {
return requireInRange(value, lowInclusive, highExclusive, "value");
}

public static int requireInRange(final int value, final int lowInclusive, final int highExclusive, final String name) throws IllegalArgumentException {
Objects.requireNonNull(name);
if (lowInclusive >= highExclusive) {
throw new IllegalArgumentException("the lower inclusive bound is greater or equal to the higher exclusive bound: " + lowInclusive + " >= " + highExclusive);
}
if (value = highE

Solution

Test Names

testRequirePositive_intIAE1() doesn't tell me anything besides what function is being tested. It is helpful to give some context as to why you think it should throw an exception. What makes testRequirePositive_intIAE1() different from testRequirePositive_intIAE2()? You have to go look at the test to know what is actually being tested. In this case, it is easy to deduce why the argument is invalid, but this will not be the case for more complicated methods.

Repetitiveness

As you pointed out, the test code is bulky and some times hard to follow. When you look at the code, there are lots of cases where each tests is doing the exact same thing, but with different inputs.

This is a perfect case for parameterized tests. You provide the data with a static method and your test class is instantiated once for each data set.

Touching on the previous point: Parameterized tests mean your actual test method can't describe why the data is intended to cause an error. So it is a good idea to include an additional String argument to provide that context.

Multiple Asserts

testRequireIndexInRangeClosed()1 has three different assertions that are unrelated. These should be different tests (probably different data inputs for a parameterized test). This issue with this is that if the first assertion fails, the following assertions are not checked. This can hide information about what is wrong with the system.

1 This is not the only instance of this in the test code.

Long Class

Just because all of the tested methods are in the same class doesn't mean all of the tests have to be in the same class too. Organize your tests just as you would with your actual code. If a class or method is too long, it likely means you should break it into smaller pieces.

Context

StackExchange Code Review Q#47797, answer score: 6

Revisions (0)

No revisions yet.