In the previous post we have gone through the minimum steps on how we can build a library.
At the current moment, it cannot be even named as “library”.
In this post I will try to cover the following issues:
- Improve tests
- Read content from a file
The from from what we start can be seen here.
1. Improve test
So, we start by improving tests. We first analyse what can we improve and then we start by doing it. After looking on the TestFile.php I noticed the following things:
- we do not test ::delete for nonexistent file or the same file twice
- writable works only for existent files. This is a false statement because it doesn’t mean that a nonexistent file is not writable. At the current moment this requires a vast implementation so we will leave it as s known issues (call it development debt)
- No tests has been done for ::write on non-writable files
So let’s add this scenarios and see how the tests will end-up:
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
class FileTest extends \PHPUnit_Framework_TestCase
{
// [...]
/**
* @param $providedFile
* @dataProvider provideFilesToDelete
* @dataProvider provisionFakeFiles
*/
public function testDelete($providedFile)
{
$file = new File($providedFile);
// $this->assertTrue($file->exists(), sprintf("%s should exists!", $providedFile));
$file->delete();
$this->assertFalse($file->exists(), sprintf("%s should be deleted!", $providedFile));
}
// [...]
}
As expected, running the tests gives as an warning. So let’s fix this issues:
- add a bool return type for ::delete method
- modify the ::testDelete method to do assertion on the delete method, not validation using another method (::exists in this case).
- split testDelete in 2 separate methods: testDeleteTrue and testDeleteFalse
After these changes, our test file should like like this:
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
class FileTest extends \PHPUnit_Framework_TestCase
{
// [...]
/**
* @param $providedFile
* @dataProvider provideFilesToDelete
*/
public function testDeleteTrue($providedFile)
{
$file = new File($providedFile);
$this->assertTrue($file->delete(), sprintf("%s should be deleted!", $providedFile));
}
/**
* @param $providedFile
* @dataProvider provisionFakeFiles
*/
public function testDeleteFalse($providedFile)
{
$file = new File($providedFile);
$this->assertFalse($file->delete(), sprintf("%s should not be deleted!", $providedFile));
}
// [...]
}
An this is what our File.php should end up:
<?php
class File
{
// [...]
/**
* Deletes the current file
*/
public function delete(): bool
{
if ($this->exists() === false) {
return false;
}
return unlink($this->filename);
}
// [...]
}
The tests passes but they are still not entirely what the should be. At the moment, the unlink() can still trigger a E_WARNING on failure (for example, permissions failure when the file is opened, for example).
To avoid quickly fixing this by adding the @ operator, I decided it can be handle more rigorously later (for now, another addition to development debts).
Let’s move on to writing content to a non-writable file. We do that by creating a new test method called testWriteFail. The test should be done on the Exception level:
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
class FileTest extends \PHPUnit_Framework_TestCase
{
// [...]
/**
* @param $providedFile
* @dataProvider provisionUnwritableFiles
*/
public function testWriteFail($providedFile)
{
$file = new File($providedFile);
$file->write(new Content('foo'));
}
// [...]
}
Now we should implement for the current test. To avoid permission errors we first validate if we can write the file. If we cannot write, we should throw an Exception. I aim to use Custom Exceptions for the purpose of more specific handling the actions.
First, we create the Exception directory inside src directory and afterwards we create a base Exception that we will use to be extended by any custom exception thrown by the library. This gives us a little flexibility.
Why custom exceptions?
Using custom exceptions gives the possibility to handle error types more specific. Let’s say, for example, that we throw a base \Exception. Upon handling, we do not know exactly who and why it threw it and we can only act in one way for all the cases. Example:
<?php
use NeedleProject\FileIo\File;
use NeedleProject\FileIo\Content\Content;
$file = new File('a_filename.ext');
try {
$file->write(new Content('my content'));
} catch (\Exception $e) {
echo "We have an error on writing the content!"
// and log the error or do whatever you need to do
}
By using the custom exception we can handle things a little different:
<?php
use NeedleProject\FileIo\File;
use NeedleProject\FileIo\Content\Content;
use NeedleProject\FileIo\Exception\PermissionDeniedException;
$file = new File('a_filename.ext');
try {
$file->write(new Content('my content'));
} catch (PermissionDeniedException $e) {
// we can try to change permissions
// and retry the process for writing
} catch (\Exception $e) {
// If we end up here we sure have an Exception that
// we didn't predict and it maybe really bad
}
Now let’s the additions from Exception directory, FileTest.php and File.php:
IOException.php
<?php
namespace NeedleProject\FileIo\Exception;
class IOException extends \Exception
{
}
PermissionDeniedException.php
<?php
namespace NeedleProject\FileIo\Exception;
class PermissionDeniedException extends IOException
{
}
TestFile.php
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
class FileTest extends \PHPUnit_Framework_TestCase
{
// [...]
/**
* @param $providedFile
* @dataProvider provisionUnwritableFiles
* @expectedException \NeedleProject\FileIo\Exception\PermissionDeniedException
*/
public function testWriteFail($providedFile)
{
$file = new File($providedFile);
$file->write(new Content('foo'));
}
// [...]
}
File.php
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\ContentInterface;
use NeedleProject\FileIo\Exception\PermissionDeniedException;
class File
{
// [...]
/**
* Write content to the current file
*
* @param \NeedleProject\FileIo\Content\ContentInterface $content
* @return \NeedleProject\FileIo\File
* @throws \NeedleProject\FileIo\Exception\PermissionDeniedException
*/
public function write(ContentInterface $content): File
{
if ($this->isWritable() === false) {
throw new PermissionDeniedException("The current file is not writable!");
}
file_put_contents($this->filename, $content->get());
return $this;
}
// [...]
}
Conclusion: By improving tests we added new logic to the File class, we created new classes (Exception classes) but most important we improved ::delete and ::write methods. We can now commit our changes with minimum visible API changes. The commit can be viewed here.
2. Read content from a file
We can now start adding features to the library. Let’s first create the method ::getContent() in File.php and let’s create the test scenarios.
File.php
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\ContentInterface;
use NeedleProject\FileIo\Exception\PermissionDeniedException;
class File
{
// [...]
/**
* @return \NeedleProject\FileIo\Content\ContentInterface
*/
public function getContent(): ContentInterface
{
return new Content('dummy_content');
}
// [...]
}
Before we write test scenarios we should analyse what do we expect from ::getContent.
- we should throw Exceptions for unreadable files (non-existent or permission issues)
- we successfully read a content as expected
So we end-up with the following test scenarios:
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
use Symfony\Component\DependencyInjection\Tests\Compiler\F;
class FileTest extends \PHPUnit_Framework_TestCase
{
// [...]
/**
* Test setUp
*/
public function setUp()
{
// create fixture
touch(static::FIXTURE_PATH . 'readable.file');
touch(static::FIXTURE_PATH . 'unreadable.file');
chmod(static::FIXTURE_PATH . 'unreadable.file', 0333);
touch(static::FIXTURE_PATH . 'unwritable.file');
chmod(static::FIXTURE_PATH . 'unwritable.file', 555);
touch(static::FIXTURE_PATH . 'delete.file');
touch(static::FIXTURE_PATH . 'content.file');
}
/**
* Test tearDown
*/
public function tearDown()
{
unlink(static::FIXTURE_PATH . 'readable.file');
unlink(static::FIXTURE_PATH . 'unreadable.file');
unlink(static::FIXTURE_PATH . 'unwritable.file');
if (file_exists(static::FIXTURE_PATH . 'delete.file')) {
unlink(static::FIXTURE_PATH . 'delete.file');
}
unlink(static::FIXTURE_PATH . 'content.file');
}
// [...]
/**
* @param $providedFile
* @dataProvider provisionUnreadableFiles
* @expectedException \NeedleProject\FileIo\Exception\PermissionDeniedException
*/
public function testGetContentUnreadableFile($providedFile)
{
$file = new File($providedFile);
$file->getContent();
}
/**
* @param $providedFile
* @dataProvider provisionFakeFiles
* @expectedException \NeedleProject\FileIo\Exception\FileNotFoundException
*/
public function testtGetContentNonExistentFile($providedFile)
{
$file = new File($providedFile);
$file->getContent();
}
/**
* We will test that we receive the desired content
* @dataProvider provideFileAndContent
*/
public function testtGetContentPass($providedFile, $providedContent)
{
file_put_contents($providedFile, $providedContent);
$file = new File($providedFile);
$content = $file->getContent();
$this->assertEquals($providedContent, $content->get(), "The content is not the same!");
}
// [...]
/**
* Provide a filename and a content
* @return array
*/
public function provideFileAndContent(): array
{
return [
[static::FIXTURE_PATH . 'content.file', 'Lorem ipsum']
];
}
}
Running test on this scenarios will obviously fail, so we need to actually implement the code according to our given scenarios.
FileNotFoundException.php
<?php
namespace NeedleProject\FileIo\Exception;
class FileNotFoundException extends IOException
{
}
File.php
<?php
namespace NeedleProject\FileIo;
use NeedleProject\FileIo\Content\Content;
use NeedleProject\FileIo\Content\ContentInterface;
use NeedleProject\FileIo\Exception\FileNotFoundException;
use NeedleProject\FileIo\Exception\PermissionDeniedException;
class File
{
// [...]
/**
* @return \NeedleProject\FileIo\Content\ContentInterface
* @throws \NeedleProject\FileIo\Exception\FileNotFoundException
* @throws \NeedleProject\FileIo\Exception\PermissionDeniedException
*/
public function getContent(): ContentInterface
{
if ($this->exists() === false) {
throw new FileNotFoundException(sprintf("%s does not exists!", $this->filename));
}
if ($this->isReadable() === false) {
throw new PermissionDeniedException(sprintf("You do not have permissions to read file %s!", $this->filename));
}
return new Content(file_get_contents($this->filename));
}
// [...]
}
At this moment, running the tests should show us the desired result.
To conclude:
The god things:
- we have improved are test coverage for ::delete and ::write
- added a new feature: reading content from a file
Things we should improve:
- handle file_get_contents correctly (upon failure, this function should return false as a value. Our current implementation does not handle this case.)
- we have a development debt in ::delete
- improve ::writable response
- correctly implement ::getContent – in the previous post I highlighted the use of Dependency Inversion but we are not using it.
The final code is posted here (The diff between the last commit and the first commit).
Also, to better keep track of our issues, we should use the tools in hand (github.com) and track our issues. They can be viewed here.