On Fri, Sep 25, 2009 at 3:06 AM, Andrea Jahn <[email protected]> wrote:
> Hi,
>
> in my Controller I have some tansactions. I'm trying to write specs, which
> test the two program pathes (an exception is thrown or no exception is
> thrown). But the test seems always to go into the path, where the exception
> is catched.
>
> Here's an example for the delete action
> (destroy method is overwritten, so :dependent => :destroy is not possible
> here)
Off topic, but ActiveRecord offers a number of ways to interact with
model and transaction life cycles. The fact that you are overriding
destroy rather than hooking into the destroy cycle means you're
bypassing a lot of functionality that you now have to implement
yourself (hence the problem you are describing in this post). If there
is any way for you to use those hooks instead, I'd strongly recommend
it.
>
> The test case "should flash the notice" fails, because not the program path
> of success is executed, but the rescue path is executed.
>
> Perhaps my exception handling is generally wrong ?
>
> Thanks
> Andrea
>
> plannings_controller.rb
> -----------------------
>
> def destroy
> @planning = Physical::Planning::Planning.find(params[:id])
>
> respond_to do |format|
> begin
> ActiveRecord::Base.transaction do
> @planning.destroy
> @planning.planned_amounts.each { |planned_amount|
> planned_amount.destroy }
> end
Even if you've overridden destroy, why not just include this code in
the custom destroy method? Then the controller could just call
@planning.destroy and that method can worry about transactions.
> # success
> flash[:notice] = "The
> #{Physical::Planning::Planning::DISPLAY_NAME.downcase} '" + @planning.title +
> "' was successfully deleted."
> format.html { redirec t_to(planning_plannings_url) }
> format.xml { head :ok }
> rescue => e
> # DB raised errors
> flash[:error] = "The
> #{Physical::Planning::Planning::DISPLAY_NAME.downcase} '" + (@planning.nil? ?
> "" : @planning.title) + "' could not be deleted. Error:" + e
> format.html { redirect_to(planning_plannings_url) }
> format.xml { render :xml => @planning.errors, :status =>
> :unprocessable_entity }
> end
> end
> end
>
>
>
> plannings_co ntroller_spec.rb
> ----------------------------
>
> describe Physical::Planning::PlanningsController, "DELETE" do
> include PlanningMacros
>
> should_require_login :put, :delete
> should_require_authorization :put, :delete
>
> context "when the user is logged in" do
> before(:each) do
> do_login
> do_authorization
>
> @planning = mock_model(Physical::Planning::Planning,
> valid_planning_attributes)
> @planned_amount = mock_model(Physical::Planning::PlannedAmount,
> valid_planned_amount_attributes)
>
> @planning.stub!(:destroy)
> @planning.stub!(:planned_amounts).and_return([...@planned_amount])
>
> Physical::Planning::Planning.stub!(:find).and_return(@planning)
>
> @plan ning.errors.stub!(:full_messages).and_return([])
> end
>
> # delete successfully
> # -------------------
>
> context "and the Planning deletes successfully" do
>
> ....
>
> it "should destroy the Physical::Planning::Planning in a transaction" do
>
> Physical::Planning::Planning.should_receive(:transaction).any_number_of_times.and_yield(
> @planning.should_receive(:destroy)
> )
This ^^ isn't doing anything. any_number_of_times includes 0, which is
the number of times Planning (the class object) receives transaction
(the controller calls ActiveRecord::Base.transaction). I'd recommend
leaving out any_number_of_times. By default, should_receive expects
exactly one call, which is the right number in this case. And it
should be on ActiveRecord::Base, given the current state of the
implementation.
This, however, is a great example of how writing examples first can
have a positive impact on design. If I were writing this, I never
would have specified transactions in a controller action. This example
would simply be:
it "should destroy the Physical::Planning::Planning in a transaction" do
@planning.should_receive(:destroy)
do_put_delete
end
Then the implementation would only need call @planning.destroy to pass
the example. The transactional nature of the destroy method would be
specified in the model spec, and now any other code that gets written
later that needs to destroy a Planning instance can just send it
destroy() and you won't have to duplicate this transaction code in
each of those cases.
HTH,
David
> do_put_delete
> end
>
> it "should destroy the PlannedAmounts of the
> Physical::Planning::Planning in a transaction" do
> Physical::Planning::Planning.should_receive
> (:transaction).any_number_of_times.and_yield(
> @planned_amount.should_receive(:destroy)
> )
> do_put_delete
> end
>
> it "should flash the notice" do
> do_put_delete
> flash[:notice].should match(/was successfully deleted/)
> end
>
> ...
>
> end
>
> # delete fails
> # ------------
> context "and the Planning fails to delete" do
>
> it "should flash error message because of database exception" do
> Physical::Planning::Planning.should_receive(:transaction).any_
> number_of_times.and_yield(
>
> @planning.should_receive(:destroy).and_raise(ActiveRecord::RecordInvalid.new(@planning))
> )
> do_put_delete
> flash[:error].should match(/could not be deleted/)
> end
> end
> end
> end
_______________________________________________
rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users
