More rspec questions

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
[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.

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…

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…

Patrick J. Collins
http://collinatorstudios.com

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.

As with anything, YMMV.

On 27 Apr 2010, at 22:51, Patrick J. Collins wrote:

gist:381384 · GitHub
end
@vcard.find_contact.should == contact

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
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

+447974 430184

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

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

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

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

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…

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