On my website each user can have the following roles:
1) Not logged in
2) Logged in
- active
- administrator
- sysadministrator
How would you write DRY specs to test each action of a controller?
Currently I am doing somethings that looks like:
--
describe ''a non admin is signed in'', :shared => true do
before(:each) do
@current_user = mock_model(User, :id => 1, :state =>
''active'')
controller.stub!(:current_user).and_return(@current_user)
@current_user.should_receive(:administrator?).and_return(false)
@current_user.should_receive(:sysadministrator?).and_return(false)
end
end
describe ''an administrator is signed in'', :shared => true
do
before(:each) do
@current_user = mock_model(User, :id => 1, :state =>
''administrator'')
controller.stub!(:current_user).and_return(@current_user)
@current_user.should_receive(:administrator?).and_return(true)
end
end
describe Admin::OrdersController, ''index'' do
describe "A non admin wants to have access" do
it_should_behave_like ''a non admin is signed in''
it "should redirect" do
get :index
response.should redirect_to(products_url)
end
end
describe "An admin wants to have access" do
it_should_behave_like ''an administrator is signed in''
it "should render index page" do
controller.should_receive(:select_date_initializer).with({},nil)
Order.should_not_receive(:admin_find_from_params)
get :index
end
it "should accept search params on index page" do
Order.should_receive(:admin_find_from_params).with(''test.host'',
{}, {})
get :index, :commit => ''Search''
end
end
end # describe Admin::OrdersController, ''index''
---
I didn''t test for the sysadministrator as it would force me to repeat
all the tests for administrator, basically a sysadministrator has
administrator rights and more. What is a better way of writing such
specs?
--
Posted via http://www.ruby-forum.com/.
On Thu, Nov 6, 2008 at 7:02 AM, Fernando Perez <lists at ruby-forum.com> wrote:> On my website each user can have the following roles: > > 1) Not logged in > > 2) Logged in > - active > - administrator > - sysadministrator > > > > How would you write DRY specs to test each action of a controller? > > Currently I am doing somethings that looks like: > -- > describe ''a non admin is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => ''active'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(false) > @current_user.should_receive(:sysadministrator?).and_return(false) > end > end > > describe ''an administrator is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => > ''administrator'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(true) > end > end > > describe Admin::OrdersController, ''index'' do > > describe "A non admin wants to have access" do > it_should_behave_like ''a non admin is signed in'' > > it "should redirect" do > get :index > response.should redirect_to(products_url) > end > end > > describe "An admin wants to have access" do > it_should_behave_like ''an administrator is signed in'' > > it "should render index page" do > controller.should_receive(:select_date_initializer).with({},nil) > Order.should_not_receive(:admin_find_from_params) > > get :index > end > > it "should accept search params on index page" do > Order.should_receive(:admin_find_from_params).with(''test.host'', > {}, {}) > > get :index, :commit => ''Search'' > end > end > > end # describe Admin::OrdersController, ''index'' > --- > > I didn''t test for the sysadministrator as it would force me to repeat > all the tests for administrator, basically a sysadministrator has > administrator rights and more. What is a better way of writing such > specs? > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >One thing I''ve been trying lately is to share shared spec (somewhat like inheritance). I too have many tests that a essentially duplicates of each other. So by factoring out commonalities and differences, I''d have something like module NonAdminSpec describe ''acting like a non-admin'', :shared => true before :each # log in as a non admin describe ''something that only applies to a NonAdmin'' do # end module -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20081106/30dd4777/attachment.html>
On Thu, Nov 6, 2008 at 10:02 AM, Fernando Perez <lists at ruby-forum.com> wrote:> On my website each user can have the following roles: > > 1) Not logged in > > 2) Logged in > - active > - administrator > - sysadministrator > > > > How would you write DRY specs to test each action of a controller? > > Currently I am doing somethings that looks like: > -- > describe ''a non admin is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => ''active'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(false) > @current_user.should_receive(:sysadministrator?).and_return(false) > end > end > > describe ''an administrator is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => > ''administrator'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(true) > end > end > > describe Admin::OrdersController, ''index'' do > > describe "A non admin wants to have access" do > it_should_behave_like ''a non admin is signed in'' > > it "should redirect" do > get :index > response.should redirect_to(products_url) > end > end > > describe "An admin wants to have access" do > it_should_behave_like ''an administrator is signed in'' > > it "should render index page" do > controller.should_receive(:select_date_initializer).with({},nil) > Order.should_not_receive(:admin_find_from_params) > > get :index > end > > it "should accept search params on index page" do > Order.should_receive(:admin_find_from_params).with(''test.host'', > {}, {}) > > get :index, :commit => ''Search'' > end > end > > end # describe Admin::OrdersController, ''index'' > --- > > I didn''t test for the sysadministrator as it would force me to repeat > all the tests for administrator, basically a sysadministrator has > administrator rights and more.And why wouldn''t you want to test that?> What is a better way of writing such specs?I''ve really moved away from shared example groups and started writing more targeted macros. So I might do something like this: def for_roles *roles roles.each do |role| before(:each) { login_as role } yield end end describe OrdersController do describe "GET index" do for_roles :admin, :sysadmin do |role| it "..." do ... end end for_roles :sysadmin do |role| it "..." do ... end end end describe "GET edit" do for_roles :admin, :sysadmin do |role| it "..." do ... end end for_roles :sysadmin do |role| it "..." do ... end end end end When you''re doing this sort of thing, it is crucial that you keep things organized so that individual actions can change independently as requirements change. This is the thing that most people fail to realize when they try to DRY things up. This scheme makes it easy when we decide to remove a privilege from admin but keep it in sysadmin. Just move that example (it "...") to the other for_roles block. WDYT?
Mark Wilden to rspec-users show details 7:46 AM (0 minutes ago) Reply - Show quoted text - On Thu, Nov 6, 2008 at 7:02 AM, Fernando Perez <lists at ruby-forum.com> wrote:> On my website each user can have the following roles: > > 1) Not logged in > > 2) Logged in > - active > - administrator > - sysadministrator > > > > How would you write DRY specs to test each action of a controller? > > Currently I am doing somethings that looks like: > -- > describe ''a non admin is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => ''active'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(false) > @current_user.should_receive(:sysadministrator?).and_return(false) > end > end > > describe ''an administrator is signed in'', :shared => true do > before(:each) do > @current_user = mock_model(User, :id => 1, :state => > ''administrator'') > controller.stub!(:current_user).and_return(@current_user) > @current_user.should_receive(:administrator?).and_return(true) > end > end > > describe Admin::OrdersController, ''index'' do > > describe "A non admin wants to have access" do > it_should_behave_like ''a non admin is signed in'' > > it "should redirect" do > get :index > response.should redirect_to(products_url) > end > end > > describe "An admin wants to have access" do > it_should_behave_like ''an administrator is signed in'' > > it "should render index page" do > controller.should_receive(:select_date_initializer).with({},nil) > Order.should_not_receive(:admin_find_from_params) > > get :index > end > > it "should accept search params on index page" do > Order.should_receive(:admin_find_from_params).with(''test.host'', > {}, {}) > > get :index, :commit => ''Search'' > end > end > > end # describe Admin::OrdersController, ''index'' > --- > > I didn''t test for the sysadministrator as it would force me to repeat > all the tests for administrator, basically a sysadministrator has > administrator rights and more. What is a better way of writing such > specs? > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >One thing I''ve been trying lately is to share shared spec (somewhat like inheritance). I too have many tests that a essentially duplicates of each other. So by factoring out commonalities and differences, I''d have something like // anybody_spec.rb module AnybodySpec describe ''anybody'', :shared => true end // nonadmin_spec.rb require ''anybody_spec'' describe ''a nonadmin'' include AnyBodySpec it_should_behave_like ''anybody'' before :each # login as a nonadmin # specs that apply to anyone, given the right setup end // admin_spec.rb require ''anybody_spec'' module AdminSpec include AnyBodySpec describe ''an admin'', :shared => true it_should_behave_like ''anybody'' # specs that apply to any admin, given the right setup end describe ''admin'' # specs that only apply to admins end // sysadmin_spec.rb require ''admin_spec'' module SysAdminSpec include AdminSpec describe ''a sysadmin'' it_should_behave_like ''an admin'' # specs that apply to any sysadmin, given the right setup end describe ''sysadmin'' # specs that only apply to sysadmins end This is all from memory, so it''s not complete. But the idea is that you can have shared specs behave_like other shared specs. before() is run from bottom to top, so you can set @instance variables to parameterize the specs, just making sure that an outer level spec doesn''t overwrite an inner level spec''s variables. I''m writing specs for a lot of different queries. There are three that group by date, week or month - they''re different in some ways and the same in a lot of others. Those three are grouped by time period, which is the same in some ways as grouping by an entity, and different from others. There are two or three other types of queries, too, each sharing some stuff and being completely different in other ways. Using nested shared examples like this has massively reduced the duplication in my spec code. And it''s also guaranteed that all the specs get run on all the code they apply to. Finally, it has suggested similar refactorings in the code under test. It seems to be working well for me. ///ark -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20081106/89c58b44/attachment-0001.html>
> And why wouldn''t you want to test that? >I want to test for it, it''s just that I don''t want to copy/paste spec like an idiot.> def for_roles *roles > roles.each do |role| > before(:each) { login_as role } > yield > end > end > > describe OrdersController do > describe "GET index" do > for_roles :admin, :sysadmin do |role| > it "..." do ... end > end > for_roles :sysadmin do |role| > it "..." do ... end > end > end> This scheme makes it easy when we decide to remove a privilege from > admin but keep it in sysadmin. Just move that example (it "...") to > the other for_roles block. > > WDYT?I like your idea very much, very clean, readable and maintainable. And thank you Mark, I will also use your idea to factor out commonalities in my specs. -- Posted via http://www.ruby-forum.com/.
Fernando Perez <lists at ruby-forum.com> writes:>> And why wouldn''t you want to test that? >> > I want to test for it, it''s just that I don''t want to copy/paste spec > like an idiot. > > >> def for_roles *roles >> roles.each do |role| >> before(:each) { login_as role } >> yield >> end >> end >> >> describe OrdersController do >> describe "GET index" do >> for_roles :admin, :sysadmin do |role| >> it "..." do ... end >> end >> for_roles :sysadmin do |role| >> it "..." do ... end >> end >> end > >> This scheme makes it easy when we decide to remove a privilege from >> admin but keep it in sysadmin. Just move that example (it "...") to >> the other for_roles block. >> >> WDYT? > > I like your idea very much, very clean, readable and maintainable. > > And thank you Mark, I will also use your idea to factor out > commonalities in my specs.A lot of the time when I''m faced with this issue - there''s a number of roles, each with a set of privileges - I find it easier to focus on the privileges rather than the roles themselves. This means creating domain-specific methods on the user object. #can_edit_orders?, #can_publish?, etc. Now when I''m specing at the controller level, I only have to deal with two cases - when the user has the privilege, and when he doesn''t. Then we write examples for all of the different roles at the model level. No duplication, no fuss. One thing I''ve noticed is that when I''m writing controller specs and there seems to be too much duplication, I can usually eliminate it by pushing some logic down the stack. In fancier terms, it means I need to encapsulate a domain concept that I''ve missed up to that point. Pat
On Thu, Nov 6, 2008 at 4:52 PM, Pat Maddox <pergesu at gmail.com> wrote:> Fernando Perez <lists at ruby-forum.com> writes: > >>> And why wouldn''t you want to test that? >>> >> I want to test for it, it''s just that I don''t want to copy/paste spec >> like an idiot. >> >> >>> def for_roles *roles >>> roles.each do |role| >>> before(:each) { login_as role } >>> yield >>> end >>> end >>> >>> describe OrdersController do >>> describe "GET index" do >>> for_roles :admin, :sysadmin do |role| >>> it "..." do ... end >>> end >>> for_roles :sysadmin do |role| >>> it "..." do ... end >>> end >>> end >> >>> This scheme makes it easy when we decide to remove a privilege from >>> admin but keep it in sysadmin. Just move that example (it "...") to >>> the other for_roles block. >>> >>> WDYT? >> >> I like your idea very much, very clean, readable and maintainable. >> >> And thank you Mark, I will also use your idea to factor out >> commonalities in my specs. > > A lot of the time when I''m faced with this issue - there''s a number of > roles, each with a set of privileges - I find it easier to focus on the > privileges rather than the roles themselves. This means creating > domain-specific methods on the user object. #can_edit_orders?, > #can_publish?, etc. Now when I''m specing at the controller level, I > only have to deal with two cases - when the user has the privilege, and > when he doesn''t. Then we write examples for all of the different roles > at the model level. No duplication, no fuss. > > One thing I''ve noticed is that when I''m writing controller specs and > there seems to be too much duplication, I can usually eliminate it by > pushing some logic down the stack. In fancier terms, it means I need to > encapsulate a domain concept that I''ve missed up to that point.+1 David> Pat
> I''ve really moved away from shared example groups and started writing > more targeted macros. So I might do something like this: > > def for_roles *roles > roles.each do |role| > before(:each) { login_as role } > yield > end > end > > describe OrdersController do > describe "GET index" do > for_roles :admin, :sysadmin do |role| > it "..." do ... end > end > for_roles :sysadmin do |role| > it "..." do ... end > end > end > > describe "GET edit" do > for_roles :admin, :sysadmin do |role| > it "..." do ... end > end > for_roles :sysadmin do |role| > it "..." do ... end > end > end > end >I can''t write my specs so that they work as expected. What does login_as look like? And where do you put this code? I am not sure mine (if working) gets initialized correctly. I think I have to write my specs from scratch. -- Posted via http://www.ruby-forum.com/.
On 2008-11-11, at 17:24, Fernando Perez wrote:>> I''ve really moved away from shared example groups and started writing >> more targeted macros. So I might do something like this: >> >> def for_roles *roles >> roles.each do |role| >> before(:each) { login_as role } >> yield >> end >> end >> >> describe OrdersController do >> describe "GET index" do >> for_roles :admin, :sysadmin do |role| >> it "..." do ... end >> end >> for_roles :sysadmin do |role| >> it "..." do ... end >> end >> end >> >> describe "GET edit" do >> for_roles :admin, :sysadmin do |role| >> it "..." do ... end >> end >> for_roles :sysadmin do |role| >> it "..." do ... end >> end >> end >> end >> > > I can''t write my specs so that they work as expected. What does > login_as > look like? And where do you put this code? I am not sure mine (if > working) gets initialized correctly. > > I think I have to write my specs from scratch.Hi Fernando. #login_as is a custom method that you need to write yourself. It should simply login as the given user, or a user with the given role. Its implementation will depend on which authentication and authorisation system you''re using. Cheers, Nick
Nick Hoffman wrote:> On 2008-11-11, at 17:24, Fernando Perez wrote: >>> describe OrdersController do >>> for_roles :admin, :sysadmin do |role| >> login_as >> look like? And where do you put this code? I am not sure mine (if >> working) gets initialized correctly. >> >> I think I have to write my specs from scratch. > > Hi Fernando. #login_as is a custom method that you need to write > yourself. It should simply login as the given user, or a user with the > given role. Its implementation will depend on which authentication and > authorisation system you''re using. > > Cheers, > NickI am using restful_authentication. I tried to look at their specs, but they are unreadable. So I am trying to throw together my own authentication mocker/stuber but with no luck. -- Posted via http://www.ruby-forum.com/.
> > I am using restful_authentication. I tried to look at their specs, but > they are unreadable. So I am trying to throw together my own > authentication mocker/stuber but with no luck.Just to clear things out, can you tell me which snippet is correct: 1) get :index response.should be_redirect 2) response.should be_redirect get :index It appears to me that 1) is correct, as 2) seems to be messing up all my specs. -- Posted via http://www.ruby-forum.com/.
Fernando Perez <lists at ruby-forum.com> writes:>> >> I am using restful_authentication. I tried to look at their specs, but >> they are unreadable. So I am trying to throw together my own >> authentication mocker/stuber but with no luck. > > Just to clear things out, can you tell me which snippet is correct: > > 1) > get :index > response.should be_redirect > > 2) > response.should be_redirect > get :index > > > > It appears to me that 1) is correct, as 2) seems to be messing up all my > specs.Yep, #1 Pat
On Nov 6, 2008, at 8:01 AM, David Chelimsky wrote:> I''ve really moved away from shared example groups and started writing > more targeted macros. So I might do something like this: > > def for_roles *roles > roles.each do |role| > before(:each) { login_as role } > yield > end > end > > describe OrdersController do > describe "GET index" do > for_roles :admin, :sysadmin do |role| > it "..." do ... end > end > for_roles :sysadmin do |role| > it "..." do ... end > end > endI was attempting to follow this example and discovered that before(scope, &block) is an alias for append_before which, as the method name indicates, appends the before block to the existing collection of before blocks. So, in your example code above it seems that login_as would get called for each role passed to for_roles before each example is executed. Since the before blocks are stored internally as an array, the last element of the array would win. Another example: require File.expand_path(File.dirname(__FILE__) + ''/../../spec_helper'') def for_roles *roles roles.each do |role| before(:each) do puts role.to_s end yield end end describe "DRY roles test" do describe "GET index" do for_roles :admin, :sysadmin do |role| it "..." do puts ''in example'' end end end end Running this sample spec from the command line generates this output: admin sysadmin in example .admin sysadmin in example . I noticed that there is a method called ''remove_after'' in before_and_after_hooks.rb but no corresponding remove_before. Looking at the code: def remove_after(scope, &block) after_each_parts.delete(block) end I noticed that this method doesn''t use the scope parameter at all and just attempts to delete the block from after_each_parts. So, I modified the method to honor the scope parameter and wrote a corresponding remove_before method: def remove_after(scope, &block) parts = after_parts_from_scope(scope) parts.delete(block) end def remove_before(scope, &block) parts = before_parts_from_scope(scope) parts.delete(block) end However, this doesn''t work as expected because if you do something like: before(:each) { login_as role } remove_before(:each) { login_as role } two different Proc objects are created by each method call so parts.delete(block) will always fail. The only way I could see around this was if there were versions of append_before and remove_before which took a Proc object as a parameter instead of converting the block so you could maintain a reference to it in the calling code, i.e. def append_before_proc(*args, block) scope, options = scope_and_options(*args) parts = before_parts_from_scope(scope) parts << block end def remove_before_proc(scope, block) parts = before_parts_from_scope(scope) parts.delete(block) end my_block = Proc.new { login_as role } append_before_proc(:each, my_block) remove_before_proc(:each, my_block) Which seems like a lot of monkey patching to get this working. So now (finally) my questions: 1) Is there a better way to do this? 2) The current version of remove_after seems broken. Should I report this as a bug? Thanks, -Jesse