Replacing Magic Numbers in Tests
When writing tests for code you just developed (or are about to develop, practicing TDD), the test often requires certain input data (provided either directly or via mocked collaborators) to describe the setting in which the test case will exercise the productive code. The first version of a test might look something like this:
@Test
public void dialNumberOfForeignCarrierIsNotObsolete() {
var carrier = carrier()
.withId(42)
.foreign()
.build();
var dialNumberBlock = dialNumberBlock()
.withId(1337)
.withOwningCarrier(42)
.build();
var dialNumber = dialNumber()
.withBlock(1337)
.build();
repository.insert(carrier);
repository.insert(dialNumberBlock);
repository.insert(dialNumber);
unitUnderTest.deleteObsoleteDialNumbers();
assertThat(collaborator.deletedNumbers(), not(contains(dialNumber)));
}
While this method tells the reader a lot about the author's literary preferences or the confidence in their skills, the integer literals 42 and 1337 convey no information about the reasons behind their particular choice. Is the concrete value of the integer relevant? Is there some common base test data these integers refer to? What is the impact of changing a value? Do literals of equal value need to be changed together because they are related in some way (where in our example the repeated values look suspiciously like foreign keys)?
None of these questions are obviously answered by the way the test is written. In the following I want to describe different strategies to improve the clarity of such a test.
Replace Literals by Reading Variables
An easy and obvious trick to communicate that two occurrences of the same value are intended rather than coincidental is to share them via a local variable:
var carrierId = 42;
var carrier = carrier()
.withId(carrierId)
.foreign()
.build();
var dialNumberBlock = dialNumberBlock()
.withOwningCarrier(carrierId)
.build();
Or if carrier is designed as a DTO it even has a way to provide its ID to interested clients.
var carrier = carrier()
.withId(42)
.foreign()
.build();
var dialNumberBlock = dialNumberBlock()
.withOwningCarrier(carrier.id())
.build();
Either way your code will end up with only a single literal for all places where the same value is required. There is no ambiguity whether two values are equal by intention or not. And even setting a different value is only a matter of updating it in a single spot. There is, however, one question remaining: Is the concrete value of the literal relevant? As the answer should probably be "no", let's look into some ways to make this answer self-evident.
Use a Counter
What if every time a test requires a new value, instead of encumbering the programmer with thinking it up, delegate this burden to some piece of code. A simple approach could be a counter:
CountingNumberProvider numbers = new CountingNumberProvider();
@Test
public void test() {
var carrier = carrier()
.withId(numbers.next())
.foreign()
.build();
}
static class CountingNumberProvider {
private int counter;
int next() {
return counter++;
}
}
Since we rely on Java's default value 0 for the otherwise uninitialised field counter, the code ends up with no more magic number at all.
Use Randomness
While using a counter solves all the concerns this article's introduction tried to raise on your mind, using only continuously enumerated test data looks artificial as most data your code will encounter probably won't have its fields as tightly packed numbers. In teams with varying levels of expertise a less experienced member might even be tempted to guess the value the counter just provided and reintroduce it as a literal. To address this, the number provider, instead of counting, can use an internal source of randomness to generate the values.
import java.util.concurrent.ThreadLocalRandom;
class RandomNumberProvider {
int next() {
return ThreadLocalRandom.current().nextInt();
}
}
Reproducing a Failure
Regarding external behaviour, this provider has some important differences. When repeatedly running the test, most of these runs will probably never have been run before in the same choices of random values. This gives raise to flaky tests if the productive code (accidentally or not) depends on the concrete input values. Imagine your shivers watching a CI build report a failing test using this provider. You run the test locally to investigate, but it passes. And it passes once again. While your IDE might provide a feature like "Run until failure", this is no desired Developer Experience (DX) as it might take arbitrarily long to choose the "right" random values to reproduce. So to ease such test executions, the test should report the values it provided to the tests. In JUnit 4 this can be achieved using a test rule.
(I am well aware that JUnit 5 has made a major leap forward in terms of customising the surroundings of the tests themselves. However, the concrete case I came to use this pattern in was an Android Espresso test, a framework still using JUnit 4).
1 class RandomNumberProvider implements TestRule {
2
3 private final List<Integer> providedValues = new ArrayList<>();
4
5 int next() {
6 var nextValue = ThreadLocalRandom.current().nextInt();
7 providedValues.add(nextValue);
8 return nextValue;
9 }
10
11 @Override
12 public Statement apply(Statement base, Description description) {
13 return new Statement() {
14 @Override
15 public void evaluate() {
16 try {
17 base.evaluate();
18 } catch (Throwable e) {
19 AssertionFailedError randomValues =
20 new AssertionFailedError(providedValues.toString());
21 randomValues.setStackTrace(new StackTraceElement[0]);
22 e.addSuppressed(randomValues);
23 throw e;
24 }
25 }
26 };
27 }
28 }
The apply() method allows wrapping around the test it executes in L17 and catches a potential failure to attach additional context about our chosen random values as a suppressed exception, a mechanism I only slightly abuse from the try-with-resources context. The suppressed exception is then included in the stacktrace of the failing test, enabling the programmer to reproduce the problem.
Value Uniqueness
For the counting provider each value was guaranteed to be returned at most once. This changes in the random provider as no checking is performed in this regard. To uphold the high standards of this blog, I am obliged to refer you to the Birthday Problem, a mathematical consideration in our case informally stating that a test run is more likely to suffer from returning the same value twice than you might expect.
But is this a big deal? It certainly depends, but for me, uniqueness has been vital as it was not unusual for my code under test to suffer from ID confusion, where an ID of entity A was handled as an ID of entity B, e.g.
dialNumber()
.withBlockId(carrierId)
.build();
Such errors are easily caught when asserting on a block ID different from the carrierId but remains hidden to the test if both have the same value.
Once aware of this issue, our provider becomes trivial to fix by introducing some bookkeeping:
class RandomNumberProvider implements TestRule {
private final Set<Integer> providedValues = new HashSet<>();
int next() {
int nextValue;
do {
nextValue = ThreadLocalRandom.current().nextInt();
} while (providedValues.contains(nextValue));
providedValues.add(nextValue);
return nextValue;
}
}
Note the change of data structure to efficiently check for value presence and the refreshingly appealing use of the otherwise rarely seen do-while loop.
The full version of this pattern that I use in my tests has some more quality of life features up its sleeve but the core idea remains the same.
Summary
There are numerous ways to reduce usage of magic numbers in your tests. You should always strive to make the statements of your tests as obvious as possible and connect equal values by referencing them. Tests that don't depend on the concrete values of their input data can make this clear by deferring the value generation to a component agnostic to the concrete test case. What will your next unit test look like?