Hello all,
In yet another attempt to learn Ruby on Rails and Rspec I have started
writing a simple life streaming app with will aggregate feeds from several
places and save them in a database for later use.
An internet search eventually led me to the following method for looping
through the feeds in the database, getting the contents of the URL and then
passing this into another model to prepare and save it.
def self.cache_all
feeds = self.find(:all)
for feed in feeds
xml = REXML::Document.new Net::HTTP.get(URI.parse(feed.url))
xml.elements.each ''//item'' do |item|
Item.prepare_and_save(feed, item)
end
end
end
Problems are now beginning to arise when trying to write the specs for this
method. I started from scratch slowly building up the specs but it has led
to an awful amount of mocks and stubs, and I am not even sure whether they
are asking the correct things of the method.
Can anyone give me some pointers on how to write useful, meaningful specs
for this method?
The other thing I have found is that I seem to have incorrectly stubbed
xml.elements.each meaning that the contents of the block are never called,
how should I be specifying this behavior?
I have pastied the complete code for the Feed model, the spec and a little
helper at: http://pastie.caboo.se/203941
Thanks in advance,
Andy
--
View this message in context:
http://www.nabble.com/Help-with-Writing-Meaningful-Specs-tp17488575p17488575.html
Sent from the rspec-users mailing list archive at Nabble.com.
On 27 May 2008, at 12:44, andypearson wrote:> xml = REXML::Document.new Net::HTTP.get(URI.parse(feed.url))It won''t make the problem go away, but you can certainly reduce the blizzard of intermediate stubs by pulling this chain out into its own method (e.g. fetch_xml_from_url) and stubbing that method once in your cache_all spec.> it has led to an awful amount of mocks and stubs, and I am not even > sure whether they > are asking the correct things of the method. > Can anyone give me some pointers on how to write useful, meaningful > specs > for this method?What part of its behaviour do you care about? (Refactoring into more fine-grained methods might help again here.) It seems as though all you''re interested in is that 1. the feed URLs get fetched from the database, 2. the feed contents get fetched from the URLs, and 3. Item.prepare_and_save gets called for each item in each feed. So, ideally, that''s what your specs for this method should be saying.> The other thing I have found is that I seem to have incorrectly > stubbed > xml.elements.each meaning that the contents of the block are never > called, > how should I be specifying this behavior?Use #and_yield, not #and_return. (And you might as well use an actual mock, rather than [@element], since you''re stubbing the only method you plan to call on it.) Cheers, -Tom
On 27 May 2008, at 12:44, andypearson wrote:> def self.cache_all > feeds = self.find(:all) > > for feed in feeds > xml = REXML::Document.new Net::HTTP.get(URI.parse(feed.url)) > xml.elements.each ''//item'' do |item| > Item.prepare_and_save(feed, item) > end > end > end > > Problems are now beginning to arise when trying to write the specs > for this > method. I started from scratch slowly building up the specs but it > has led > to an awful amount of mocks and stubs, and I am not even sure > whether they > are asking the correct things of the method. > > Can anyone give me some pointers on how to write useful, meaningful > specs > for this method?Hi Andy I think the problem comes from two things - trying to specify the details of an algorithm instead of checking it transforms the data correctly, and having too much logic in a class method. To take some Feed-specific logic out, try making an accessor method, such as Feed#uri (I can''t think of a good name for it), then you could replace one line with this: xml = REXML::Document.new = Net::HTTP.get(feed.uri) Alternatively, go one step further and have a method in Feed that does the Net::HTTP.get, so you could write this: xml = feed.xml_source Obviously you will still need the corresponding specs in the Feed instance methods, but at least you''ve encapsulated the logic related to fetching the URI/XML. The other thing is the "item" elements. I don''t know what xml.elements.each yields, but I''m guessing it''s either plain text (XML) or some REXML object. If it''s plain XML fine, but if it''s an REXML object, you don''t want your code depending on that (you''re tying yourself down to a specific library). Instead, consider parsing the XML in Feed.cache_all. I''m imagining something like this: feed.prepare_and_save_item(:item_attr_1 => "foo", :item_attr_2 => "bar", ...)> The other thing I have found is that I seem to have incorrectly > stubbed > xml.elements.each meaning that the contents of the block are never > called, > how should I be specifying this behavior?Just noticed that Tom replied and pointed out the issue. But if you refactor the code like I describe above, you can avoid the issue. Just stub feed.xml_source (or whatever) to return some sample valid feed XML, and check that Feed.cache_all correctly extracts the item data. I can''t say that my way is better or worse than the way you are going about it. But I (where possible) I always test the output of third- party libraries rather than my use of them. It avoids loads of stubs that don''t, in any case, prove you''re using the library correctly so you can see immediately if your code is likely to work in the wild. Hope this helps Ashley -- http://www.patchspace.co.uk/ http://aviewfromafar.net/