Showing posts with label phpunit. Show all posts
Showing posts with label phpunit. Show all posts

2015-02-23

Unit testing headers in PHP

There's plenty of questions about unit testing code that sets headers using PHPUnit. PHPUnit officially assumes that neither the test code nor the tested code emit output or send headers:

(from https://github.com/sebastianbergmann/phpunit/issues/720#issuecomment-10399612)

And rightfully so. Headers are essentially global state, which is hard to unit test. However, if you're willing to install the PECL extension Runkit, you can actually unit test headers if you really want to.

I use the setupBeforeClass hook to only run the code once before the entire class runs. If you only have one method that you're testing it can all get stuck in the single test. I check if the runkit extension is loaded to avoid breaking the unit test suite if someone tries to run it that doesn't have runkit installed. The test that actually looks at the headers is marked with a requires annotation, which will mark that test as skipped if runkit isn't loaded. If every test in your test class tests headers, you can move that up to the class level if you'd like. My particular class had some tests that didn't need runkit and I wanted those to still run for people that didn't have runkit loaded.

2012-03-12

Unit testing code that uses static methods

Static calls are death to testability. There are ways of mocking out static calls made inside a class, but if you have some code that makes static calls outside a class, it's much more difficult. The best way is to refactor the static class to an instance class so you can mock things out.


But if you're working in a legacy code base with large and ugly static classes, refactoring them may be overly difficult or hard to justify from a business perspective. But you still want to test the new code that you're writing that uses those static classes. A coworker showed me one way to do it. It's still hacky and not as good as fixing the static class properly.

Basically instead of refactoring the crufty static code, you do the refactoring in the class you're writing. Here's an example class that uses the static Calculator class from above:


You can't mock out the Calculator class in this case. But you can make changes to your Foo class that will let you control the output from the static class. You can add protected methods to your class that call the static methods, and then mock those methods for testing and test the mock of the class you're testing.


Obviously this example is very simplistic, but in a more complicated class you could have the static class throw exceptions or different output or whatever.

Now you have two problems

This technique is far from perfect. If you were to change the behavior of the protected methods of your class, the unit tests would still pass, but the code would no longer work the same in production.

2011-12-19

One assertion per test


There's a right way and a wrong way to write unit tests. I've seen lots of wrong ways (like unit tests with no assertions) and differing shades of rightness. One thing that isn't really "wrong" but is not as right is writing your test cases with multiple assertions per test case.

Here's a simple example of a test with multiple assertions:
function testSomethingThatReturnsAnArray() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Foo', $value[0]);
    $this->assertEquals('Bar', $value[1]);
    $this->assertEquals('Mitz', $value[2]);
}
This could easily be rewritten to multiple assertions:
function testSomethingThatReturnsAnArrayFirstElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Foo', $value[0]);
}

function testSomethingThatReturnsAnArraySecondElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Bar', $value[1]);
}

function testSomethingThatReturnsAnArrayThirdElement() {
    $value = somethingThatReturnsAnArray();
    $this->assertEquals('Mitz', $value[2]);
}
What does this gain? You can immediately see whether somethingThatReturnsAnArray() is broken for all of the tests or just the first. With the first example, if $value[0] is not 'Foo', the test will immediately fail and you won't even test whether the next two elements are correct or not.


But the method I'm testing is slow!


If somethingThatReturnsAnArray() is so slow that you don't want to run it three times, you've got two options:

  • Make the method faster.
  • Only run it once per test suite run.
Obviously making the method faster would benefit your code the most overall, but it isn't always possible. But you can always just run it once, either by saving its output as a member variable of the test suite or making the tests depend on each other.

Saving the output is easy:
$result = null;
public function testSomethingSlowThatReturnsComplexResult() {
    $this->result = somethingThatReturnsComplexResult();
    $this->assertEquals('Foo', $this->result[0]);
}

public function testSomethingSlowThatReturnsComplexResultSecond() {
    $this->assertEquals('Bar', $this->result[1]);
}
But there is a problem. Tests shouldn't require a certain order unless it is explicitly set. If you move the second test method above the first it will start failing. You could have a helper method that only executes the function once:
$result = null;
private function runSomethingSlow() {
    if (!$this->result) {
        $this->result = somethingThatReturnsComplexResult();
    }
    return $this->result;
}

public function testSomethingSlowThatReturnsComplexResult() {
    $result = runSomethingSlow();
    $this->assertEquals('Foo', $result[0]);
}

public function testSomethingSlowThatReturnsComplexResultSecond() {
    $result = runSomethingSlow();
    $this->assertEquals('Bar', $result[1]);
}

Now the slow function only is executed a single time, and the test methods don't have an unnamed dependency on any other tests.


But my method is really complicated!

If you have a very complicated method that has lots of dependencies, you may be tempted to test actions on all of the dependencies at the same time as well as the return value so you don't have to have all of the boilerplate code repeated. I've heard some otherwise very talented coders make this argument, which always surprises me. Test code is not really different from production code in that if you find that you're repeating code several times, it should be factored out into a method.

Let's try testing everything about this method:
function compareAppleToOrange(Apple $apple, Orange $orange) {
    return array(
        'sizeDifference' => $apple->getSize()
                - $orange->getSize(),
        'peelDifference' => $apple->getPeelWidth()
                - $orange->getPeelWidth(),
    );
}
You could test it all in one big test:
function testCompareAppleToOrange() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));

    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(2));

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));

    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(3));

    $expected = array(
        'sizeDifference' => 1,
        'peelDifference' => -1,
    );
    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals($expected, $result);
}

Better yet, you can test each piece on its own:
function testCompareAppleToOrangeSize() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));

    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->any())
        ->method('getPeelWidth');

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));

    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals(1, $result['sizeDifference']);
}

function testCompareAppleToOrangePeel() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));

    $apple->expects($this->any())
        ->method('getSize');
    $apple->expects($this->once())
        ->method('getPeelWidth')
        ->will($this->returnValue(2));

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));

    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    $result = compareAppleToOrange($apple, $orange);
    $this->assertEquals(-1, $result['peelDifference']);
}
This allows you to change the peel calculation without touching the test for size, or the size calculation without touching the peel calculation.

If you wanted to really split the test up, you can go even further since the mocks  provide some assertions of their own with the expects() calls. We'll split up the sizeDifference tests:

/**
 * @return array Array from apples to orange comparison.
 */
function testCompareAppleToOrangeSizeCalls() {
    $apple = $this->getMock('Apple',
        array('getSize', 'getPeelWidth'));
    $apple->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(4));
    $apple->expects($this->any())
        ->method('getPeelWidth');

    $orange = $this->getMock('Orange',
        array('getSize', 'getPeelWidth'));
    $orange->expects($this->once())
        ->method('getSize')
        ->will($this->returnValue(3));
    $orange->expects($this->any())
        ->method('getPeelWidth');

    return compareAppleToOrange($apple, $orange);
}

/**
 * @depends testCompareAppleToOrangeSizeCalls
 * @param array $result Result of comparison.
 */
function testCompareAppleToOrangeSize($result) {
    $this->assertEquals(1, $result['sizeDifference']);
}
Each split gives you more information on a failure and makes debugging test failure easier as well as reducing the changes required for refactoring.

2011-11-10

Building rock solid software in the real world

http://www.flickr.com/photos/preef/32995286/
Recently (2011-11-08) I gave a talk at the Dallas PHP meetup about building rock solid software as a team. For my first experience talking in front of a crowd since high school, I thought it went pretty well. Several people have asked for me to post my slides (which I did), but they were made in a way that doesn't really help people out if they didn't see the talk. The talk was recorded and is available on Ustream, but I thought it might be helpful to do a blog post on the topics I covered as well.

This post is mainly meant to aggregate links to the topics that I talked about.


Tools


I covered several tools. All of these should be available to the developers as build targets and run in your continuous build. Lint and your unit tests should be run as part of your submission process.

  • lint - The bare minimum, it just detects syntax errors in your scripts. Code that doesn't pass the lint test won't pass any other tests or manual QA.
  • PHPUnit - Standard unit testing framework. There is plenty of information about it elsewhere.
  • PHP Code Sniffer - Detects code smells that should be fixed. Many bad programming practices can be written as "sniffs" along with most rules from your smile guide.
  • PHP Mess Detector - Statically analyzes your code for possible bugs or coding practices that tends to hide bugs.
  • PHP Copy Paste Detector - Scans your code to find large similar blocks which can be factored out to a common method.
  • PHP Dead Code Detector - Scans your code to find code that can not be reached. For example, code after a return statement.
  • Code coverage - Adding the xdebug extension to your system allows PHPUnit to calculate how much of your code is run by unit tests.


Code reviews

I talked about two different code review packages:
And I talked about three different ways of doing reviews:
  • Pre-review - Code doesn't get submitted until a peer reviews it. Keeps bad stuff out of your code base.
  • Post-review - Code gets submitted, then gets peer reviewed. Comments made about the code may never get resolved, but code reviews don't slow down getting code into production.
  • Public shaming - Put the code up on a projector and discuss as a team. Great way to destroy programmer morale.
I mentioned a few points about what to look for in a code review:
  • anything the tools couldn't catch
    • logic errors (like ifs that don't make sense)
    • loops with off-by-one errors
    • performance problems (SQL in a loop)
    • things to refactor (large methods)
  • or things they missed
    • Style problems (not really wrong, but you know, wrong)
    • Typos (variable names, documentation)
    • Tests that don't have assertions
    • Methods without tests


Style guides

There's two ways to choose a style guide:
  1. Roll your own - Look at your existing code and build the style guide from what you're already doing.
  2. Use existing - Such as Zend or Pear.

2010-09-28

Unit Testing cURL Code in PHP

My team is writing an API that will be used from other parts of our infrastructure. We're using cURL to make connections from the other parts to the API. But sometimes our network and API server are a bit flaky, which makes code that calls the API brittle if we don't handle those transient errors.

Instead of using PHP's native curl_* functions, we've created a simple class to wrap them. This helps some of the more junior team members as well, hiding some of the nitty-gritty details of making HTTP connections:


/**
 * Class abstracting the cURL library for easier use.
 *
 * Usage:
 *     $curl = new Curl();
 *     $curl->setUrl('http://www.google.com/#')
 *         ->setData('&q=testing+curl')
 *         ->setType('GET');
 *     $curl->send();
 *     echo $curl->getStatusCode(), PHP_EOL;
 *     echo $curl->getResponse(), PHP_EOL;
 */
class Curl {
    /**
     * @var string Body returned by the last request.
     */
    protected $body;

    /**
     * @var resource Actual CURL connection handle.
     */
    protected $ch;

    /**
     * @var mixed Data to send to server.
     */
    protected  $data;

    /**
     * @var integer Response code from the last request.
     */
    protected $status;

    /**
     * @var string Request type.
     */
    protected $type;

    /**
     * @var string Url for the connection.
     */
    protected $url;


    /**
     * Constructor.
     */
    public function __construct() {
        $this->body = null;
        $this->ch = curl_init();
        $this->data = null;
        $this->status = null;
        $this->type = 'GET';
        $this->url = null;
        curl_setopt($this->ch, CURLOPT_RETURNTRANSFER,
            true);
        curl_setopt($this->ch, CURLOPT_USERAGENT,
            'Curl Client');
    }

    /**
     * Return the body returned by the last request.
     * @return string
     */
    public function getBody() {
        return $this->body;
    }

    /**
     * Return the current payload.
     * @return mixed
     */
    public function getData() {
        return $this->data;
    }

    /**
     * Set the payload for the request.
     *
     * This can either by a string, formatted like a query
     * string:
     *      foo=bar&mitz=fah
     * or a single-dimensional array:
     *      array('foo' => 'bar', 'mitz' => 'fah')
     * @param mixed $data
     * @return Curl
     */
    public function setData($data) {
        if (is_array($data)) {
            $data = http_build_query($data);
        }
        $this->data = $data;
        return $this;
    }

    /**
     * Return the status code for the last request.
     * @return integer
     */
    public function getStatusCode() {
        return $this->status;
    }

    /**
     * Return the current type of request.
     * @return string
     */
    public function getType() {
        return $this->type;
    }

    /**
     * Set the type of request to make (GET, POST, PUT,
     * DELETE, etc)
     * @param string $type Request type to send.
     * @return Curl
     */
    public function setType($type) {
        $this->type = $type;
        curl_setopt($this->ch, CURLOPT_CUSTOMREQUEST,
            $type);
        return $this;
    }

    /**
     * Return the connection's URL.
     * @return string
     */
    public function getUrl() {
        return $this->url;
    }

    /**
     * Set the URL to make an HTTP connection to.
     * @param string $url URL to connect to.
     * @return Curl
     */
    public function setUrl($url) {
        $this->url = $url;
        curl_setopt($this->ch, CURLOPT_URL, $url);
        return $this;
    }

    /**
     * Send the request.
     * @return Curl|null
     */
    public function send() {
        if (!$this->url) {
            return null;
        }
        if ('GET' == $this->type) {
            $this->url .= '?' . $this->data;
        } else {
            curl_setopt($this->ch, CURLOPT_POSTFIELDS,
                $this->data);
            if ('PUT' == $this->type) {
                $header = 'Content-Length: '
                    . strlen($this->data);
                curl_setopt($this->ch, CURLOPT_HTTPHEADER,
                    array($header));
            }
        }
        $this->body = curl_exec($this->ch);
        $this->status = curl_getinfo($this->ch,
            CURLINFO_HTTP_CODE);
        return $this;
    }
}



First, here's an example class that calls to the API to pull a customer record. Assume the API returns a JSON object when called with http://api/customer/42. We pass in a Curl object to actually make the connection, then parse the response to fill our customer object's members.


class Customer {
    /**
     * @var integer Database ID.
     */
    public $id;

    /**
     * @var string Customer's name.
     */
    public $name;

    /**
     * Load a customer from the API.
     * @param integer $id ID of the customer to load.
     * @param Curl $curl Curl object.
     */
    public function load($id, Curl $curl) {
        $curl->setUrl('http://api/customer/' . (int)$id)
            ->send();
        $customer = json_decode($curl->getResponse());
        $this->id = $customer->id;
        $this->name = $customer->name;
    }
}


In a perfect world, calling load() will always populate the id and name members of the customer. In the real world, the API server could be down, or the network could die, or random alpha particles could mess up the request, so the code should handle those error messages. But setting up a test network to create those conditions is expensive and probably a waste of time, so we'll create a stub for the Curl object that allows us to break the API calls in interesting ways.


/**
 * Stub for the Curl class.
 *
 * Allows testing code that uses cURL without actually
 * making any HTTP connections.
 */
class StubCurl
extends Curl {
    /**
     * Constructor.
     */
    public function __construct() {
        $this->body = null;
        $this->data = null;
        $this->status = null;
        $this->type = 'GET';
        $this->url = null;
    }

    /**
     * Set the response 'returned' by the server.
     * @param string $body
     * @return StubCurl
     */
    public function setBody($body) {
        $this->body = $body;
    }

    /**
     * Set the HTTP status 'returned' by the server.
     * @param integer $code
     * @return StubCurl
     */
    public function setStatusCode($status) {
        $this->status = $status;
    }

    /**
     * Set the request method (GET, POST, etc).
     * @param string $type
     * @return StubCurl
     */
    public function setType($type) {
        $this->type = $type;
        return $this;
    }

    /**
     * Set the URL to connect to.
     * @param string $url
     * @return StubCurl
     */
    public function setUrl($url) {
        $this->url = $url;
        return $this;
    }

    /**
     * 'Send' the cURL request.
     *
     * Obviously doesn't actually send any requests.
     * @return StubCurl
     */
    public function send() {
        if (!$this->url) {
            return null;
        }
        if ('GET' == $this->type) {
            $this->url .= '?' . $this->data;
        }
        return $this;
    }
}


By passing a StubCurl object in to Customer::Load(), we can make the server return any status (or no status at all):


require_once 'Curl.php';
require_once 'Customer.php';
require_once 'StubCurl.php';

class CustomerTest
extends PHPUnit_Framework_TestCase {
    private $customer;

    protected function setUp() {
        $this->customer = new Customer();
    }

    /**
     * Test loading with a good API connection.
     * @covers Customer::load
     * @test
     */
    public function testLoadFound() {
        $curl = new StubCurl();
        $curl->setStatusCode(200)
            ->setBody('{"id":42,"name":"Bob King"}');
        $this->customer->load(42, $curl);
        $this->assertEquals('Bob King',
            $this->customer->name);
    }

    /**
     * Test loading a customer that isn't found.
     * Add a test for whatever behavior you want the
     * customer object to take if the API returns a 404
     * Not Found error. In this case, we're going to want
     * the customer object to throw an exception.
     * @covers Customer::load
     * @expectedException NotFoundException
     * @test
     */
    public function testLoadNotFound() {
        $curl = new StubCurl();
        $curl->setStatusCode(404);
        $this->customer->load(42, $curl);
    }

    /**
     * Test loading a customer from bad JSON.
     * The API returns a garbled response body.
     * @covers Customer::load
     * @expectedException BadResponseException
     * @test
     */
    public function testBadResponse() {
        $curl = new StubCurl();
        $curl->setStatusCode(200)
            ->setBody('{"id":_*()*(*$#@#$');
        $this->customer->load(42, $curl);
    }
}


Then the Customer class needs to have its load() method beefed up to handle the errors:


/**
 * Load a customer from the API.
 * @param integer $id ID of the customer to load.
 * @param Curl $curl Curl object.
 * @throws BadResponseException
 * @throws NotFoundException
 */
public function load($id, Curl $curl) {
    $curl->setUrl('http://api/customer/' . (int)$id)
        ->send();
    if (404 == $curl->getStatusCode()) {
        throw new NotFoundException();
    }
    $customer = json_decode($curl->getResponse());
    if (null === $customer) {
        throw new BadResponseException();
    }
    $this->id = $customer->id;
    $this->name = $customer->name;
}


Obviously you'll want to handle more errors in a production environment, maybe logging bad responses and server errors for later analysis.

2010-08-23

Forcing PHPUnit tests on Subversion commit

All of my PHP projects use PHPUnit for unit testing. One of my coworkers just refuses to understand the point of testing. He doesn't get Test Driven Development (TDD), and rarely runs the unit tests for his classes. This means that he frequently breaks my build. I can't force him to change his behavior since he doesn't work for me, but I can change the way my systems work. And I control the Subversion server.
I wanted to force unit tests to pass before allowing submission, but couldn't find anything about running PHPUnit tests in a Subversion pre-commit hook. So I wrote my own. Hopefully this will help someone out there:
To use this, copy it to the hooks directory in your Subversion repository and name it 'pre-commit'. Make sure the paths defined in the script are correct for your system. Make it executable (chmod +x pre-commit).