everydayrails / rails-4-1-rspec-3-0

Code samples for Everyday Rails Testing with RSpec, Rails 4.1/RSpec 3.0 edition
272 stars 229 forks source link

Ch.9 Avoid useless stubbing #57

Open JunichiIto opened 9 years ago

JunichiIto commented 9 years ago

contacts_controller_spec stubs like this:

        allow(Contact).to receive(:persisted?).and_return(true)
        allow(Contact).to receive(:order).with('lastname, firstname').and_return([contact])
        allow(Contact).to receive(:find).with(contact.id.to_s).and_return(contact)
        allow(Contact).to receive(:save).and_return(true)

However persisted? and save are useless because they are not class method. The spec would pass even if they return false.

        allow(Contact).to receive(:persisted?).and_return(false) # Not called
        allow(Contact).to receive(:order).with('lastname, firstname').and_return([contact])
        allow(Contact).to receive(:find).with(contact.id.to_s).and_return(contact)
        allow(Contact).to receive(:save).and_return(false) # Not called

Moreover, this example tests GET #show, so order method is not used either. After all, required stub is only this one:

        allow(Contact).to receive(:find).with(contact.id.to_s).and_return(contact)

I think beginners cannot distinguish these rules. They would believe all of them are required. So the code should be fixed to teach them valid stubbing.

P.S.

allow_any_instance_of(Contact).to receive(:save).and_return(true) is okay for Contact instance but RSpec does not recommend using allow_any_instance_of method:

https://www.relishapp.com/rspec/rspec-mocks/v/3-2/docs/working-with-legacy-code/any-instance