63

In my unit tests, I often throw arbitrary values at my code to see what it does. For example, if I know that foo(1, 2, 3) is supposed to return 17, I might write this:

assertEqual(foo(1, 2, 3), 17)

These numbers are purely arbitrary and have no broader meaning (they are not, for example, boundary conditions, though I do test on those as well). I would struggle to come up with good names for these numbers, and writing something like const int TWO = 2; is obviously unhelpful. Is it OK to write the tests like this, or should I factor the numbers out into constants?

In Are all magic numbers created the same?, we learned that magic numbers are OK if the meaning is obvious from context, but in this case the numbers actually have no meaning at all.

Kevin
  • 2,668

11 Answers11

81

When do you really have numbers which have no meaning at all?

Usually, when the numbers have any meaning, you should assign them to local variables of the test method to make the code more readable and self-explaining. The names of the variables should at least reflect what the variable means, not necessarily its value.

Example:

const int startBalance = 10000;
const float interestRate = 0.05f;
const int years = 5;

const int expectedEndBalance = 12840;

assertEqual(calculateCompoundInterest(startBalance, interestRate, years),
            expectedEndBalance);

Note that the first variable is not named HUNDRED_DOLLARS_ZERO_CENT, but startBalance to denote what's the meaning of the variable but not that its value is in any way special.

5gon12eder
  • 7,236
Philipp
  • 23,488
21

If you're using arbitrary numbers just to see what they do, then what you're really looking for is probably randomly generated test data, or property-based testing.

For example, Hypothesis is a cool Python library for this sort of testing, and is based on QuickCheck.

Think of a normal unit test as being something like the following:

  1. Set up some data.
  2. Perform some operations on the data.
  3. Assert something about the result.

Hypothesis lets you write tests which instead look like this:

  1. For all data matching some specification.
  2. Perform some operations on the data.
  3. Assert something about the result.

The idea is to not constrain yourself to your own values, but pick random ones that can be used to check that your function(s) match their specifications. As an important note, these systems will generally remember any input that fails, and then ensure that those inputs are always tested in the future.

Point 3 can be confusing to some people so let's clarify. It doesn't mean that you're asserting the exact answer - this is obviously impossible to do for arbitrary input. Instead, you assert something about a property of the result. For example, you might assert that after appending something to a list it becomes non-empty, or that a self-balancing binary search tree is actually balanced (using whatever criteria that particular data structure has).

Overall, picking arbitrary numbers yourself is probably pretty bad - it doesn't really add a whole bunch of value, and is confusing to anyone else who reads it. Automagically generating a bunch of random test data and using that effectively is good. Finding a Hypothesis or QuickCheck-like library for your language of choice is probably a better way to accomplish your goals while remaining understandable to others.

Dan Oberlam
  • 1,291
  • 1
  • 9
  • 22
11

Your unit test name should provide most of the context. Not from the values of the constants. The name/documentation for a test should be giving the appropriate context and explanation of whatever magic numbers are present within the test.

If that is not sufficient, a slight bit of documentation should be able to provide it (whether through the variable name or a docstring). Keep in mind the function itself has parameters that hopefully have meaningful names. Copying those into your test to name the arguments is rather pointless.

And last, if your unittests are complicated enough that this is hard/not practical you probably have too complicated of functions and might consider why this is the case.

The more sloppily you write tests, the worse your actual code will be. If you feel the need to name your test values to make the test clear, it strongly suggests your actual method needs better naming and/or documentation. If you find the need to name constants in tests I would look into why you need this - likely the problem is not the test itself but implementation

enderland
  • 12,201
  • 4
  • 53
  • 64
10

This depends heavily on the function you are testing. I know lots of cases where the individual numbers do not have a special meaning on their own, but the test case as a whole is constructed thoughtfully and so has a specific meaning. That is what one should document in some way. For example, if foo really is a method testForTriangle which decides if the three numbers might be valid lengths of the edges of a triangle, your tests might look like this:

// standard triangle with area >0
assertEqual(testForTriangle(2, 3, 4), true);

// degenerated triangle, length of two edges match the length of the third
assertEqual(testForTriangle(1, 2, 3), true);  

// no triangle
assertEqual(testForTriangle(1, 2, 4), false); 

// two sides equal
assertEqual(testForTriangle(2, 2, 3), true);

// all three sides equal
assertEqual(testForTriangle(4, 4, 4), true);

// degenerated triangle / point
assertEqual(testForTriangle(0, 0, 0), true);  

and so on. You might improve this and turn the comments into a message parameter of assertEqual which will be displayed when the test fails. You can then improve this further and refactor this into a data driven test (if your testing framework supports this). Nevertheless you do yourself a favor if you put a note into the code why you picked this numbers and which of the various behaviours you are testing with the individual case.

Of course, for other functions the individual values for the parameters might be of more importance, so using a meaningless function name like foo when asking for how to deal with the meaning of parameters is probably not the best idea.

Doc Brown
  • 218,378
6

Why do we want to use named Constants instead of numbers?

  1. DRY - If I need the value at 3 places, I only want to define it once, so I can change it at one place, if it changes.
  2. Give meaning to numbers.

If you write several unit tests, each with an assortment of 3 Numbers (startBalance, interest, years ) - I would just pack the values into the unit-test as local variables. The smallest scope where they belong.

testBigInterest()
  var startBalance = 10;
  var interestInPercent = 100
  var years = 2
  assert( calcCreditSum( startBalance, interestInPercent, years ) == 40 )

testSmallInterest()
  var startBalance = 50;
  var interestInPercent = .5
  var years = 1
  assert( calcCreditSum( startBalance, interestInPercent, years ) == 50.25 )

If you use a language which allows named parameters, this is of course superflous. There I would just pack the raw values in the method call. I can't imagine any refactoring making this statement more concise:

testBigInterest()
  assert( calcCreditSum( startBalance:       10
                        ,interestInPercent: 100
                        ,years:               2 ) = 40 )

Or use a testing-Framework, which will allow you to define the test cases in some array or Map format:

testcases = { {
                Name: "BigInterest"
               ,StartBalance:       10
               ,InterestInPercent: 100
               ,Years:               2
              }
             ,{ 
                Name: "SmallInterest"
               ,StartBalance:       50
               ,InterestInPercent:  .5
               ,Years:               1
              }
            }
Falco
  • 1,299
3

...but in this case the numbers actually have no meaning at all

The numbers are being used to call a method so surely the above premise is incorrect. You may not care what the numbers are but that is beside the point. Yes, you could infer what the numbers are used for by some IDE wizardry but it would be far better if you just gave the values names - even if they just match the parameters.

Robbie Dee
  • 9,823
3

If you want to test a pure function on one set of inputs that aren't boundary conditions, then you almost certainly want to test it on a whole bunch of sets of inputs that aren't (and are) boundary conditions. And to me that means there should be a table of values to call the function with, and a loop:

struct test_foo_values {
    int bar;
    int baz;
    int blurf;
    int expected;
};
const struct test_foo_values test_foo_with[] = {
   { 1, 2, 3, 17 },
   { 2, 4, 9, 34 },
   // ... many more here ...
};

for (size_t i = 0; i < ARRAY_SIZE(test_foo_with); i++) {
    const struct test_foo_values *c = test_foo_with[i];
    assertEqual(foo(c->bar, c->baz, c->blurf), c->expected);
}

Tools like those suggested in Dannnno's answer can help you construct the table of values to test. bar, baz, and blurf should be replaced by meaningful names as discussed in Philipp's answer.

(Arguable general principle here: Numbers are not always "magic numbers" that need names; instead, numbers might be data. If it would make sense to put your numbers into an array, perhaps an array of records, then they're probably data. Conversely, if you suspect you might have data on your hands, consider putting it into an array and acquiring more of it.)

zwol
  • 2,620
1

Firstly let’s agree that “unit test” is often used to cover all automated tests a programmer writes, and that it is pointless to debate what each test should be called….

I have worked on a system where the software took a lot of inputs and worked out a “solution” that had to fulfil some constraints, while optimizing other numbers. There were no right answers, so the software just had to give a reasonable answer.

It did this by using lots of random numbers to get a starting point, then using a “hill climber” to improve the result. This was run many times, picking the best result. A random number generator can be seeded, so that it always gives the same numbers out in the same order, hence if the test set a seed, we know that the result would be the same on each run.

We had lots of tests that did the above, and checked that the results were the same, this told us that we had not changed what that part of the system did by mistake while refactoring etc. It did not tell us anything about the correctness of what that part of the system did.

These tests were costly to maintain, as any change to the optimising code would break the tests, but they also found some bugs in the much larger code that pre-processed the data, and post-processed the results.

As we “mocked” the database, you could call these tests “unit tests”, but the “unit” was rather large.

Often when you are working on a system with no tests, you do something like the above, so that you can confirm your refactoring don’t change the output; hopefully better tests are written for new code!

Ian
  • 4,623
1

Tests are different from production code and, at least in units tests written in Spock, which are short and to the point, I have no issue using magic constants.

If a test is 5 lines long, and follows the basic given/when/then scheme, extracting such values into constants would only make code longer and harder to read. If the logic is "When I add a user named Smith, I see the user Smith returned in user list", there is no point in extracting "Smith" to a constant.

This of course applies if you can easily match values used in the "given" (setup) block to those found in "when" and "then" blocks. If your test setup is separated (in code) from the place the data is used, it might indded be better to use constants. But since tests are best self-contained, setup is usually close to place of use and the first case applies, meaning magic constants are quite acceptable in this case.

1

I think in this case the numbers should be termed Arbitrary Numbers, rather than Magic Numbers, and just comment the line as "arbitrary test case".

Sure, some Magic Numbers can be arbitrary too, as for unique "handle" values (which should be replaced with named constants, of course), but can also be precalculated constants like "airspeed of an unladen European sparrow in furlongs per fortnight", where the numeric value is plugged in without comments or helpful context.

RufusVS
  • 121
0

I won't venture as far as to say a definitive yes/no, but here are some questions you should ask yourself when deciding whether it's OK or not.

  1. If the numbers don't mean anything, why are they there in the first place? Can they be replaced by something else? Can you do verification based on method calls and flow instead of value assertions? Consider something like Mockito's verify() method that checks whether or not certain method calls were made to mock objects instead of actually asserting a value.

  2. If the numbers do mean something, then they should be assigned to variables that are named appropriately.

  3. Writing the number 2as TWO might be helpful in certain contexts, and not so much in other contexts.

    • For instance: assertEquals(TWO, half_of(FOUR))makes sense to someone who reads the code. It is immediately clear what you are testing.
    • If however your test is assertEquals(numCustomersInBank(BANK_1), TWO), then this doesn't make that much sense. Why does BANK_1 contain two customers? What are we testing for?