Al Chou
2007-Nov-20 06:00 UTC
[rspec-users] confirming that a model instance was correctly created from POST params
I''m wondering whether I''m wasting my time trying to verify
that an addition I''ve made to legacy code is in fact setting a new
attribute on a model.
Substruct''s (http://dev.subimage.com/projects/substruct) OrderHelper
contains this method:
1 def create_order_from_post
2 @use_separate_shipping_address =
params[:use_separate_shipping_address]
3
4 @order_user = OrderUser.find_or_create_by_email_address(
5 params[:order_user][:email_address]
6 )
7 @order_user.valid?
8
9 @order = Order.new(params[:order])
10 @order.valid?
11
12 # Look up billing address and update if customer is logged in.
13 if @customer
14 @billing_address = @customer.billing_address
15 @billing_address.attributes = params[:billing_address]
16 @shipping_address = @customer.billing_address
17 @shipping_address.attributes = params[:shipping_address]
18 else
19 @billing_address = OrderAddress.new(params[:billing_address])
20 @shipping_address = OrderAddress.new(params[:shipping_address])
21 end
22 @billing_address.valid?
23
24 if @use_separate_shipping_address
25 @shipping_address.valid?
26 end
27
28 @order_account = OrderAccount.new(params[:order_account])
29 @order_account.valid?
30
31 OrderUser.transaction do
32 @order_user.save!
33 Order.transaction do
34 @order = @order_user.orders.build(params[:order])
35 @order.order_number = Order.generate_order_number
36 @order.save!
37 end
38 OrderAddress.transaction do
39 # Addresses
40 @billing_address =
@order_user.order_addresses.create(params[:billing_address])
41 @billing_address.order_id = @order.id
42 @billing_address.is_shipping = true
43 @billing_address.save!
44 if @use_separate_shipping_address then
45 @shipping_address =
@order_user.order_addresses.create(params[:shipping_address])
46 @shipping_address.is_shipping = true
47 @billing_address.is_shipping = false
48 @billing_address.save!
49 @shipping_address.order_id = @order.id
50 @shipping_address.save!
51 end
52 end
53 OrderAccount.transaction do
54 @order_account = OrderAccount.new(params[:order_account])
55 @order_account.order_id = @order.id
56 @order_account.order_user_id = @order_user.id
57 @order_account.order_address_id = @billing_address.id
58 @order_account.save!
59 end
60 end
61 end
I am adding to the OrderAddress model the concept of whether an address is a
business address (as opposed to a home/residential address):
describe OrderHelper do
it ''should record the fact that the address is a business
address'' do
1 order = mock_model( Order, :null_object => TRUE )
2 order.stub!( :valid? ).and_return( TRUE )
3 Order.stub!( :new ).and_return( order )
4 order.stub!( :order_number= )
5 order.stub!( :save! )
6
7 params[:order_user] = Hash.new
8 params[:order_user][:email_address] = ''''
9 order_user = mock_model( OrderUser )
10 order_user.stub!( :valid? ).and_return( TRUE )
11 order_user.stub!( :save! )
12 orders = mock( ''orders'' )
13 orders.stub!( :build ).and_return( order )
14 order_user.stub!( :orders ).and_return( orders )
15 OrderUser.stub!( :find_or_create_by_email_address ).and_return(
order_user )
16 order_user.stub!( :order_addresses ).and_return( OrderAddress )
17
18 order_address = mock_model( OrderAddress ) #, :null_object =>
TRUE )
19 order_address.stub!( :valid? ).and_return( TRUE )
20 OrderAddress.stub!( :new ).and_return( order_address )
21 order_address.stub!( :order_id= )
22 order_address.stub!( :is_shipping= )
23 order_address.stub!( :save! )
24 order_address.should_receive( :save )
25
26 order_account = mock_model( OrderAccount )
27 order_account.stub!( :valid? ).and_return( TRUE )
28 order_account.stub!( :order_id= )
29 order_account.stub!( :order_user_id= )
30 order_account.stub!( :order_address_id= )
31 order_account.stub!( :save! )
32 OrderAccount.stub!( :new ).and_return( order_account )
33
34 assigns[:use_separate_shipping_address] = FALSE
35 params[:is_business_address] = TRUE
36 create_order_from_post
37
38 assigns[:billing_address].is_business_address.should be_true
end
end
What I am trying to demonstrate to myself is that the addition of a boolean
"is_business_address" column to the order_addresses table via a
migration allows line 40 of the create_order_from_post method to set this
property when an OrderAddress instance is created. But my example as shown
tells me that assigns[:billing_address] is nil when I run it. Also, I realize
that line 16 of the example isn''t returning the right thing; it should
be an AssociationCollection of OrderAddress. That''s where I''m
starting to wonder if I''m spending too much time on something that is
probably simple enough that it''s already working but disproportionately
hard to write an RSpec example for (because the existing create_order_from_post
method has no tests or examples and thus isn''t easy to test -- witness
the 32 lines, modulo blank lines, of stubbing just to get the example to run).
Suggestions on how to write the example, or advice on whether to continue to
try, are requested.
Al
____________________________________________________________________________________
Never miss a thing. Make Yahoo your home page.
http://www.yahoo.com/r/hs
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/rspec-users/attachments/20071119/cf3f5baf/attachment-0001.html
Jarkko Laine
2007-Nov-20 08:04 UTC
[rspec-users] confirming that a model instance was correctly created from POST params
On 20.11.2007, at 8.00, Al Chou wrote:> What I am trying to demonstrate to myself is that the addition of a > boolean "is_business_address" column to the order_addresses table > via a migration allows line 40 of the create_order_from_post method > to set this property when an OrderAddress instance is created. But > my example as shown tells me that assigns[:billing_address] is nil > when I run it. Also, I realize that line 16 of the example isn''t > returning the right thing; it should be an AssociationCollection of > OrderAddress. That''s where I''m starting to wonder if I''m spending > too much time on something that is probably simple enough that it''s > already working but disproportionately hard to write an RSpec > example for (because the existing create_order_from_post method has > no tests or examples and thus isn''t easy to test -- witness the 32 > lines, modulo blank lines, of stubbing just to get the example to > run). > > Suggestions on how to write the example, or advice on whether to > continue to try, are requested.A couple of points: * I have a similar situation with shipping and billing addresses. I solved it by adding attr_accessor :single_address to the customer model. That gives you the option of using "check_box :single_address" in the form for the customer and it will automatically be passed to the model when you update its attributes. I''ve noticed it cleans up the controller code a lot. * for line 16 in the spec, you need to do something like this: order_addresses = [] order_addresses.stub!(:create).and_return(order_address) # order_address must be created before this order_user.stub!( :order_addresses ).and_return( order_addresses ) * You should put all the stubbing and mocking in a before(:each) block, they don''t really belong to the actual spec. * What you have in your helper is basically business domain stuff. Helpers in Rails are meant for streamlining view code. I just refactored some code that does similar things that yours does, and noticed that it fits perfectly in the model. So in your case, you could add e.g. a class method Order.create_with_addresses_and_user (params[:order], params[:order_user], params[:billing_address], params [:shipping_address], params[:order_account]). In that case, all you''d have to spec in the controller is that the class method is called with correct params. All the other spec''ing would go into the model''s specs. * Unless your db supports nested transactions (very few do), there''s no point in putting nested transaction blocks in your code. * You can shorten your mocks a bit by defining the stubbed methods already in the mock_model call: order_account = mock_model( OrderAccount, :valid? => true, :order_id= => true, :save! => true ) and so on. Also, once your specs are more than just the one, you start finding patterns there and you can easily refactor the stubbing into spec helpers so that you only need a line or two to create a comprehensive mock model in your specs. HTH, //jarkko P.S. in Ruby true and false are normally written in lower case. -- Jarkko Laine http://jlaine.net http://dotherightthing.com http://www.railsecommerce.com http://odesign.fi
Al Chou
2007-Nov-20 17:33 UTC
[rspec-users] confirming that a model instance was correctly created from POST params
Thanks, Jarkko. Substruct is not my code, so I don''t know how OrderHelper got to be so big. I''m not sure I want to refactor its contents into Order just to make it easier to write the examples for my addition, but at least I have that option because the project is open source! I''ll have a detailed look at your suggestions later today, hopefully. Rails programming is not my day job, unfortunately! Al ----- Original Message ---- From: Jarkko Laine <jarkko at jlaine.net> To: rspec-users <rspec-users at rubyforge.org> Sent: Tuesday, November 20, 2007 12:04:13 AM Subject: Re: [rspec-users] confirming that a model instance was correctly created from POST params On 20.11.2007, at 8.00, Al Chou wrote:> What I am trying to demonstrate to myself is that the addition of a > boolean "is_business_address" column to the order_addresses table > via a migration allows line 40 of the create_order_from_post method > to set this property when an OrderAddress instance is created. But > my example as shown tells me that assigns[:billing_address] is nil > when I run it. Also, I realize that line 16 of the example isn''t > returning the right thing; it should be an AssociationCollection of > OrderAddress. That''s where I''m starting to wonder if I''m spending > too much time on something that is probably simple enough that it''s > already working but disproportionately hard to write an RSpec > example for (because the existing create_order_from_post method has > no tests or examples and thus isn''t easy to test -- witness the 32 > lines, modulo blank lines, of stubbing just to get the example to > run). > > Suggestions on how to write the example, or advice on whether to > continue to try, are requested.A couple of points: * I have a similar situation with shipping and billing addresses. I solved it by adding attr_accessor :single_address to the customer model. That gives you the option of using "check_box :single_address" in the form for the customer and it will automatically be passed to the model when you update its attributes. I''ve noticed it cleans up the controller code a lot. * for line 16 in the spec, you need to do something like this: order_addresses = [] order_addresses.stub!(:create).and_return(order_address) # order_address must be created before this order_user.stub!( :order_addresses ).and_return( order_addresses ) * You should put all the stubbing and mocking in a before(:each) block, they don''t really belong to the actual spec. * What you have in your helper is basically business domain stuff. Helpers in Rails are meant for streamlining view code. I just refactored some code that does similar things that yours does, and noticed that it fits perfectly in the model. So in your case, you could add e.g. a class method Order.create_with_addresses_and_user (params[:order], params[:order_user], params[:billing_address], params [:shipping_address], params[:order_account]). In that case, all you''d have to spec in the controller is that the class method is called with correct params. All the other spec''ing would go into the model''s specs. * Unless your db supports nested transactions (very few do), there''s no point in putting nested transaction blocks in your code. * You can shorten your mocks a bit by defining the stubbed methods already in the mock_model call: order_account = mock_model( OrderAccount, :valid? => true, :order_id= => true, :save! => true ) and so on. Also, once your specs are more than just the one, you start finding patterns there and you can easily refactor the stubbing into spec helpers so that you only need a line or two to create a comprehensive mock model in your specs. HTH, //jarkko P.S. in Ruby true and false are normally written in lower case. -- Jarkko Laine http://jlaine.net http://dotherightthing.com http://www.railsecommerce.com http://odesign.fi _______________________________________________ rspec-users mailing list rspec-users at rubyforge.org http://rubyforge.org/mailman/listinfo/rspec-users ____________________________________________________________________________________ Get easy, one-click access to your favorites. Make Yahoo! your homepage. http://www.yahoo.com/r/hs -------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/rspec-users/attachments/20071120/5bb8f732/attachment-0001.html