[RSpec] Testing a helper with or without model layer?

Testing RSpec
1

Hi guys,

I wanted to get a more experienced opinion about a test I had to do.

I posted the code at pastie with comments for best visual readability:

http://pastie.org/private/qvvrxubslvia2nv6l3km4q

Any feedback is appreciated

Thanks

2

On Aug 3, 2010, at 6:34 AM, Bruno C. wrote:

Hi guys,

I wanted to get a more experienced opinion about a test I had to do.

I posted the code at pastie with comments for best visual readability:

http://pastie.org/private/qvvrxubslvia2nv6l3km4q

Even though the code is in a helper, it depends heavily on the model.
This exhibits a code smell called Feature Envy, in which one object (the
helper) does some computation but another object (the CfgInterface
model) has all the data. Based on that, one might argue this method
belongs on the model anyhow, in which case the argument of stubbing out
the model layer makes little sense.

There are also several expectations that overlap. It doesn’t matter that
the class is a Hash - what matters is that it behaves like a hash, which
is demonstrated in other expectations. The fact that the keys exist is
also demonstrated in the expectations that access those keys.

Based on those comments, I’d probably do something like
gist:506366 · GitHub. Then I’d figure out how to swap in
factories for the fixtures - right now there’s no way to understand what
‘CONS_ALL_ACCOUNT’ means without looking elsewhere.

HTH,
David

3

On Aug 3, 2010, at 9:23 AM, Bruno C. wrote:

helper was more heavy on the processing and still very data dependent ?
Should I use fixtures on the helper or should I stub/mock whatever I
need to avoid the model layer within the helper ?

It really depends on how deep the helper is reaching into the model.
Ideally, when you’re stubbing a layer, you want to be able to stub one
thing. In this example, there are many things that need to be stubbed. A
common example is a display formatter for a person’s name:

it “concats the first and last name” do
person = double(‘person’, :first_name => “Joe”, :last_name => “Smith”)
helper.full_name_for(person).should == “Joe S.”
end

Here we’re just stubbing a couple of values on one object - simple. In
this case it makes sense to just stub the model.

4

David C. wrote:

It really depends on how deep the helper is reaching into the model.
Ideally, when you’re stubbing a layer, you want to be able to stub one
thing. In this example, there are many things that need to be stubbed. A
common example is a display formatter for a person’s name:

it “concats the first and last name” do
person = double(‘person’, :first_name => “Joe”, :last_name => “Smith”)
helper.full_name_for(person).should == “Joe S.”
end

Here we’re just stubbing a couple of values on one object - simple. In
this case it makes sense to just stub the model.

Thanks again David.

5

Hi David

Even though the code is in a helper, it depends heavily on the model.
This exhibits a code smell called Feature Envy, in which one object (the
helper) does some computation but another object (the CfgInterface
model) has all the data. Based on that, one might argue this method
belongs on the model anyhow, in which case the argument of stubbing out
the model layer makes little sense.

I see your point and in this case you are probably right but what if my
helper was more heavy on the processing and still very data dependent ?
Should I use fixtures on the helper or should I stub/mock whatever I
need to avoid the model layer within the helper ?

There are also several expectations that overlap. It doesn’t matter that
the class is a Hash - what matters is that it behaves like a hash, which
is demonstrated in other expectations. The fact that the keys exist is
also demonstrated in the expectations that access those keys.

Good point, thanks.

Based on those comments, I’d probably do something like
gist:506366 · GitHub. Then I’d figure out how to swap in
factories for the fixtures - right now there’s no way to understand what
‘CONS_ALL_ACCOUNT’ means without looking elsewhere.

‘CONS_ALL_ACCOUNT’ actually has no impact when using stub/mocks,
‘CONS_ALL_ACCOUNT’ was the input parameter the model was using to fetch
data from the BD, in the second form of the test with stubs/mocks, this
input parameter has no meaning since I’m controlling all return
parameters.