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

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 = %{Manage Samples}
helper.manage_samples_link.should == link
end
end
end

If the above doesn’t format well, here’s a gist:

welcome_helper.rb · GitHub

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.

On Sep 17, 12:48 pm, “Doug E.” [email protected] wrote:

return nil if current_user.blank?

welcome_helper_spec.rb

helper.manage_samples_link.should == link

I stubbed current_user, but I haven’t mocked the User object and its

trying to wrap my head around what rspec tests go where and how to
rspec-users mailing list
[email protected]://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] GitHub - ryanb/cancan: Authorization Gem for Ruby on Rails.

On Sep 18, 11:52 am, “Doug E.” [email protected] wrote:

I’m trying to understand BDD and proper testing technique. I’m testing
current_user.role?(:admin)
describe WelcomeHelper do
end
returning the link only for those the admin, submitter, and reviewer
(1) that I get the right link; and
Thanks.
The presence of 3 clauses in your if statement suggests to me that you

#can_manage_samples? is an appropriate way to do so. Your helper test

this:
end
link_to “Manage Samples”, manage_samples_path
it “should not return manage_samples_link for nil user” do

Here’s how I understand this: With a mocked User#can_manage_samples?
through the stack to isolate the problem?
following:
require ‘spec_helper’
it “should return manage_samples_link for authorized user” do

do stuff to Samples to be able to get to the Manage Samples page.
Again, thanks.

Doug


rspec-users mailing list
[email protected]://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

On Fri, Sep 17, 2010 at 16:48, Doug E. [email protected] wrote:

 current_user.role?(:reviewer) ||
 current_user.role?(:admin)

link_to “Manage Samples”, manage_samples_path
end
end
end

Your helper method depends on the user currently logged in, but
hardwires
itself to how you have implemented “current user”. I’d prefer to make
the
dependency explicit, not just because that makes checking the behavior
easier.

module WelcomeHelper
def manage_samples_link
manage_samples_link_for_user(current_user)
end
end

I think you’ll find stubbing the :user parameter here must easier than
stubbing Rails’ #current_user method.

On Sep 17, 9:16 pm, Myron M. [email protected] wrote:

should be shown. I’m using rails 2.3.8 and rspec version 1.3.0. My
end
it "should return manage samples link for user with role
If the above doesn’t format well, here’s a gist:

Doug
that into an appropriate method on your user model:

[1]GitHub - ryanb/cancan: Authorization Gem for Ruby on Rails.


rspec-users mailing list
[email protected]://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 = %{Manage Samples}
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