Showing posts with label tdd. Show all posts
Showing posts with label tdd. Show all posts

Friday, October 26, 2007

Handoff 1.0

Having used it on my current project for the last few months and improved the unit test coverage, I've released version 1.0.0 of the Handoff gem. You can get the gem file at RubyForge or just a wait a while and do

sudo gem install handoff -v 1.0.0

My first priority for future development is to improve the output when specifying delegation that hasn't been implemented yet. (Currently you just get one NoMethodError.) After that I may work on a couple of features requested by Ali, but it's a real hassle TDD'ing a library for making test assertions, so the going will likely be slow.

For those of you who don't work with me, Handoff is a tiny little gem providing a fluent interface for asserting on simple delegation. It aims to make asserting the behavior as simple as implementing it (with Forwardable). See the rdocs for examples.

Friday, October 19, 2007

Some Rambling on xUnit Testing Style

On our drive back to Brooklyn from the client site last night, Patrick and I were talking about testing style.

I mentioned that it had bothered me for a long time that the base class we extend in (most) xUnit frameworks is called "TestCase" even though any good test class describes multiple test cases. [To help fend off semantic confusion, here's my idea of test case identity: One test case is a set of inputs and stimuli to the code under test. There may be several things to assert about what goes on during a test case or the state of things after its run, but the inputs and stimuli are constant. When you vary them, you have another test case.]

Since we try to minimize the number of assertions in a given test (ideally keeping it to one, though in contrast to Jay I'm personally fine with that one assertion being complex) we often have multiple test methods that assert on the same test case (by the definition above) and could therefore share setup and teardown code. But since we also need tests to exercise other scenarios, we can't use the framework's setup method to create the scenario unless we're willing to set up all the stuff we need in all our tests, what you might call a "superset fixture," which feels wrong. Incidentally, I think the reason not many people use the term "fixture" for the stuff you set up for your tests is that it's always been a pretty weak concept in practice: either you have a mess of objects that various tests will use in various ways or you're setting up so little that it's barely worth talking about.

It may be practical to set up a superset fixture when doing simple state-based testing, but if you're dealing with a "wide" object, it means a really noisy setup.* When using mocks it's actually impossible to fully set up more fixture than you'll need in every test: a mock set up but not used will fail the tests that don't satisfy its expectations.

So besides having the name 'TestCase' that doesn't seem to make sense, we have these facilities for setting up and tearing down that we don't leverage much.

Then I had what I thought might be an important insight right there in the car. Maybe the class was called a TestCase and had just one setup method because it was originally intended to describe just one case, with each test method just asserting something different about the scenario created in the setup. If so, the setup could even include the stimulus of the code under test, reducing test methods to nothing but assertions. Maybe what's made both the naming and the use of shared fixture-setup seem awkward all this time is that we've tied ourselves to creating one TestCase class per production class, when all along we could have had a TestCase class for each scenario we wanted to test, with most having a very small number of test methods.

Here's a super-simple example of the sort of thing I was imagining, though my imaginings were a lot more abstract.

require 'test/unit'
require 'set'

class Set::EmptyTest < Test::Unit::TestCase
  def setup
    @set = Set.new
  end
  
  def test_size_is_zero
    assert_equal 0, @set.size
  end
  
  def test_empty
    assert @set.empty?
  end
end

class Set::AdditionTest < Test::Unit::TestCase
  def setup
    @set = Set.new
    @set.add 5
  end
  
  def test_size_is_one
    assert_equal 1, @set.size
  end
  
  def test_contains_added_item
    assert @set.include?(5)
  end
  
  def test_not_empty
    assert !@set.empty?
  end
end

class Set::DeletionTest < Test::Unit::TestCase
  def setup
    @set = Set.new [:abc, 5]
    @set.delete 5
  end
  
  def test_size_is_one
    assert_equal 1, @set.size
  end
  
  def test_no_long_contains_deleted_item
    assert !@set.include?(5)
  end
  
  def test_still_contains_other_item
    assert @set.include?(:abc)
  end
  
  def test_not_empty
    assert !@set.empty?
  end
end

All these little test cases could be a maintenance headache if they each lived in their own file, but it might not be too bad if you gave up the one-class-per-file convention. Although I'm not a good student of history, I knew xUnit frameworks started with one in Smalltalk, and it seemed like this one-TestCase-class-per-scenario approach might have been really convenient in a development environment where all code was organized hierarchically without the bother of source files that might need to be moved, renamed, etc when changing tests. I've only run Squeak long enough to build a trivial Seaside application, so I was speculating, but I could imagine it being pretty handy to organize tests with one package per class under test, then a class per test case, each with a setup, then a test method for each assertion to be verified.

In Ruby we would also do some metaprogramming to reduce the noise and make the test code more intentional. Maybe something like this.

testcase_for 'an empty set' do
  
  setup { @set = Set.new }
  
  test('size is zero') { assert_equal 0, @set.size }
  
  test('empty') { assert @set.empty? }
end

testcase_for 'adding an item to a set' do
  setup do
    @set = Set.new
    @set.add 5
  end
  
  test('size is one') { assert_equal 1, @set.size }
  
  test('contains added item') { assert @set.include?(5) }
  
  test('not empty') { assert !@set.empty? }
end

You probably noticed this looks a lot like RSpec contexts, which gets at why I was so excited. I wondered if Kent Beck's original intent had been something much closer to BDD, and it had just taken the rest of us a long time to catch up.

So when I got home I went digging around for articles about unit testing style and found surprisingly little. I also looked for anything about the original intent of the framework. (Googling these topics was a little depressing because of all the weak information, plagiarism, and content spam.)

My search stopped when I found the Kent Beck article where he originally presented the unit testing framework pattern we now know so well. (I believe that was first published in The Smalltalk Report in October 1994. Thanks to Farley for digging up that obscure nugget.) I was disappointed to find that the example in that first article conforms pretty much exactly to the classic form of unit test we've all seen before, including a setup method that sets up more fixture than any one test method uses.

In case you find the Smalltalk a little painful to read, here's my translation of Beck's example test case to Ruby. (The examples above, you'll now see, are based on Beck's.)

require 'test/unit'
require 'set'

class SetTest < Test::Unit::TestCase
  
  def setup
    @empty = Set.new
    @full = Set.new [:abc, 5]
  end
  
  def test_add
    @empty.add 5
    assert @empty.include?(5)
  end
  
  def test_delete
    @full.delete 5
    assert @full.include?(:abc)
    assert !@full.include?(5)
  end
  
  def test_illegal
    begin
      @full[0]
      fail
    rescue NoMethodError
      # expected
    end
  end
end

So that was a bit of a letdown. On the other hand, I did finally learn why the base class is called a TestCase when it represents so many different test cases.

As a test writer, you tend not to think of your TestCase subclass as a normal class. All the instantiation and running is in the framework, and none of your code ever interacts with TestCase instances, so their life-cycle (their very existence as normal objects) is usually irrelevant to you as a user of the framework. From the framework's point of view, however, their life-cycle is central.

As you may or may not know, your xUnit runner creates one instance of your TestCase class for each test method, passing to the constructor the name of the test method the new instance will run. Sure, well written setup and teardown methods would allow the runner to use one instance for all the test methods, but that would require the test writer not to accidentally leave state hanging around in the instance. Why put that burden on the framework user when the framework can just as easily start with a completely clean slate every time?

So the framework creates one TestCase instance per test method, each of which is a test case of its own. It works in the intuitive sense of "test case" as well as in OO terms. Score one for the forefathers of agile software development!

I'm interested to hear what styles people have used or seen in unit test/spec suites. Have you tried creating multiple TestCases per class to keep individual test case classes more maintainable? Did it work out? What about BDD specs? Do you find having the structure of the test or spec map directly to the purpose of the code (as opposed to having private test helper methods scattered around your source file) advantageous? How far have you gone with keeping test setup all inline in the tests themselves? Let me know.

* Yes, wide objects that are hard to set up for test are a smell that the code under test may be poorly factored, but if there's anything we've learned driving back and forth from Brooklyn to central New Jersey every week, it's that you often have to live with bad smells, though you should always be on the lookout for a route that avoids them without making you late.

Friday, July 13, 2007

Towards More Readable Tests

Making code readable is probably my number two priority after making it function correctly. I want every developer on the team to be able to look at code written by anyone else on the team and understand its intent. Of course that goes for the tests too.

We've had a few new developers join the team recently, and I've heard more than one of them say "Huh, I've never seen that before" about a pattern that shows up in some of our unit tests. It's something I've done and liked, but on further reflection, I'm starting to lean away from it.

Here's a test that could be clearer. (Let's take the interaction being tested as a given and focus on how the tests read. (Let's also not assume I have any interest in fishing. I don't know how I landed on this example.) The mocking library is Mocha.)

test "catches, kills, and bags tasty fish" do
  river = mock
  fish = mock
  river.expects(:give_it_up!).returns(fish)
  fish.expects(:tasty?).returns(true)
  fish.expects(:die!)
  bag = mock
  bag.expects(:put).with(fish)
  
  fisher = Fisher.new(river, bag)
  fisher.fish
end

Here's what makes it hard to read:

  river = mock                              # setup
  fish = mock                               # setup
  river.expects(:give_it_up!).returns(fish) # expectations
  fish.expects(:tasty?).returns(true)       # expectations
  fish.expects(:die!)                       # expectations
  bag = mock                                # setup
  bag.expects(:put).with(fish)              # expectations

As you read that, you have to change gears back and forth as you read lines that create testing fixtures interspersed with lines that specify the interaction we're working on. No good.

After reading a few tests like that (and tests with the same problem magnified), lines like these

  river = mock
  fish = mock
  # ... snip
  bag = mock

start to feel like pure noise. I suppose that was my mindset when I started changing those sorts of tests to look like the following.

test "catches, kills, and bags tasty fish" do
  (river = mock).expects(:give_it_up!).returns(fish = mock)
  fish.expects(:tasty?).returns(true)
  fish.expects(:die!)
  (bag = mock).expects(:put).with(fish)
  
  fisher = Fisher.new(river, bag)
  fisher.fish
end

Here, the creation of mocks is all inlined so that you can read straight through the interaction. The assignment of local variables embedded at the first occurence of each item in the interaction (thanks to Ruby's lack of variable declaration) isn't too distracting once you're accustomed to seeing it. Each test takes up less screen real estate than before, and the world's all around better than it was.

Or at least that's how I felt based on the before and after states of some pretty long tests I had to work with.

But here's something that's no better than before (probably worse): look at the test above and tell me which collaborators come into play when a Fisher catches a tasty fish, as opposed to the following, in which the fish gets thrown back.

test "catches and releases non-tasty fish" do
  (river = mock).expects(:give_it_up!).returns(fish = mock)
  fish.expects(:tasty?).returns(false)
  river.expects(:put).with(fish)
  
  fisher = Fisher.new(river, :ignored_bag)
  fisher.fish
end

It requires some pretty annoying code scanning when mock creation is embedded at various funny spots along the way. Those lines that felt like noise when they interrupted the specification of the interaction suddenly seem a lot more useful.

Having reconsidered, I now think the clearest presentation is closer to the first version, but different in a key way.

test "catches and releases non-tasty fish" do
  river = mock
  fish = mock
  
  river.expects(:give_it_up!).returns(fish)
  fish.expects(:tasty?).returns(false)
  river.expects(:put).with(fish)
  
  fisher = Fisher.new(river, :ignored_bag)
  fisher.fish
end

test "catches, kills, and bags tasty fish" do
  river = mock
  fish = mock
  bag = mock
  
  river.expects(:give_it_up!).returns(fish)
  fish.expects(:tasty?).returns(true)
  fish.expects(:die!)
  bag.expects(:put).with(fish)
  
  fisher = Fisher.new(river, bag)
  fisher.fish
end

Like the list of characters at the beginning of a script, these tests tell the reader who's involved in the interaction before jumping into the story, which makes the story much easier to follow. The tests take up more lines, but are easier for the reader to comprehend, particularly one who isn't interested in all the details. (Imagine looking through your codebase before a big refactoring wondering "how often do we use that FishingBag I want to change?" If collaborators are called out, you can quickly focus on the relevant interactions and read just those in detail.)

In the case of the longer tests that pushed me towards the inline style, the list of players would have been quite a bit uglier than the one above. That shouldn't have discouraged me from keeping it separated from the rest of the test: it should have told me that the object I was trying to test had too much on its plate. When something like that happens, hopefully I can figure out a design that won't require more players than I can easily keep track of all at once. If not, at least the next developer who comes along will have a clean tally of all the collaborators and a fighting chance of improving the design herself.

Good luck!

Friday, July 06, 2007

Moving to RSpec

RSpec's been on my list of tools to adopt for a while now. My current project is unlikely to migrate from Test::Unit, and there's no plan for me to migrate off that project, so I decided to switch Nestegg over to use it. It's not the sort of hard-core dive-in that's going to force me to get fluent, but it's a nice first baby step. (I'm mean really baby: the whole gem is only one module.)

Here's what the migration looked like.

I started by running newgem -t rspec fakegem to get a template for my spec directory and Rakefile, then pulled the spec-related stuff that generated into nestegg.

The spec folder had

  • spec_helper.rb, which just does require 'spec',
  • spec.opts, which just hash --colour, and
  • fakegem_spec.rb, which is a do-nothing spec.

The Rakefile had this up top

begin
  require 'spec/rake/spectask'
rescue LoadError
  puts 'To use rspec for testing you must install rspec gem:'
  puts '$ sudo gem install rspec'
  exit
end

and this down below

desc "Run the specs under spec"
Spec::Rake::SpecTask.new do |t|
  t.spec_opts = ['--options', "spec/spec.opts"]
  t.spec_files = FileList['spec/**/*_spec.rb']
end

desc "Default task is to run specs"
task :default => :spec

After a sudo gem install rspec, I was up and running. The default rake target ran my Test::Unit suite, then the do-nothing spec. Sweet.

I moved the do-nothing spec to nestegg/nesting_exception_spec.rb and copied each of my test names over (which was especially effortless since my Test::Unit suite was using RSpec-ish test declarations). I also remembered reading that you could pass a class to the describe method, so I landed with this

describe Nestegg::NestingException do
  
  it "includes cause in backtrace" do
    violated "Be sure to write your specs"
  end
  
  it "includes cause's backtrace after cause in backtrace" do
    violated "Be sure to write your specs"
  end
  
  it "removes duplicated backtrace elements from nesting exception" do
    violated "Be sure to write your specs"
  end
  
  it "defaults cause to current raised exception ($!)" do
    violated "Be sure to write your specs"
  end
  
end

Then I started at the top and copied the body of the first test into the spec. I'd anticipated some pain related to the helper methods that lived in my test, but those turned out to be completely portable. The only thing I had to modify in each test was the assertion. With special thanks to the very helpful Test::Unit Cheat Sheet, here are the before-and-afters.

  • assert_true e.backtrace.include?("cause: StandardError: #{cause.message}")
    became
    e.backtrace.should include("cause: StandardError: #{cause.message}")
  • assert_equal ["cause: StandardError: #{cause.message}", "line_one", "line_two"], e.backtrace[-3..-1]
    became
    e.backtrace[-3..-1].should == ["cause: StandardError: #{cause.message}", "line_one", "line_two"]
  • assert_match(/#{__FILE__}:\d+:in `test_.+'$/, e.backtrace[0])
    assert_equal "cause: StandardError: msg", e.backtrace[1]
    became
    e.backtrace[0..1].should == ["#{__FILE__}:#{line}", "cause: StandardError: msg"]
    Note that on that one I'd done a Regexp match in the Test::Unit version, because I didn't want to put the test method name in the content of the test. Since RSpec apparently doesn't create methods out of examples, the backtrace line I'm interested in is free of any method name. To enable a simple equality check, I saved the line number on which the exception was raised in a temporary variable.
  • assert_equal expected_cause, raised_error.cause
    became
    raised_error.cause.should == expected_cause

See here for the whole shebang. The original test is here. The two files are incredibly similar, no?

Just like that, Nestegg was converted. I was a little sad I hadn't used mocha, since that could have meant switching to RSpec's mocking library. Oh well. That'll give me something to look forward to when I migrate Handoff.