4
\$\begingroup\$

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?

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://php.net/manual/en/function.realpath.php
  public function GetRealpath ()
  {
    $return = realpath($this->GetFilename());

    if ($return === false)
    {
      $lastError = error_get_last();

      throw new Exception(
        'Unable to determine the real path. '
          .'It might be due to a lack of permissions.',
        403  ,
        new Exception(
          'message: "'.$lastError['message'].'"'.PHP_EOL
            .'file: "'.$lastError['file'].'"'.PHP_EOL
            .'line: `'.$lastError['line'].'`'.PHP_EOL,
          $derniereErreur['type']));
    }

    return $return;
  }
}

Qyy.G.en.PHP.File/UnitTests/Qyy_G_en_FileTest.php:

require_once dirname(__FILE__) . '/../Qyy_G_en_File.php';

/**
 * Test class for Qyy_G_en_File.
 */
class Qyy_G_en_FileTest extends PHPUnit_Framework_TestCase
{

  /**
   * @var Qyy_G_en_File
   */
  protected $object0;

  /**
   * @var Qyy_G_en_File
   */
  protected $object1;

  /**
   * @var Qyy_G_en_File
   */
  protected $object2;

  /**
   * @var array
   */
  protected $filenames;

  /**
   * Sets up the fixture.
   */
  protected function setUp()
  {
    $this->filenames = array(
      '../readme.md',
      '../.gitignore',
      '../README',
      'foo.bar');

    $this->testNewObject0();
    $this->testNewObject1();
    $this->testNewObject2();
  }

  public function testNewObject0 ()
  {
    $this->object0 = new Qyy_G_en_File($this->filenames[0]);

    $this->assertEquals(true, is_a($this->object0, 'Qyy_G_en_File'));
  }

  public function testNewObject1 ()
  {
    $this->object1 = new Qyy_G_en_File($this->filenames[1]);

    $this->assertEquals(true, is_a($this->object1, 'Qyy_G_en_File'));
  }

  public function testNewObject2 ()
  {
    $this->object2 = new Qyy_G_en_File($this->filenames[2]);

    $this->assertEquals(true, is_a($this->object2, 'Qyy_G_en_File'));
  }

  /**
   * @expectedException InvalidArgumentException
   */
  public function testNewObject3 ()
  {
    new Qyy_G_en_File($this->filenames[3]);
  }

  /**
   * @depends testNewObject0
   * @depends testNewObject1
   * @depends testNewObject2
   */
  public function testGetFilename()
  {
    for($i = 0; $i <= 2; $i++)
    {
      $this->assertEquals(
        $this->filenames[$i],
        $this->{'object'.$i}->GetFilename());
    }
  }

  /**
   * @depends testNewObject0
   * @depends testNewObject1
   * @depends testNewObject2
   */
  public function testGetBasename()
  {
    for($i = 0; $i <= 2; $i++)
    {
      $this->assertEquals(
        basename($this->filenames[$i]),
        $this->{'object'.$i}->GetBasename());
    }
  }

  /**
   * @depends testNewObject0
   */
  public function testGetBasenameNoSuffix0 ()
  {
    $this->assertEquals('readme', $this->object0->GetBasenameNoSuffix());
  }

  /**
  * @depends testNewObject1
  * @expectedException LengthException
  */
  public function testGetBasenameNoSuffix1 ()
  {
    $this->object1->GetBasenameNoSuffix();
  }

  /**
   * @depends testNewObject2
   */
  public function testGetBasenameNoSuffix2 ()
  {
    $this->assertEquals('README', $this->object2->GetBasenameNoSuffix());
  }

  /**
   * @depends testNewObject0
   */
  public function testGetSuffix0 ()
  {
    $this->assertEquals('md', $this->object0->GetSuffix());
  }

  /**
   * @depends testNewObject1
   */
  public function testGetSuffix1 ()
  {
    $this->assertEquals('gitignore', $this->object1->GetSuffix());
  }

  /**
   * @depends testNewObject2
   * @expectedException LengthException
   */
  public function testGetSuffix2 ()
  {
    $this->object2->GetSuffix();
  }

  /**
   * @depends testNewObject0
   * @depends testNewObject1
   * @depends testNewObject2
   */
  public function testGetDirname ()
  {
    for($i = 0; $i <= 2; $i++)
    {
      $this->assertEquals(
        dirname($this->filenames[$i]),
        $this->{'object'.$i}->GetDirname());
    }
  }

  /**
   * @depends testNewObject0
   * @depends testNewObject1
   * @depends testNewObject2
   */
  public function testGetRealpath ()
  {
    for($i = 0; $i <= 2; $i++)
    {
      $this->assertEquals(
        realpath($this->filenames[$i]),
        $this->{'object'.$i}->GetRealpath());
    }
  }
}
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.