Tuesday, July 3rd, 2012

Unit Testing with Static Variables

By , Staff Engineer

Unit testing is an obvious best practice, but it is not easy to introduce to legacy code.  I’ve found some great advice here, particularly in Clean Code by Robert Martin.  However, one issue he didn’t cover was testing code with lots of static variables.  Aside from being a general bad practice and hard to mock, static variables can make unit tests unreliable.

The fundamental problem is that PHP web requests run in isolation but PHPUnit tests do not.  Normally, you can assume that all classes are reloaded per request, but PHPUnit normally runs all tests in the same memory space, meaning that static variables now persist across test runs.  This introduces dependencies between tests and causes the dreaded situation in which a test passes individually but fails in a suite.  This is a debugging nightmare.

Three obvious solutions are to eliminate the static variables, write pure unit tests that mock all dependencies, or to use with PHPUnit’s  –process-isolation flag to remove any possible contamination between tests.  All three are great goals, but require immense refactoring and time to achieve.  Meanwhile, we need to add test coverage to our legacy code before we can refactor it.  At Box, we decided to compromise: enable the tests to clean up after themselves.

Let’s look at a trivial example:

class Foo
{
private static $threshold = 5;

public static function is_past_threshold($value)
{
return $value > self::$threshold;
}
}

class Foo_Test extends Box_Base_Test_Case
{
public function test_is_past_threshold_returns_true_when_value_is_past_threshold()
{
// we don't want to depend on threshold being 5,
<p dir="ltr">// ...so set it explicitly in the test</p>
$this->set_static_variable($class = 'Foo', $name = 'threshold', 100);

$is_past_threshold = Foo::is_past_threshold(101);
$this->assertTrue($is_past_threshold, '101 is larger than threshold');
}
}

This is great, except that Foo::$threshold is now set to 100 for all subsequent tests.  If a test assumes the threshold is still 5, then it could fail if run after this test, but will pass on its own.  Unreliable tests are the bane of agile development.

So, how do achieve test reliability without embarking on a perilous journey? Our solution is to cache the original values of the variables that we are changing. This involves changing the base test class method to set a static variable and changing the base class tear down method. See below:

class Box_Base_Test_Case
{
....

protected function set_static_variable($class, $variable_name, $new_value)
{
// if we haven't already done so, remember the original value for later
if (!array_key_exists($class, $this->values_to_restore)
<p dir="ltr">|| array_key_exists($variable_name, $this->values_to_restore[$class]))</p>
{
$this->values_to_restore[$class][$variable_name] =
<p dir="ltr">$this->get_static_variable($class, $variable_name);</p>
}

// (uses an easy helper to abstract the messy reflection code)
Reflection_Helper::set_static_variable($class, $name, $new_value);
}

protected function tearDown()
{
…

// Restore the values
foreach ($this-> values_to_restore as $class => $variables_to_restore)
{
foreach ($variables_to_restore as $name => $original_value)
{
Reflection_Helper::set_static_variable($class, $name, $original_value);
}
}

$this->values_to_restore = array();
}

The result is an ability to test code that otherwise is untestable. All the while keeping our tests stable and fast.  With this test coverage in place, we can now safely refactor our legacy code to follow best practices and hopefully avoid this situation altogether.

By ,

Staff Engineer

See all of Ben's articles.