Skip to content

Instantly share code, notes, and snippets.

@BaylorRae
Created September 28, 2012 23:10
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save BaylorRae/65a049ad1d049fbe8070 to your computer and use it in GitHub Desktop.
Save BaylorRae/65a049ad1d049fbe8070 to your computer and use it in GitHub Desktop.

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.

Extract the tests from a real environment

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.

  1. In Fetch_Test::setUp() assign $this->fetch to a stubbed version of Fetch.
  2. 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.

  1. You don't need to add a second class or stub the method.
  2. It still works with real files.
  3. You can test the content in public/vendors/js and make sure it's correct.

Don't put content from test files in the working application

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment