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
--
Posted via http://www.ruby-forum.com/.
On Oct 20, 2008, at 1:00 AM, Harry Bishop wrote:> 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.Not necessarily. Have you looked into using a null_object mock? Scott
Scott Taylor wrote:> On Oct 20, 2008, at 1:00 AM, Harry Bishop 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? > > ScottYes, 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 6:53 AM, Harry Bishop <lists at ruby-forum.com> wrote:> Scott Taylor wrote: >> On Oct 20, 2008, at 1:00 AM, Harry Bishop 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?Can you please post the controller code?> > TIA, > HR > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
David Chelimsky wrote:> On Mon, Oct 20, 2008 at 6:53 AM, Harry Bishop <lists at ruby-forum.com> > 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 7:24 AM, Harry Bishop <lists at ruby-forum.com> wrote:> David Chelimsky wrote: >> On Mon, Oct 20, 2008 at 6:53 AM, Harry Bishop <lists at ruby-forum.com> >> 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 > endWhere is @current_user defined in the controller?> > HR > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
Harry Bishop wrote:> David Chelimsky wrote: >> On Mon, Oct 20, 2008 at 6:53 AM, Harry Bishop <lists at ruby-forum.com> >> 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 -- Posted via http://www.ruby-forum.com/.
@current_user is retrieved in the application controller with retrieve_user. -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 7:30 AM, Harry Bishop <lists at ruby-forum.com> 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?> -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
David Chelimsky wrote:> On Mon, Oct 20, 2008 at 7:30 AM, Harry Bishop <lists at ruby-forum.com> > 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 8:02 AM, Harry Bishop <lists at ruby-forum.com> wrote:> David Chelimsky wrote: >> On Mon, Oct 20, 2008 at 7:30 AM, Harry Bishop <lists at ruby-forum.com> >> 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]) > endretrieve_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> > 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 > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
David Chelimsky 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 -- Posted via http://www.ruby-forum.com/.
> David Chelimsky 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 8:29 AM, Harry Bishop <lists at ruby-forum.com> wrote:> David Chelimsky 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.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> > HR > > > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
David Chelimsky 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 10:09 AM, Harry Bishop <lists at ruby-forum.com> wrote:> David Chelimsky 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.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 Dennis http://www.continuousthinking.com http://www.mutuallyhuman.com
Zach Dennis 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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 11:34 AM, Harry Bishop <lists at ruby-forum.com> wrote:> Zach Dennis 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_userThat''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?> > HR > > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
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 -- Posted via http://www.ruby-forum.com/.
On Mon, Oct 20, 2008 at 12:01 PM, Harry Bishop <lists at ruby-forum.com> 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.> > Thanks, > HR > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
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 -- Posted via http://www.ruby-forum.com/.
On Tue, Oct 21, 2008 at 9:39 PM, Harry Bishop <lists at ruby-forum.com> wrote:> 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.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> HR
David Chelimsky wrote:> On Tue, Oct 21, 2008 at 9:39 PM, Harry Bishop <lists at ruby-forum.com> > 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, > DavidDavid, 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 -- Posted via http://www.ruby-forum.com/.