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.

18
  • 10
    If you're putting values in and expecting to be able to read those same values back, I'd say magic numbers are fine. So if, say, 1, 2, 3 are 3D array indices where you previously stored the value 17, then I think this test would be dandy (as long as you have some negative tests as well). But if it's the result of a calculation, you should make sure that anyone reading this test will understand why foo(1, 2, 3) should be 17, and magic numbers probably won't achieve that goal.
    – Joe White
    Jun 20, 2016 at 3:04
  • 25
    const int TWO = 2; is even worse than just using 2. It's conforming to wording of the rule with intent to violate it's spirit.
    – Agent_L
    Jun 20, 2016 at 12:06
  • 4
    What is a number that "doesn't mean anything"? Why would it be in your code if it meant nothing?
    – Tim Grant
    Jun 20, 2016 at 16:00
  • 6
    Sure. Leave a comment before a series of such tests, e.g., "a small selection of manually determined examples". This, in relation to your other tests which are clearly testing boundaries and exceptions, will be clear.
    – davidbak
    Jun 20, 2016 at 17:28
  • 5
    Your example is misleading - when your function name would be really foo, it would not mean anything, and so the parameters. But in reality, I am pretty sure the function does not have that name, and the parameters don't have names bar1, bar2, and bar3. Make a more realistic example where the names have a meaning, then it makes much more sense to discuss if the test data values need a name, too.
    – Doc Brown
    Jun 20, 2016 at 20:16

11 Answers 11

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.

17
  • 3
    @Kevin - what language are you testing in? Some testing frameworks let you set up data providers which return an array of arrays of values for testing
    – HorusKol
    Jun 20, 2016 at 1:48
  • 10
    Though I agree with the idea, beware that this practice can introduce new errors too, like if you accidentally extract a value like 0.05f to an int. :) Jun 20, 2016 at 5:44
  • 5
    +1 - great stuff. Just because you don't care what a particular value is, that does not mean it isn't still a magic number...
    – Robbie Dee
    Jun 20, 2016 at 9:25
  • 2
    @PieterB: AFAIK that's the fault of C and C++, which formalised the notion of a const variable. Jun 20, 2016 at 10:26
  • 2
    Have you named your variables the same as the named parameters of calculateCompoundInterest? If so, then the extra typing is a proof-of-work that you've read the documentation for the function you're testing, or at least copied the names given to you by your IDE. I'm not sure how much this tells the reader about the intent of the code, but if you pass the parameters in the wrong order at least they can tell what was intended. Jun 20, 2016 at 10:37
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.

12
  • 12
    Random testing may find bugs that are hard to reproduce but testing randomly hardly finds reproducible bugs. Be sure to capture any test failures with a specific reproducible test case. Jun 20, 2016 at 8:28
  • 5
    And how do you know that your unit test isn't bugged when you "assert something about the result" (in this case, recompute what foo is computing)...? If you were 100% sure your code gives the right answer, then you would just put that code in the program and not test it. If you're not, then you need to test the test, and I think everyone sees where this is going.
    – user198399
    Jun 20, 2016 at 12:25
  • 2
    Yeah, if you pass random inputs into a function you have to know what the output would be to be able to assert that it is working correctly. With fixed/chosen test values you can of course work it out by hand, etc. but surely any automated method of detemining if the result is correct is subject to the exact same problems as the function you are testing. You either use the implementation you have (which you can't because you are testing whether it works) or you write a new implementation which is just as likely to be buggy (or moreso else you'd use the more likely to be correct one).
    – Chris
    Jun 20, 2016 at 12:35
  • 7
    @NajibIdrissi - not necessarily. You could, for example, test that applying the inverse of the operation you're testing to the result gives back the initial value you started with. Or you could test expected invariants (e.g. for all interest calculations at d days, the calculation at d days + 1 month should be a known monthly percentage rate higher), etc.
    – Jules
    Jun 20, 2016 at 12:36
  • 12
    @Chris - In many cases, checking results are correct is easier than generating the results. While this isn't true in all circumstances, there are lots where it is. Example: adding an entry to a balanced binary tree should result in a new tree that is also balanced... easy to test, quite tricky to implement in practice.
    – Jules
    Jun 20, 2016 at 12:38
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

8
  • This answer appears to be about the difficulty of inferring the purpose of a test whereas the actual question is about magic numbers in method parameters...
    – Robbie Dee
    Jun 20, 2016 at 8:57
  • @RobbieDee the name/documentation for a test should be giving the appropriate context and explanation of whatever magic numbers are present within the test. If not, either add documentation or rename the test to be more clear.
    – enderland
    Jun 20, 2016 at 11:09
  • It would still be better to give the magic numbers names. If the number of parameters were to change, the documentation risks becoming out of date.
    – Robbie Dee
    Jun 20, 2016 at 14:07
  • 1
    @RobbieDee 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.
    – enderland
    Jun 20, 2016 at 14:22
  • "Hopefully" huh? Why not just code the thing properly and do away with what is ostensibly a magic number as Philipp has already outlined...
    – Robbie Dee
    Jun 20, 2016 at 20:56
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.

1
  • Sensible solution. Mar 27, 2017 at 13:52
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
              }
            }
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.

1
  • 1
    This isn't necessarily true, though -- as in the example of the most recent unit test I wrote (assertEqual "Returned value" (makeKindInt 42) (runTest "lvalue_operators")). In this example, 42 is just a placeholder value that is produced by the code in the test script named lvalue_operators and then checked when it is returned by the script. It has no meaning at all, other than that the same value occurs in two different places. What would be an appropriate name here that actual gives any useful meaning?
    – Jules
    Jun 20, 2016 at 12:48
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.)

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

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!

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.

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?

Not the answer you're looking for? Browse other questions tagged or ask your own question.