[rspec-users] stub_chain together with should_receive

2,115 views
Skip to first unread message

medihack

unread,
Nov 23, 2010, 8:12:46 PM11/23/10
to rspec...@rubyforge.org
Hello.

I am trying to test if in a method calling chain one of the methods
get a specific parameter. In the below code for example MyModel must
receive the parameter 0 for the method "offset". Unfortunately the
code below does not work. It seems it is not possible to mix
should_receive and stub_chain. How could I solve this?

MyModel.should_receive(:offset).with(0).stub_chain(:tag_counts, :offset, :limit, :order).and_return([])
# does not work!

Regards,
Kai
_______________________________________________
rspec-users mailing list
rspec...@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

David Chelimsky

unread,
Nov 24, 2010, 8:38:46 AM11/24/10
to rspec-users
On Nov 23, 2010, at 7:12 PM, medihack wrote:

> Hello.
>
> I am trying to test if in a method calling chain one of the methods
> get a specific parameter. In the below code for example MyModel must
> receive the parameter 0 for the method "offset". Unfortunately the
> code below does not work. It seems it is not possible to mix
> should_receive and stub_chain. How could I solve this?
>
> MyModel.should_receive(:offset).with(0).stub_chain(:tag_counts, :offset, :limit, :order).and_return([])
> # does not work!

As you're finding out, chains like this are hard to test. That, and the fact that writing code before tests significantly increases the likelihood that the code will be hard to test.

There is no easy way to do what you're trying to do with this code. You'd have to create a series of test doubles, like this:

offset_return_value = double
offset_return_value.stub_chain("limit.order") { [] }
tag_counts_return_value = double
tag_counts_return_value.should_receive(:offset).with(0).and_return(offset_return_value)
MyModel.stub(:tag_counts) { tag_counts_return_value }

The problem with this sort of structure, besides it being hard to read, is that you can't change anything about the implementation of the method that is invoking this chain without changing this example and likely many others.

I'd recommend wrapping that chain in a method and specifying how that method behaves when a 0 makes its way to the offset method vs how it behaves otherwise.

HTH,
David

>
> Regards,
> Kai
> _______________________________________________
> rspec-users mailing list
> rspec...@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

Cheers,
David

medihack

unread,
Nov 25, 2010, 10:44:58 AM11/25/10
to rspec...@rubyforge.org
Hello.

I am trying to test if in a method calling chain one of the methods
get a specific parameter. In the below code for example MyModel must
receive the parameter 0 for the method `offset`. Unfortunately the
code below does not work. It seems it is not possible to mix

should_receive and stub_chain. How could I solve this? I am using
RSpec 2.

does not work:


MyModel.should_receive(:offset).with(0).stub_chain(:tag_counts, :offset, :limit, :order).and_return([])

also does succeed (expectation is never called):
MyModel.stub_chain(:tag_counts, :offset, :limit, :where, :order).and_return([])
MyModel.should_receive(:offset).with(0) { MyModel }

does succeed, but is IMHO quite unhandy (you can't stub_chain in a
before filter):
MyModel.stub(:tag_counts) { MyModel }
MyModel.should_receive(:offset).with(0) { MyModel }
MyModel.stub_chain(:limit, :where, :order).and_return([])

medihack

unread,
Nov 26, 2010, 3:24:11 AM11/26/10
to rspec...@rubyforge.org
David, sorry for double posting (it seems I am working too much and
forgetting about what I already asked) ... and thanks for your answer.
How about a bit more convenient way for future releases. Something
like:
MyModel.stub_chain(:tag_counts, { :offset =>
0 }, :limit, :order).and_return([])
it could also work for multi parameters:
MyModel.stub_chain(:tag_counts, { :my_method => [:param1, :param2,
param3] }, :limit, :order).and_return([])
What do you think?

David Chelimsky

unread,
Nov 26, 2010, 6:26:03 AM11/26/10
to rspec-users
On Nov 26, 2010, at 2:24 AM, medihack wrote:

> David, sorry for double posting (it seems I am working too much and
> forgetting about what I already asked) ... and thanks for your answer.
> How about a bit more convenient way for future releases. Something
> like:
> MyModel.stub_chain(:tag_counts, { :offset =>
> 0 }, :limit, :order).and_return([])
> it could also work for multi parameters:
> MyModel.stub_chain(:tag_counts, { :my_method => [:param1, :param2,
> param3] }, :limit, :order).and_return([])
> What do you think?

I would be opposed to this feature.

There is an old guideline in TDD that suggests that you should listen to your tests because when they hurt there is usually a design problem. Tests are clients of the code under test, and if the test hurts, then so do all of the other clients in the codebase. Shortcuts like this quickly become an excuse for poor designs. I want it to stay painful because it _should hurt_ to do this.

I understand that chains like this are common in Rails apps thanks to good ideas like composable finders (which generally do not violate Demeter), but I don't think the parallel chains should appear in client code or in specs. e.g. if this is a controller spec, the model should expose a single method that wraps this, and if it's the model spec, the spec should just call the method that wraps the chain with different inputs and and specify the expected outcomes.

Even if I were in favor of the concept, the example above is confusing because it is a stub that becomes a message expectation. Constrained stubs like this (e.g. double.stub(:method).with(:arg).and_return(value)) are confusing enough as it is because they don't look like message expectations, but they act sort of like them. I've considered deprecating "with" on stubs and pushing toward an inline fake implementation instead:

account.stub(:withdraw) do |*args|
if args.first == Money.new(200, :USD)
# do something
else
# do something else
end
end

RSpec already supports this, and it makes the intent of the spec much more clear and localized than:

account.stub(:withdraw).with(Money.new(200, :USD)).and_return(TransactionResult.new)

In this case, if account receives withdraw() with anything other than Money.new(200, :USD), the outcome needs to be defined elsewhere in the spec, and these things quickly become spread out and difficult to understand.

That's my 2¢, but feel free to try to convince me otherwise :)

Cheers,
David

medihack

unread,
Nov 28, 2010, 3:06:20 AM11/28/10
to rspec...@rubyforge.org
> That's my 2¢, but feel free to try to convince me otherwise :)

Ok, I'll give my best ... how about a dollar? ;-)

> I understand that chains like this are common in Rails apps thanks to good ideas like composable finders (which generally do not violate Demeter), but I don't think the parallel chains should appear in client code or in specs. e.g. if this is a controller spec, the model should expose a single method that wraps this, and if it's the model spec, the spec should just call the method that wraps the chain with different inputs and and specify the expected outcomes.

I dislike the idea to extract a single line of chained method calls
into its own method, just because it is easier to test then. It is not
used anywhere else and the method it is in is not very large either
(those would be reasons to extract that code in its own method). By
the way, it is part of the model. I would agree if it were part of the
controller.

> Even if I were in favor of the concept, the example above is confusing because it is a stub that becomes a message expectation.

I absolutely agree. I also thought about that when I read my post for
a second time. How about: MyModel.should_receive_chain(...)

David Chelimsky

unread,
Nov 28, 2010, 3:01:08 PM11/28/10
to rspec-users

On Nov 28, 2010, at 2:06 AM, medihack wrote:

>> That's my 2¢, but feel free to try to convince me otherwise :)
>
> Ok, I'll give my best ... how about a dollar? ;-)
>
>> I understand that chains like this are common in Rails apps thanks to good ideas like composable finders (which generally do not violate Demeter), but I don't think the parallel chains should appear in client code or in specs. e.g. if this is a controller spec, the model should expose a single method that wraps this, and if it's the model spec, the spec should just call the method that wraps the chain with different inputs and and specify the expected outcomes.
>
> I dislike the idea to extract a single line of chained method calls
> into its own method, just because it is easier to test then.

We have a fundamental philosophical disagreement here. I see testability as having inherent value, "just because it is easier to test" is a good enough reason to consider a change.

> It is not
> used anywhere else and the method it is in is not very large either
> (those would be reasons to extract that code in its own method).

I'm confused about where it _is_ being used? What code actually calls this chain?

> By
> the way, it is part of the model. I would agree if it were part of the
> controller.
>
>> Even if I were in favor of the concept, the example above is confusing because it is a stub that becomes a message expectation.
>
> I absolutely agree. I also thought about that when I read my post for
> a second time. How about: MyModel.should_receive_chain(...)

Need to ponder that. Will follow up with some thoughts.

Reply all
Reply to author
Forward
0 new messages