HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Mock user table class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usermockclasstable

Problem

I've just created my first tests using mock classes. So far my tests are running much faster and now I guess I'm solely focusing on a single class (in this case I'm testing UserTable and mocking the DatabaseAdapter instance I inject into it)

I haven't proceeded to do the write tests for the other methods of this class (just testing "create" method for now), I was hoping someone could comment on my tests for this method. It simply creates a new record in the user table. I'm trying to test all possibilities that I should be testing. I'm a little new to this and not quite sure if I've got it right yet.

```
dbAdapter = $this->getMockBuilder('framework\Db\DatabaseAdapter')
->disableOriginalConstructor()
->getMock();

// create our instance of the class to be tested
$this->userTable = new UserTable;
}

// create method

public function testCreateMethodWhenSuccessfullyInsertsRow()
{
// set mock class

// set the mock object's methods for this test
$this->dbAdapter->expects( $this->any() )
->method('insert')
->will( $this->returnValue(true) );

$this->userTable->setDatabaseAdapter($this->dbAdapter);

// perform test

// assert user is created when valid argument is passed
$result = $this->userTable->create( array() );

$this->assertTrue($result);
}

public function testCreateMethodWhenFailsToInsertsRow()
{
// set mock class

// set the mock object's methods for this test
$this->dbAdapter->expects( $this->any() )
->method('insert')
->will( $this->returnValue(false) );

$this->userTable->setDatabaseAdapter($this->dbAdapter);

// perform test

// assert user is created when valid argument is passed
$result = $this->userTable->create( array() );

$this->assertFalse($result);
}

/**
* @expectedException InvalidArgumentExcepti

Solution

Except for some (mostly experience related issues) I commend your efforts. Well done!
The Short Version
Specific Points

  • use app\models\User; is not needed as the User class is not used (or mocked) in this test.



  • The comments before your asserts don't always seem to match what you are actually asserting.


I think you'd be better of using those comments as the test method name rather that having them as a comment in the function.

  • On the subject of names, I'd suggest using words like should and when to make the test method name state exactly what it expects the system under test to do. That way when a test fails you'll rarely have to look in the test to understand what is broken.



  • The first two test methods are basically testing the same thing. Whether or not the output of the create method matches that of the insertRow mock.


This could be simplified into one single test method fed by a dataProvider, more on this below.

  • You don't need to catch the result of the create in the methods that test for exceptions. Unused variables in code are bad and test code is still code.



Although it is impossible to tell without the class under test, you seem to have covered all logic paths:

  • Insert Succeeds



  • Insert Fails



  • Valid Arguments Provided (implicitly through previous methods)



  • Invalid Arguments Provided



  • DbAdapter Provided (implicitly)



  • DbAdapter Not Provided



You could consider making the implicit tests more explicit.

Incorporating these points your code would look a bit like this:


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

    /**
     * create our instance of the class to be tested
     */
    final public function setUp()
    {
        $this->userTable = new UserTable;
    }

    /////////////////////////////////// TESTS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     * 
     * @dataprovider provideBooleans
     */
    final public function testUserTableShouldOnlyCreateUserWhenInsertRowSucceeds($insertRowStatus)
    {
        $userTable = $this->userTable;

        // set mock class
        $mockDbAdapter = $this->getMockAdapter();
        
        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue($insertRowStatus) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // assert user is created when valid argument is passed
        $actual = $userTable->create( array() );

        $this->assertEquals($insertRowStatus, $actual);
    }

    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     * 
     * @expectedException InvalidArgumentException
     *
     * @dataprovider provideInvalidArgumentTypes
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenValidArguments($invalidArgumentType)
    {
        $userTable = $this->userTable;
        // set mock class
        $mockDbAdapter = $this->getMockAdapter();

        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue(false) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // pass invalid argument, should be an array of values
        $userTable->create($invalidArgumentType);
    }

    /**
     * @covers ::create
     * 
     * @expectedException RuntimeException
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenDatabaseAdapter()
    {
        $userTable = $this->userTable;
        $userTable->create( array() );
    }
    
    ////////////////////////////// MOCKS AND STUBS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function getMockAdapter()
    {
        // create mock database adapter
        $mockDbAdapter = $this->getMockBuilder('framework\Db\DatabaseAdapter')
            ->disableOriginalConstructor()
            ->getMock()
        ;
    }

    /////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function provideBooleans() 
    {
        return array(
            'true' => array(true),
            'false' => array(false),
        );
    }
    
    final public function provideInvalidArgumentTypes() 
    {
        return array(
            'string' => array('invalid argument type'),
            'boolean - false' => array(false),
            'boolean - true' => array(true),
            'null' => array(null),
        );
    }    
}


The Long Version

Should you feel in the mood for a bit more depth and education, I have a more thorough review below.
These are all things I'd point out to my teammates, if this code had passed my desk at work.

This may be a bit much to take in, keep in mind I'm a rather critical developer an

Code Snippets

<?php

namespace app\models;

/**
 * Tests for the UserTable class
 *
 * @coversDefaultClass app\models\UserTable
 * @covers ::<!public>
 */
class UserTableTest extends PHPUnit_Framework_TestCase
{
    ////////////////////////////////// FIXTURES \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    protected $userTable;


    /**
     * create our instance of the class to be tested
     */
    final public function setUp()
    {
        $this->userTable = new UserTable;
    }

    /////////////////////////////////// TESTS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     * 
     * @dataprovider provideBooleans
     */
    final public function testUserTableShouldOnlyCreateUserWhenInsertRowSucceeds($insertRowStatus)
    {
        $userTable = $this->userTable;

        // set mock class
        $mockDbAdapter = $this->getMockAdapter();
        
        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue($insertRowStatus) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // assert user is created when valid argument is passed
        $actual = $userTable->create( array() );

        $this->assertEquals($insertRowStatus, $actual);
    }

    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     * 
     * @expectedException InvalidArgumentException
     *
     * @dataprovider provideInvalidArgumentTypes
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenValidArguments($invalidArgumentType)
    {
        $userTable = $this->userTable;
        // set mock class
        $mockDbAdapter = $this->getMockAdapter();

        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue(false) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // pass invalid argument, should be an array of values
        $userTable->create($invalidArgumentType);
    }

    /**
     * @covers ::create
     * 
     * @expectedException RuntimeException
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenDatabaseAdapter()
    {
        $userTable = $this->userTable;
        $userTable->create( array() );
    }
    
    ////////////////////////////// MOCKS AND STUBS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function getMockAdapter()
    {
        // create mock database adapter
        $mockDbAdapter = $this->getMockBuilder('framework\Db\DatabaseAdapter')
            ->disableOriginalConstructor()
            ->getMock()
        ;
    }

    /////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function provideBooleans() 
    {
        return array(
            'true' => array(true),
            'false' => array(false),
        );
   
/**
  * @expectedException        MyException
  * @expectedExceptionMessage MyClass::ERROR_MESSAGE
  */

Context

StackExchange Code Review Q#59925, answer score: 6

Revisions (0)

No revisions yet.