Code emphasis for tests that teach
In product code, not repeating yourself is almost always a good idea. In tests, it’s not so clear-cut. Repeating yourself has the same maintenance dangers as it does for code, but not repeating yourself has two additional downsides:
-
A common knock against well-factored object-oriented code is that no object does anything; they all just forward work on to other objects. That structure turns out to be useful once you’re immersed in the system, but it does make systems harder to learn.
One purpose of tests is to explain the code to the novice. Remove duplication too aggressively, and the tests do a poor job of that.
-
Another purpose of tests is to make yourself think. One way to do that is to force yourself to enumerate possibilities and ask “What should happen in this case?” That’s one of the reasons that I, when acting as a tester, will turn a state diagram into a state table. A state diagram doesn’t make it easy to see whether you’ve considered the effect of each possible event in each possible state; a state table does. (It’s not as simple as that, though: it’s hard to stay focused as you work through a lot of identical cases looking for the one that’s really different. It’s like the old joke that ends “1 is a prime, 3 is a prime, 5 is a prime, 7 is a prime, 9 is a prime…”)
If you factor three distinct assertions into a single master assertion, it’s easy to overlook that the second shouldn’t apply in some particular case. When you factor three distinct setup steps into one method, you can more easily fail to ask what should happen when the second setup step is left out.
So as I balance the different forces, I find myself writing test code like this:
|
There’s duplication there. In an earlier version, I’d in fact reflexively factored it out, but then decided to put it back. I think the tests are better for that, and I’m willing to take the maintenance hit.
Nevertheless, there’s a problem. It’s not obvious enough what’s different about the two tests. What to do about that?
Consider explaining the evolution of a program over time in a book. Authors don’t usually show a minimal difference between before and after versions. Instead, they show both versions with a fair amount of context, somehow highlighting the differences. (When I write, I tend to bold changed words.) I wish I could highlight what’s special about each test in my IDE, so that it would look like this:
|
Something for IDE writers to implement.
November 26th, 2007 at 6:49 am
I’m not keen on this kind of duplication even in test code. How about defining some helper methods with names like these…
def when_not_logged_in
def when_logged_in_as_aaron
def attempt_update_of_quentins_details
def assert_hacking_attempt_is_handled
David
November 26th, 2007 at 9:54 pm
It is one of those balancing things, but my favorite tests look roughly like Brian Marick’s in terms of how much duplication they have. I just haven’t found that modest amounts of duplication in tests has the same costs as in code. For example, if there are two copies of a piece of code, even if it is just a line or two, bugs tend to lurk, because the two aren’t really quite identical, or because a future fix is made to only one, or because the duplication is a symptom of not really having thought out what two business processes have in common (for example, is your domain language missing a term?). I could probably make up a contrived example of test duplication making it harder to see what the functionality of the system is, but I don’t remember it happening (other than the obvious cases of tests getting too long and being unreadable for that reason).
A good test will read something like: “do A, B, and C. assert that X and Y happened”. If everything is factored out in the name of removing duplication, there might be a lot of browsing of test code to find the A, B, C, X, and Y.
November 27th, 2007 at 8:43 am
David: Normally, I’m inclined to hide things like
put :update, {:id => blah blah blah}behind methods likewhen_not_logged_in. These particular tests are Rails “functional” tests of a RESTy interface, which means they’re about what kind of HTTP gets sent in and what happens in response. Being more specific seems appropriate.In my graphical business-facing tests, I do use more abstract language like “Sasha is still not logged in.”
I originally had a method named something like
assert_hacking_attempt_is_handledbut split it into three parts because I wanted to be clear about what it meant for a hacking attempt to be handled and make myself think about whether each of the three parts applied to each situation. For example, you can do more when you notice one logged-in user is trying to edit someone else. Unlike a not-logged-in user, you have an email address. Thinking thoughts like that is harder when you bundle all the responses behindhacking_attempt_is_handled.November 27th, 2007 at 11:34 am
I’ve just started some black-box tests for a REST interface and I find myself resisting the urge to refactor the HTTP calls to functions. Here is some real code, at least as of right now:
def page_delete_json
req = Net::HTTP::Delete.new(@@page_loc, initheader = {’Content-Type’ => ‘text/x.socialtext-wiki’})
req.basic_auth @@user, @@pass
response = Net::HTTP.new(@@host, @@port).start {|http| http.request(req) }
assert_equal(’204′, response.code)
assert_equal(’No Content’, response.message)
end
def page_put_json
req = Net::HTTP::Put.new(@@page_loc, initheader = {’Content-Type’ => ‘text/x.socialtext-wiki’})
req.basic_auth @@user, @@pass
req.body = “test get page json”
response = Net::HTTP.new(@@host, @@port).start {|http| http.request(req) }
assert_equal(’201′, response.code)
assert_equal(’Created’, response.message)
end
def test_page_get_json
puts “starting test get page json”
page_delete_json
page_put_json
req = Net::HTTP::Get.new(@@page_loc,initheader = {’Accept’ => ‘application/json’})
req.basic_auth @@user, @@pass
response = Net::HTTP.new(@@host, @@port).start {|http| http.request(req) }
parsed = JSON.parse(response.body)
assert_equal(parsed.keys, [”last_editor”, “name”, “uri”,”revision_id”, “last_edit_time”,”tags”,”modified_time”,”revision_count”,”page_id”,”page_uri”])
assert_equal([’devnull1@socialtext.com’],parsed.values_at(”last_editor”))
assert_equal([’TestGetPageJSON’],parsed.values_at(”name”))
assert_equal([’testgetpagejson’],parsed.values_at(”uri”))
assert_equal([’testgetpagejson’],parsed.values_at(”page_id”))
end
For one thing, if I move the HTTP calls out to their own functions, I’m only going to save one or two LOC per method. For another thing, my Accept and Content-Type headers can be any one of about four values, and calling the right function with the right headers looks from here like more trouble than it’s worth. If I keep all the guts exposed, it’s a heck of a lot easier to tinker with data types and HTTP functions as I go along.
When I get more tests, I might change my mind, but for now I like having all the wiring exposed where everyone can see it.
December 2nd, 2007 at 12:57 pm
Weekly Agile Reading. Pack 7…
Manually filtered top of the most interesting writing published since last Saturday. Of course, most interesting from my personal point of view….