before_action/after_action
November 6th, 2007
A while back there was either a feature request in the rspec tracker, or a suggestion on one of the rspec mailing lists, for a feature to DRY up controller specs.
The idea was that this pattern feels a bit clunky:
describe PersonController, "handling failed POST to create" do
def do_post
post :create, invalid_arguments
end
it "should redisplay the create form" do
do_post
response.should render_template("people/new")
end
it "should try to create a Person" do
Person.should_receive(:create).with(invalid_arguments).and_return(false)
do_post
end
end
And it would be nice to have something that was more expressive using tags like this:
describe PersonController, "handling failed POST to create" do
def do_post
post :create, invalid_arguments
end
it "should redisplay the create form", :after => :do_post do
response.should render_template("people/new")
end
it "should try to create a Person", :before => :do_post do
Person.should_receive(:create).with(invalid_arguments).and_return(false)
end
end
I didn’t add this to rspec_on_rails because I personally find it harder to read. It also doesn’t support situations where you want to stub something before the action and then set a state-based expecation after the action.
But the problem is still present, and it would be nice to have something a bit less clunky.
Well – here’s what I’ve been experimenting with. This is NOT part of RSpec, and I may not want to include it in RSpec because I think it’s somewhat particular to my personal style (as opposed to a style that I think is “right” for BDD), but it’s easy enough to add to your own projects.
Here’s what the specs look like:
describe PersonController, "handling failed POST to create" do
def do_post
post :create, invalid_arguments
end
it "should redisplay the create form" do
after_post do
response.should render_template("people/new")
end
end
it "should try to create a Person" do
during_post do
Person.should_receive(:create).with(invalid_arguments).and_return(false)
end
end
end
I really like this even though it actually turns out to be a bit more verbose. I think it speaks very clearly about what is going on – especially “during_post”, which describes very well when the Person.should_receive the :create message.
Here’s the code in spec_helper.rb that supports this pattern:
[:get, :post, :put, :delete, :render].each do |action|
eval %Q{
def before_#{action}
yield
do_#{action}
end
alias during_#{action} before_#{action}
def after_#{action}
do_#{action}
yield
end
}
end
This supports controller and view specs (hence including :render).
Please try it out and let me know what you think.
November 6th, 2007 at 11:55 am
I agree that the original pattern feels clunky, and that we should do something to fix it. And the :before => do_post syntax isn’t clear whether the do_post runs before the spec, or if the spec should run before the do_post.
it "should redisplay the create form" do response.should render_template("people/new") end
it "should try to create a Person" do before_the_before_clause do Person.should_receive(:create).with(invalid_arguments).and_return(false) end end end
I’m not sure how hard it would be to implement that though. I would guess that it would require a serious architectural change to RSpec.
it "should redisplay the create form" do response.should render_template("people/new") end
it "should try to create a Person" do Person.should_have_received(:create).with(invalid_arguments) end end
I understand that we’d probably have to re-write a lot of the mocking and stubbing code, but I think that this expresses intent a lot better, and probably solves the root of the problem you’re trying to solve. If something like this exists, I’d be interested in learning about it. If not, I’d be willing to help make it come about.
November 6th, 2007 at 11:55 am
I actually like the verbosity of this style; this is definitely something I would adopt. I’d actually like to see this implemented into RSpec. I’ll ask around and see what others have to say. So far, I like it!
November 6th, 2007 at 11:55 am
I think I might have been the one to originally suggest the tags version. I like your version better.
I’m going to start trying it out.
November 6th, 2007 at 11:55 am
@craig & @nicholas:
November 6th, 2007 at 11:55 am
@david:
November 6th, 2007 at 11:55 am
@David:
November 6th, 2007 at 11:55 am
@nicholas:
November 6th, 2007 at 11:55 am
craig: @should_receive(:foo).any_number_of_timesdoesn’t actually make any assertions.anymeans zero or more. Obviously, it reads a bit oddly, because it looks like it is making an assertion, but for all practical purposes it isn’t. My only use case forshould_receive(:foo).any_number_of_timesis whenstub!(:foo)isn’t powerful enough to get the job done.than
should_receive(:foo).any_number_of_timescurrently does? If not, thenupon_receiving(:foo)should be a drop-in replacement, and the clunkierany_number_of_timesshould be deprecated (IMHO).any strong suggestions… but I was thinking about something like the following:
it "should redisplay the create form" do response.should render_template("people/new") end
it "should try to create a Person" do @expectation.should_have_received(:create) end end
But does that read well?
November 6th, 2007 at 11:55 am
@nicholas:
November 6th, 2007 at 11:55 am
@craig:
method call than
should_have_received. It doesn’t need the method name to be reiterated either. But I can’t come up with anything better at the moment… Oh well. Also, you are right thatshould_returndoesn’t match up well withupon_receiving. I likewill_returnthe best, there.an assertion in the spec is more likely. I do it both ways, depending on the situation. So I’d like both to be supported.
November 6th, 2007 at 11:55 am
David,