This is an awesome tutorial on writing tests for a PHP framework. Most articles on Test Driven Development in PHP focus on a couple of standalone files and I was very excited to see that you worked with an actual, production ready PHP framework.
However, there are a few things that I would like to point out. I'm by no means an expert in TDD and if you disagree with anything I say please let me know in a reply. After watching the video there were a few things that I thought should be changed to improve the tests and decided to add them as a comment.
What I mean is create your own environment that you have complete control of. In your tests you were working with real data that was irrelavent to the tests itself. Instead you should have used your own data that was independant of the class.
Near the end you extracted and stubbed the Asset::fetch()
method, but I don't think that was the best solution. Here are a couple of alternatives, the second being my personal choice.
- In
Fetch_Test::setUp()
assign$this->fetch
to a stubbed version ofFetch
. - Modify
Fetch_Task::$paths
to use local files.
By using local files you can still use file_get_contents()
without running into issues. But, in addition you force your class to use the environment you create.
Here's as example. The following will load "jQuery" from a local file. Because Fetch_Task
isn't parsing the file, you can put whatever content you want in it.
public function setUp() {
Fetch_Task::$paths = array(
'jquery' => dirname(__FILE__) . '/fetch-factories/jquery.js' // create this file with some fake content
);
// Fetch_Task now uses the local file for jquery
$this->fetch = new Fetch_Task;
}
Here are the reasons "stubbing" the source list is better.
- You don't need to add a second class or stub the method.
- It still works with real files.
- You can test the content in
public/vendors/js
and make sure it's correct.
Something I noticed that almost made me faint was watching you add the following lines to your tearDown()
method.
public function tearDown() {
File::cleandir(static::$cssDir);
File::cleandir(static::$jsDir);
}
I realize that these days if you're not using a SCM then "you're probably doing something wrong". But, I pitty the person, I won't call them a fool :), that runs the tests and accidentally loses all the vendor assets they've downloaded.
Here's how I would work with temporary files, put them in a tmp
directory. I'm not familiar with Laravel and there may be a better place do something like this, but you get the point.
public function setUp() {
// there may be a better place to put the files
Fetch_Task::$cssDir = 'storage/tmp/fetch_task_tests/css';
Fetch_Task::$jsDir = 'storage/tmp/fetch_task_tests/js';
// ...
}
public function tearDown() {
File::cleandir(Fetch_Task::$cssDir);
File::cleandir(Fetch_Task::$jsDir);
}
I know this is a tutorial and the code in tutorials don't need to be perfect. But I wanted to share some of the things that I've learned that other's may find useful. As I mentioned earlier, if you disagree with something I said please let me know in a reply. I love criticism; fire away!