Monday, July 11, 2011

Editing Functional Tests in Rails

There was a time I was afraid of breaking my code. And then there was a time I was less afraid of breaking my code, but I became obsessed with fixing every little error and bug that I ended up wasting time on something relatively trivial when I could be working on more productive matters.

Case in point:

I've been working through the Depot application with Agile Web Development with Rails. This latest chapter had me working through creating a mailer and writing tests for it. Creating the mailer went well, and I can use my own gmail account to send mail through my Depot application! SUCCESS!

However, I got hung up on the functional tests. And I know what the problem is, but not how to fix it.

Here are the errors I get:

1) Error:
test_should_destroy_line_item(LineItemsControllerTest):
ActiveRecord::RecordNotFound: Couldn't find LineItem with ID=980190962 [WHERE ("line_items".cart_id = 980190963)]
app/controllers/line_items_controller.rb:79:in `destroy'
  test/functional/line_items_controller_test.rb:45:in `test_should_destroy_line_item'
  test/functional/line_items_controller_test.rb:44:in `test_should_destroy_line_item'

2) Error:
test_order_shipped(NotifierTest):
ActionView::Template::Error: undefined method `protect_against_forgery?' for #<#:0x1030519d8>
    app/views/line_items/_line_item.html.erb:9:in `_app_views_line_items__line_item_html_erb___1505233713_2172764620_6072364'
    app/views/notifier/order_shipped.html.erb:8:in `_app_views_notifier_order_shipped_html_erb___2127495373_2172826120_0'
    app/mailers/notifier.rb:13:in `order_shipped'
    test/functional/notifier_test.rb:13:in `test_order_shipped'


I have an idea why I'm getting the first error. I've been following the optional exercises at the end of each chapter and I changed the :destroy method in line_items_controller without updating the functional test. Here's the OFFENSIVE code:

def destroy
  @cart = current_cart
  @line_item = @cart.remove_product(@cart.line_items.find(params[:id]))

    respond_to do |format|
      format.html { redirect_to store_url }
      format.js
      format.xml { head :ok }
    end
  end
end


Any idea how to fix this? I'm not super concerned about it, but I'd like to know if I'm on the right track and I think this would be an easy fix. The second error I'm less sure about. I don't get why I'm getting an error for protect_against_forgery? since I'm not calling that method anywhere. Is it a default Rails thing that I'm overlooking?

Anyway, my current strategy has been simply to comment out the tests until later. Which may never come, depending on a variety of factors! I have a sneaking suspicion writing tests will never be one of my strong suits.

Getting back to my original point, it made more sense simply to comment these tests out rather than spend a bunch of time fixing them, especially since my application DOES work. Still, it's a good idea to get a handle on writing tests. I'm doing this for Future Tyler.

8 comments:

  1. It can't find @cart.line_items.find(params[:id]), has it already been destroyed earlier in the test? Is it using fixtures?

    The second one is that Rails apps protect against CSRF/XSS attacks by putting a token in your forms when using form_for and form_tag. Not sure what is causing it in your case, though.

    What do the tests look like?

    Maybe a push to Github is in order (make sure you don't commit your Gmail credentials).

    ReplyDelete
  2. Whoops, good call on not submitting my email information. I would have totally forgotten that without your reminder. Maybe should've stuck with the default? Or created a dummy account just for this kind of situation?

    I've pushed my most recent changes.

    https://github.com/illbzo1/Depot

    ReplyDelete
  3. So in your line_items_controller test, you are setting up a line item like this:

    @line_item = line_items(:one)

    Then you are testing whether posting a delete request to that line item actually removes it here:

    assert_difference('LineItem.count', -1) do
    delete :destroy, :id => @line_item.to_param
    end

    But your delete method no longer blindly takes a line_item id and deletes it; now it searches through the line items in the cart and only removes the one with that ID.

    Your test does not add the line_item(:one) to the cart, the cart is simply created as empty. So the test is searching the empty cart for the line_item in your fixture, and not finding it.

    So it raises an exception doing @cart.line_items.find( the id of the fixture line item )

    The test is replicating someone trying to remove an item from their cart that isn't even in there.

    To properly test the happy scenario, lets add the item to the cart and put that cart in our session as a precondition:

    test "should destroy line_item" do
    cart = Cart.create
    cart.line_items << @line_item
    session[:cart_id] = cart.id

    assert_difference('LineItem.count', -1) do
    delete :destroy, :id => @line_item.to_param
    end

    assert_redirected_to store_url
    end

    A more advanced scenario might say your user has the cart open in two windows, and removed the line item in one, but clicks remove again in the other, that would also cause an exception, so you could check for that in Cart.remove_product, but I wouldn't go too far from what the book covers until you are finished.

    The second issue is that in your html template for order_shipped, you use button_to, which creates a form element. This doesn't work in email, and the mailer class can't handle it anyway; so remove the button_to from your _line_item.html.erb or just loop over the line items and create table rows directly in your mailer template.

    ReplyDelete
  4. So in your line_items_controller test, you are setting up a line item like this:

    @line_item = line_items(:one)

    Then you are testing whether posting a delete request to that line item actually removes it here:

    assert_difference('LineItem.count', -1) do
    delete :destroy, :id => @line_item.to_param
    end

    But your delete method no longer blindly takes a line_item id and deletes it; now it searches through the line items in the cart and only removes the one with that ID.

    Your test does not add the line_item(:one) to the cart, the cart is simply created as empty. So the test is searching the empty cart for the line_item in your fixture, and not finding it.

    So it raises an exception doing @cart.line_items.find( the id of the fixture line item )

    The test is replicating someone trying to remove an item from their cart that isn't even in there.

    To properly test the happy scenario, lets add the item to the cart and put that cart in our session as a precondition:

    test "should destroy line_item" do
    cart = Cart.create
    cart.line_items << @line_item
    session[:cart_id] = cart.id

    assert_difference('LineItem.count', -1) do
    delete :destroy, :id => @line_item.to_param
    end

    assert_redirected_to store_url
    end

    A more advanced scenario might say your user has the cart open in two windows, and removed the line item in one, but clicks remove again in the other, that would also cause an exception, so you could check for that in Cart.remove_product, but I wouldn't go too far from what the book covers until you are finished.

    The second issue is that in your html template for order_shipped, you use button_to, which creates a form element. This doesn't work in email, and the mailer class can't handle it anyway; so remove the button_to from your _line_item.html.erb or just loop over the line items and create table rows directly in your mailer template.

    ReplyDelete
  5. Awesome, both of these suggestions worked like a charm! I swapped the button_to for a link_to, so a user can still remove line items from his cart. There isn't any issue with this method, is there? I still get the same functionality as button_to, right?

    ReplyDelete
  6. That should work just fine, but do you really want your purchasers to be able to remove items from their cart (from an email) when the order has already shipped?

    ReplyDelete
  7. Aha, good point! Fairly sure the book will cover shipping products later (I hope so, since it gets into deployment) so I don't want to get ahead of myself and code for that particular issue just yet.

    But how would you suggest fixing this problem? Right now I'm thinking disabling editing a cart if it's been marked as shipped. Should be easy enough to do it this way in the carts controller.

    Also, could you give me more info on moving email certifications into a yml then loading with an initializer? Not having much luck finding info about how to do that.

    ReplyDelete
  8. Definitely disable adding to & removing from the cart once it has shipped (and maybe even after the order is sent).

    If you followed this guide to setting up emailing from gmail:
    http://edgeguides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration-for-gmail

    Then it should shake out pretty much like this:

    https://gist.github.com/1079519

    You can omit a lot of the options you haven't seen before, you really just need delivery_method and smtp_settings, but this gives you a lot of flexibility over setting different settings for development and production. The default_url_options is that base url when using link_to in mailers, and raise_delivery_errors is usually set in config/environments/*.rb, so have them in one place and remove from the other.

    ReplyDelete