[rspec-devel] If you set an expectation on something that's already stubbed, should it return the stubbed value?

Pat Maddox pergesu at gmail.com
Sat Sep 13 13:44:08 EDT 2008

"Zach Dennis" <zach.dennis at gmail.com> writes:

> On Fri, Sep 12, 2008 at 10:44 AM, Pat Maddox <pergesu at gmail.com> wrote:
>> Hey guys,
>> Here's a pretty simple spec
>> describe AccountService do
>>  before(:each) do
>>    @account = stub("account", :balance => 12345)
>>    @service = AccoutnService.new
>>  end
>>  it "should check the balance" do
>>    @account.should_receive(:balance)
>>    @service.get_balance(@account)
>>  end
>> end
>> Ignore the fact that it's totally lame :)
>> Right now, in the example, @account.balance will return nil.  Any
>> other specs that call it will get back 12345.  What do you guys think
>> about making it return the same value by default, instead of nil?  So
>> basically, instead of totally shadowing that method, we simply tighten
>> up the constraints on the mock object by adding an expectation.
>> Pros:
>> - Less verbose - it expresses intent better, I think.  You don't
>> really care what it returns in that case.  Just that it gets that
>> method called, and everything works
>> - No duplication.  I can't think of a single instance where I stubbed
>> a call, and then in my expectation I wanted it to return nil.  I
>> *always* duplicate it
>> Cons:
>> - Less verbose :)  maybe some people would think it's not explicit enough?
>> Personally, I'm all for it.  What do you guys think?
> -1. This works against clearly expressing the intent in each example.
> I vote for keeping the explicit values used by an example (if that is
> what is being tested) inside each example. Clarity over saving a few
> keystrokes on this one,

I actually think it expresses the intent *more* clearly.  Consider this
example from a controller spec:

it "should find the book" do
  get :show, :id => "1"

"and_return" simply adds noise to the example.  The giveaway is that the
description doesn't say "should find and return the book."  We could
change that obviously...but *why* do we want to specify what book gets
returned?  So that we can display it on the page, of course!  So let's
write an example for that:

it "should assign the book to the view" do
  get :show, :id => "1"
  assigns[:book].should == @mock_book

Now we've got one example that says we should find the proper book, and
another one that says we should assign it to the view.  And with these
two, we can be confident everything is set up properly because there'd
be no way for @mock_book to make it to the view were it not the same
object returned by Book.find.

This is less an issue of duplication than one of Least Surprise, in my
opinion.  If you look at .should_receive...and_return in the example I
gave, you might naturally think "well of COURSE it returns the mock book
- I already stubbed it!"  Book is set up to do the right thing, we're
just tightening the constraints on it for a single example in order to
verify the desired behavior.  Also related to Least Surprise is what
happens right now if you forget to specify a return value: you get back
nil and tests fail.  When you've run into this problem several times
before it's no big deal...but for someone new to the framework it's a
head-scratcher.  And good luck if you accidentally return the wrong
object (think @mock_book2 vs @mock_book1) - you won't get somewhat
obvious errors about nils, instead you'll just git different behavior

Finally, consider how much *better* your intent is expressed when you DO
add and_return to return a different value - you know in that particular
example that you're expecting the behavior to be different as a result
of the indirect input.

Here's my modified pros and cons list:

* Less noise in should_receive examples
* Example is less coupled to setup - changing the value returned by the
  stub doesn't make you change the value for the mock
* Principle of least surprise - no weird errors from forgetting to
  return the correct value from the mock
* Setting and_return clearly points to when you're overriding the
  default behavior

* Might break some existing examples

Honestly I think that the con is not likely to be much of a problem.  I
bet 99% of examples that use the stub-in-setup/expect-in-example pattern
use the same value both times.  And even if it does break your test, I'd
say you were asking for it anyway:

before(:each) do
  @mock_book = mock_model("book")
  Book.stub!(:find).and_return @mock_book

it "should render a 404 when the book is not found" do
  get :show, :id => "1"
  response.code.should == "404"

Why on earth were you relying on the reader's knowledge of subtleties of
the mocking framework?  Express your intent!

Book.should_receive(:find).with("1").and_return nil

Ahhhhh... that's better.


More information about the rspec-devel mailing list