[rspec-users] Speccing private class methods

David Chelimsky dchelimsky at gmail.com
Thu Dec 21 12:14:27 EST 2006


On 12/21/06, Ashley Moran <work at ashleymoran.me.uk> wrote:
>
> On 21 Dec 2006, at 12:59, David Chelimsky wrote:
>
> > In theory you shouldn't be specifying anything about private methods.
>
> Hmmm...
>
> Here is my original thinking:
>   - I've written a method somewhere (private or otherwise) that is
> specified independently
>   - I know the algorithm must use this method to behave correctly
>   - therefore I should specify that the main method calls the
> auxillary one
>
> Not testing the private methods seems to increase the granularity of
> the specs too much.  There's a lot of stuff we do has complex but
> decomposable algorithms, and it makes sense to test the parts of
> these even if they are not part of the public interface.  Not doing
> so seems similar to not unit testing ActiveRecord models because you
> can test their behaviour in the controller specs (which is what Rails
> does with Test::Unit)

"Not testing the private methods seems to increase the granularity of
the specs too much."

I assume you mean that it decreases the granularity, yes?

There is a school of thought that you should only test through public
APIs of your classes. So in your case, there would be tests on the
public method that would specify how it should behave. As your
algorithm becomes more complex you would refactor out the private
method internally.

There are divergent schools of thought as to whether the new private
method gets its own test. The one to which I subscribe is that if the
method is so interesting that it needs its own test, it probably
belongs in another class because the current class is probably
violating the Single Responsibility Principle. If so, then I'd move
the method to a new class and make it public on that class.

Bear in mind that these are all guidelines and schools of thought.
There is no RIGHT way.

>
> > The only reason we support partial mocks at all (which is a mocking
> > no-no) is because Rails forces us to.
>
> Why is this so wrong and what about about Rails forces it?

Partial mocking is riddled with pitfalls. The most obvious one is the
scenario in which you mock method :a and a new method :b comes into
play later which uses :a internally. Now :b won't behave the way you
expect any longer because you're mocking :a. These things lead to
long, painful debug sessions.

Rails forces us to use partial mocks because ActiveRecord uses static
finder methods and forces us to use inheritance for models instead of
composition. Another alternative would be to introduce a ModelSearch
class into your controllers. So instead of this:

def index
  @people = Person.find(:all)
end

you might have this:

def index
  @people = model_finder.find(:all, Person)
end

def model_finder
  @model_finder ||= ModelFinder.new
end

def model_finder=(model_finder)
  @model_finder = model_finder
end

Now you can set the model_finder in your specs and you don't have to
do any partial mocking of the Person static methods. Not recommending
this. It's got its own drawbacks, but it's just another way you could
handle things.

> I'm new
> to mocking and I'm not sure I'm clear on the definition.  (Does the
> following code count as partial mocking, seeing how "date" is
> required in the CapFutureResidual  model class file?)
>
> Here is a spec I wrote today that tests a really simple (private)
> method:
>
> context "CapFutureResidual.registration_date_recent?" do
>    setup do
>      Date.stub!(:today).and_return(Date.new(2006, 12, 20))
>    end
>
>    specify "should find the current date" do
>      Date.should_receive(:today).with(:no_args).and_return(Date.new
> (2006, 12, 20))
>      CapFutureResidual.send(:registration_date_recent?, Date.new
> (2006, 06, 25))
>    end
>
>    specify "should define a data under six months old (approx) as
> recent" do
>      CapFutureResidual.send(:registration_date_recent?, Date.new
> (2006, 06, 23)).should == true
>    end
>
>    specify "should define a date over six months old (approx) as not
> recent" do
>      CapFutureResidual.send(:registration_date_recent?, Date.new
> (2006, 06, 22)).should == false
>    end
> end
>
> Again, I don't see how specifying Date.should_receive(:today) is
> different from specifying that a private method is called, or should
> I not  be doing this either...
>
> >
> > You won't find a solution to your problem within RSpec, but you can
> > do this:
> >
> > specify "(2) call_private_method should call private_method" do
> >   begin
> >     MyClass.send :public, 'private_method'
> >     MyClass.should_receive(:private_method).and_return("this is
> > private")
> >   ensure
> >     MyClass.send :private, 'private_method'
> >   end
> > end
> >
> > So, for this spec only, you can access the method to spec that it gets
> > called, but then restore the method to its private state.
>
> Hmmm... not sure if I'd like to write specs like this... might just
> drop that spec entirely and include the behaviour of the private
> method in the spec for the calling method.

That's what I'd do. If this makes your specs too complicated for the
one method, then maybe it's trying to do too much.

Hope this all helps.

David

>
>
> Ashley
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


More information about the rspec-users mailing list