Are my Unit Tests any good?

This is the first time I have written Unit Tests. I am testing code that I have already written and would like to move to TDD once I have managed to get units written for everything that is already in my application.

Just wanted to be sure that the tests that I have written look good, or if there is a different approach I should be taken. These are for CakePHP 2.0

```
Player = ClassRegistry::init('Player');
}

/**
* testGetPlayers method
*
* @return void
*/
function testGetPlayers() {

//Specific Player, All Stats, No Position Exclusion
$result = $this->Player->GetPlayers(11, 'all', null);
$expected = array(
'Player' => array ('id' => 11, 'name' => 'Forward 2', 'status' => 'doubtful', 'note' => '', 'club_id' => '2', 'position_id' => '4'),
'Club' => array ('id' => 2, 'name' => 'Club 2', 'abbreviation' => 'clb2'),
'Position' => array ('id' => 4, 'name' => 'Forward', 'abbreviation' => 'fw', 'min' => 1, 'max' => 3, 'need' => 2, 'db_abbr' => 'fw'),
'Stat' => array (
array ('id' => 11, 'rank' => 11, 'goals' => 0, 'saves' => 0, 'penaltysaves' => 0, 'assists' => 0, 'cleansheet' => 0, 'redcard' => 0, 'yellowcard' => 0, 'foulsconc' => 0, 'foulswon' => 0, 'tackleslost' => 0, 'tackleswon' => 0, 'shotontarget' => 0, 'totalshot' => 0, 'appearances' => 0, 'attcreated' => 0, 'totalpasses' => 0, 'accpasses' => 0, 'minutesplayed' => 0, 'conceded' => 0, 'player_id' => '11', 'team_id' => 0, 'period_id' => '1'),
array ('id' => 23, 'rank' => 11, 'goals' => 0, 'saves' => 0, 'penaltysaves' => 0, 'assists' => 0, 'cleansheet' => 0, 'redcard' => 0, 'yellowcard' => 0, 'foulsconc' => 0, 'foulswon' => 0, 'tackleslost' => 0, 'tackleswon' => 0, 'shotontarget' => 0, 'totalshot' => 0, 'appearances' => 0, 'attcreated' => 0, 'totalpasses' => 0, 'accpasses' => 0, 'minutesplayed' => 0, 'conceded' => 0, 'player_id' => '11', 'team_id' => 0, 'period_id' => '2')
)
);
$this->assertEquals($

First of all, I'd create a distinct method for every assertEquals() call. It helps defect localization. If the first assert throws an error you'll have no idea that the 2nd, 3rd and the others are OK or not. Having only one assert call per test method is a good practice. The comments should be the name of these methods. (Comment usually is a code smell.)

The $expected arrays are rather complicated. If the Player class get a new field (e.g. nickname) you probably have to modify all of your tests which could be a huge pain. I suggest using custom assert methods. For example, instead of

//Get All Available Players with No Stats Filtered by Position
$result = $this->Player->GetAvailablePlayers(1, false, null, array('def', 'mid', 'fw'));
$expected = array(...)
$this->assertEquals($expected, $result);

try

function testGetAllAvailablePlayersWithNoStatsFilteredByPosition() {
    $result = $this->Player->GetAvailablePlayers(1, false, null, 
        array('def', 'mid', 'fw'));
    $this->assertPlayerCount("playerCount", 2, $result);
    $this->assertPlayerIdEquals("player id 1", 2, $result[0]);
    $this->assertPlayerIdEquals("player id 2", 3, $result[1]);
}

(Of course you have write the assertPlayerCount and assertPlayerIdEquals methods. The first string parameter is an assertion message. It's useful if you have more than one assert call in one test method.)

In this test class you use Shared Fixture. If it's possible try using fresh fixture with Inline Setup. From
Test Automation Strategy, The Three Major Fixture Strategies:


This Shared Fixture strategy is often used to improve the execution
speed of tests that use a Persistent Fresh Fixture but it comes with a
fair amount of baggage. These test require the use of one of the
fixture construction and tear down triggering patterns. They also
involve tests that interact with each other, whether by design or
consequence, and this often leads to Erratic Tests (page X) and High
Test Maintenance Costs.

(The cited text has links, check them.)

If you have enough time it's worth reading the whole XUnit Test Patterns book. Reading just the Part I (and Part II.) is also beneficial.

(Please note that I haven't written any PHP code in last few years, so the sample codes probably don't compile etc.)

StackExchange Code Review Q#5701, answer score: 4