Skip to content
This repository has been archived by the owner on Aug 3, 2021. It is now read-only.

Ch.9 Avoid useless stubbing #57

Open
JunichiIto opened this issue May 3, 2015 · 0 comments
Open

Ch.9 Avoid useless stubbing #57

JunichiIto opened this issue May 3, 2015 · 0 comments

Comments

@JunichiIto
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant