I personally feel that conventions should be best practices and not inevitable parts of frameworks. Conventions are good, but they kill testability. So while they can save you some time you would have had to spend on configuration otherwise, they also limit the granularity of your interfaces and break testability.
My recent example of not testable controllers and how it could have been fixed was very well received amongst fellow Symfony2 developers, so that gives me enough confidence to propose something else.
There is another major part of the framework that can hardly be tested as it relies on Symfony’s internals and cannot use DIC for own configuration. Console Commands. They are registered by manual scan of bundles’ Console directory. They therefore cannot be configured through DIC with all dependencies moved to their interface definition and just get the generic Container instance instead.
Or can they? The answer is: “Yes, they can”.
And it wouldn’t be a lot of work to switch that. All we need to do is register each command in DIC as a service, and use tags to specify that this service is a command:
Let’s look at how we could then test one of the least testable Symfony2 commands - the Symfony\Bundle\FrameworkBundle\Command\AssetsInstallCommand. This command copies public assets like JavaScript and CSS files from the bundles’ Resources/public directories into a publicly accessible web directory, that is passed to it as the only parameter.
Since I’m gonna be testing the already existing class, the test will not be as elegant as it could have been:
While writing this test, I found out the command wasn’t testable because of a hard-coded mkdir function call that I couldn’t mock out. In order to fix it, I found the already existent Symfony\Bundle\FrameworkBundle\Util\Filesystem::mkdirs() function that wraps it, and makes it mockable, which I then proceeded to use. The only other changes I had to introduce were - get rid of Container dependency, and add Symfony\Bundle\FrameworkBundle\Command\AssetsInstallCommand::setKernel and Symfony\Bundle\FrameworkBundle\Command\AssetsInstallCommand::setFilesystem methods for direct injection of primary dependencies.
So here it is - the modified AssetsInstallCommand, that is fully unit-tested:
And here is the result of running it in PHPUnit:

As always, feedback is much appreciated, and I have nice DISQUS Comments for that very purpose now!
Happy Coding!
P.S. While I was posting this, and embedding my thoughts in public gists, Kris Wallsmith suggested to use the tags to specify command names as well, which is a very interesting suggestion.
P.P.S. Henrik Bjørnskov was very happy when I shared this idea with him and contributed most of the initial implementation of this feature here.
P.P.P.S Code that I provided in the post is available on my GitHub repository, and is built on top of Henrik’s efforts.
No comments:
Post a Comment