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(p) 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 Hoffman wrote:> Property.stub!(:find).and_return mock_property1, mock_property2try Property.stub!(:find).and_return( [mock_property1, mock_property2] ) ____________________________________________________________ Want to help others? Become a certified physical therapist. Click now. http://thirdpartyoffers.netzero.net/TGL2241/fc/Ioyw6i4vHJTUahrDj893hIpJrosoTWmfqTn2L5VZAiHpeGUCV1fp6d/
On Fri, Sep 5, 2008 at 4:44 PM, Nick Hoffman <nick at deadorange.com> wrote:> 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(p) > 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_property2This 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?> 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 > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
On 2008-09-05, at 18:22, Jonathan Linowes wrote:> On Sep 5, 2008, at 5:44 PM, Nick Hoffman 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 Sat, Sep 6, 2008 at 2:32 PM, Nick Hoffman <nick at deadorange.com> wrote:> On 2008-09-05, at 18:22, Jonathan Linowes wrote: >> >> On Sep 5, 2008, at 5:44 PM, Nick Hoffman 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?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 :) 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> > Thanks, > Nick > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
On 2008-09-06, at 15:58, David Chelimsky 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 :)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(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.RentalMap#add_properties does have feature envy. However...> 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.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
> > 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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20080906/d2ed751d/attachment.html>
On Sat, Sep 6, 2008 at 4:48 PM, Nick Hoffman <nick at deadorange.com> wrote:> On 2008-09-06, at 15:58, David Chelimsky 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 :) > > 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(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. > > RentalMap#add_properties does have feature envy. However... > >> 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. > > 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.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> > Thanks again for your opinions, David! > Nick > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
On Sat, Sep 6, 2008 at 5:25 PM, Mark Wilden <mark at mwilden.com> 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.> > ///ark > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
On Sat, Sep 6, 2008 at 9:57 PM, David Chelimsky <dchelimsky at gmail.com> wrote:> On Sat, Sep 6, 2008 at 4:48 PM, Nick Hoffman <nick at deadorange.com> wrote: >> On 2008-09-06, at 15:58, David Chelimsky 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 :) >> >> 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(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. >> >> RentalMap#add_properties does have feature envy. However... >> >>> 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. >> >> 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. > > 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 > endOr, 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.> > 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 > >> >> Thanks again for your opinions, David! >> Nick >> _______________________________________________ >> rspec-users mailing list >> rspec-users at rubyforge.org >> http://rubyforge.org/mailman/listinfo/rspec-users >> >
On Sat, Sep 6, 2008 at 8:00 PM, David Chelimsky <dchelimsky at gmail.com>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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20080907/62b63cb9/attachment.html>