Get to a different controller

pkoebbe · January 5, 2010, 6:45pm

I’m trying to implement a base controller that other controllers descend
from, and am having a bit of difficulty in testing the sole “feature” of
the base controller.

gist.github.com

https://gist.github.com/phillipkoebbe/269544

base_controller.rb

class Admin::BaseController < ApplicationController
	before_filter :require_admin
	
	protected
	
	def require_admin
		redirect_to error_path(403) unless current_user.is_administrator?
	end
end

base_controller_spec.rb

require 'spec_helper'

describe Admin::BaseController do
	should_be_an_instance_of Admin::BaseController
	should_descend_from ApplicationController

	context 'before_filter' do
		it 'should respond to :require_admin' do
			controller.should respond_to(:require_admin)
		end

This file has been truncated. show original

home_controller_spec.rb

describe Admin::HomeController do
	should_be_an_instance_of Admin::HomeController
	should_descend_from Admin::BaseController
	it_should_behave_like 'administrative controllers'
end

There are more than three files. show original

In the “not redirecting when user is an admin” context, I keep getting
an error that “no action responded to ”. Obviously,
there are no actions in this controller, nor should there be. I want to
hit an action in a descendant controller and make sure that it goes
through.

How do I do that?

Thanks,
Phillip

pkoebbe · January 5, 2010, 9:15pm

The spec has Admin::BaseController as the described type. So of course
it’s going to test against that. If you want to test a different class,
you need to describe that instead!

pkoebbe · January 5, 2010, 9:30pm

Subclass it in your spec with

class TestController < Admin::BaseController
def index
end
end

…then use the TestController in your tests for Admin::BaseController.

That might mean you’ll need to add special routing for TestController
which is annoying but can be done.

On 5 Jan 2010, at 17:36, Phillip K. wrote:

that it goes through.

How do I do that?

Thanks,
Phillip

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

+447974 430184

pkoebbe · January 5, 2010, 9:40pm

On Tue, Jan 5, 2010 at 1:46 PM, Matt W. [email protected] wrote:

is annoying but can be done.
From The RSpec Book:

http://media.pragprog.com/titles/achbd/code/rails_controllers/messages/15/spec/controllers/application_controller5_spec.rb

pkoebbe · January 5, 2010, 10:01pm

Pat M. wrote:

The spec has Admin::BaseController as the described type. So of course it’s going to test against that. If you want to test a different class, you need to describe that instead!

Hi Pat,

Right. But, I’m not really wanting to test a different class. My
intention is to put the require_admin in the base_controller and have
all the admin controllers descend from it, so I don’t have to duplicate
the before_filter. I was trying to be simple and make a request to a
controller that descends from base_controller, thinking that if it got
through and didn’t redirect to the error page, it worked. How would you
suggest I test that

Peace,
Phillip

pkoebbe · January 5, 2010, 10:42pm

On Tue, Jan 5, 2010 at 2:53 PM, Phillip K.
[email protected]wrote:

which is annoying but can be done.

Thanks, Matt. Between your post and David’s, I’ll see if I can put it all
together.

Phillip - one thing to keep in mind is that subclasses can always
override
super class behaviour. So on the one hand, we want to minimize
duplication
and only spec one thing once. On the other hand, if we’re spec’ing that
every controller should behave some way, we should spec every
controller.
I’m not advocating one approach or the other, just pointing out a couple
of
costs/benefits.

pkoebbe · January 5, 2010, 10:32pm

which is annoying but can be done.
Thanks, Matt. Between your post and David’s, I’ll see if I can put it
all together.

Peace,
Phillip

pkoebbe · January 6, 2010, 1:58am

El 05/01/2010, a las 21:52, Phillip K.
escribió:

duplicate the before_filter. I was trying to be simple and make a
request to a controller that descends from base_controller, thinking
that if it got through and didn’t redirect to the error page, it
worked. How would you suggest I test that

I test inherited stuff with shared behaviors. It might be something
you could use here.

Basically, I have a bunch of behavior in my ApplicationController, for
example, and in my spec/controllers/application_controller_spec.rb
file I have a bunch of blocks like this:

describe ApplicationController, ‘protected methods’, :shared =>
true do
…
end

describe ApplicationController, ‘parameter filtering’, :shared =>
true do
…
end

And I then group them all together in one more shared behavior:

describe ApplicationController, :shared => true do
it_should_behave_like ‘ApplicationController protected methods’
it_should_behave_like ‘ApplicationController parameter filtering’
…
end

And finally in all subclasses which inherit I can now just do:

describe ArticlesController do
it_should_behave_like ‘ApplicationController’
…
end

So the behavior inherited from the superclass is specified, but it is
also tested independently by the inheriting subclasses.

I imagine the same idea might be adaptable in some way for your use
case.

Cheers,
Wincent

pkoebbe · January 6, 2010, 4:28pm

Wincent C. wrote:

end
it_should_behave_like ‘ApplicationController parameter filtering’
So the behavior inherited from the superclass is specified, but it is
also tested independently by the inheriting subclasses.

I imagine the same idea might be adaptable in some way for your use case.

Cheers,
Wincent
Hi Wincent,

Thanks for the input. I like the idea of the shared behavior so it gets
tested at all points of execution. [And I really enjoyed David’s “Yay!”,
too. ] I am going to keep that in mind as I go forward. In this
particular case, my dilemma was how to test the behavior in the
base_controller to begin with. The problem stemmed from the fact that
there were no actions in the controller to call so I couldn’t do a get
on it. With Matt and David’s help, I have achieved what I was originally
after. The gist is updated if you are interested:

gist.github.com

https://gist.github.com/phillipkoebbe/269544

base_controller.rb

class Admin::BaseController < ApplicationController
	before_filter :require_admin
	
	protected
	
	def require_admin
		redirect_to error_path(403) unless current_user.is_administrator?
	end
end

base_controller_spec.rb

require 'spec_helper'

describe Admin::BaseController do
	should_be_an_instance_of Admin::BaseController
	should_descend_from ApplicationController

	context 'before_filter' do
		it 'should respond to :require_admin' do
			controller.should respond_to(:require_admin)
		end

This file has been truncated. show original

home_controller_spec.rb

describe Admin::HomeController do
	should_be_an_instance_of Admin::HomeController
	should_descend_from Admin::BaseController
	it_should_behave_like 'administrative controllers'
end

There are more than three files. show original

I’m not sure how to share the behavior now, though. As I look at the
context that implements the testing of the before_filter, sharing it
doesn’t seem to make sense since it creates a bogus derived class to do
the testing. If I share the behavior to real derived controllers, it
doesn’t seem to me that they will, in fact, be tested.

Do you have any thoughts on that?

Peace,
Phillip

pkoebbe · January 6, 2010, 2:03am

On Tue, Jan 5, 2010 at 6:44 PM, Wincent C. [email protected]
wrote:

Hi Pat,
could use here.
…
And finally in all subclasses which inherit I can now just do:

describe ArticlesController do
it_should_behave_like ‘ApplicationController’
…
end

So the behavior inherited from the superclass is specified, but it is also
tested independently by the inheriting subclasses.

Yay!

pkoebbe · January 7, 2010, 3:56am

Wincent C. wrote:

only respond to a subset; but even if they only respond to one (“show”
or “index”, say) you have enough of a common basis to test that the
require_admin before filter is actually hit and does what you think it
should (ie. you only need to hit “show” or “index”, there is no need
to test all actions).

Cheers,
Wincent

Are you basically saying that you wouldn’t worry about testing the
before_filter in the base_controller at all? Maybe part of the reason I
am not “getting this” is my lack of familiarity with shared behaviors.
If I share the behavior in the base_controller_spec, will it get tested
when I run that spec? If it won’t, then my concern is moot. If it will,
then I’m just as confused as I was before.

Peace,
Phillip

pkoebbe · January 7, 2010, 8:29am

On Wed, Jan 6, 2010 at 8:53 PM, Phillip K.
[email protected]wrote:

Are you basically saying that you wouldn’t worry about testing the
before_filter in the base_controller at all? Maybe part of the reason I am
not “getting this” is my lack of familiarity with shared behaviors. If I
share the behavior in the base_controller_spec, will it get tested when I
run that spec? If it won’t, then my concern is moot. If it will, then I’m
just as confused as I was before.

A shared example group is a lot like a module. It is only run when it is
invoked in a non-shared example group:

SPECS:

shared_examples_for “any controller” do
it “does something shared” { … }
end

describe SpecificController do
it_should_behave_like “any controller”
it “does something specific” { … }
end

OUTPUT:

SpecificController
does something shared
does something specific

Does that clear anything up?

Cheers,
David

pkoebbe · January 6, 2010, 11:25pm

El 06/01/2010, a las 16:17, Phillip K.
escribió:

describe ApplicationController, ‘protected methods’, :shared =>

…
Hi Wincent,

base_controller.rb · GitHub

I’m not sure how to share the behavior now, though. As I look at the
context that implements the testing of the before_filter, sharing it
doesn’t seem to make sense since it creates a bogus derived class to
do the testing. If I share the behavior to real derived controllers,
it doesn’t seem to me that they will, in fact, be tested.

Do you have any thoughts on that?

Well, there is more than one way to skin a cat, but the thing I like
about my proposed solution is that:

the specification of the behavior appears in the “describe” block
that corresponds to the controller where the behavior is implemented
but given that the implementing controller is an abstract one, you
actually test the behavior where it is actually exercised (ie. in the
subclasses)
you don’t need to make a fictional controller which isn’t actually
part of your application purely for testing purposes

Looking at the gist you pasted it looks like it could be very
straightforward to test, especially if you’re using a RESTful access
pattern.

Your admin controllers all inherit from your abstract base class, and
if they’re RESTful you know before you even start which actions
they’ll respond to (the usual index, new, create etc). Perhaps they
only respond to a subset; but even if they only respond to one (“show”
or “index”, say) you have enough of a common basis to test that the
require_admin before filter is actually hit and does what you think it
should (ie. you only need to hit “show” or “index”, there is no need
to test all actions).

Cheers,
Wincent

pkoebbe · January 7, 2010, 8:39am

El 07/01/2010, a las 03:53, Phillip K.
escribió:

in the subclasses)
they’ll respond to (the usual index, new, create etc). Perhaps they
Are you basically saying that you wouldn’t worry about testing the
before_filter in the base_controller at all?

Exactly. The base_controller is never directly instantiated and its
“behaviour” is only ever manifested in the context of its subclasses,
so you can test the behavior by testing that it exists in the
subclasses.

As you have seen, you can’t really test the abstract base_controller
directly itself because it has no actions.

Maybe part of the reason I am not “getting this” is my lack of
familiarity with shared behaviors. If I share the behavior in the
base_controller_spec, will it get tested when I run that spec? If it
won’t, then my concern is moot. If it will, then I’m just as
confused as I was before.

Shared behaviors are just a convenient way to define in behaviors once
in a single place, which will be exercised in multiple places, so that
you can keep things DRY.

If you run the file with the “describe foo :shared => true” block in
it, nothing will actually happen.

Your assertions get exercised only when you run other “describe”
blocks containing “it_should_behave_like …”

They can be useful not just in cases like this where you have an
abstract superclass. You can use them wherever you have a bunch of
common behavior across different classes.

Cheers,
Wincent

pkoebbe · January 7, 2010, 11:24am

2010/1/7 Phillip K. [email protected]

Are you basically saying that you wouldn’t worry about testing the
before_filter in the base_controller at all? Maybe part of the reason I am
not “getting this” is my lack of familiarity with shared behaviors. If I
share the behavior in the base_controller_spec, will it get tested when I
run that spec? If it won’t, then my concern is moot. If it will, then I’m
just as confused as I was before.

You are using your base controller like an Abstract Class. In classic OO
you
can’t test such classes because you can’t instantiate them. Things are a
little different here in Rails, but the parallel holds because you can’t
route this class. The classic way to test an abstract class is to
instantiate a concrete class from it and test that. This is what you’ve
done, created a concrete test class with a route. The only problem is
you
think the class is bogus, but its not, its just what you want, you can
see
that by how easy it is to spec. Once you’ve specified how this class
works
you should be able to share that specification using the techniques
Wincent
has shown. Saying that your other classes should behave like this
special
test class is perfectly fine.

All best

Andrew

pkoebbe · January 7, 2010, 11:11am

On 7 Jan 2010, at 07:22, Wincent C. wrote:

but given that the implementing controller is an abstract one,
Your admin controllers all inherit from your abstract base class,

directly itself because it has no actions.

If you run the file with the “describe foo :shared => true” block in
it, nothing will actually happen.

Your assertions get exercised only when you run other “describe”
blocks containing “it_should_behave_like …”

They can be useful not just in cases like this where you have an
abstract superclass. You can use them wherever you have a bunch of
common behavior across different classes.

OK I’m going to bite on this one.

Shared behaviours are indeed useful and a really nice idea. When I
first started working with RSpec I used them exactly like this - I
wrote no specs for abstract classes or modules, and wrote shared
behaviour specs which I mixed in to all the classes that used the
abstract class or module. I told myself: “this is much better - I’m
not being distracted by the implementation details (inheritance,
mixins) I’m just specifying the behaviour of the class”.

I think that method probably works great for small models, but I have
found that over time on the Songkick codebase, I’ve come to rather
dislike the shared behaviours. This is mainly, I think, because when I
get a failure, the error message doesn’t point me very clearly at
which class was actually being tested when the failure occurred. I
also think I resent them because I know that for a popular module, I
might be running the exact same specs over the exact same code several
times, for no purpose.

What I now prefer to do is keep the interface between a module (or
even an abstract class) and the class it’s mixed into quite clean, and
specify that re-usable unit in isolation, with an example using a
temporary class created in the test. I might have one or two specs in
the class which mixes in the module to prove that the module is mixed
in and working, but most of the specs for the behaviour of that module
itself will live alongside the module and run only once.

As David said earlier there are strengths and weaknesses to both
approaches so you have to find your own path here. I just wanted to
share my experience.

cheers,
Matt

+447974 430184

pkoebbe · January 7, 2010, 7:00pm

On Thu, Jan 7, 2010 at 5:09 AM, Matt W. [email protected] wrote:

corresponds to the controller where the behavior is implemented
pattern.
Wincent
directly itself because it has no actions.

OK I’m going to bite on this one.
I think that method probably works great for small models, but I have found
that over time on the Songkick codebase, I’ve come to rather dislike the
shared behaviours. This is mainly, I think, because when I get a failure,
the error message doesn’t point me very clearly at which class was actually
being tested when the failure occurred.

I don’t really see a correlation between size and usefulness of shared
behaviours either through rspec’s it_should_behave_like or a custom
macro. When you describe a class in a spec that class is prepended to
all of the example descriptions. For example:

describe “a bar”, :shared => true
it “should serve up tequila” { … }
end

describe Foo do
it_should_behave_like “a bar”
end

will produce:

Foo should serve up tequila

If the top-level describe for an object isn’t descriptive enough to
communicate what class/module/etc is being spec’ed then I see what
you’re saying as a problem because the backtrace of a failed shared
example won’t help either, since the backtrace stems from the file
that the shared example is stored in rather than the actual spec using
it (I wonder if there is a way around this).

I also think I resent them because I
know that for a popular module, I might be running the exact same specs over
the exact same code several times, for no purpose.

This doesn’t bother me a whole lot. While I don’t want to needlessly
run examples multiple times to ensure something is working, I like err
on the side of my objects are behaving correctly rather than an object
includes a module. One of my reasons for this is because in the past
I’ve unfortunately had to find the hard way classes that were buggy
because of module inclusion order and one module overwrote another
module’s method, yada yada yada. I have personally also gotten into a
groove for writing my own little shared macros and it makes it super
simple to build new classes which re-use that functionality and know
it actually works.

What I now prefer to do is keep the interface between a module (or even an
abstract class) and the class it’s mixed into quite clean, and specify that
re-usable unit in isolation, with an example using a temporary class created
in the test. I might have one or two specs in the class which mixes in the
module to prove that the module is mixed in and working, but most of the
specs for the behaviour of that module itself will live alongside the module
and run only once.

This is excellent advice regardless of using shared examples, although
it isn’t always practical (clean interfaces are always practical, what
isn’t is creating new specs with test classes). Some modules are
simply a grouping of certain behaviour that relies on other behaviour
existing on an object. Creating a new test class to put in a spec can
require a good amount of time and thinking based on the functionality
and where what you’re module’s functionality falls into the overall
chain of dependencies.

There are times to be disciplined and put in that time and thinking.
At other times, it’s important to not misplace a bunch of time and
energy if what you’re working on doesn’t warrant it. Unfortunately,
making knowing when to make good decisions often comes from making bad
ones. So as Corey H. would suggest, practice practice practice!

As David said earlier there are strengths and weaknesses to both approaches
so you have to find your own path here. I just wanted to share my
experience.

Ditto.

–
Zach D.
http://www.continuousthinking.com (personal)
http://www.mutuallyhuman.com (hire me)
http://ideafoundry.info/behavior-driven-development (first rate BDD
training)
@zachdennis (twitter)

pkoebbe · January 7, 2010, 4:09pm

To all involved,

Great discussion! Thank you so much for taking the time out of your
busy lives to contribute. Please believe me when I say that I
appreciate it very much!

Here is the latest version of my code:

gist.github.com

https://gist.github.com/phillipkoebbe/269544

base_controller.rb

class Admin::BaseController < ApplicationController
	before_filter :require_admin
	
	protected
	
	def require_admin
		redirect_to error_path(403) unless current_user.is_administrator?
	end
end

base_controller_spec.rb

require 'spec_helper'

describe Admin::BaseController do
	should_be_an_instance_of Admin::BaseController
	should_descend_from ApplicationController

	context 'before_filter' do
		it 'should respond to :require_admin' do
			controller.should respond_to(:require_admin)
		end

This file has been truncated. show original

home_controller_spec.rb

describe Admin::HomeController do
	should_be_an_instance_of Admin::HomeController
	should_descend_from Admin::BaseController
	it_should_behave_like 'administrative controllers'
end

There are more than three files. show original

I like the idea of shared examples (trying to use the right
terminology!). It took me a bit to realize that the shared examples
have to be required somewhere. [I didn’t see anything on rspec.info about it, but did not consult the RSpec Book. Maybe it’s there.] I
decided on the shared_controller_examples.rb file for now, but might
change it later. I have some further refactoring to do regarding the
chain of requiring files, but I’m pretty pleased at the moment.

I can go both ways on where to test implementation: at the source or
the execution. I worked on a Rails project last year that used Single
Table Inheritance, and there was quite a bit of functionality defined
in the base class with some additional functionality in the derived
classes. We opted to test the different parts where they were, not
where they would be exercised. It worked out well there. But in this
case, I am convinced that shared examples are the way to go. I do
not like creating classes in my specs just to run a test. That’s
akin to having routes or actions in my application that would be used
only during testing. I can’t bring myself to do it.

Peace,
Phillip