Hi everyone,
I still am on my quest to understand Rspec, and I have a few new questions...
If I have a "complex" method which calls other methods, say something
like:
-----
class Foo
def complex_method
setup
setup_something_else
mini_method1
mini_method2
mini_method3
end
private
def setup
@somevar = Something.something_else
end
def setup_something_else
@someothervar = SomethingElse.something_else
end
def mini_method1
# do something
end
... etc
end
-----
One way to properly test this would be to write a spec that calls
complex_method and it''s output returns what I am expecting.. but if
it''s
broken, it becomes difficult for me to figure out why without writing
individual tests for the mini_methods.
However, I cannot do tests for mini_methods because they are private, and 2ndly
they require initializer methods (setup/setup_something_else) to be called.
So I am just wondering, what is the proper technique to test these
private mini_methods?
Patrick J. Collins
http://collinatorstudios.com
On Tue, Apr 27, 2010 at 2:20 PM, Patrick J. Collins <patrick at collinatorstudios.com> wrote:> Hi everyone, > > I still am on my quest to understand Rspec, and I have a few new questions... > > If I have a "complex" method which calls other methods, say something like: > > ----- > > class Foo > > ?def complex_method > ? ? ? ? setup > ? ?setup_something_else > > ? ? ? ? mini_method1 > ? ? ? ? mini_method2 > ? ? ? ? mini_method3 > ?end > > ?private > > ?def setup > ? ?@somevar = Something.something_else > ?end > > ?def setup_something_else > ? ?@someothervar = SomethingElse.something_else > ?end > > ?def mini_method1 > ? ?# do something > ?end > > ?... etc > end > > ----- > > One way to properly test this would be to write a spec that calls > complex_method and it''s output returns what I am expecting.. ?but if it''s > broken, it becomes difficult for me to figure out why without writing > individual tests for the mini_methods. >Yep. You need small focused specs that each test one bit of behavior so that when they fail you know why.> However, I cannot do tests for mini_methods because they are private, and 2ndly > they require initializer methods (setup/setup_something_else) to be called. >What if they didn''t? Is there a different way you could design this so that the interesting bits (The small methods) didn''t depend so much on the other bits around them? BTW, those kind of dependencies are what design experts call coupling. Over the years many of us have come to believe that less coupling =better design. The fact that your spec is telling you that this is hard to test is a really important clue that the design could be improved.> So I am just wondering, what is the proper technique to test these > private mini_methods? >Make them public. Move them to their own classes that encapsulate the stuff they need to know about (e.g. the "setup".) When you test the higher level methods you can mock this stuff. When you test the small methods you should call them directly. There are a number of other techniques that might be helpful too. If you have a more specific example of what you are trying to do we might be able to give you more specific advice.
Hi,> What if they didn''t? Is there a different way you could design this so > that the interesting bits (The small methods) didn''t depend so much on > the other bits around them? >Well this is for importing vCards... http://gist.github.com/381384 So for example, I would like to just make a spec that does: ... before(:all) do card_data = File.read(RAILS_ROOT + "/spec/fixtures/vcard/kirk_no_photo.vcf") @vcard = Vcard.create!(:data => card_data) Contact.all.map(&:destroy) end describe "finding a contact" do it "should find the contact when an email for the contact exists" do email = Email.create!(:address => "kirk at enterprise.com") contact = Contact.create!(:first_name => ''James'', :last_name => ''Kirk'') contact.emails << email @vcard.find_contact.should == contact end end ... But, like I said, I am not quite sure how to structure it so that my @card ivar gets set..> Make them public. Move them to their own classes that encapsulate the > stuff they need to know about (e.g. the "setup".) When you test the > higher level methods you can mock this stuff. When you test the small > methods you should call them directly.So is it bad practice then to use private methods? I have a habit of doing that-- but from what you''re saying there is no way to test those things individually.... Patrick J. Collins http://collinatorstudios.com
On Tue, Apr 27, 2010 at 2:51 PM, Patrick J. Collins <patrick at collinatorstudios.com> wrote:> Hi, > >> What if they didn''t? Is there a different way you could design this so >> that the interesting bits (The small methods) didn''t depend so much on >> the other bits around them? >> > > Well this is for importing vCards... > http://gist.github.com/381384 > > So for example, I would like to just make a spec that does: > > ... > > ?before(:all) do > ? ?card_data = File.read(RAILS_ROOT + "/spec/fixtures/vcard/kirk_no_photo.vcf") > ? ?@vcard = Vcard.create!(:data => card_data) > ? ?Contact.all.map(&:destroy) > ?end > > ?describe "finding a contact" do > > ? ?it "should find the contact when an email for the contact exists" do > ? ? ?email = Email.create!(:address => "kirk at enterprise.com") > ? ? ?contact = Contact.create!(:first_name => ''James'', :last_name => ''Kirk'') > ? ? ?contact.emails << email > > ? ? ?@vcard.find_contact.should == contact > ? ?end > ?end > > ... > > > But, like I said, I am not quite sure how to structure it so that my @card ivar gets set.. >Based on the above I think the Vcard is a good opportunity for a mock. That would most likely imply that you create the Vcard somewhere else and pass it into this method. Also, you should directly test the implementation of the Vcard outside of this spec (i.e. in it''s own spec.)>> Make them public. Move them to their own classes that encapsulate the >> stuff they need to know about (e.g. the "setup".) When you test the >> higher level methods you can mock this stuff. When you test the small >> methods you should call them directly. > > So is it bad practice then to use private methods? ?I have a habit of doing > that-- but from what you''re saying there is no way to test those things individually.... >No, it''s not bad practice to use private methods. However, private methods should mostly be helpers that encapsulate some small calculation. If the method is interesting enough to be tested on its own then it probably should be part of the public API. If the method is not interesting enough to be tested on its own then it should be private and tested through the public method that calls it. As with anything, YMMV.
On 27 Apr 2010, at 22:51, Patrick J. Collins wrote:> Hi, > >> What if they didn''t? Is there a different way you could design this >> so >> that the interesting bits (The small methods) didn''t depend so much >> on >> the other bits around them? >> > > Well this is for importing vCards... > http://gist.github.com/381384 > > So for example, I would like to just make a spec that does: > > ... > > before(:all) do > card_data = File.read(RAILS_ROOT + "/spec/fixtures/vcard/ > kirk_no_photo.vcf") > @vcard = Vcard.create!(:data => card_data) > Contact.all.map(&:destroy) > end > > describe "finding a contact" do > > it "should find the contact when an email for the contact exists" > do > email = Email.create!(:address => "kirk at enterprise.com") > contact = Contact.create!(:first_name => ''James'', :last_name => > ''Kirk'') > contact.emails << email > > @vcard.find_contact.should == contact > end > end > > ... > > > But, like I said, I am not quite sure how to structure it so that my > @card ivar gets set.. > >> Make them public. Move them to their own classes that encapsulate the >> stuff they need to know about (e.g. the "setup".) When you test the >> higher level methods you can mock this stuff. When you test the small >> methods you should call them directly. > > So is it bad practice then to use private methods? I have a habit > of doing > that-- but from what you''re saying there is no way to test those > things individually....It''s not bad practice to use private methods - lots of small methods in your classes is a good thing! As Adam says though, when a class gets hard to write microtests for, that''s generally a sign that the design needs a re-think. This subject has been discussed at length on this list before. Here''s a good example: http://groups.google.com/group/rspec/browse_thread/thread/9e3d879f712ce4f2/030af755918967dd> > Patrick J. Collins > http://collinatorstudios.com > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-userscheers, Matt http://mattwynne.net +447974 430184
> Also, you should directly test the implementation of the Vcard outside of > this spec (i.e. in it''s own spec)Could you explain what you mean by that? Patrick J. Collins http://collinatorstudios.com
> Based on the above I think the Vcard is a good opportunity for a mock. > That would most likely imply that you create the Vcard somewhere else > and pass it into this method. Also, you should directly test theOk, and regarding mocking-- Something that is still very unclear to me is how can I affect instance variables through mocking? If I do: def master_method @foo = "foo" slave_method end def slave_method @foo end .. it "should be foo" do foo = mock_model(Foo) foo.slave_method.should == "foo" end ... This will be false because @foo is set in the msater method which I am bypassing because I just want to test the slave method. So how can I set an instance variabel within my spec so that @foo will be properly set? Should I use setter/getter methods in this case?? def master_method foo = "foo" slave_method end def slave_method foo end def foo=(new_foo) @foo = new_foo end def foo @foo end ......... ? Is that the way to go??? Patrick J. Collins http://collinatorstudios.com
If something in the code you are testing depends on the return value
of a method then you would use a stub. e.g.:
foo = mock(Foo)
foo.stub!(:slave_method).and_return("foo")
However, in some cases what matters is not what the method returns but
the fact that slave_method gets called. i.e.:
foo.should_receive(:slave_method)
There is plenty of info about this in the RSpec docs.
Re-reading your example, I don''t think I would mock Vcard. From what I
can see Vcard reads data that you got from a file and creates a list
of Contact objects from it. The only interesting thing about Vcard is
that it is able to parse this data in string form. So, I would write a
spec for that (i.e. what are the interesting permutations of strings
that Vcard can parse? And what representation should result from
parsing them?)
The second interesting thing is that I can create new Contacts and put
them into the collection that Vcard maintains. I would consider
separating the parsing from maintaining the list as these seem like
different responsibilities.
There really doesn''t seem to be anything else going on here from the
information you''ve given. Contact and Email seem like pure values, so
there probably isn''t any point in mocking them. However, if they
contain some interesting behavior then that should be tested as well.
On Tue, Apr 27, 2010 at 4:02 PM, Patrick J. Collins
<patrick at collinatorstudios.com> wrote:>> Based on the above I think the Vcard is a good opportunity for a mock.
>> That would most likely imply that you create the Vcard somewhere else
>> and pass it into this method. Also, you should directly test the
>
> Ok, and regarding mocking-- ?Something that is still very unclear to me is
how
> can I affect instance variables through mocking? ?If I do:
>
>
> def master_method
> ?@foo = "foo"
>
> ?slave_method
> end
>
> def slave_method
>
> ?@foo
>
> end
>
> ..
>
> it "should be foo" do
>
> ?foo = mock_model(Foo)
>
> ?foo.slave_method.should == "foo"
>
> end
>
> ...
>
>
> This will be false because @foo is set in the msater method which I am
> bypassing because I just want to test the slave method. ?So how can I set
an
> instance variabel within my spec so that @foo will be properly set? ?Should
I
> use setter/getter methods in this case??
>
> def master_method
> ?foo = "foo"
>
> ?slave_method
> end
>
> def slave_method
>
> ?foo
>
> end
>
> def foo=(new_foo)
> ?@foo = new_foo
> end
>
> def foo
> ?@foo
> end
>
> ......... ? ?Is that the way to go???
>
> Patrick J. Collins
> http://collinatorstudios.com
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
> If something in the code you are testing depends on the return value > of a method then you would use a stub. e.g.:Right, but what I am asking is--- if all of my "slave methods" are relying on stored card data from the @card instance variable-- how is the best way to have access to the variable if it''s only defined in the master method? if I have a method that does @card.addresses.each do |address| ... end and I want to test that a contact does in fact get a new address created after that method is called with a vcard that has a valid address. I mean, one way to do this would be the getter/setter way-- have a card=/card method.. Another way would be to just pass card into each method def process_addresses(card) card.addresses.each... .. But then I start feeling like my code is going to be less-elegant due to the fact that I am trying to write things that can be tested. Passing in a variable is definitely less elegant than using an instance variable, and a getter/setter is somewhat too (in my opinion). Patrick J. Collins http://collinatorstudios.com
On Tue, Apr 27, 2010 at 4:56 PM, Patrick J. Collins <patrick at collinatorstudios.com> wrote:>> If something in the code you are testing depends on the return value >> of a method then you would use a stub. e.g.: > Right, but what I am asking is--- ?if all of my "slave methods" are relying on > stored card data from the @card instance variable-- ?how is the best way to > have access to the variable if it''s only defined in the master method? > > if I have a method that does > > @card.addresses.each do |address| > ?... > end > > and I want to test that a contact does in fact get a new address created after > that method is called with a vcard that has a valid address. >It might be that the problem is with the way you are stating your test. It seems to me that the code you wrote above is the least interesting part. We can just assume that iterating over addresses works, it is what we do with them that is interesting. So that leaves us with two ideas: 1) is an address that a vcard contains valid? 2) can I add an address to a contact. I would go a step further and say that adding an address to card is not very interesting (Just like iterating over addresses, adding them should work out of the box assuming Ruby itself isn''t broken.) So if the only interesting part of the above is that vcards can parse addresses then there should be a public method for that which you test by passing in a string and getting out an address object.> I mean, one way to do this would be the getter/setter way-- have a card=/card > method.. > > Another way would be to just pass card into each method > > def process_addresses(card) > > card.addresses.each... > > > .. ?But then I start feeling like my code is going to be less-elegant due to > the fact that I am trying to write things that can be tested. ?Passing in a > variable is definitely less elegant than using an instance variable, and a > getter/setter is somewhat too (in my opinion). >I wouldn''t say that it was a good idea to create a way to pass in a card just for the sake of the test. However, I am presuming that at some point you are going to want to do something with the cards. Otherwise, why have them in the first place. So, if there is some logic that gets the cards, and you have a Spec for that, then you will need to set them somehow so that that Spec works.
> I would go a step further and say that adding an address to card is > not very interesting (Just like iterating over addresses, adding them > should work out of the box assuming Ruby itself isn''t broken.)Well, I ended up restructuring my code and I wrote tests for everything I thought were important that would help me know when something is broken. Maybe you''d say some of these cases aren''t necessary-- I don''t know.. But when I was going through my code and making sure it worked, the tests seemed to be very helpful for me... Here''s how it looks now: http://gist.github.com/383233 I solved my @contact.photo = problem by approaching it a different way... Though I still wish someone could tell me what the deal was with that mock expectation error. No one seemed to reply to that question... Anyway, everything passes so I guess I can move on to the next thing... Patrick J. Collins http://collinatorstudios.com
With only a cursory look that looks good to me. If I have some time later I will look more carefully and maybe suggest some refactorings, but you look to be on the right track. The important thing is just to practice and continually improve. Best, Adam On Wed, Apr 28, 2010 at 11:37 PM, Patrick J. Collins <patrick at collinatorstudios.com> wrote:>> I would go a step further and say that adding an address to card is >> not very interesting (Just like iterating over addresses, adding them >> should work out of the box assuming Ruby itself isn''t broken.) > > Well, I ended up restructuring my code and I wrote tests for everything I > thought were important that would help me know when something is broken. ?Maybe > you''d say some of these cases aren''t necessary-- I don''t know.. ?But when I was > going through my code and making sure it worked, the tests seemed to be very > helpful for me... > > Here''s how it looks now: > http://gist.github.com/383233 > > I solved my @contact.photo = problem by approaching it a different way... > Though I still wish someone could tell me what the deal was with that mock > expectation error. ?No one seemed to reply to that question... > > Anyway, everything passes so I guess I can move on to the next thing... > > Patrick J. Collins > http://collinatorstudios.com > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >