Ch.5, p.55 - Suble bug in spec

[olerass]

The _redirects to contacts#show" test on page 55 has a subtle bug:

it "redirects to contacts#show" do
 post :create, contact: attributes_for(:contact, phones_attributes: @phones)
 expect(response).to redirect_to contact_path(assigns[:contact])
end

Look at what RSpec will output if ContactsController#create doesn't assign @contact:

2) ContactsController POST #create with valid attributes redirects to contacts#show
     Failure/Error: expect(response).to redirect_to contact_path(nil)
       Expected response to be a redirect to <http://test.host/contacts/> but was a redirect to <http://test.host/contacts/1>.
       Expected "http://test.host/contacts/" to be === "http://test.host/contacts/1".
     # ./spec/controllers/contacts_controller_spec.rb:101:in `block (4 levels) in <top (required)>'

This tells us nothing about what the failure is (well, it does as it says contact_path(nil) which hints to the problem, but the message is still confusing). It is misinforming because it says we expect to be redirected to "http://test.host/contacts/" but we were redirected to "http://test.host/contacts/1" which is the correct url.

The problem is subtle. By using contact_path(assigns[:contact]) the test implicitly expects assigns[:contact] to be not nil. The problem is the implicitness; if assigns[:contact] returns nil contact_path(nil) returns /contacts/ which is equivalent to contacts_path. I don't know if this is a bug or a feature in Rails - I don't see why contact_path(nil) should ever be a valid path since you can always use contacts_path instead? Anyway, this is why the test fails with a wrong message.

To solve this one should at least make the assumption explicit by explicitly expecting assigns[:contact] to be not nil:

it "redirects to contacts#show" do
  post :create, contact: attributes_for(:contact, phones_attributes: @phones)

  assigned_contact = assigns(:contact)
  >> expect(assigned_contact).to_not be_nil <<
  expect(response).to redirect_to contact_path(assigned_contact)
end

This test will fail in the correct way when @contact isn't assigned and also expresses the intent better.

A related question is whether expecting @contact to be not nil is good enough. Do we not want to test that it's assigned the correct contact and not just any contact? For example with a test like this:

it "assigns the new contact to @contact" do
  contact_attrs = attributes_for(:contact)
  post :create, contact: contact_attrs

  assigned_contact = assigns(:contact)
  expect(assigned_contact).to_not be_nil 
  expect(assigned_contact.attributes).to include(contact_attrs.stringify_keys)
end

Note that this doesn't test for correctness of phones - I couldn't figure out how to do that.

Book version: 2013-10-29 (PDF)