Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Unit testing code that relies on constant values

Consider the following (totally contrived) example:

public class Length {
    private static final int MAX_LENGTH = 10;
    private final int length;
    public Length(int length) {
        if (length > MAX_LENGTH)
            throw new IllegalArgumentException("Length too long");
        this.length = length;
    }
}

I would like to test that this throws an exception when called with a length greater than MAX_LENGTH. There are a number of ways this can be tested, all with disadvantages:

@Test(expected = IllegalArgumentException.class)
public void testMaxLength() {
    new Length(11);
}

This replicates the constant in the testing case. If MAX_LENGTH becomes smaller this will silently no longer be an edge case (though clearly it should be paired with a separate case to test the other side of the edge). If it becomes larger this will fail and need to be changed manually (which might not be a bad thing).

These disadvantages can be avoided by adding a getter for MAX_LENGTH and then changing the test to:

new Length(Length.getMaxLength());

This seems much better as the test does not need to be changed if the constant changes. On the other hand it is exposing a constant that would otherwise be private and it has the significant flaw of testing two methods at once - the test might give a false positive if both methods are broken.

An alternative approach is to not use a constant at all but, rather, inject the dependency:

interface MaxLength {
    int getMaxLength();
}

public class Length {
    public static void setMaxLength(MaxLength maxLength);
}

Then the 'constant' can be mocked as part of the test (example here using Mockito):

MaxLength mockedLength = mock(MaxLength.class);
when(mokedLength.getMaxLength()).thenReturn(17);
Length.setMaxLength(mockedLength);
new Length(18);

This seems to be adding a lot of complexity for not a lot of value (assuming there's no other reason to inject the dependency).

At this stage my preference is to use the second approach of exposing the constants rather than hardcoding the values in the test. But this does not seem ideal to me. Is there a better alternative? Or is the lack of testability of these cases demonstrating a design flaw?

like image 375
sprinter Avatar asked Nov 23 '15 04:11

sprinter


1 Answers

As Tim alluded to in the comments, your goal is to make sure that your software behaves according to the specifications. One such specification might be that the maximum length is always 10, at which point it'd be unnecessary to test a world where length is 5 or 15.

Here's the question to ask yourself: How likely is it that you'll want to use your class with a different value of the "constant"? I've quoted "constant" here because if you vary the value programmatically, it's not really a constant at all, is it? :)

  • If your value will never ever change, you could not use a symbolic constant at all, just comparing to 10 directly and testing based on (say) 0, 3, 10, and 11. This might make your code and tests a little hard to understand ("Where did the 10 come from? Where did the 11 come from?"), and will certainly make it hard to change if you ever do have reason to vary the number. Not recommended.

  • If your value will probably never change, you could use a private named constant (i.e. a static final field), as you have. Then your code will be easy enough to change, though your tests won't be able to automatically adjust the way your code would.

    • You could also relax to package-private visibility, which would be available to tests in the same package. Javadoc (e.g. /** Package-private for testing. */) or documentation annotations (e.g. @VisibleForTesting) may help make your intentions clear. This is a nice option if your constant value is intended to be opaque and unavailable outside of your class, like an URL template or authentication token.

    • You could even make it a public constant, which would be available to consumers of your class as well. For your example of a constant Length, a public static final field is probably best, on the assumption that other pieces of your system may want to know about that (e.g. for UI validation hints or error messages).

  • If your value is likely to change you could accept it per-instance, as in new Length(10) or new Length().setMaxLength(10). (I consider the former to be a form of dependency injection, counting the constant integer as a dependency.) This is also a good idea if you wanted to use a different value in tests, such as using a maximum length of 2048 in production but testing against 10 for practicality's sake. To make a flexible length validator, this option is probably a good upgrade from a static final field.

  • Only if your value is likely to change during your instance's lifetime would I bother with a DI-style value provider. At that point, you can query the value interactively, so it doesn't behave at all like a constant. For "length", that'd be obvious overkill, but maybe not for "maximum allowed memory", "maximum simultaneous connections", or some other pseudo-constants like that.

In short, you'll have to decide how much control you need, and then you can pick the most straightforward choice from there; as a "default", you may want to make it a visible field or constructor parameter, as those tend to have good balance of simplicity and flexibility.

like image 107
Jeff Bowman Avatar answered Sep 30 '22 20:09

Jeff Bowman