How should I spec this

Question

The following spec works but I know it shouldn't be like this. I am having a hard time getting my head around rspec, in particular mocks and stubs.

This is the model code

class RecipeFermentable < ActiveRecord::Base
  belongs_to :recipe
  belongs_to :product

  def set_attributes()
    attrs = product.product_attributes
    self.ppg = attrs.find_by_name(:ppg.to_s).value
    self.ecb = attrs.find_by_name(:ecb.to_s).value
  end
end

And this is the spec I have written

  it "should set the attributes from the product" do
    @product_attribute1 = mock_model(ProductAttribute, :name => :ppg, :value => 40)
    @product_attribute2 = mock_model(ProductAttribute, :name => :ecb, :value => 1)

    @product = Product.new
    @product.product_attributes << @product_attribute1
    @product.product_attributes << @product_attribute2
    @recipe_fermentable = RecipeFermentable.new
    @recipe_fermentable.product.should_receive(:product_attributes).and_return(@product_attributes)
    @product_attributes.stub(:find_by_name).with(:ppg.to_s).and_return(@product_attribute1)
    @product_attributes.stub(:find_by_name).with(:ecb.to_s).and_return(@product_attribute2)

    @recipe_fermentable.set_attributes

    @recipe_fermentable.ppg.should eql(40)
    @recipe_fermentable.ecb.should eql(1)
 end

For a start my spec is way bigger than my method, and I am using a real Product. Some pointers on the way to write a goodbetter spec for this would be really helpfull. Also if anyone knows of a good resource for learning rspec using mocks and stubs, please could you add some links.

Thanks

Answer 1

I would change a couple of things here:

• Much of the code in your it is just providing context, so it should be in your before(:each) block.
• You are setting a message expectation, but it doesn't really seem like you are testing for that. I think the expectation should be switched to a stub instead. Another test could be it 'should call product_attributes', where you would in fact test that expectation - I am not advocating you do this since you would be testing implementation and not behavior, but just making the point.
• You are returning @product_attributes in that message expectation, and using it right after to stub the find_by_name calls. However, you never defined @product_attributes. I assume that should be a mock object, and I'm not sure what it really is in that context. Maybe it is nil, and you are stubbing a couple of methods on it.

With those two changes, here's where we are:

before(:each) do
  @product                = mock_model(Product)
  @product_attribute_ppg  = mock_model(ProductAttribute, :name => :ppg, :value => 40)
  @product_attribute_ecb  = mock_model(ProductAttribute, :name => :ecb, :value => 1)
  @product_attributes     = mock('product_attributes')
  @product_attributes.stub!(:find_by_name).with(:ppg.to_s).and_return(@product_attribute_ppg)
  @product_attributes.stub!(:find_by_name).with(:ecb.to_s).and_return(@product_attribute_ecb)
  @product.stub!(:product_attributes).and_return(@product_attributes)

  @recipe_fermentable = RecipeFermentable.new
  @recipe_fermentable.stub!(:product).and_return(@product)
end

it 'should set the attributes from the product' do
  @recipe_fermentable.set_attributes
  @recipe_fermentable.ppg.should eql(40)
  @recipe_fermentable.ecb.should eql(1)
end

With all of that all of the way, I don't completely agree with your approach here. I think that you are repeating data and moving away from DB normalization. Unless there's a real reason for that (could be that your way ahead and for performance reasons you had to do this), I would suggest the following instead:

class RecipeFermentable < ActiveRecord::Base
  def ppg
    #rescue nil here so that if attributes is nil, or find_by_name('ppg') is nil, things don't blow up
    product.attributes.find_by_name('ppg').value rescue nil
  end

  #other
end

A couple of resources for testing:

• RSpec book
• xUnit patterns: not RSpec nor Ruby, but a must.