There’s been a bunch of hullabaloo about our recent deprecation of the Shoulda
macros should_change and should_not_change. A lot of people have asked,
“why remove it?” Short answer: we don’t use it anymore. Long answer: it’s
confusing and may encourage bad programming practices.
Consider this example of should_change:
context "updating a post" do
setup do
@post = Post.create(:title => "old")
put :update, :post => {:title => "new"}, :id => @post.to_param
end
should_change("the post title", :from => "old", :to => "new") { @post.title }
end
This reads well and seems to be fairly straightforward. Sadly, this doesn’t
work because of how should_change works internally (hence the confusing
part). It would actually need to be written like this:
context "given a post" do
setup do
@post = Post.create(:title => "old")
end
context "updating" do
setup do
put :update, :post => {:title => "new"}, :id => @post.to_param
end
should_change("the post title", :from => "old", :to => "new") { @post.title }
end
end
The @post instance variable needs to be assigned a context above the context
containing should_change. I told you it was confusing.
Now, onto the bad practice aspect. This tests that the post was saved! Come
on, we don’t want to do that here. That’s why we write Cucumber scenarios!
We’re testing the controller, right? If that’s the case, then we shouldn’t
care whatsoever if update_attributes worked with specific data passed to it,
just that it was called and that we handle the result accordingly.
context "updating a post" do
setup do
@post = Factory :post
Post.stubs(:find => @post)
end
context "when the update is successful" do
setup do
@post.stubs(:update_attributes => true)
put :update, :post => {:title => "new"}, :id => @post.to_param
end
should set_the_flash.to(/updated/)
should redirect_to(posts_path)
end
context "when the update is unsuccessful" do
setup do
@post.stubs(:update_attributes => false)
put :update, :post => {:title => "new"}, :id => @post.to_param
end
should render_template(:edit)
should assign_to(:post).with(@post)
end
end
This is more inline with what we really want to test; that the controller
handled update_attributes correctly when the update worked and when it didn’t.
Leave the model logic and whether it should save or not in the model (and its
tests), and test the controller to ensure it handles whether the model was
updated.
I mentioned before that Cucumber scenarios should cover overall integration. Something like these would work:
Scenario: Update a post successfully
Given I am signed in
And I authored a post titled "Great Scott"
When I go to edit the "Great Scott" post
And I fill in "Title" with "Willard Scott"
And I press "Update"
Then I should see the flash message "Your post was updated successfully"
And I should have authored "Willard Scott"
Scenario Example: Update a post unsuccessfully
Given I am signed in
And I authored a post titled "Great Scott"
When I go to edit the "Great Scott" post
And I fill in "<required field>" with ""
And I press "Update"
Then I should see error messages
When I fill in "<required field>" with "<valid value>"
And I press "Update"
Then I should see "Your post was updated successfully"
Examples:
| required field | valid value |
| Title | Awesome Post Name |
| Body | Awesome story |
I’m sure this isn’t the only way people were using should_change, but
hopefully this clarifies how deprecating the methods will encourage better
tests.
Do you use should_change and think it’s by far the best way to test certain
pieces of code? I’d like to see some people who’ll sorely miss it post some
real code (Pastebin or Gist).
