[rspec-users] [Rails] rails view helpers, BDD, and what to mock

Myron Marston myron.marston at gmail.com
Sat Sep 18 15:31:15 EDT 2010



On Sep 18, 11:52 am, "Doug E." <d... at emeryit.com> wrote:
> On Sep 17, 9:16 pm, Myron Marston <myron.mars... at gmail.com> wrote:
>
>
>
>
>
> > On Sep 17, 12:48 pm, "Doug E." <d... at emeryit.com> wrote:
>
> > > Hi,
>
> > > I'm trying to understand BDD and proper testing technique. I'm testing
> > > a rails view helper method that checks user roles to see if a link
> > > should be shown. I'm using rails 2.3.8 and rspec version 1.3.0.  My
> > > helper looks like this:
>
> > > # welcome_helper.rb
> > > module WelcomeHelper
> > >  def manage_samples_link
> > >   return nil if current_user.blank?
> > >   if current_user.role?(:submitter) ||
> > >       current_user.role?(:reviewer) ||
> > >       current_user.role?(:admin)
> > >     link_to "Manage Samples", manage_samples_path
> > >   end
> > >  end
> > > end
>
> > > The spec code looks like this:
>
> > > # welcome_helper_spec.rb
> > > require 'spec_helper'
>
> > > describe WelcomeHelper do
> > >  %w{ submitter reviewer admin }.each do |rolename|
> > >   it "should return manage samples link for user with role
> > > #{rolename}" do
> > >     role = Role.new(:rolename => rolename)
> > >     user = User.new(:roles => [role])
> > >     helper.stub(:current_user).and_return(user)
> > >     link = %{<a href="/manage_samples">Manage Samples</a>}
> > >     helper.manage_samples_link.should == link
> > >   end
> > >  end
> > > end
>
> > > If the above doesn't format well, here's a gist:
>
> > >  https://gist.github.com/afd222a63193bf000ab8
>
> > > My question is, What should I be testing here?
>
> > > I stubbed current_user, but I haven't mocked the User object and its
> > > role? method. While I understand that this is the proper place to test
> > > the User#role? method, don't I want to know that this helper is
> > > returning the link only for those the admin, submitter, and reviewer
> > > users and not for, for instance, 'editors'?
>
> > > If I mock out user and stub it with something like:
>
> > >  user.stub!(:role?).and_return(true)
>
> > > Am I really testing anything other than the content of the link?
>
> > > It seems to me that there are two things to test:
>
> > > (1) that I get the right link; and
> > > (2) the logic; that is, that the correct users get the link
>
> > > And, it seems that if I don't use a User object, I don't really know
> > > whether the logic is correct or, secondarily, if some later change to
> > > the User#role? method has broken my helpers, views, etc.
>
> > > Is this correct? I apologize if this is too basic a question, but I'm
> > > trying to wrap my head around what rspec tests go where and how to
> > > structure them, and leave integration testing to cucumber.
>
> > > Thanks.
>
> > > Doug
>
> > > PS My apologies to the moderators if you've been getting spammed with
> > > different versions of this post as I've stumbled about trying to get
> > > my account set up.
> > > _______________________________________________
> > > rspec-users mailing list
> > > rspec-us... at rubyforge.orghttp://rubyforge.org/mailman/listinfo/rspec-users
>
> > The presence of 3 clauses in your if statement suggests to me that you
> > have too much business logic in this helper.  I'd recommend you move
> > that into an appropriate method on your user model:
>
> > class User
> >   def can_manage_samples?
> >     [:submitter, :reviewer, :admin].any? { |r| role?(r) }
> >   end
> > end
>
> > This should not be difficult to test in your user model spec.
>
> > Now your helper can be greatly simplified:
>
> > module WelcomeHelper
> >  def manage_samples_link
> >   link_to "Manage Samples", manage_samples_path if
> > current_user.try(:can_manage_samples?)
> >  end
> > end
>
> > This is far easier to test, and I believe that stubbing
> > #can_manage_samples? is an appropriate way to do so.  Your helper test
> > just needs to test 3 cases:
>
> > 1.  No link should be rendered when current_user is nil.
> > 2.  No link should be rendered when current_user.can_manage_samples?
> > returns false.
> > 3.  The link should be rendered when current_user.can_manage_samples?
> > returns true.
>
> > Note that you may want to look into something like cancan[1] to help
> > with this (although, I've never used cancan myself).
>
> > Myron
>
> > [1]http://github.com/ryanb/cancan
> > _______________________________________________
> > rspec-users mailing list
> > rspec-us... at rubyforge.orghttp://rubyforge.org/mailman/listinfo/rspec-users
>
> Hi Myron,
>
> I believe I understand what you're suggesting. My code now looks like
> this:
>
> # app/models/user.rb
>   class User < ActiveRecord::Base
>
>     has_and_belongs_to_many :roles
>
>     # stuff cut out for brevity
>
>     def role?(role)
>       roles.detect { |r| r.rolename == role.to_s }
>     end
>
>     def can_manage_samples?
>       [ :submitter, :reviewer, :admin ].any? { |r| role? r }
>     end
>   end
>
> # app/helpers/welcome_helper.rb
>   module WelcomeHelper
>     def manage_samples_link
>       if current_user.try(:can_manage_samples?)
>         link_to "Manage Samples", manage_samples_path
>       end
>     end
>   end
>
> # spec/helpers/welcome_helper_spec.rb
>   require 'spec_helper'
>
>   describe WelcomeHelper do
>
>     context "Manage Samples link" do
>       it "should not return manage_samples_link for nil user" do
>         helper.stub(:current_user).and_return(nil)
>         helper.manage_samples_link.should be_nil
>       end
>
>       it "should not return manage_samples_link for role-less user" do
>         user = mock_model(User)
>         user.stub(:can_manage_samples?).and_return(false)
>         helper.stub(:current_user).and_return(user)
>         helper.manage_samples_link.should be_nil
>       end
>
>       it "should return manage_samples_link for permitted user" do
>         user = mock_model(User)
>         user.stub(:can_manage_samples?).and_return(true)
>         helper.stub(:current_user).and_return(user)
>         link = %{<a href="/manage_samples">Manage Samples</a>}
>         helper.manage_samples_link.should == link
>       end
>     end
>   end
>
> Here's how I understand this: With a mocked User#can_manage_samples?
> method, the test focuses not on the User class code, which should be
> tested at the model level. What this test says is, 'If
> User#can_manage_samples? is working correctly, then the correct users
> will see the link.
>
> What happens if when the app is in production and I've forgotten
> writing this test, I'm told that some user who should be authorized to
> is not seeing this link? The view test will still be passing because
> I've told the User object to return what I want it to. Is the idea
> that I should return to my integration tests and move back down
> through the stack to isolate the problem?
>
> It seems to me that the really challenging part of BDD/TDD is learning
> the boundaries of what's being tested and writing code that is
> testable. I appreciate your help, very much.
>
> As for CanCan, I am in fact using it, and based on this exchange, if I
> wanted to use CanCan for determining whether to print this link, I
> would set up the test up in the following way. Since CanCan adds,
> among other methods, a 'can?' method which is included in the
> controller and available to helpers and views, I would have the
> following:
>
> # app/helpers/welcome_helper.rb
> module WelcomeHelper
>   def manage_samples_link
>     link_to "Manage Samples", manage_samples_path if can? :alter,
> Sample
>   end
> end
>
> # spec/helpers/welcome_helper_spec.rb
> require 'spec_helper'
>
> describe WelcomeHelper do
>   context "Manage Samples link" do
>     it "should not return manage_samples_link for no or unauthorized
> user" do
>       helper.should_receive(:can?).with(:alter,
> Sample).and_return(false)
>       helper.manage_samples_link.should be_nil
>     end
>
>     it "should return manage_samples_link for authorized user" do
>       helper.should_receive(:can?).with(:alter,
> Sample).and_return(true)
>       link = %{<a href="/manage_samples">Manage Samples</a>}
>       helper.manage_samples_link.should == link
>     end
>   end
> end
>
> My CanCan Ability implementation handles the case of a nil user,
> creating an empty User object, as ryanb shows in his examples.
>
> The reason I'm using roles and not CanCan for rendering this link and
> navigating to the management page is off-topic, but it is this:
>
> What I want to do is determine whether a user in general can do
> something more than perform the :read action on a Sample, like
> :submit, :edit, :delete, and so on. CanCan works at the class or
> instance level, and while reviewers and admins can do stuff to any
> Sample, submitters can only do stuff to certain samples, the ones they
> created. I don't want to hit the database to find out whether a
> submitter has created any samples. I just want all users who can
> do stuff to Samples to be able to get to the Manage Samples page.
>
> So I would create a general "do stuff to samples" action and
> give submitters, reviewers, and admins authorization to it. I call it
> :alter, because :manage is a special CanCan action that maps to any
> action (e.g., admins `can :manage, User').
>
> But, I decided, in this case I'm just interested in a class of
> users. If down the line I realize I need something more complex for
> allowing access to this page, then I can add it.
>
> Again, thanks.
>
> Doug
> _______________________________________________
> rspec-users mailing list
> rspec-us... at rubyforge.orghttp://rubyforge.org/mailman/listinfo/rspec-users



> What this test says is, 'If User#can_manage_samples? is working
> correctly, then the correct users will see the link.

That's one way to say it, but I'd rephrase it a bit.  The specs are
testing the following behavior: only users who are allowed to manage
samples get the "manage samples" link.  I don't think the helper, or
its specs, should have any knowledge of the logic that determines
whether or not a user is allowed to manage samples.  You've decoupled
your helper and its specs from this logic, so that this logic can be
implemented and tested separately, and the helper specs won't break if/
when you need to change the logic.  Your tests are less brittle now.

> What happens if when the app is in production and I've forgotten
> writing this test, I'm told that some user who should be authorized to
> is not seeing this link? The view test will still be passing because
> I've told the User object to return what I want it to. Is the idea
> that I should return to my integration tests and move back down
> through the stack to isolate the problem?

Hopefully you have integration tests, and moving down the stack is one
easy way to find the problem.  The fact that the helper spec passes,
even though the feature doesn't quite work right, doesn't bother me at
all--I view the helper spec as being an isolated unit test for the
helper.  And looking at the helper in isolation, it's still working
right.  The bug would be in the logic of User#can_manage_samples?, and
you would want to update the user spec to have a test for this new
case, and update the method appropriately.

It's good when a bug causes a test failure, but it's not good when a
bug in a single place causes multiple test failures.  I strive, as
much as possible, to design my code and test suite so that a bug or
logic change will cause at most 2 test failures: 1 failure in the
appropriate unit test (often a model spec, but it could be a
controller, view, helper or some other spec), and 1 failure in an
integration test.  Having lots of test failures would just slow me
down.

I don't really have a comment on your CanCan stuff--I haven't used it
myself, but I definitely intend to one of these days :).

Myron


More information about the rspec-users mailing list