Reviewing my unit testing (in PHP)

This week-end, I finally found some time to learn unit testing (it's about time, I know).

So I've tried to do this in PHP, with PHPUnit 3.6.

I've wrote a small, simple and useless class wrapper for files, and a basic unit testing class for it.

As I'm a self learner, and nobody around me can help me about that, is it possible for someone to review it, and tell me what is right and wrong, and how I can improve my tests?

• Here is the Github repository with all the code

• I paste thereafter the class and the test class

Qyy.G.en.PHP.File/Qyy_G_en_File.php:

```
// TODO: doc
class Qyy_G_en_File
{
protected $filename;

// TODO: doc
function __construct ($filename)
{
if (!file_exists($filename))
{
throw new InvalidArgumentException(
'This file does not exist or permissions are not set correctly: '
.$filename,
404);
}

$this->filename = $filename;
}

// TODO: doc
public function GetFilename ()
{
return $this->filename;
}

// TODO: doc
// http://php.net/manual/en/function.basename.php
public function GetBasename ()
{
return basename($this->GetFilename());
}

// TODO: doc
// http://php.net/manual/en/function.pathinfo.php
public function GetBasenameNoSuffix ()
{
$return = pathinfo($this->GetFilename(), PATHINFO_FILENAME);

if (is_null($return) || empty($return))
{
throw new LengthException('This file seems to start with a dot.', 404);
}

return $return;
}

// TODO: doc
// http://php.net/manual/en/function.pathinfo.php
public function GetSuffix ()
{
$return = pathinfo($this->GetFilename(), PATHINFO_EXTENSION);

if (is_null($return) || empty($return))
{
throw new LengthException('There is no suffix for this file.', 404);
}

return $return;
}

// TODO: doc
// http://php.net/manual/en/function.dirname.php
public function GetDirname ()
{
return dirname($this->GetFilename());
}

// TODO: doc
// http://ph

All in all, it looks like you're off to a great start. I'm going to skip around a bit in what I recommend however there are definitely a few things that could be cleaned up.

public function testNewObject3 ()
{
    // I'm not sure you can define what the exception should
    // describe in the doc-block however you can by using the method
    $this->setExpectedException('InvalidArgumentException', 'File does not exist');
    new Qyy_G_en_File($this->_getNonExistantFile());
}

// I would use something like this to describe a bit more of what the filename means
private function _getNonExistantFile()
{
    return 'Foo.bar';
}

In your setUp() method, you call several other tests to prepare future tests. With unit testing, you want to keep the tests separate. In your code, if testNewObject2() fails, testGetSuffix1() will also fail, even though it is unrelated.

To reduce code duplication, use private methods for specific needs. Maybe something like this:

public function testNewObjectWithShortExtension()
{
    new Qyy_G_en_File('read.me');
}

public function testGetSuffixWhenShort()
{
    $Qyy = $this->_createQyyFile('read.me');        
    $this->assertEquals('md', $Qyy->GetSuffix());
}

Also, I would add that some very notable programmers have emphasized the rule of a single assertion per unit test. As such, give some thought to this as well.

Lastly, I would recommend being a bit more descriptive than object0, filename1, etc. Try to give more meaning to the variables. It'll help immensely in 6 months when you look back at your code and sit there wondering what the purpose of this or that was.

StackExchange Code Review Q#5154, answer score: 2