[rspec-users] Help needed

Ashley Moran ashley.moran at patchspace.co.uk
Sun May 18 04:40:23 EDT 2008

On 17 May 2008, at 18:34, Jens Carroll wrote:
> Hi All,
> I am new to rspec and it seems that I don't understand some basics.

Hi Jens,

Jarkko answered your real question but there are other things worth  
pointing out, more about style than actually making the specs work.

> ---- spec -------
> module XmlImportSpecHelper
>  def mock_xml_import
>    xml_file = RAILS_ROOT + "/spec/fixtures/import-member.xml"
>    xml = File.read(xml_file)
>    @xml_import = XmlImport.new
>    @xml_import.should_receive(:open).exactly(1).times.
>      with("any-file-name.xml").
>      and_return(xml)
>  end
> end

You shouldn't have (hoho) a should_receive here.  You're using the  
mock_xml_import in the before step, which is intended to set up mocks  
and stubs shared across the examples ("it" blocks).  The code  
"@xml_import.should_receive(:open)..." should have its own example.

Also, since you're creating a *real* XmlImport, it doesn't make sense  
to call it "mock_xml_import", maybe "new_xml_import" would be clearer.

> describe XmlImport do
>  include XmlImportSpecHelper
>  before(:each) do
>    mock_xml_import
>    @xml_import.parse_xml("any-file-name.xml")
>    @country = mock_model(Country)
>    Country.stub!(:find_by_short).and_return(@country)
>  end
>  it "should find country object for DE" do
> Country.should_receive(:find_by_short).with("DE").and_return(@country)
>    @xml_import.user.country.should equal(@country)
>  end
> end

Be clear what you are specifying here.  Your call to #parse_xml is in  
the before block, which means you can't set expectations on it.  I  
like to nest my methods in their own inner describe blocks, eg:

   describe XmlImport do
     before(:each) do
       @xml_import = XmlImport.new

     describe "#parse_xml" do
       it "should open the XML file" do
         # ...

       it "should find country object for DE" do
         # ...

Be aware also that while "xml_import.should_receive(:open)" may work,  
it's a bit of a quirk of Ruby.  The real method is define on the  
Kernel module, which is included into Object (so you get access to it  
everywhere).  Since you know you're dealing with files, I'd use  
File.open, and specify that File.should_receive(:open) instead.   
Otherwise you may start thinking it's ok to specify interpendencies  
between methods in the class being specified.

Finally, this line:

>   @xml_import.user.country.should equal(@country)

is known as a trainwreck- that is, multiple method calls strung  
together.  It's usually a sign that something is modelled wrong.  In  
this case, it's because your XmlParser class is doing too much.

This section

>   (@doc/:member).each do |data|
>     retrieve_and_save_user(data)
>   end

Is the culprit.  The XmlParser should parse XML, not retrieve and save  
user data (I know this, because it's called XmlParser, not  
UserDataRetrieverAndSaver, basically.)  A more OO approach would be to  
make User responsible for this:

   (@doc/:member).each do |data|

That way, the spec for User#update_from_xml would have one less level  
of indirection, ie just

   @user.country.should equal(@country)

Hope this is useful,


More information about the rspec-users mailing list