Hi, I used to have the following method: def Paypal.process_ipn(params) ... paypal = create!(params) paypal.order.update_from_ipn(completed?) end That method obviously is not easily specable because of the double dot method call, and when specing it, it would hit the DB for nothing. I used to actually spec the associated order status to know if everything went on well which breaks the isolation of unit tests. Bad bad bad. ----------- So I refactored it as following: def Paypal.new_from_ipn(params) ... new(params) end So that it doesn''t hit the DB when I spec the method, and specing the method can now be done in isolation, as I only set some values returned by IPN to something more compliant with my DB storage and then in the controller that calls this method: paypal = Paypal.new_from_ipn(params) paypal.update_order if paypal.save! So the paypal instance still gets saved, but instead of getting saved in the model it occurs in the controller, no big deal. Also I added a new instance method to Paypal class to abid to Demeter''s law: def update_order order.update_from_paypal_ipn!(completed?) end But now I am wondering how to spec this instance method, I thought of the following: it "should update the associated order" do @paypal = Paypal.new(:payment_status => ''Completed'') @order = @paypal.order @order.expects(:update_from_paypal_ipn!).with(true) @paypal.update_order end Is that spec acceptable? I mean I am not sure about the following two lines: @order = @paypal.order @order.expects(:update_from_paypal_ipn!).with(true) I thought about this other solution which is slightly more complicated: @order = Order.new @paypal = Paypal.new(:payment_status => ''Completed'') @paypal.stubs(:order).returns(@order) @order.expects(:update_from_paypal_ipn!).with(true) @paypal.update_order So which is best? And what do you think of my refactoring and my new specs? Did I improve the code or is it just crap? -- Posted via http://www.ruby-forum.com/.
On Wed, Apr 15, 2009 at 9:36 AM, Fernando Perez <lists at ruby-forum.com> wrote:> Hi, > > I used to have the following method: > > def Paypal.process_ipn(params) > ?... > ?paypal = create!(params) > ?paypal.order.update_from_ipn(completed?) > end > > That method obviously is not easily specable because of the double dot > method call, and when specing it, it would hit the DB for nothing. I > used to actually spec the associated order status to know if everything > went on well which breaks the isolation of unit tests. Bad bad bad. > > ----------- > > So I refactored it as following: > > def Paypal.new_from_ipn(params) > ?... > ?new(params) > end > > So that it doesn''t hit the DB when I spec the method, and specing the > method can now be done in isolation, as I only set some values returned > by IPN to something more compliant with my DB storage and then in the > controller that calls this method: > > paypal = Paypal.new_from_ipn(params) > paypal.update_order if paypal.save! > > > So the paypal instance still gets saved, but instead of getting saved in > the model it occurs in the controller, no big deal. Also I added a new > instance method to Paypal class to abid to Demeter''s law: > > def update_order > ?order.update_from_paypal_ipn!(completed?) > end > > But now I am wondering how to spec this instance method, I thought of > the following: > > it "should update the associated order" do > ?@paypal = Paypal.new(:payment_status => ''Completed'') > ?@order = @paypal.order > ?@order.expects(:update_from_paypal_ipn!).with(true) > > ?@paypal.update_order > end > > > Is that spec acceptable? I mean I am not sure about the following two > lines: > > @order = @paypal.order > @order.expects(:update_from_paypal_ipn!).with(true) > > I thought about this other solution which is slightly more complicated: > ?@order = Order.new > ?@paypal = Paypal.new(:payment_status => ''Completed'') > ?@paypal.stubs(:order).returns(@order) > ?@order.expects(:update_from_paypal_ipn!).with(true) > ?@paypal.update_order > > So which is best? > > And what do you think of my refactoring and my new specs? Did I improve > the code or is it just crap? > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >I would probably move the call to update_order into a before/after_create on Paypal. Then I would write a spec on Paypal that checks that the right thing happened. I would not likely use a mock unless Order#update_from_paypal_ipn! is very complex. describe Paypal, "from IPN" do it "should update its order when created" do paypal = Paypal.new_from_ipn :ipn => {:completed => true} paypal.save! paypal.order.should be_completed end end class Paypal after_create :update_order def update_order order.update_from_paypal_ipn! completed? end end By doing it in a callback you get * transaction semantics for free * ability to use resource_controller * simpler model API If you only want to update the order when Paypal.new_from_ipn is called (and not Paypal.new, for example) then just put an ivar in Paypal and check for it in the callback. So to recap, I would test this behavior via the Paypal examples, because that''s where the behavior originates. I may or may not mock the call to order.update_from_paypal depending on how complex it is. Does that make sense? Pat
> So to recap, I would test this behavior via the Paypal examples, > because that''s where the behavior originates. I may or may not mock > the call to order.update_from_paypal depending on how complex it is. > > Does that make sense?Argh! I had sent you an answer but for some reason my session expired and the message didn''t go through. Anyway, thank you Pat for your message, I like to check out your blog to fetch some tips on how to spec correctly. The callback is a good idea, I don''t use them often enough. order.update_from_paypal is a simple method that just sets the order state to ''paid'' or ''unpaid'' depending on the payment status. I use other payment gateways so it is paypal agnostic. Now that I test my code I tend to write smaller methods. -- Posted via http://www.ruby-forum.com/.
By the way in Rails I am now finding myself replacing: update_attributes, create! and their friends with something that looks like: new(...) save! Then in the spec I stub the save! method so that it doesn''t hit the DB, and then I can easily compare the object attributes if they are as expected. Pros: - specs are lightning fast Cons: - data isn''t actually inserted in DB, so there is a 0.000001% chance that the object has bad attributes that would raise an error if it was actually saved in DB. But that would mean that my spec is false as I myself set the comparison value. Is it clever or not to do something like that? Maybe I can use that idea sometimes, and the other times it is safer to really save the object in DB? -- Posted via http://www.ruby-forum.com/.
Joaquin Rivera Padron wrote:> hey there, > maybe you should take a look at solutions that fake your database in > memory > for such cases, saving your time doing all that stubbing and mockingYeah you are right. I am refactoring (messing up?) code because I have a DB constraint, so instead of rewriting the code I should find a better DB to use during tests. What''s up will nullDB? I once saw the developper post a few message on the mailing-list. Anyone using it with success? -- Posted via http://www.ruby-forum.com/.
> What''s up will nullDB? I once saw the developper post a few message on > the mailing-list. Anyone using it with success?I saw a post by Pat Maddox and he talked about Sqlite in-memory, so I decided to give it a try using this tutorial: http://www.mathewabonyi.com/articles/2006/11/26/superfast-testing-how-to-in-memory-sqlite3/ When I run ''rake test'', I have 1 failure in one of my test file. if I test that file alone, there is no failure. The test that fails doesn''t hit the DB at any time though, so it''s really strange. So for now I will forget about hitting the database and in-memory databases, and just focus on writing my specs, but it''s definitely an interesting idea. -- Posted via http://www.ruby-forum.com/.