A Good Spec Should Be Easy to Read

Submitted by swarut on Thu, 06/21/2012 - 14:46

I am working on a test-intensive project. Yesterday, I just implement one spec for the code I just wrote. However, I made a very big mistake. Here is a part of the spec code.

it 'should set iCal alarms based on user preferences' do
  user.prefs = User.default_preferences
  user.save!
 
  EventRsvp.make!(user: user, event: event, joining: 'Y')
 
  alarms = subject.alarms
  alarms.should_not be_empty
  alarms.first.description.should eq event.title
  alarms.first.action.should eq 'DISPLAY'
  alarms.first.trigger.should eq '-PT15M'
 
  feed.events.inject(0){ |count, item| count += item.alarms.present? ? 1 : 0}.should == 1
end

The thing I miss is that the code is hard to read. It took one a while to understand what the last statement feed.events.inject(0){ |count, item| count += item.alarms.present? ? 1 : 0}.should == 1 does. As spec is a description of the software behavior, other people who are not the code owner should be able to read and understand it easily. This is an important objective, and I broke it T_T. Finally, I got my code improved by my senior. Here is the better one he made.

conntext 'given user has iCal reminder preference enabled' do
  subject { decorated.to_ics(user) }
  before(:each) do
    user.prefs = User.default_preferences.merge(
      ical_notifications: { user_notification: true }
    )
    user.save!
  end
 
  context 'and RSVPed to the event' do
    before { EventRsvp.make!(user: user, event: event, joining: 'Y') }
    it 'should set feed alarms' do
      alarms = subject.alarms
      alarms.should_not be_empty
      alarms.first.description.should eq event.title
      alarms.first.action.should eq 'DISPLAY'
      alarms.first.trigger.should eq '-PT15M'
    end
  end
  context 'and did not RSVP to the event' do
    it 'should not set feed alarms' do
      alarms = subject.alarms
      alarms.should be_empty
    end
  end
end