Dreading Controller Specs

Hi there,

I was look for a little advice really. I’ve been using RSpec for about 4
months now and I find it an absolute joy for model work and a really
nice tool it makes everything so much more readable and nicer to
organise

However, I seem to dread spec-ing out controllers, they end up being
quite untidy, I think maybe I am approaching them in the wrong way as it
probably shouldn’t be as hard as I am making it.

For example I had a problem over the past few days where I am creating a
way of logging in a customer to our site through a token. There is a
UserController, the customer does a get to the action token_login, we
authenticate the token, check a few other things and then set a session
variable to keep them logged in.

-----8<----------------
it “should find a single sign on” do
mock_user(:generate_security_token => ‘newtoken’)
ms = mock_model(MusicService, :users => [mock_user])
sso = mock_model(SingleSignOn, :music_service => ms)

SingleSignOn.should_receive(:find_by_secret_and_remote_host).with(@secret,
@remote_host).and_return(sso)

    ms.users.stub!(:find_by_email).and_return(mock_user)

    post :request_token, :secret => @secret, :email_address => 

@email
assigns(:single_sign_on).should equal(sso)
end
-----8<-----------------------

To be honest I feel very uncomfortable about the way this is arranged as
I am mocking several objects and their associations. I am trying to
stick to the “don’t touch the database” way of testing for controllers
and I feel that I am almost writing the implementation in the test just
by the way I am mocking / stubbing and therefore it makes the test
pointless as its just a confirmation of how I’ve written it.

Can anyone give me any ideas of how to do this properly?

How far do you go do you go with your mocking?

I almost am tempted to simplify the controller by using only the User
model and moving most of the checks out of the controller action
entirely and putting all into User, although that would mean that the
user model, single_sign_on and music_service would then be really
tightly coupled which wouldn’t be great either. I then don’t feel
comfortable because this approach would be as a reaction to making the
test simpler rather than making the controller code work which is the
whole point of the test.

Any help would be greatly appreciated. Cheers.

RobL

On 21 Oct 2008, at 10:45, Rob L. wrote:

I almost am tempted to simplify the controller by using only the
User model and moving most of the checks out of the controller
action entirely and putting all into User, although that would mean
that the user model, single_sign_on and music_service would then be
really tightly coupled which wouldn’t be great either. I

The “Rails Way” (assuming you are using Rails) is to make your
controllers do virtually nothing - find/create an object, call a
method on it, decide which view to render and that’s it. So, actually
what you suggest would be best.

However, rather than talking to your models directly, the controller
could talk to a “presenter” object, which does the “glue
work” (finding the associated models, calling the relevant methods in
the correct order and packaging up the results) - you can then RSpec
your presenter in the same way as you would a model.

This makes your controller specs (and implementations) trivial:

it “should find a single sign-on” do
@presenter = mock ‘SingleSignOnPresenter’
@presenter.should_receive(:request_token).with(secret,
email_address).and_return(:whatever)

post :request_token, :secret => ‘secret’, :email_address =>
'[email protected]

response.should redirect_to(some_path)
end

I actually use helpers (given_a_single_sign_on_presenter and
expect_to_request_a_token) instead of setting up the mocks and
expectations within the spec, just to make it a bit more readable.

Then you can RSpec your SingleSignOnPresenter separately, in much the
same way as you would spec a model, and keep the associations (and
implementation details) away from your controller.

Baz.

Rahoul B.
Web design and development: http://www.3hv.co.uk/
Nottingham Forest: http://www.eighteensixtyfive.co.uk/
Serious Rails Hosting: http://www.brightbox.co.uk/
Lifecast: http://www.madeofstone.net/

On 21 Oct 2008, at 13:08, Rahoul B. wrote:

the same way as you would spec a model, and keep the associations
(and implementation details) away from your controller.

Doesn’t this just end up shifting the ugly mocking code into the
Presenter specs though?

The stock answer to this question is to move this logic down into the
model layer, so that the interface the Presenter / Controller uses to
work with the database is simpler. This is what people call ‘listening
to your tests’ - if it’s hard to mock, it’s probably indicative of a
problem in your design.

However, I worry about this ‘skinny controller, fat model’ advice, it
still doesn’t feel like the final answer. To me, ActiveRecord classes
already have too many responsibilities, without making them also
orchestrate calls to other models.

I have been thinking about this a lot lately, and I am starting to
feel like I also need a Service layer between the Controller /
Presenters and the ‘Data Access Layer’ (Models) to orchestrate the work.

cheers,
Matt

On Tue, Oct 21, 2008 at 5:45 AM, Rob L.
[email protected] wrote:

However, I seem to dread spec-ing out controllers, they end up being quite
untidy, I think maybe I am approaching them in the wrong way as it probably
shouldn’t be as hard as I am making it.

For what it’s worth, Rob, I’m totally with you. The generated code
for controller specs has always felt…well, wrong to me. And it’s
been a frustration every time I’ve sat down to try to write new
controller specs the same way, taking many times longer than it takes
to write the controller.

The bad part has been the work required to set up the stubs and mocks
– for just the reason you cite. Conforming to the “single
expectation per test” pattern means I have to figure out and stub
every method that gets called and make it return a reasonable value,
and then I have to mock each call at least once to confirm that it
gets called. By this point I’ve essentially written the model
interface twice, which feels like extraordinary extra work – and it
also feels brittle. Reasonable changes in the model require
unreasonable maintenance in the controller stubs and mocks.

The only reason for all of this work is the principle of code
isolation. You’re supposed to make sure you’re only running the code
in the unit you’re testing – but because controllers sit at the heart
of your app, of course they’re going to have a great deal of
interaction with everything else. That isn’t wrong and it doesn’t
necessarily make the controllers too fat. But it makes the testing
fat. Just stubbing the relationship between model collections and
objects is complex. It also looks screwy – and it isn’t really a
test of the controller. But it has to be done if you’re not going to
talk to the model.

There are some cheats, of course. You can make blanket responses to
stuff you don’t feel like mocking. The null_object option to RSpec
mocks is such a cheat; so’s stub_everything in Mocha. But to me they
feel like copouts, and they return null by default, which is most
often the wrong behavior, so they don’t save work. Mock_model and
stub_model are intended for views, and stub_model doesn’t isolate you
from the model. There are some extension plugins that do some of the
mocking/stubbing “grunt work” for you, but that reduces transparency
and they don’t know about anything but the most common ‘formula’
methods.

The conclusion I’m starting to reach is that it often isn’t worth it.
All these hours of work to avoid talking with actual models… But if
you just plain used the models, so what? Yes, your controller specs
could fail if your model is buggy or unimplemented. But if your
model’s properly spec’ed you’ll get failures on the bug there too,
pinning the problem down, and the implementation constraint simply
means you can’t write models last. You’d implement methods on them
before or simultaneous with the controllers that call them. That’s
not a bad order of things.

So now I’m experimenting with live models. I’m using simple, basic
factory methods (I use fixture_replacement2) to create objects for my
controller specs to operate on, saved or unsaved as necessary, and
using Mocha to inject expectations into the actual objects for
specific examples. I may look at not_a_mock for that, too, now that
it’s being talked about here. This may not be as philosophically
pristine as total isolation, but it’s simpler and cleaner. You don’t
have to replicate fake model complexity it in the controller spec.
Most of the lines in the controller spec are once again about the
controller, not about setting up models. It’s also slower, but
only a bit. Even with autospec, waiting on my tests to begin is
already slow enough that I don’t feel it makes a huge difference. I’m
documenting my approach, too, and what I’ve been thinking and learning
about RSpec from a “thoughtful beginner” perspective, and hope to have
something I can post on that soon. (That documentation is, in fact,
one of the motivations for my current project.)

That’s my take. It’s working for me so far, but with the caveat that
I haven’t carried a project through to completion with this approach.
If any of the cognoscenti can offer reasons why this is a horribly
bad idea that’ll blow up in my face sooner or later, I’m open to being
convinced. I also offer my apologies if this is a topic that comes up
on this list over and over again, and if my little rant here is a
common and tired one.


Have Fun,
Steve E. ([email protected])
ESCAPE POD - The Science Fiction Podcast Magazine
http://www.escapepod.org

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 21 Oct 2008, at 16:51, Matt W. wrote:

it still doesn’t feel like the final answer. To me, ActiveRecord
classes already have too many responsibilities, without making them
also orchestrate calls to other models.

I have been thinking about this a lot lately, and I am starting to
feel like I also need a Service layer between the Controller /
Presenters and the ‘Data Access Layer’ (Models) to orchestrate the
work.

Personally I dislike the name “Presenter” - and much prefer Service,
Builder or Adapter depending upon what its doing; everyone else seems
to call it a presenter however.

But the point of the “Presenter/Service/Whatever” is precisely so that
neither the controller nor the models have to orchestrate the calls
between associated models. If you think of it that way then I think
it deals with your points above:

  • the presenter/service’s role is to coordinate the models - so its
    specs are purely about mocking the associations and the calls
    inbetween them
  • the presenter/service isn’t a model (not ActiveRecord::Base) - so
    it’s not adding extra responsibilities to the models
  • it is pretty much a service layer sat between controllers and models

Rahoul B.
Web design and development: http://www.3hv.co.uk/
Nottingham Forest: http://www.eighteensixtyfive.co.uk/
Serious Rails Hosting: http://www.brightbox.co.uk/
Lifecast: http://www.madeofstone.net/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkj+NDoACgkQu0BNRvjN8xRIsQCfQMkAVClEQOqPmdF9dPDm8Afq
o1sAnRF5gYkDI1qgfM8G2S+PpdLOUHaz
=fIkf
-----END PGP SIGNATURE-----

On Tue, Oct 21, 2008 at 3:57 PM, Rahoul B. [email protected]
wrote:

  • the presenter/service’s role is to coordinate the models - so its specs
    are purely about mocking the associations and the calls inbetween them
  • the presenter/service isn’t a model (not ActiveRecord::Base) - so it’s not
    adding extra responsibilities to the models
  • it is pretty much a service layer sat between controllers and models

There seems to be some overload on the word “Presenter” in Railsspeak.
As best I can tell, people are using it to refer to two or more
totally different patterns. Initially I thought presenters were for
encapsulating limited-domain controller and view logic in such a way
that they could be embedded in other controllers, thus allowing
composition in views. Things like sidebar widgets, or components of a
dashboard, or a reusable address box, etc. Stuff that benefits from
maintaining its own data so you can’t just a partial, but it’s still
more about the view than the model.

On Googling this post, however, I’ve found that some people do use it
that way, but other people talk about the Presenter pattern as a class
to aggregate data before sending it to (or split data after receiving
it from) the controller. I.e., what you’re saying. I think Jay
Fields confused the issue by talking about presenters in this way, but
saying you’re doing it for the sake of the view.

All of this has convinced me that the name “Presenter” sucks and
nobody should use it. My thinking this is unlikely to change
anything, but at least people ought to agree on what they are. What
you’re talking about, Rahoul, at least one or two people are calling a
“Conductor”:
http://blog.new-bamboo.co.uk/2007/8/31/presenters-conductors-on-rails

Personally I prefer handling it in the model, and letting models act
as compositions of other models if need be, but that’s just me.


Have Fun,
Steve E. ([email protected])
ESCAPE POD - The Science Fiction Podcast Magazine
http://www.escapepod.org

Matt W. [email protected] writes:

If we assume that the work to be done against the database or other
sub-systems is non-trivial, then we should not directly call the
persistence layer (= Models in Railsspeak) from the Controller, but
delegate that to another class. It seems like Service is a much more
appropriate name than Presenter here, since this work nothing to do
with presentation. I buy the thing about conductors, and I like the
new-bamboo guys, but I do wonder whether there’s some wheel
reinvention going on there.

I think any app with a rich domain model benefits from a service layer
that uses that model. When building Rails apps, stuff is relatively
simple and the controllers are the service layer. That’s how I think
of it, anyway. Most of the time that’s sufficient, but sometimes it
gets complex enough that you want to split out some of the logic.
Whenever I’ve had to do that, I’ve created some kind of Service object
to handle the coordination of domain objects, and I let the controller
handle all the HTTP shit and pass required info into the Service. Now,
I haven’t had to do that often - I think I’ve written like 5 or 6 of
these Service objects in the last 3 years of daily Rails work - but when
I have, I’ve been happy with the testability and maintenance benefits.
Separation of concerns ftw!

Pat

On Oct 22, 2008, at 4:32 PM, Pat M. wrote:

I think any app with a rich domain model benefits from a service layer
that uses that model. When building Rails apps, stuff is relatively
simple and the controllers are the service layer. That’s how I
think
of it, anyway. Most of the time that’s sufficient, but sometimes it
gets complex enough that you want to split out some of the logic.

Reflecting on this thread, I’m realizing I put a lot of these kind of
things into application.rb. Maybe I should think about how to make
them into objects in a service layer, but presently they’re doing
fine where they are: split out from the controller, and independently
testable. hmm…

On 21 Oct 2008, at 22:13, Stephen E. wrote:

  • it is pretty much a service layer sat between controllers and
    more about the view than the model.
    Right on - that was going to be my response too. Martin F.
    actually retired the ‘Model View Presenter’ pattern precisely because
    of this confusion.[1]

If we say that a Controller should be responsible for handling HTTP
requests, and co-ordinating the appropriate (persistence /
presentation / etc) work, that seems like enough responsibility.

If we assume that the work to be done against the database or other
sub-systems is non-trivial, then we should not directly call the
persistence layer (= Models in Railsspeak) from the Controller, but
delegate that to another class. It seems like Service is a much more
appropriate name than Presenter here, since this work nothing to do
with presentation. I buy the thing about conductors, and I like the
new-bamboo guys, but I do wonder whether there’s some wheel
reinvention going on there.

If we also assume that the data to be presented is non-trivial,
composed of data from multiple database tables, then we need some
object to model the data in this presentation domain. I think this is
where Presenter comes in, although I’m still not sure this is really
an appropriate name. This class can be facade over the various
persistence-layer objects needed for the specific presentation task.

On another project, in another language, in a galaxy far, far, away,
we sprouted a whole layer of POCO presenation ‘models’ like this, and
it felt like such a relief once we’d broken out this extra layer.

Does this make sense to anyone else?

cheers,
Matt

[1]Retirement note for Model View Presenter Pattern

Jonathan L. [email protected] writes:

things into application.rb. Maybe I should think about how to make
them into objects in a service layer, but presently they’re doing
fine where they are: split out from the controller, and independently
testable. hmm…

Okay, but I just want to restate that you usually don’t need that extra
layer. Like I said, I’ve done that maybe four or five times over the
past three years. Basically, if it seems to fit okay in the controller,
there’s no need to create a service layer. It’s really only when the
controller stuff gets awkward to test.

Pat