Test Smell: Hard Coded Values

This is a series of short blog posts on the topic of test architecture. Most topics evolved from my talk “Dealing with Testing Fatigue” which highlights typical anti-patterns in test code, why they are problematic and how to improve them.

Hard coded test values are scalar values or value objects that are used directly in fixture setup, as parameters in the test exercise or as expected values in the verification. That is, they are not assigned to a named constant or variable.

Problem

The problem with hard coded test data is that it’s often interdependent: You expect a certain value because you have used it in the setup. Take for example this test from Magento:

public function testGetTranslationFileTimestamp()
{
    $this->fileManagerMock->expects($this->once())
        ->method('getTranslationFileTimestamp')
        ->willReturn(1445736974);
    $this->assertEquals(1445736974, $this->model->getTranslationFileTimestamp());
}

public function testGetTranslationFileTimestamp()

{

$this->fileManagerMock->expects($this->once())

->method('getTranslationFileTimestamp')

->willReturn(1445736974);

$this->assertEquals(1445736974, $this->model->getTranslationFileTimestamp());

}

The timestamp 1445736974 is configured as file timestamp in a mock. It’s a short test and the assertion is right in the next line, so it is quite obvious that we are looking for the same timestamp used in the mock configuration (other than with the Mystery Guest smell where test data is hidden somewhere else).

It becomes a problem when we have multiple different test values and maybe some of them are set up in a general fixture in the setUp() method.

When developers need to make up values, they usually don’t have much variance. For strings and numbers, 90% of the time it’s “test,” “foo”, “bar”, “0”, “1”, “2”, or “42”. Okay, the “90” is made up as well.

Now there might be multiple hard coded “1” and “2” values in one test with entirely different meanings (e.g. store ID, customer ID, product ID. So again, cause and effect becomes unclear.

Possible solutions

If there is interdependency only within a single test, introduce a variable in the test:

public function testGetTranslationFileTimestamp()
{
    $translationFileTimestamp = 1445736974;
    $this->fileManagerMock->expects($this->once())
        ->method('getTranslationFileTimestamp')
        ->willReturn($translationFileTimestamp);
    $this->assertEquals($translationFileTimestamp, $this->model->getTranslationFileTimestamp());
}

public function testGetTranslationFileTimestamp()

{

$translationFileTimestamp = 1445736974;

$this->fileManagerMock->expects($this->once())

->method('getTranslationFileTimestamp')

->willReturn($translationFileTimestamp);

$this->assertEquals($translationFileTimestamp, $this->model->getTranslationFileTimestamp());

}

If it is used in a general fixture across tests, use a constant instead

class ShoppingCartTest
{
    private const DEFAULT_STORE_ID = 1;
    private const CURRENT_CUSTOMER_ID = 1;

    // ...
}

class ShoppingCartTest

{

private const DEFAULT_STORE_ID = 1;

private const CURRENT_CUSTOMER_ID = 1;

// ...

}

But really only if the tests share this general fixture, i.e. it has been set up in the setUp() method or using shared fixtures via annotations on the class level, as Magento allows it in integration tests. If two or more tests just happen to create a customer with ID 1, a local variable is suited better, because it makes clear that there is no interdependency between the tests.

If input and output values are not identical but still depend on each other, try to make this dependency clear in the test as well.

Take this example of a test for tax calculation:

$item = new Item("Item Name", 5, 6.99, 0.19, true);
$this->assertEquals(1.3281, $item->taxValue());

1 2	$item = new Item("Item Name", 5, 6.99, 0.19, true); $this->assertEquals(1.3281, $item->taxValue());

It’s short, but not quite clear how all these values play together. If we know that the third argument of the Item constructor is the item price, the fourth is the tax rate and the expected result of taxValue() is the precise product of both, we can express it as follows:

$itemPrice = 6.99;
$taxRate = 0.19;
$item = $this->createItemWithPriceAndTax($itemPrice, $taxRate);
$this->assertEquals(
    $itemPrice * $taxRate,
    $item->taxValue(),
    "Tax value should be {$itemPrice} * {$taxRate}"
);

$itemPrice = 6.99;

$taxRate = 0.19;

$item = $this->createItemWithPriceAndTax($itemPrice, $taxRate);

$this->assertEquals(

$itemPrice * $taxRate,

$item->taxValue(),

"Tax value should be {$itemPrice} * {$taxRate}"

);

The item has other required attributes, but in this test we are only interested in price and tax rate, so we move instantiation into a creation method that takes the relevant values as arguments and fills the rest with defaults (see test smell Irrelevant Information). This reduces superfluous hard coded data and leaves us with two named values which are relevant to the test.

The information is also added to the assertion message, so that if it fails, we get something like

Failed asserting that 1 is equal to 1.3281.
Tax value should be 6.99 * 0.19

1 2	Failed asserting that 1 is equal to 1.3281. Tax value should be 6.99 * 0.19

Conclusion

Give those values a name by extracting them to variables or constants! You might have heard that temporary variables are evil, but hard coded values are even more evil. If I have to choose, I go for the solution that expresses its intent better, especially in tests. Name the child!

Test Smell: Hard Coded Values

Problem

Possible solutions

Conclusion

Further Reading

Author: Fabian Schmengler

Submit a Comment Cancel reply