rubymonsters / text-to-squares

Railsgirls Berlin Learning Group Project
13 stars 14 forks source link

the test is still not working :( #155

Closed tyranja closed 11 years ago

tyranja commented 11 years ago

Fehlermeldung: AbstractController::DoubleRenderError: Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like "redirect_to(...) and return".

  test "text can be destroyed by its owner" do
    user = User.create!(:name => "Testimus Maximus")
    @text.user = user
    @text.save!

    session[:user_id] = user.id
    delete :destroy, :id => @text.id

    assert_response :success
  end
fidothe commented 11 years ago

a DoubleRenderError means that render is being called twice in your controller method. Assuming that this is the current version of your test:

https://github.com/tyranja/text-to-squares/blob/ty_destroy_action/app/controllers/texts_controller.rb#L93

then I can see the problem. You've got a render call in an if-block, and a respond_to block. Calling render doesn't work like return - where execution of the current method stops immediately and the value of return would be returned by the method, even though it looks and feels like it should. (I remember this being really confusing when I was starting out with Rails.) If someone hits the unauthorised branch of the if statement, render will be called and then execution will continue through the respond_to block, where either redirect_to or head will be called, and will in turn call render behind the scenes.

On 6 Dec 2012, at 21:10, tyranja wrote:

Fehlermeldung: AbstractController::DoubleRenderError: Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like "redirect_to(...) and return".

test "text can be destroyed by its owner" do user = User.create!(:name => "Testimus Maximus") @text.user = user @text.save!

session[:user_id] = user.id
delete :destroy, :id => @text.id

assert_response :success

end — Reply to this email directly or view it on GitHub.

Matt Patterson matt@constituentparts.com | +44 20 7193 4195 | +49 30 3471 2856

http://constituentparts.com/

Constituent Parts Limited | Company number 6366606, registered in England & Wales

tyranja commented 11 years ago

Wrote it like that:

  def destroy
    @text = Text.find(params[:id])

    if user_can_edit?(current_user, @text)
      @text.destroy
        respond_to do |format|
          format.html { redirect_to texts_url }
          format.json { head :no_content }
        end
    else
      render :text => "No waffles for you", :status => 403
    end
  end

but the destroy text test doesnt work.

fidothe commented 11 years ago

Aha! I just checked out your branch and ran the tests. The failure on tests/functional/texts_controller_test.rb at line 85 is one of the generated-by-scaffold tests, and it's wrong now (it assumes anyone can delete) so that test can be deleted :-)

The other failure is your fault ;-)

You're calling delete :destroy, which (by default) will mean that the format.html block will fire in your respond_to.

So, you're asserting that the response should be 200 OK with assert_response :success but the format.html block is calling redirect_to.

If you were calling delete :destroy, :id => @text.id, :format => :json in that test it would be passing, because the format.json block is calling head :no_content which will return an empty response, but it'll have the status code 200.

You can switch to using assert_redirected_to in your test and all will be well :-)

tyranja commented 11 years ago

Wow, I dont really get that. I will have to ask in the course for translation ;)

Is the test wrong or the code? And I tried curl and there it seems to work.

fidothe commented 11 years ago

https://github.com/tyranja/text-to-squares/blob/ty_destroy_action/test/functional/texts_controller_test.rb#L84-L90

The test above was created by rails generate scaffold and is junk and can be deleted - it's failing because it's junk.

The second failing test is one you wrote: https://github.com/tyranja/text-to-squares/blob/ty_destroy_action/test/functional/texts_controller_test.rb#L93-L102

This one is failing because you're checking that it returns status 200 in line 101:

https://github.com/tyranja/text-to-squares/blob/ty_destroy_action/test/functional/texts_controller_test.rb#L101

But in the controller you're redirecting once you've successfully deleted a text, so returning status code 302 and not 200.

https://github.com/tyranja/text-to-squares/blob/ty_destroy_action/app/controllers/texts_controller.rb#L92

That's why that test is failing - you need to change it to use assert_redirected_to instead of assert_response :success

To complicate things a bit, the destroy action is making use of respond_to, which i'm not sure we've covered much (if at all). The normal (render HTML) happy path through that code will end in a redirect, but the .json happy path ends in returning status 200 and an empty response body.