2nd attempt at mocking and stubbing

So, after reading all of those fantastic emails discussing testing
behaviour vs state, I decided to try mocking and stubbing a couple of
methods. I think I did well on my first one, but I’m not sure what the
best way to spec the following method is:

1 class RentalMap

166 def add_properties
167 Property.find(:all, :conditions => ‘latitude NOT NULL AND
longitude NOT NULL’).each do |p|
168 add_marker p.address, p.latitude, p.longitude,
generate_marker_contents§
169 end
170 end

In my spec, I figured I would:
-Create a RentalMap object.
-Mock two properties.
-Setup expectations on the two mock properties.
-Stub the call to Property#find .
-Stub the call to RentalMap#add_marker .
-Stub the call to RentalMap#generate_marker_contents .

Here’s what I wrote:

185 describe ‘#add_properties’ do
186 it ‘should add properties to the map’ do
187 map = RentalMap.new @map_name, @latitude,
@longitude
188 mock_property1 = mock ‘property’
189 mock_property2 = mock ‘property’
190
191 Property.stub!(:find).and_return mock_property1,
mock_property2
192 map.stub!(:add_marker).and_return ‘?’
193
194 mock_property1.should_receive(:address ).and_return ‘400
Bloor Street’
195 mock_property1.should_receive(:latitude ).and_return 12.34
196 mock_property1.should_receive(:longitude).and_return 56.78
197 mock_property2.should_receive(:address ).and_return ‘500
Bloor Street’
198 mock_property2.should_receive(:latitude ).and_return 12.45
199 mock_property2.should_receive(:longitude).and_return 56.89
200 map.stub!(:generate_marker_contents).twice.and_return ‘Some
contents for the first property’, ‘Some contents for the second
property’
201
202 map.should_receive(:add_marker).with mock_property1.address,
mock_property1.latitude, mock_property1.longitude,
map.generate_marker_contents
203 map.should_receive(:add_marker).with mock_property2.address,
mock_property2.latitude, mock_property2.longitude,
map.generate_marker_contents
204
205 map.add_properties
206 end
207 end

However, this fails with:

Spec::Mocks::MockExpectationError in ‘RentalMap#add_properties should
add properties to the map’
Mock ‘property’ received unexpected message :each with (no args)
/Users/nick/src/myapp.podoboo.com/app/models/rental_map.rb:167:in
`add_properties’
./spec/models/rental_map_spec.rb:205:

I don’t think that I should be stubbing Array#each , but I’m not sure
what I should change. Any suggestions?

Thanks!
Nick

On Sep 5, 2008, at 5:44 PM, Nick H. wrote:

Property.stub!(:find).and_return mock_property1, mock_property2

try
Property.stub!(:find).and_return( [mock_property1, mock_property2] )

On Fri, Sep 5, 2008 at 4:44 PM, Nick H. [email protected]
wrote:

NOT NULL’).each do |p|
-Stub the call to RentalMap#add_marker .
191 Property.stub!(:find).and_return mock_property1, mock_property2
This should be:

Property.stub!(:find).and_return [mock_property1, mock_property2]

The way you had it returns mock_property1 the first time Property is
sent :find and mock_property2 the 2nd.

Make sense?

On 2008-09-05, at 18:22, Jonathan L. wrote:

On Sep 5, 2008, at 5:44 PM, Nick H. wrote:

Property.stub!(:find).and_return mock_property1, mock_property2

try
Property.stub!(:find).and_return( [mock_property1, mock_property2] )

Thanks Jonathan and David. That was a silly mistake.

With that fixed, a new question has popped up. My specs for the method
I previously mentioned are now:

185 describe ‘#add_properties’ do # {{{
186 it ‘should add properties to the map’ do
187 map = RentalMap.new @map_name, @latitude,
@longitude
188 mock_property1 = mock ‘property’
189 mock_property2 = mock ‘property’
190 marker1_contents = ‘Some contents for the first property’
191 marker2_contents = ‘Some contents for the second property’
192
193 Property.stub!(:find).and_return [mock_property1,
mock_property2]
194 map.stub!(:add_marker).and_return “Doesn’t matter”
195
196 mock_property1.stub!(:address ).and_return ‘400 Bloor Street’
197 mock_property1.stub!(:latitude ).and_return 12.34
198 mock_property1.stub!(:longitude).and_return 56.78
199 mock_property2.stub!(:address ).and_return ‘500 Bloor Street’
200 mock_property2.stub!(:latitude ).and_return 12.45
201 mock_property2.stub!(:longitude).and_return 56.89
202 map.stub!(:generate_marker_contents).twice.and_return
marker1_contents, marker2_contents
203
204 map.should_receive(:add_marker).with mock_property1.address,
mock_property1.latitude, mock_property1.longitude, marker1_contents
205 map.should_receive(:add_marker).with mock_property2.address,
mock_property2.latitude, mock_property2.longitude, marker2_contents
206
207 map.add_properties
208 end
209 end # }}}

However, lines 190 and 191 feel a bit like I’m setting up fixture
data. But the only alternative that I can think of will fail:
-Comment out lines 190 and 191.
-On line 202, replace “marker*_contents” with the strings on lines 190
and 191.
-Lines 204 and 205 will have to call #generate_marker_contents for the
last argument.
-Lines 204 and 205 will then consume the two calls to
#generate_marker_contents that I expect on line 202.

Can anyone suggest a better/different way of doing this?

Thanks,
Nick

On 2008-09-06, at 15:58, David C. wrote:

Well, without changing the underlying semantics, you can clean up the
syntax a bit like this:

mock_property1 = stub(‘property’, :address => ‘400 Bloor Street’,
:latitude => 12.34, :longitude => 56.78)

Of course, that doesn’t address your question :slight_smile:

That makes the specs much easier to read!

Here’s another thought that doesn’t answer your question - this line
in the code:

add_marker p.address, p.latitude, p.longitude,
generate_marker_contents§

is resulting in a situation where you have to do a lot of setup in
order to isolate the object under test. The line exhibits the Feature
Envy Code Smell, which is the odor emitted by one object depending on
another object’s data.

RentalMap#add_properties does have feature envy. However…

you can write code examples with the details of its own data rather
than having to stub so much of its data for use in the current
example.

I believe that the process of adding a marker (IE: Property) to a
RentalMap should be a feature of the RentalMap model, rather than the
Property model. From a Property’s perspective, a property has nothing
to do (IE: no interactions) with RentalMaps. From a RentalMap’s
perspective, a RentalMap has lots to do with Properties, because it
needs to ask a Property for information so that it can add a marker to
itself.

To make an analogy, do I put on a shirt, or does a shirt put itself on
me?

Thanks again for your opinions, David!
Nick

On Sat, Sep 6, 2008 at 2:32 PM, Nick H. [email protected]
wrote:

192
marker1_contents, marker2_contents
However, lines 190 and 191 feel a bit like I’m setting up fixture data. But
the only alternative that I can think of will fail:
-Comment out lines 190 and 191.
-On line 202, replace “marker*_contents” with the strings on lines 190 and
191.
-Lines 204 and 205 will have to call #generate_marker_contents for the last
argument.
-Lines 204 and 205 will then consume the two calls to
#generate_marker_contents that I expect on line 202.

Can anyone suggest a better/different way of doing this?

Well, without changing the underlying semantics, you can clean up the
syntax a bit like this:

mock_property1 = stub(‘property’, :address => ‘400 Bloor Street’,
:latitude => 12.34, :longitude => 56.78)

Of course, that doesn’t address your question :slight_smile:

Here’s another thought that doesn’t answer your question - this line
in the code:

add_marker p.address, p.latitude, p.longitude,
generate_marker_contents(p)

is resulting in a situation where you have to do a lot of setup in
order to isolate the object under test. The line exhibits the Feature
Envy Code Smell, which is the odor emitted by one object depending on
another object’s data.

If I were coding by example, I’d probably start w/ a simple example like
this:

describe ‘#add_properties’ do
it ‘should add one property to the map’ do
map = RentalMap.new @map_name, @latitude, @longitude
mock_property = mock ‘property’
Property.stub!(:find).and_return [mock_property]

mock_property.should_receive(:add_marker_to).with(map)

map.add_properties

end
end

This defines how I’d like to be able to “talk” to the Property, and
results in an implementation like this:

p.add_marker_to(map)

Now all the additional behaviour is pushed to the Property, for which
you can write code examples with the details of its own data rather
than having to stub so much of its data for use in the current
example.

That all make sense?
David

On Sat, Sep 6, 2008 at 4:48 PM, Nick H. [email protected]
wrote:

That makes the specs much easier to read!

another object’s data.
Property.stub!(:find).and_return [mock_property]
p.add_marker_to(map)
has lots to do with Properties, because it needs to ask a Property for
information so that it can add a marker to itself.

Sounds like you’re letting adherence to a mental model overrule what
the code is actually telling you. The code is telling you that the
model is flawed. I’d listen to it.

Personally, I don’t see any conceptual, philosophical problem with a
Property knowing it is on a Map - doesn’t have to be a RentalMap, just
a Map. If you can accept that, then you can take the next step and do
a little double dispatch:

class Property
def add_to(map)
map.add_marker address, latitude, longitude, contents
end
end

The map could be RentalMap, a ListingMap, an EventsMap, etc, etc.

To make an analogy, do I put on a shirt, or does a shirt put itself on me?

You are human. Programs are not. Seeing as a Person object and a Shirt
object can be programmed to behave as we wish, I’d suggest that there
might be fewer dependencies and a simpler series of instructions if
the shirt object put itself on the person object.

While Objects can sometimes do a decent job of modeling the real
world, that’s not why they exist. They were born to make software more
… soft. Guidelines like Law of Demeter, Tell Don’t Ask and DRY
didn’t come out of nowhere. None of them are 100% applicable all the
time, but when they are violated, their resulting cries should at
least be a red flag.

FWIW,
David

On Sat, Sep 6, 2008 at 5:25 PM, Mark W. [email protected] wrote:

203
204 map.should_receive(:add_marker).with mock_property1.address,
mock_property1.latitude, mock_property1.longitude, marker1_contents
205 map.should_receive(:add_marker).with mock_property2.address,
mock_property2.latitude, mock_property2.longitude, marker2_contents

It looks like this is copying certain property properties to the map. Would
it be better for the map to simply hold a reference to the property?

That might clean up the interactions a bit, but then you’ve still got
the map asking the properties for its data.

I think the property knows it’s location, it should provide that
information.

On Sat, Sep 6, 2008 at 9:57 PM, David C. [email protected]
wrote:

Envy Code Smell, which is the odor emitted by one object depending on
mock_property = mock ‘property’

interactions) with RentalMaps. From a RentalMap’s perspective, a RentalMap
a little double dispatch:

class Property
def add_to(map)
map.add_marker address, latitude, longitude, contents
end
end

Or, better yet:

class Property
def add_marker_to(map)
map.add_marker Marker.new(address, latitude, longitude, contents)
end
end

That reduces the surface contact between the Property and the Map even
more.

203
204 map.should_receive(:add_marker).with mock_property1.address,
mock_property1.latitude, mock_property1.longitude, marker1_contents
205 map.should_receive(:add_marker).with mock_property2.address,
mock_property2.latitude, mock_property2.longitude, marker2_contents

It looks like this is copying certain property properties to the map.
Would
it be better for the map to simply hold a reference to the property?

///ark

On Sat, Sep 6, 2008 at 8:00 PM, David C.
[email protected]wrote:

class Property
def add_marker_to(map)
map.add_marker Marker.new(address, latitude, longitude, contents)
end
end

That reduces the surface contact between the Property and the Map even
more.

The surface contact between those objects is indeed reduced. But now the
map
has to ask the marker for this information, rather than the property.
The
number of surface contact points just increased by one.

Plus, the information is still copied instead of referenced. Which means
that if someone made a typo in the property’s address, they’d have to
remember to copy it to the map again.

Anyway, there are lots of ways to skin cats, and it’s interesting to
discuss
them (if not necessarily rspecy).

///ark