Design in TDD

There’s been a recent discussion in the Melbourne XP Enthusiast Group (MXPEG) mail list about whether or not TDD ‘works’, with part of the thread also being about Story Driven Development (SDD). Some of the folks have reported a lot of success using SDD and its associated tests (that I would call acceptance and integration tests), to the point where they don’t use unit tests at all. The impression I get is that with this approach they’re working predominantly top down. I’m glad this works for some people, but I don’t think it would work for me, and I wanted to take a few minutes to say why.

I think it’s important to have both unit tests (developer tests) and acceptance tests (customer tests), whether they’re implemented in the same tools by the same people or quite different tools by quite different people. Notice though that I said “implemented” – to me the two groups of tests have different owners, different perspectives, and serve different purposes.

Customer tests – they tell us if the application is sick or not, but they don’t provide any indication of where or why.

Developer tests – they tell us exactly where something is going wrong, but don’t necessarily tell us what the impact on the overall application would be.

I would like to have both of these, not one or the other. I also find that I personally don’t design entirely either top down or bottom up. I do some top down stuff to get an idea of the problem, then I use that to hypothesise about a design that will meet the requirements. Once I have some confidence that my design looks reasonable, I implement parts of it bottom up, and I want unit tests to confirm that my implementation of each component works the way I expected. I keep switching back and forth between top down and bottom up as I go.

The thing I do badly is that I don’t write acceptance tests until the end (if at all). I know this isn’t what I’m supposed to do, I know it probably detracts from my overall quality, but it is what I find myself doing. So far I haven’t found a tool that makes writing those tests easy enough for me (personal opinion only).

I wish I had said this…

… but Bob Martin did :

One of the developers asked the question point blank: “What do you do when your managers tell you to make a mess?” I responded: “You don’t take it. Behave like a doctor who’s hospital administrator has just told him that hand-washing is too expensive, and he should stop doing it.”

Coupling rollback actions with transaction actions

The system I’m currently developing needs to import a variety of data from another system. The data arrives in batches, and within a batch some items might be valid and some might be invalid – I expect that this is a common problem. An item in the batch might also update many different objects within my domain model, and I won’t necessarily know if the item is invalid until I try to commit the changes to the domain model. It became fairly clear that I had some recurring code patterns in my application, so of course I wanted to extract them into some abstraction and Ruby’s reflection gave me a nice way to capture this.

What I needed was to be able to associate a piece of code that created or modified some domain objects with the code that cleaned up the changes to the domain model on rollback (assuming that the database transaction handled the persistence parts). I could have used blocks, but I find that unlabelled blocks aren’t very expressive so I decided to use methods (and their names) instead of blocks. Here’s an example of using my method, #within_transaction_with_rollback; in the example :create_reference_objects and :clear_reference_objects are methods defined somewhere else in the class.

  def process
    within_transaction_with_rollback(:create_reference_objects, :clear_reference_objects)
  end

And here’s the implementation of my method, for those who are interested. I’m sure that there are plenty of other possible implementations

module Transactions

    def within_transaction_with_rollback(transaction_method, rollback_method)
      begin
        within_transaction(transaction_method)
      rescue
        self.send(rollback_method)
      end
    end

    def within_transaction(create_method)
      self.class.transaction do
        self.send(create_method)
      end
    end

end

Interaction between finder_sql and Active Scaffold

I came across an interesting interaction in Rails that depended on two things that I didn’t know (of course, that list is infinite). The two things were:

1. Active Record ignores finder_sql when you do an eager load of an association (though I can’t find a definitive reference for this, it is the observed behaviour);
2. Active Scaffold uses eager loading by default

I didn’t have any specs that explicitly tested that my model could eagerly load its associations, but everything else seemed to be working, so I was surprised that Active Scaffold was unhappy. The solution was to tell Active Scaffold not to use eager loading on that particular association, following the instructions in the FAQ under “my database is choking” (which kind of describes what was happening to me, if the choking was fatal).

Need a Rails-savvy, Java tainted, agile bigot?

My involvement in a project has finished unexpectedly early, so I find myself available for a new gig without anything in the pipe – which is unusual for me. If you have anything that you think I could help with, especially short- or part-time, drop me a line (steve at cogentconsulting dot com dot au).

Rspec View specs and integrate_views

We’ve recently had the pleasure of having Craig Ambrose working on a project for us, and as you’d expect when you bring someone new into the team, we’ve been doing some storming. One of the topics that comes up repeatedly within development teams I’m familiar with, including ours, is how fanatically we should adhere to clear separation of unit and integration testing.

I’m fairly laissez faire about the issue. I’m quite happy to write tests for Rails models that actually invoke ActiveRecord and interact with the database. I’ll also happily set up a network of associated objects in the database rather than mock out everything except the class under test, though I do have some ill-defined limit that makes me uncomfortable.

I’m more strict about controller tests – I want the controller itself to be wafer thin, and I’ll generally only test that the controller sets the right instance variables (along with flash notices etc). The tests end up being fairly small, don’t touch the view at all, and I think even the evangelical TDD folk would call them unit tests.

However, Craig rightly points out a couple of problems with this. I’m typically working with server side functionality (if I had to say what I was best out, I’d initially claim domain modelling), so I write crap view code and I rarely write tests for it (someone’s just going to come along and rewrite it to be presentable anyway). It’s a fairly pathetic defence of my laziness, but anecdotally it seems lots of people don’t write view specs. So there’s a whole area of my application that’s not tested very well.

Craig also points out that one of the biggest sources of defects for him is that his controller doesn’t set up what the view expects (or you can express if from the opposite perspective if you prefer) – the failure occurs during the interaction between the controller and the view. These sorts of failures are quite difficult to find with unit tests alone, and probably won’t get picked up by distinct controller specs and view specs.

We could write separate integration specs, but RSpec gives a simple way to catch at least the most egregious of these problems. Unit testing purists might object, but the laissez faire’sts won’t mind. Put “integrate_views” into your descriptions. When integrate_views is specified, RSpec renders the real view rather than mocking out the rendering, and if objects are missing or badly misconfigured you’ll get a rendering exception.

I remembered the integrate_views option from earlier versions of RSpec – it’s not particularly conspicuous in the current version’s documentation, but it can be very useful.

Using the latest attributes in a Rails migration

Occasionally I need to make some change to a table via migrations, and then immediately use the attributes associated with the latest table inside the migration. Most of the time you can do this by defining the aspects of the class inside the migration – for example:

class AddDepartmentToProducts < ActiveRecord::Migration

  class Product < ActiveRecord::Base
    belongs_to :department
  end

  def self.up
    add_column :products, :department_id, :integer
    # do something with product.department
  end

  def self.down
    #...
  end

end

There’s also another way that’s not as neat, but gives you finer grained control, for example if you need to have different definitions of the class in the up and down migrations:

class AddDepartmentToProducts < ActiveRecord::Migration

  def self.up
    add_column :products, :department_id, :integer
    Object.class_eval <<-end_eval
    class Apparel21::Product < ActiveRecord::Base
      belongs_to :department
    end
  end_eval
    # do something with product.department
  end

  def self.down
    #...
  end

end

ActiveScaffold reverse associations to models in modules

I really enjoy using ActiveScaffold, and encourage everyone to use it to generate administration style interfaces. I came across an apparent defect today that I wanted to share.

I have a set of models that are contained in a module, and I’m using AS to do administration for them. Mostly fine. However there’s a problem when I click a link on an element of a many relationship that’s being displayed in a list. In this image,

ranges is the result of a has_many relationship, and when I click on the range ‘Summer Selection’, I get this:

and the following stacktrace:

ActionView::TemplateError (undefined method `reflect_on_all_associations' for Range:Class) on line #19 of vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:
16:       # determine what constraints we need
17:       if column.through_association?
18:         @constraints = {
19:           association.source_reflection.reverse => {
20:             association.through_reflection.reverse => parent_id
21:           }
22:         }

    vendor/plugins/active_scaffold/lib/extensions/reverse_associations.rb:26:in `reverse_matches_for'
    vendor/plugins/active_scaffold/lib/extensions/reverse_associations.rb:12:in `reverse'
    vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:19:in `_run_erb_47vendor47plugins47active_scaffold47frontends47default47views47_nested46rhtml'
    vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:11:in `each'
    vendor/plugins/active_scaffold/frontends/default/views/_nested.rhtml:11:in `_run_erb_47vendor47plugins47active_scaffold47frontends47default47views47_nested46rhtml'
    vendor/rails/actionpack/lib/action_view/base.rb:637:in `send'
    vendor/rails/actionpack/lib/action_view/base.rb:637:in `compile_and_render_template'
    vendor/rails/actionpack/lib/action_view/base.rb:365:in `render_template'
    vendor/rails/actionpack/lib/action_view/base.rb:316:in `render_file'
    vendor/rails/actionpack/lib/action_view/base.rb:331:in `render_without_active_scaffold'

The problem is that class should be MyModule::Range, not Range.

AS is looking up the class using code on ActiveRecord::Reflection::AssociationReflection, from the file activescaffold/lib/extensions/reverse_associations.rb. The fix (apparently – I haven’t tested this exhaustively yet) is to change the code that sends “class_name.constantize” to the association to send “klass” instead (you need to do this in two places). The completed code is attached below. I hope this helps someone else!

	module ActiveRecord
	  module Reflection
	    class AssociationReflection #:nodoc:
	      def reverse_for?(klass)
	        reverse_matches_for(klass).empty? ? false : true
	      end

	      attr_writer :reverse
	      def reverse
	        unless @reverse
	          # Following line changed for compatibility with associations on classes in modules
	          # reverse_matches = reverse_matches_for(self.class_name.constantize)
	          reverse_matches = reverse_matches_for(self.klass)
	          # grab first association, or make a wild guess
	          @reverse = reverse_matches.empty? ? self.active_record.to_s.pluralize.underscore : reverse_matches.first.name
	        end
	        @reverse
	      end

	      protected

	        def reverse_matches_for(klass)
	          reverse_matches = []

	          # stage 1 filter: collect associations that point back to this model and use the same primary_key_name
	          klass.reflect_on_all_associations.each do |assoc|
	            # skip over has_many :through associations
	            next if assoc.options[:through]

	            ## Following line changed for compatibility with associations on classes in modules
	            # next unless assoc.options[:polymorphic] or assoc.class_name.constantize == self.active_record
	            next unless assoc.options[:polymorphic] or assoc.klass == self.active_record
	            case [assoc.macro, self.macro].find_all{|m| m == :has_and_belongs_to_many}.length
	              # if both are a habtm, then match them based on the join table
	              when 2
	              next unless assoc.options[:join_table] == self.options[:join_table]

	              # if only one is a habtm, they do not match
	              when 1
	              next

	              # otherwise, match them based on the primary_key_name
	              when 0
	              next unless assoc.primary_key_name.to_sym == self.primary_key_name.to_sym
	            end

	            reverse_matches < 1

	          reverse_matches
	        end

	    end
	  end
	end

Uses of OpenID

I’m a user of OpenID, but this video from Google is an excellent introduction to the concept, the uses, and the pros and cons.

Particularly interesting to me as a developer were the ideas of using OpenID as a ‘lightweight’ authentication mechanism for sites that want a low barrier for registration and access, and recognizing multiple openids so that a site can access id-specific features from each. Highly recommended!

Rspec 1.1.2 and Textmate

A quick warning to everyone – if you upgrade to RSpec 1.1.2 and you use Textmate, you’ll probably need to update your textmate bundle as well.

cd ~/Library/Application\ Support/TextMate/Bundles/
svn co svn://rubyforge.org/var/svn/rspec/trunk/RSpec.tmbundle