From functional tests to unit tests
2012-10-17Sometimes I run into tests that look like unit tests, but are really functional, integration, or smoke tests instead. An example of what that might look like is below. I'm using pseudocode to avoid messing with auth and such:
class TwitterService
{
public function getUser($screenName)
{
$response = file_get_contents('https://api.twitter.com/1.1/users/show.json?screen_name=' . $screenName);
return json_decode($response);
}
}
Then the test case would be something like this:
class TwitterServiceTest extends PHPUnit_Framework_TestCase
{
$protected $object;
public function setUp()
{
$this->object = new TwitterService();
}
public function testTwitterUserShowReturnsName()
{
$user = $this->object->getUser('rsarver');
$this->assertAttributeExists('name', $user);
$this->assertAttributeEquals('Ryan Sarver', 'name', $user);
}
}
So at first glance this may seem like a reasonable test, but this is not a true unit test because it relies on an external service. What would happen if the Twitter API was down? Your test would fail, through no fault of your own code. I ran into this a few months back with a test that was calling a service internal to our network that happened to be down for maintenance. In addition, this type of test adds extra latency to your test run because you have to wait for an HTTP call to return. If your tests are slow, people won't run them.
A good rule of thumb is, if you're going to run your unit tests, turn off your network connection first. Any test that suddenly fails is not a unit test. You obviously don't have to do this every time, but it's good to try it at least once or twice.
If you do find tests like this, you have a few options. First of all, the affected code is probably still performing a useful test, it just doesn't belong with the unit tests. This test is most likely a smoke test for your external service integration. In the meantime, you can still test the "name retrieval" portion of the code by wrapping the call to the Twitter API in a class that we can mock to return a predetermined response.
Here we're injecting a dependency into the TwitterService
, and this collaborating object actually handles the HTTP traffic. This is better separation of concerns, and it allows you to update the test case to provide the TwitterService
with a mocked implementation of your HTTP adapter. I didn't create an HTTP Adapter below, just assume I'm using something like pecl_http, Buzz, or Guzzle:
class TwitterService
{
/*
* @var HttpInterface
*/
protected $adapter = null;
public function __construct(HttpInterface $httpAdapter)
{
$this->adapter = $httpAdapter;
}
public function getUser($screenName)
{
$response = $this->adapter->get('https://api.twitter.com/1.1/users/show.json?screen_name=' . $screenName);
return json_decode($response);
}
}
class TwitterTest extends PHPUnit_Framework_TestCase
{
$protected $object;
public function setUp()
{
$mockHttpAdapter = $this->getMock('HttpAdapter');
$mockHttpAdapter->expects($this->atLeastOnce())
->method('getUser')
->with('rsarver')
->will($this->returnValue('...json blob omitted...'));
$this->object = new TwitterService();
}
public function testTwitterGetUserReturnsName()
{
$user = $this->object->getUser('rsarver');
$this->assertAttributeExists('name', $user);
$this->assertAttributeEquals('Ryan Sarver', 'name', $user);
}
}
This follows a second rule of thumb about writing clean unit tests: Every object in your test suite that is not actively being tested (or a value object like DateTime) should be a mock. This helps ensure your unit tests aren't testing anything you don't specifically tell them to test.
TL;DR
- Unit tests should run without a network connection. If they don't, they're not unit tests
- Any object in your test suite that is not actively being tested (or is a value object) should be a mock