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?
On Tue, Apr 27, 2010 at 2:20 PM, Patrick J. Collins [email protected] wrote:
def complex_method
def setup
… 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.
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…
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 => "[email protected]")
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…
On Tue, Apr 27, 2010 at 2:51 PM, Patrick J. Collins [email protected] wrote:
So for example, I would like to just make a spec that does:
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.
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.
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:
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??
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).
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
On Tue, Apr 27, 2010 at 4:56 PM, Patrick J. Collins [email protected] wrote:
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.
… 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:
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…
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
This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.