Help needed

Testing RSpec
1

Hi All,

I am new to rspec and it seems that I don’t understand some basics.
I need to have a XML import which should parse through XML data
and saves all that in various mysql tables. The XML part works just
fine and I can test this with rspec. However when I try to execute

it “should find country object for DE” do

I get an error. @user.country is a one-to-many relation in the user
table. It seems easy but I don’t get it (hmmmm feels like I am still a
newbie).

The error I get is:

‘XmlImport should find country object for DE’ FAILED
expected #<Country:0x…fdb71beba @name=“Country_1001”>, got nil (using
.equal?)

Any help very appreciated.
Thanks
Jens

— model —

require ‘hpricot’
class XmlImport #< ActiveRecord::Base

attr_reader :doc, :user

def parse_xml(file)
@doc = Hpricot.XML(open(file))

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

end

def retrieve_and_save_user(data)
# email address must be unique for members
@user = User.find_or_initialize_by_email(email)

 # save user if not existing
 if @user.new_record?
   @user.country =

Country.find_by_short((data/:countryShort).inner_html)
end
end
end

---- 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

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

2

Hi Jens,

On 17.5.2008, at 20.34, Jens Carroll wrote:

table. It seems easy but I don’t get it (hmmmm feels like I am still
Thanks
@doc = Hpricot.XML(open(file))

save user if not existing

module XmlImportSpecHelper

@country = mock_model(Country)
Country.stub!(:find_by_short).and_return(@country)

In order to have the above stubbing and mocking work, it must be done
before you use them in the actual code. It seems to me that
parse_xml is where you want to use them.

end

it “should find country object for DE” do

Country.should_receive(:find_by_short).with(“DE”).and_return(@country)

Same here. should_receive must always be defined before you run the
code that is expected to do something. In this case, parse_xml and
thus Country.find_by_short is run without any stubbing so it will go
to the database and probably find nothing.

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

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

–
Jarkko L.

http://www.railsecommerce.com
http://odesign.fi

3

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.

 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.

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
end

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

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

end

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|
User.update_from_xml(data)
end

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,
Ashley

–
http://www.patchspace.co.uk/

4

Jarkko L. schrieb:

(using .equal?)

def retrieve_and_save_user(data)

@xml_import.should_receive(:open).exactly(1).times.
before(:each) do

Jarkko L.
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

Hi Jarkko,

Thanks a lot. The next time when I get a nil object I’d better ask
myself if the mocking and stubbing
has been before the actual code. The real problem was that my whole
rspec test got quite big. I need to break it down in smaller pieces not
to lose the overview.

Have a nice day
Jens

5

Ashley M. schrieb:

 with("any-file-name.xml").

Also, since you’re creating a real XmlImport, it doesn’t make sense

  it "should open the XML file" do

it’s a bit of a quirk of Ruby. The real method is define on the

of indirection, ie just

@user.country.should equal(@country)

Hope this is useful,
Ashley

Hi Ashley,

Your hints are most appreciated. Wow looks like I have to change quite a
bit.

I was not aware that “@xml_import.user.country.should equal(@country)”
is already
a trainwreck. I think I have even longer ones - refactoring might be the
magic
word for my code now.

Every hint was really useful for me.

Thanks a million for taking the time to analyze my code.

Have a nice day
Jens

6

On 18 May 2008, at 18:28, Jens Carroll wrote:

I was not aware that “@xml_import.user.country.should
equal(@country)” is already
a trainwreck. I think I have even longer ones - refactoring might be
the magic
word for my code now.

Well I guess it’s more the wrong sort of leaves on the line :slight_smile: It’s
not so much the length of the line that’s the problem, that’s just a
code smell. The real issue is that it makes more sense for the user
(domain model) to update itself off the XML, rather than have an XML
parser (utility class) go poking around inside the user.

Having had a second look at it, I’d be inclined to say that
XmlImport#parse_xml should really be a class method of User (something
like User.import_all_from_xml). The only bit of this method that
isn’t domain logic is the line

@doc = Hpricot.XML(open(file))

which should probably be in the controller, or wherever this code gets
called. You don’t want the domain logic depending on filesystem
technicalities and more than the utility code fiddling with the model.

Ashley

–
http://www.patchspace.co.uk/