Forum: RSpec Where is current_user?

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Harry B. (Guest)
on 2008-10-20 09:00
Hi,
  I have a controller that uses the current_user attributes to determine
if a motion is showable.  I can see the motion track through from a
fixture to the show action, but not the current_user.  My rspec test is

describe MotionsController, "handling GET /motions/1" do
  fixtures :people, :motions

  before(:each) do
    @current_user = people(:someuser)
    @request.session[:user_id] = @current_user.id
    @motion = motions(:four)
    controller.stub!(:logged_in?).and_return(true)
    controller.stub!(:retrieve_user).and_return(@current_user)
  end

  it "should match motion and user" do
    showable = controller.is_showable?(@current_user, @motion)
    puts "..Motion is showable = #{showable}"  # is_showable? shows as
true.
  end

  def do_get
    get :show, :id => @motion.id
  end

  it "should be successful" do
    do_get
    response.should be_success
  end
end

In the MotionsController show action the motion is collected with the
params[:id] but the @current_user doesn't show.
The @current_user and @motion are supplied to is_showable? in the
controller for which they have attributes that are compared.
Any reason why @current_user isn't showing up in the show action?

If I take out the controller stubs in before(:each) then the redirect
from session shows as a 302 but the process doesn't proceed to the show
action.

A mock would require rebuilding the User model for all the checks done
in the controller and I am trying to avoid that by using a fixture
called from the database to test the true interaction between user and
motion.

Also, my protected qualifier on is_showable? works in the controller for
a browser initiated request by not with the rspec test.  I removed
protected to get the test done, but how is that supposed to be handled?

Thanks very much for your support,
HR
Scott T. (Guest)
on 2008-10-20 09:40
(Received via mailing list)
On Oct 20, 2008, at 1:00 AM, Harry B. wrote:

>  before(:each) do
> true.
> end
> action.
>
> A mock would require rebuilding the User model for all the checks done
> in the controller and I am trying to avoid that by using a fixture
> called from the database to test the true interaction between user and
> motion.

Not necessarily.  Have you looked into using a null_object mock?

Scott
Harry B. (Guest)
on 2008-10-20 15:53
Scott T. wrote:
> On Oct 20, 2008, at 1:00 AM, Harry B. wrote:
>
>>  before(:each) do
>> true.
>> end
>> action.
>>
>> A mock would require rebuilding the User model for all the checks done
>> in the controller and I am trying to avoid that by using a fixture
>> called from the database to test the true interaction between user and
>> motion.
>
> Not necessarily.  Have you looked into using a null_object mock?
>
> Scott

Yes, I tried null object but it doesn't get past is_showable? which says
it has a nil object passed in.
The idea is to use @current_user to determine if @motion is shown on the
view.  So how come @current_user isn't available in the controller?

TIA,
HR
David C. (Guest)
on 2008-10-20 16:00
(Received via mailing list)
On Mon, Oct 20, 2008 at 6:53 AM, Harry B. <removed_email_address@domain.invalid>
wrote:
>>> called from the database to test the true interaction between user and
>>> motion.
>>
>> Not necessarily.  Have you looked into using a null_object mock?
>>
>> Scott
>
> Yes, I tried null object but it doesn't get past is_showable? which says
> it has a nil object passed in.
> The idea is to use @current_user to determine if @motion is shown on the
> view.  So how come @current_user isn't available in the controller?

Can you please post the controller code?
Harry B. (Guest)
on 2008-10-20 16:24
David C. wrote:
> On Mon, Oct 20, 2008 at 6:53 AM, Harry B. <removed_email_address@domain.invalid>
> wrote:
>>>> called from the database to test the true interaction between user and
>>>> motion.
>>>
>>> Not necessarily.  Have you looked into using a null_object mock?
>>>
>>> Scott
>>
>> Yes, I tried null object but it doesn't get past is_showable? which says
>> it has a nil object passed in.
>> The idea is to use @current_user to determine if @motion is shown on the
>> view.  So how come @current_user isn't available in the controller?
>
> Can you please post the controller code?

Hi,
The controller show action is:

 def show
    @motion = Motion.find(params[:id])
    if is_showable?(@current_user, @motion)
      (@vote = @motion.votes.find_by_shortname(@current_user.shortname))
if is_votable?(@current_user, @motion)

      respond_to do |format|
        format.html # show.html.erb
        format.xml  { render :xml => @motion }
        end
    else
    flash[:error] = "You are not in that group."
    redirect_to motions_path
    end
  end

HR
David C. (Guest)
on 2008-10-20 16:27
(Received via mailing list)
On Mon, Oct 20, 2008 at 7:24 AM, Harry B. <removed_email_address@domain.invalid>
wrote:
>>> Yes, I tried null object but it doesn't get past is_showable? which says
>    @motion = Motion.find(params[:id])
>    redirect_to motions_path
>    end
>  end

Where is @current_user defined in the controller?
Harry B. (Guest)
on 2008-10-20 16:28
Harry B. wrote:
> David C. wrote:
>> On Mon, Oct 20, 2008 at 6:53 AM, Harry B. <removed_email_address@domain.invalid>
>> wrote:
>>>>> called from the database to test the true interaction between user and
>>>>> motion.
>>>>
>>>> Not necessarily.  Have you looked into using a null_object mock?
>>>>
>>>> Scott
>>>
>>> Yes, I tried null object but it doesn't get past is_showable? which says
>>> it has a nil object passed in.
>>> The idea is to use @current_user to determine if @motion is shown on the
>>> view.  So how come @current_user isn't available in the controller?
>>
>> Can you please post the controller code?
>
> Hi,
 The controller show action is:

  def show
     @motion = Motion.find(params[:id])
     if is_showable?(@current_user, @motion)
       (@vote =
@motion.votes.find_by_shortname(@current_user.shortname))
 if is_votable?(@current_user, @motion)

       respond_to do |format|
         format.html # show.html.erb
         format.xml  { render :xml => @motion }
         end
     else
     flash[:error] = "You are not in that group."
     redirect_to motions_path
     end
   end

 HR

and is_showable? here:

  def is_showable?(user, motion)
    user.group_member?( motion.group_name )
  end
Harry B. (Guest)
on 2008-10-20 16:30
@current_user is retrieved in the application controller with
retrieve_user.
David C. (Guest)
on 2008-10-20 16:37
(Received via mailing list)
On Mon, Oct 20, 2008 at 7:30 AM, Harry B. <removed_email_address@domain.invalid>
wrote:
> @current_user is retrieved in the application controller with
> retrieve_user.

I don't see where retrieve_user is getting called in the rspec example
code or in the show action. Maybe it's not actually getting called
anywhere?
Harry B. (Guest)
on 2008-10-20 17:02
David C. wrote:
> On Mon, Oct 20, 2008 at 7:30 AM, Harry B. <removed_email_address@domain.invalid>
> wrote:
>> @current_user is retrieved in the application controller with
>> retrieve_user.
>
> I don't see where retrieve_user is getting called in the rspec example
> code or in the show action. Maybe it's not actually getting called
> anywhere?

I guess that is what's happening, although I have this line in the
before(:each)

controller.stub!(:retrieve_user).and_return(@current_user)

and the show action has a :login_required which calls :logged_in?
since MotionsController is a subclass of ApplicationController doesn't
running the rspec test invoke the methods here:

class ApplicationController < ActionController::Base
  helper :all # include all helpers, all the time
  before_filter :retrieve_user

  protected

  def retrieve_user
    return unless session[:user_id]
    @current_user = Person.current_auth_record(session[:user_id])
  end

  def logged_in?
    @current_user.is_a?(Person)
  end
  helper_method :logged_in?

  def login_required
    return true if logged_in?
    session[:return_to] = request.request_uri
    redirect_to new_session_path and return false
  end

end
David C. (Guest)
on 2008-10-20 17:08
(Received via mailing list)
On Mon, Oct 20, 2008 at 8:02 AM, Harry B. <removed_email_address@domain.invalid>
wrote:
> I guess that is what's happening, although I have this line in the
>  before_filter :retrieve_user
>
>  protected
>
>  def retrieve_user
>    return unless session[:user_id]
>    @current_user = Person.current_auth_record(session[:user_id])
>  end

retrieve_user, the real method, sets an instance variable that other
methods expect to be set rather than returning a value. When this is
stubbed with a *return value* of the user, the instance variable never
gets set inside the controller.

I'd add a current_user method that returns @current_user, and then
stub *that* in the code examples:

# in ApplicationController

def current_user
  @current_user
end

# in MotionsController

def show
  ...
  if is_showable?(current_user, @motion)
  ...
end

# in example

controller.stub!(:current_user).and_return(@current_user)

HTH,
David
Harry B. (Guest)
on 2008-10-20 17:29
David C. wrote:
> retrieve_user, the real method, sets an instance variable that other
> methods expect to be set rather than returning a value. When this is
> stubbed with a *return value* of the user, the instance variable never
> gets set inside the controller.
>
> I'd add a current_user method that returns @current_user, and then
> stub *that* in the code examples:

 Hi David,
I understand your response now that its pointed out what is happening
between controller and rspec, however, this means changing my code to
test it.  This strikes me as backward. Isn't there another way to get
the @current_user "set" for use in the controller?

BTW - the actual code runs fine.

HR
Harry B. (Guest)
on 2008-10-20 17:39
> David C. wrote:

>>
>> I'd add a current_user method that returns @current_user, and then
>> stub *that* in the code examples:

  Hi David,

I found that I can stub out is_showable?(@current_user, @motion) and the
test passes.  I was trying to use the code logic to do this, but now see
that @current_user won't be seen by the show action as it is set in the
actual code.
Thanks for your help.
HR
David C. (Guest)
on 2008-10-20 17:52
(Received via mailing list)
On Mon, Oct 20, 2008 at 8:29 AM, Harry B. <removed_email_address@domain.invalid>
wrote:
> I understand your response now that its pointed out what is happening
> between controller and rspec, however, this means changing my code to
> test it.  This strikes me as backward.

When things are hard to test they are hard to maintain, so
maintainability requires testability. When a simple change makes
something easier to test, that change brings a lot of value. That's
the spirit of rspec, BDD and even TDD.

> Isn't there another way to get
> the @current_user "set" for use in the controller?

Sure, but it's fugly:

controller.instance_eval { @current_user = people(:someuser) }

> BTW - the actual code runs fine.

Since maintainability requires testability, just because it works
doesn't mean it's maintainable.

FWIW,
David
Harry B. (Guest)
on 2008-10-20 18:09
David C. wrote:

> When things are hard to test they are hard to maintain, so
> maintainability requires testability. When a simple change makes
> something easier to test, that change brings a lot of value. That's
> the spirit of rspec, BDD and even TDD.

Thanks David,
I am new to rspec and am enjoying the interchange between build a test
then build the code.  In this case I'm late to the party.  This
particular setup for @current_user makes sense to me but I understand
your point about maintainability.

In trying to understand what rspec is doing, my thinking was that since
MotionsController is a subclass of ApplicationController any instance
variable set in ApplicationController was available to
MotionsController. I think you are telling me that rspec doesn't invoke
this. So I need to think about the implications here.

Yes, I want the advantages of rspec and having maintainable code.

HR
Zach D. (Guest)
on 2008-10-20 18:44
(Received via mailing list)
On Mon, Oct 20, 2008 at 10:09 AM, Harry B. 
<removed_email_address@domain.invalid>
wrote:
> particular setup for @current_user makes sense to me but I understand
> your point about maintainability.
>
> In trying to understand what rspec is doing, my thinking was that since
> MotionsController is a subclass of ApplicationController any instance
> variable set in ApplicationController was available to
> MotionsController. I think you are telling me that rspec doesn't invoke
> this. So I need to think about the implications here.

The @current_user instance variable was being set by the
ApplicationController#retrieve_user method, but your spec was stubbing
out the #retrieve_user method. This means that the original
ApplicationController#retreieve_user method is not going to get called
because you have explicitly told RSpec to stub out that method and
return the current user from your spec. Since the original method is
not going to get called the @current_user instance variable in the
controller never gets set.


--
Zach D.
http://www.continuousthinking.com
http://www.mutuallyhuman.com
Harry B. (Guest)
on 2008-10-20 20:34
Zach D. wrote:

> The @current_user instance variable was being set by the
> ApplicationController#retrieve_user method, but your spec was stubbing
> out the #retrieve_user method. This means that the original
> ApplicationController#retreieve_user method is not going to get called
> because you have explicitly told RSpec to stub out that method and
> return the current user from your spec. Since the original method is
> not going to get called the @current_user instance variable in the
> controller never gets set.
>
Hi Zack,
I put the stub in to advance the process to the MotionsController
because without it the process hangs in ApplicationController with a
redirect to show.
My test log shows the 302 but show doesn't get called.  Is there some
way around that?

So far David's method of making retrieve_user return @current_user
and stubbing the protected method is_showable? works but doesn't
recognize the inheritance that should be there for @current_user.

HR
David C. (Guest)
on 2008-10-20 20:54
(Received via mailing list)
On Mon, Oct 20, 2008 at 11:34 AM, Harry B. 
<removed_email_address@domain.invalid>
wrote:
>>
> Hi Zack,
> I put the stub in to advance the process to the MotionsController
> because without it the process hangs in ApplicationController with a
> redirect to show.
> My test log shows the 302 but show doesn't get called.  Is there some
> way around that?
>
> So far David's method of making retrieve_user return @current_user

That's not what I recommended. I recommended adding a new method named
current_user that returns @current_user, so you can refer to just
current_user in method calls and stub that out if you want to control
its value from the code example.

> and stubbing the protected method is_showable? works but doesn't
> recognize the inheritance that should be there for @current_user.

Inheritance?
Harry B. (Guest)
on 2008-10-20 21:01
Hi David,

Yes, I understand.  I was trying to answer Zack with regard to my
attempt to set the @current_user and what happened.

I will be using your method as a test shortly.

Thanks,
HR
David C. (Guest)
on 2008-10-20 21:03
(Received via mailing list)
On Mon, Oct 20, 2008 at 12:01 PM, Harry B. 
<removed_email_address@domain.invalid>
wrote:
> Hi David,
>
> Yes, I understand.  I was trying to answer Zack with regard to my
> attempt to set the @current_user and what happened.
>
> I will be using your method as a test shortly.

Cool.
Harry B. (Guest)
on 2008-10-22 06:39
Thanks David,
Your method works well and rspec succeeds now.

Is the preferred way of using code with rspec to not rely on instance
variables set in a parent during execution but to rely on the method
only construct to be able to interact?

I see the way this works for the outline you provided.  I am wondering
what the implications are with respect to writing ruby code with the
assumption parent instance variables will be inherited by children.  The
approach you are providing would indicate that an attr_read method is
preferred so that the retrieve method can be stubbed since the instance
variable doesn't get set when tested.

Is this a general rule of practice with rspec?  As a beginner I'm trying
to get the larger picture here and appreciate your insight with this.

HR
David C. (Guest)
on 2008-10-22 16:06
(Received via mailing list)
On Tue, Oct 21, 2008 at 9:39 PM, Harry B. <removed_email_address@domain.invalid>
wrote:
> approach you are providing would indicate that an attr_read method is
> preferred so that the retrieve method can be stubbed since the instance
> variable doesn't get set when tested.
>
> Is this a general rule of practice with rspec?  As a beginner I'm trying
> to get the larger picture here and appreciate your insight with this.

This is more of a design thing than an rspec thing. While hierarchies
can be useful for sharing behaviour and the concept of class, sharing
state up and down a hierarchy can be quite confusing and error prone.

* Variable names are far more likely to change than method names. The
implication is that you're more likely to get bitten because somebody
else changed an instance variable name in another class in the
hierarchy (above OR below) than if a method name changes. People just
take greater care when changing method names.
* To understand that an instance variable is shared up and down a
hierarchy I have to look at the code, whereas to understand that an
instance method is overridden, I simply need look at the instance
methods, which I easily do in irb, or looking at rdoc.
* Overriding methods goes in one direction in a hierarchy, while
resetting instance variables can go in either direction and lead to
some nasty bugs.

And, of course, relying on methods instead of variables also makes it
easier to mock and stub :) For me, that is very important.

FWIW,
David
Harry B. (Guest)
on 2008-10-22 19:24
David C. wrote:
> On Tue, Oct 21, 2008 at 9:39 PM, Harry B. <removed_email_address@domain.invalid>
> wrote:
>> approach you are providing would indicate that an attr_read method is
>> preferred so that the retrieve method can be stubbed since the instance
>> variable doesn't get set when tested.
>>
>> Is this a general rule of practice with rspec?  As a beginner I'm trying
>> to get the larger picture here and appreciate your insight with this.
>
> This is more of a design thing than an rspec thing. While hierarchies
> can be useful for sharing behaviour and the concept of class, sharing
> state up and down a hierarchy can be quite confusing and error prone.
>
> * Variable names are far more likely to change than method names. The
> implication is that you're more likely to get bitten because somebody
> else changed an instance variable name in another class in the
> hierarchy (above OR below) than if a method name changes. People just
> take greater care when changing method names.
> * To understand that an instance variable is shared up and down a
> hierarchy I have to look at the code, whereas to understand that an
> instance method is overridden, I simply need look at the instance
> methods, which I easily do in irb, or looking at rdoc.
> * Overriding methods goes in one direction in a hierarchy, while
> resetting instance variables can go in either direction and lead to
> some nasty bugs.
>
> And, of course, relying on methods instead of variables also makes it
> easier to mock and stub :) For me, that is very important.
>
> FWIW,
> David

David,
  I agree with this philosophy.  Sometimes in using a language it isn't
always clear which approach will be a better payoff.  I appreciate your
advice here.
HR
This topic is locked and can not be replied to.