Hey Clay,
I like you was convinced mocks should never return values - you're either
testing message passing or state. In fact I think I learned this from you
when we were working together. I also had done a lot of watching of the
Greg Moeck confreaks. Recently I was explaining to my fellow developer at
Reverb the same concept but we had a good case for where it is warranted. I
believe I was convinced the other way but I am open to other opinions on
how to do this:
We're writing a test that interacts with an external api adapter (the
adapter is ours but it hits an external api). We're doing a unit test on
the boundary to the third party api. What we want to test is that the third
party api is hit, and that it returns something (because our class behaves
differently).
Now the code below could use 'stub' instead of 'should_receive'. However my
friend made a good point that using should_receive.and_return in this case
both exposes the intent (we want the third party service to be called) as
well as creates a better whine in the test if the service isn't hit.
context " the charge succeeds" do
specify do
statement.should_receive(:close)
Reverb::Billing::BraintreeClient.should_receive(:charge).with(statement).and_return(success)
Reverb::BillingService.run
end
end
context " the charge fails" do
specify do
statement.should_not_receive(:close)
Reverb::Billing::BraintreeClient.
should_receive(:charge).with(statement).and_return(failure)
Reverb::BillingService.run
end
end
Of course you can plug in stub here but are you really showing the intent
that the external service should be hit? Additionally the should_receive is
noisier and indicates that the third party service was not called, so it
more clearly shows the error in the test vs the stub.
I am still convinced that 99% of the time this is not warranted but in this
case - ensuring we're hitting the external api, and branching on the
result, I could see its use.
Yan
On Monday, February 11, 2013 11:20:06 PM UTC-6, clay s wrote:
>
> On Monday, February 11, 2013 4:05:12 PM UTC-8, Myron Marston wrote:
>
>> I consider the use of `should_receive` with a return value to be a code
>> smell since I generally care about command/query separation.
>>
>
> Combining should_receive with a return value is not in any way related to
> command/query separation. You could replace the mock with a stub or (if the
> return value isn't being used by the code under test) remove the return
> value, and the test would be equivalent, and would still violate (or not
> violate) command/query separation just as much as before.
>
> In fact, in the most recent relevant interaction I had with a fellow
> developer about this, he had mocked SomeModule.display_name (a method with
> *no side-effects*), and provided a return value. He never tested that the
> return value was used correctly, because he felt it would be too cumbersome
> to check that a CSV file was created with the right data. Hence the mock
> was some kind of (basically meaningless) reassurance to him.
>
> > If using `should_receive(:foo).and_return(value)` bothers you that much,
> then feel free not to use it
>
> It's not about some subjective distaste for it. It's a dishonest statement
> that the method has some side-effect that must be triggered. Indeed, if my
> friend had found himself unable to return a value from a mock, it would
> have encouraged him to make a realization about the nature of his test.
>
> you may want to specify that a particular message is sent to the
>> collaborator only once, and `should_receive(:message).once` is a fine way
>> to do that.
>>
>
> This is semantically different than a should_receive; specifically, it's a
> "should *not* receive more than once". You could argue that the semantic
> clarity isn't valuable enough to warrant something like
> double.should_cache(:foo), but I'd certainly welcome it as part of a change
> to make it impossible to return values from mocks.
>
>
>> I think you're instincts that it's often not needed (and that confusing
>> stubs with mocks is bad) are correct. But there are still cases where it's
>> useful, and RSpec provides it.
>>
>
> I speculate that the *only* case in which it's useful is when a value
> should be cached, and therefore need not be called more than once. In which
> case something like the should_cache method would solve the problem.
>
> Thanks.
>
--
You received this message because you are subscribed to the Google Groups
"rspec" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msg/rspec/-/UzpiHrtwvRkJ.
For more options, visit https://groups.google.com/groups/opt_out.
