September 1, 2007

rails - save! broke the contract - mocking to rescue

Posted by Sudhindra Rao

Detail descriptions of mockist and classicist are on Martin Fowler's blog.


Consider

   class Person < ActiveRecord::Base
       #has name, address, home, business, cell phone numbers
      .... more object behaviour
   end


The intended behaviour is to do some system specific tasks incognito - lets say send an email to all his friends.

So we alias the save method as follows

   alias :save_with_notification, :save


   def save_with_notifications
      send_emails
      save
   end

This works well for save. But when somewhere we call save! it all breaks - i.e. our save_with_notifications is not called and nobody gets notified.

The root cause(as analysed by David Vollbracht (not this one)) of this problem was this piece of rails code

   def save
      create_or_update
   end

   # Attempts to save the record, but instead of just returning

   # false if it couldn't happen, it raises a

   # RecordNotSaved exception

   def save!

      create_or_update || raise(RecordNotSaved)

   end


save! does not call save.

On further investigation I found functional tests around the base class for active_record.

But what was missed was a good unit test with mocking like so

   test "save! should successfully save object by calling save" do
      base = Base.new
      base.expects(:save).returns(true)
      assert_nothing_raised base.save!
   end

   test "save! should throw on exception if save fails" do
      base = Base.new
      base.expects(:save).returns(false)
      assert_raises RecordNotSaved, base.save!
   end

Here is a place where mocking does a much better job at testing a specific contract that is a convention and was missed due to black box testing.

This is not the only way to fix this problem though.

The real problem is that the requirements did not capture the fact of aliasing save or save! methods.

Testing this scenario functionally is perfectly possible though the testing could get more involving.

The tests in that case would make sure that aliasing of save method does not break save!.

But the above mocked tests are as simple as it gets and they reflect the simpler(obvious?) underlying requirement of save! calling save.

Note: Formatting code on blogger sucks

Welcome