A few days ago I wrote in my portuguese blog an article trying to demonstrate why the “one assertion per test” pattern is an excellent one. If you dont know much about this pattern, I suggest you to follow this link which is a post from Jay Fields talking about it.
Some people gave me some feedback in the comments and one of them pointed me to this post from the guys from thoughtbot where among other practices, they criticized the one I was trying to evangelize
I like arguments and in this case arguments are present, but I think they aren’t valid, we’ll see why.
6 database records created
The author of the posts shows a code written in Shoulda where before each test he creates a record in the database and claims that the pattern is bad because to follow it he needed to create six database records instead of just one. Now I ask, did anyone tell anybody to create a record before each test? In my first post talking about the pattern I wrote an example which contains the following code:
before :all do
email = Email.new("firstname.lastname@example.org")
Note that I wrote before :all and not before :each, that’s because independent of the pattern, the tests need to be smart. In the case you couldn’t specify in Shoulda a block to be executed before all tests, that’s a problem of the framework and not of the pattern.
TDD should feel rhythmic
In the same code talked before, the author wrote some tests consisting of one line (Shoulda macros) and others consisting of three lines. Then he argues that the pattern in discussion is bad because the code hasn’t rhythm and TDD should have rhythm. Does this have any sense? The pattern doesn’t talk about having tests in one line or three, just about having one assertion per test. This doesn’t have any relation with the pattern, I might have the same behavior without following it, like this:
should 'include the user id and user name' do
assert_select 'id', @user.id.to_s
assert_select 'name', @user.name
What about now? Can we say that Shoulda is bad because I wrote a code that I consider it has no rhyme? Are then Shoulda macros not that good because they take they rhythm of the code?
Test names are not intention-revealing
I’m not sure if I understood this argument very well but I think the author claimed that the more tests you have, the more you have to give specific names to them. Assuming I’m right in my understanding, the intent of the pattern is precisely this, to create tests that specify what they are testing and what the code should do. When you run your tests, I surely think it’s better to receive this:
a GET to /users/:id.xml should respond with success
a GET to /users/:id.xml should render template show
a GET to /users/:id.xml should find the correct User
a GET to /users/:id.xml should respond with xml for user
a GET to /users/:id.xml should include the user id
a GET to /users/:id.xml should include the user name
I did write “user show xml” because the second code wrote by the author is a single test with the name “test_user_show_xml”. “user show xml” doesn’t tell us anything about the system. If I read this test I will think that the user must show a xml. The first case tells us much more about the system, it’s an executable specification.
If context/describe/should/it blocks want to make things shorter then why aren’t we using a library (Test::Unit) that makes them one line?
What relation does this have with the pattern? Again, the pattern just tells about having one assertion per test, not about leaving the code smaller. Moreover, I think this argument is good for the pattern. There are people who say that they don’t follow the pattern because they prefer to write less code. In this case, if you want to write less code then you should use Test::Unit
The author shows then the code written in Test::Unit that I talked about before, with a lot of asserts in only one test. I think this test is really bad because it doesn’t specify anything at all and to understand it you need to read the code, not just the title.
Finally, the post mentioned that the pattern is a solution for a problem that doesn’t exist, but the problem does really exists and it’s this:
Writing non-executable documentation of a code is very expensive because every change in the behavior of the code generates also a change in the documentation and that creates double work. Besides the double work, the non-executable documentation does not guarantee that the code does what it should. The solution to this is to create executable documentation and executable documentation means tests that specify and to create tests that specify the easiest solution is to follow “the one assert per test” pattern.