My first model test in PHPUnit

Question

I've just created a test to create my table gateway class. I've written about 8 tests and all are passing. I'm hoping anyone can offer any advice on what to do next to make these better. This is quite a small app and just one of my own, but at work I want to write tests for much bigger applications so concerned with performance and writing tests faster.

I'm querying a test database for this. As you can see I clean the user table on every test.

<?php

use app\models\UserTable;

/**
*    UserTableTest
*/
class UserTableTest extends PHPUnit_Framework_TestCase
{
    protected $userTable;

    public function setUp()
    {

        // get the db adapter and set the userTable object

        $dbAdapter = new framework\Db\DatabaseAdapter(array(
            'dsn' => $GLOBALS['DB_DSN'],
            'user' => $GLOBALS['DB_USER'],
            'password' => $GLOBALS['DB_PASSWD'],
        ));

        $this->userTable = new UserTable($dbAdapter);

        // clean table on every test so we start clean
        $this->userTable->delete(array('id <> 999999999'));
    }

    public function testCreateUser()
    {
        // get default values for new user
        $values = $this->getUserValues();

        // count number of records before create
        $before = count($this->userTable->fetchAll());

        // assert user is created
        $result = $this->userTable->create($values);
        $this->assertTrue($result);

        // check users has been incremented
        $after = count($this->userTable->fetchAll());
        $this->assertGreaterThan($before, $after);
    }

    /**
    * @depends testCreateUser
    */
    public function testCreateUserDoesntInsertDuplicateEmail()
    {
        // new user to be created
        $values = $this->getUserValues();

        // assert user is created
        $result = $this->userTable->create($values);
        $this->assertTrue($result);

        // try again with same email (should be false)
        $result = $this->userTable->create($values);
        $this->assertFalse($result);
    }

    /**
    * @depends testCreateUser
    */
    public function testFetchUserByCriteria()
    {
        // create new user
        $values = $this->getUserValues();
        $user = $this->createAndFetchUser($values);

        $this->assertEquals($user['email'], $values['email']);
    }

    /**
    * @depends testCreateUser
    * @depends testFetchUserByCriteria
    */
    public function testFindUserByPrimaryKey()
    {
        // create new user
        $values = $this->getUserValues();
        $user = $this->createAndFetchUser($values);

        // retreive by pk
        $foundUser = $this->userTable->find($user['id']);

        $this->assertEquals($user['id'], $foundUser['id']);
    }

    /**
    * @depends testCreateUser
    * @depends testFetchUserByCriteria
    */
    public function testCreateUserAutomaticallySetsCreatedAndUpdatedDatetime()
    {
        // new user to be created
        $values = $this->getUserValues(array(
            'date_created' => null,
            'date_updated' => null,
        ));
        $user = $this->createAndFetchUser($values);

        $this->assertTrue(! empty($user['date_created']));
        $this->assertTrue(! empty($user['date_updated']));
    }

    /**
    * @depends testCreateUser
    * @depends testFetchUserByCriteria
    */
    public function testUpdateUserByCriteria()
    {
        // new user to be created
        $values = $this->getUserValues();
        $user = $this->createAndFetchUser($values);

        // attempt to update row

        $values = array('email' => '[email protected]');
        $where = array('id = ?', array($user['id']));
        $result = $this->userTable->update($values, $where);

        $this->assertTrue($result);

        // fetch row and verify new column value

        $user = $this->userTable->fetch($where, array(
            'limitMax' => 1,
        ))[0];

        $this->assertEquals($user['email'], $values['email']);
    }

    /**
    * @depends testCreateUser
    * @depends testUpdateUserByCriteria
    * @depends testFetchUserByCriteria
    */
    public function testUpdateUserAutomaticallySetsUpdatedDatetime()
    {
        // create user to retreive
        $values = $this->getUserValues();
        $user = $this->createAndFetchUser($values);

        sleep(1); // delay so that dates will be different

        // attempt to update row

        $values = array('email' => '[email protected]');
        $where = array('id = ?', array($user['id']));
        $result = $this->userTable->update($values, $where);

        // fetch row and check the dates
        $user = $this->userTable->fetch($where, array(
            'limitMax' => 1,
        ))[0];

        $this->assertGreaterThan($user['date_created'], $user['date_updated']);
    }

    /**
    * @depends testCreateUser
    * @depends testFetchUserByCriteria
    */
    public function testDeleteUserByCriteria()
    {
        // create user to be deleted
        $values = $this->getUserValues();
        $user = $this->createAndFetchUser($values);

        $where = array('id = ?', array($user['id']));

        $result = $this->userTable->delete($where, array(
            'limitMax' => 1,
        ));

        $this->assertTrue($result);

        // let's try and fetch it

        $users = $this->userTable->fetch($where, array(
            'limitMax' => 1,
        ));

        $this->assertEmpty($users);
    }

    // protected functions

    protected function getUserValues(array $values=array())
    {
        // defaults
        $user = array(
            'name' => 'Martyn',
            'email' => md5(time()) . '@gmail.com',
            'password' => 'password',
            'date_created' => date("Y-m-d H:i:s"),
            'date_updated' => date("Y-m-d H:i:s"),
        );

        // overwrite defaults above with those passed in
        foreach($values as $key => $value) {
            if(array_key_exists($key, $user)) $user[$key] = $value;
        }

        return $user;
    }

    public function createAndFetchUser($values)
    {
        // create new user
        $result = $this->userTable->create($values);

        // fetch the new user
        $user = $this->userTable->fetch(array('email = ?', array($values['email'])), array(
            'limitMax' => 1,
        ))[0];

        return $user;
    }
}

Answer 1

First off: writing tests in such a way that they pass defeats the point of writing tests. You write tests, to see if your code behaves as expected. You say you wrote 8 tests, and all of them are passing. That's a slightly worrying attitude. You should say "I wrote a test, and my coding is passing".

Anyway: if you want to improve your tests, you really ought to begin by reading the manual. It's great to see people actually writing tests, but from your code, I deduce you've not yet spent enough time reading PHPUnit's actual documentation.
I don't see you using/mentioning fixtures, and you extend what is obviously a DB test from the generic PHPUnit_Framework_TestCase class.

If you are testing DB-related code, use the correct test-cases

There is this aptly named PHPUnit_Extensions_Database_TestCase that better fits your requirements. For obvious reasons. Use it, to create an abstract base test-case class. This abstract test class should handle the connection stuff, and enforce all the child classes to load actual fixtures.
Just like in coding, in tests, you should keep your logic and your data separated. The test data can be generated during the build process, for example, or written manually. Either way, it shouldn't be mixed in with the testing code.

If you do mix in the testing data with the testing code, programmers being lazy and all, you're in with a very real chance that failing tests will be "fixed" by tinkering with the data used for testing, rather than the code itself. Unit tests are about code-coverage, not data-coverage...

SRP in test methods, too
The SRP (Single Responsability Principle) applies to test-methods, too. Your testCreateUser method, for example is testing just a bit too much to my liking. You've defined another test-method that depends on testCreateUser, let that method test if the user data was inserted correctly. Then, create another test method that depends on that, which actually checks to see if the inserted data is correct.

After that, check all possible scenario's concerning new inserts and updates. All possible scenario's.

Test failing calls

When I say "all possible scenario's", I really do mean all scenario's. At first glance, I don't see you testing your code's behaviour in cases where an InvalidMethodCall or InvalidArgument exception could (and indeed should) be thrown. Add tests to make sure your code behaves as expected in case something goes wrong. Something along the lines of:

/**
 * @expectedException     MyException
 */
public function testBadEmailSet()
{
    $model = new User();//my user data-model
    $model->setEmail('this is not an email address');//test passes if exception is thrown
}

Connecting to a test-DB
All in all, I'm not entirely against this practice. But, as ever, you should keep a few things in mind when doing so:

• All changes to the actual DB have to be applied to your test DB, too
• DB migrations are a pain, and easily forgotten (ie keeping DB and test DB in sync is easily overlooked, double the nr of DB's == twice as likely to have a build fail)
• Builds will be slower, and the server load will whenever you deploy/build/test
• Murphy's law: builds fail. Fact. What if migrations fail mid-way? Maintaining databases is a full-time job, just ask any DBA.
• designated test db's are useful if you want to test db-specific things like "I tested code X, but will it perform as expected on a cluster?". That's when separate test DB's come in.

Other than that, keep at it. Writing tests can be tedious, but it's all part of the job. And when things do, inevitably go wrong, they are a godsend.

Answer 2

Slow vs. fast tests

This test is testing not only your table gateway class, but also your database. Thus it is going to be slow because it has to prepare the test environment before each test and clean up afterwards. This kind of test is still good to have, it would catch integration errors between your gateway class and your database environment. I call them integration tests to distinguish them from the faster unit tests. The definition of a unit test by Michael Feathers helps drawing the line.

A test is not a unit test if:

• It talks to the database
• It communicates across the network
• It touches the file system
• It can't run at the same time as any of your other unit tests
• You have to do special things to your environment (such as editing config files) to run it.

Tests that do these things aren't bad. Often they are worth writing, and they can be written in a unit test harness. However, it is important to be able to separate them from true unit tests so that we can keep a set of tests that we can run fast whenever we make our changes.

The point here is to split execution of your tests into two or more stages: first run unit tests, then if they all pass run the integration tests, then if they all pass run the other tests (system, acceptance, etc.) and so on. This way you preserve a short feedback loop to quickly notice failed tests. Moreover you are not bored by having to run long tests all the time (which leads to people not running the tests at all).

In your case, I would suggest you first test the table gateway class in isolation by passing it a mock database adapter. Then use this mock object to assert that the gateway generates the correct queries. This will run much faster than anything connected to a database. For example, you can use PHPUnit to run your unit tests, then use Codeception (thanks to @EliasVanOotegem for the suggestion) to run integration tests, thus splitting into two test runs.