At dinner tonight we had a wonderful discussion about testing. One topic was why SetUp and Teardown (S&T) are wrong, which I've previously blogged about. My biggest issue with S&T is that it's used to enforce DRY at the expense of readability.
Image a test that did not use S&T, it would do exactly what you wanted all in one test method (with possible helper private methods). There would be no where else to look for logic, variable declarations, or mock verifies. Wouldn't it be nice if all tests were that readable.
Unfortunately, somewhere along the way we started sacrificing readability. Also, we started blindly following rules like DRY without considering context. Sure, DRY is a great rule, but what if the duplicate code only appears within the class? What if it were only 1 line, like a variable declaration? Do you really gain anything by abstracting it to a SetUp method? Yes, you gain the ability to only make changes in one place. But, if you could make the change in one place, ensure that it changed everything correctly, and not sacrafice readability. Does this magical feature exist? Find and Replace (in this file/class only). What about more than 1 line of duplicate code? Once upon a time this was handled with private methods and the flow of a test was easy to follow. Times were simpler then, but now we do "the simplest thing that could possibly work", or do we?
Tests are procedural by naturue. Allowing developers to read tests in a procedural manner just makes sense.
I find setup and teardown very useful. E.g.,:
ReplyDeleteclass DBTestCase < Test::Unit::TestCase
def connection
@connection || connection.new.auto_commit(false)
end
def teardown
@connection.rollback if @connection
super
end
end
class MyTest < DBTestCase
def test_my_stuff
connection.do("INSERT ...")
...
end
end
(Imagine the indentation, since a pre tag isn't allowed in comments.)
So I just do whatever I want on the DB, and all my test's changes are rolled back after the unit test, without thinking about it.
Sam: The problem is that often SetUp becomes a "slut" method and creates many objects that are needed for certain tests but not all tests. Also, in practice I often see mock.verify in teardown. The largest problem is that others don't use SetUp the way you state and the tests become unreadable.
ReplyDeleteI don't totally disagree with you; however, I don't think it's possible to argue that Template Method is actually more readable than just putting the code directly in the method.
Curt: It's not the lack of knowledge concerning use driving my statement. Actually, it's my experience that's been so painful that drove me to take a stance on it. In theory S&T could be helpful. However, in practice it's rarely as simple as your example.
All: SetUp is a great place for ObjectMother and for declaring and setting up multiple mocks. The problem is that if you need more than one mock or you need an ObjectMother than you have a design issue anyway. If the classes were more testable and loosly coupled you wouldn't need SetUp.
Point taken. I still maintain my stance though. Also, the nice thing about private methods is that they only return one thing (stolen from Christian Taubman). SetUp often sets up multiple objects. Too much FM for readability. Along the same lines I dislike class variables in tests because they dilute the readability and behave differently in different xUnit frameworks.
ReplyDeleteJay: in practice it is always that simple. Otherwise I don't put the code in setup, neh?
ReplyDeleteAs for using it to set up objects that are not used in all tests: well, move the other tests to another test class, then. There's no rule that you can't have two or three test classes testing a single class.
We are really, really stuck in the rather arbitrary x-unit way of doing tests; you have just one class testing another, and you have several test methods within that class, each starting with "test".
Why don't we have a separate class for every test? Think about that one for a while. Then think about how you'd deal with setup/teardown-type stuff. And then maybe it will become clear how setup and teardown should be used. When I look at it that way, it makes sense to me, but then I am forced to admit that the way tests are currently set up, while convenient in certain ways, does not make it obvious how setup and teardown should be used well.
ReplyDeleteAs for using it to set up objects that are not used in all tests: well, move the other tests to another test class, then. There's no rule that you can't have two or three test classes testing a single class.
Totally agree that this is how it should be done. But, that's not what I see in practice.
We are really, really stuck in the rather arbitrary x-unit way of doing tests; you have just one class testing another, and you have several test methods within that class, each starting with "test".
I'm all for new ideas on testing. That's why I'm excited about BDD . Although, my statements are specific enough to say that I'm requiring a class to be tested only by another class. I do break my tests up when I feel it makes sense logically, but I still don't use SetUp anywhere.
Why don't we have a separate class for every test? Think about that one for a while.
Sure, and I'll have protected helper methods in a testbase class. Though, this violates my readability requirements and I'd never do this in practice. But, while throwing ideas out, what if it were One assertion per test? That could improve readability in that _the_ assertion is the focus of the test.
Then think about how you'd deal with setup/teardown-type stuff. And then maybe it will become clear how setup and teardown should be used.
It is clear how it _should_ be used. Unfortunately, _in general_ it's not used that way.
When I look at it that way, it makes sense to me, but then I am forced to admit that the way tests are currently set up, while convenient in certain ways, does not make it obvious how setup and teardown should be used well.
The idea of the post isn't to change anyone's mind about SetUp. However, if they question it for a minute you never know what might happen.
That link to the one-assertion-per-test page is great! I like that way of doing things. But I'm sure you must disapprove, since it uses setup. :-)
ReplyDeleteThe idea of your post may not have been to change someone's mind, but it looks like it's changed mine; I'm certainly thinking about all of this in quite a different way now.
But as for:
Sure, and I'll have protected helper methods in a testbase class. Though, this violates my readability requirements and I'd never do this in practice.
So helper methods like assert_equal violate your readability requirements? You must be very happy with methods such as connection in my DbTestCase above.