Forum: RSpec Dreading Controller Specs

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.
05d8aae3736c731bcfc2c2a5c2a5f316?d=identicon&s=25 Rob Lacey (braindeaf)
on 2008-10-21 11:53
(Received via mailing list)
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
Afe1e6b75aace67db4b3ac064256b0f1?d=identicon&s=25 Rahoul Baruah (Guest)
on 2008-10-21 14:09
(Received via mailing list)
On 21 Oct 2008, at 10:45, Rob Lacey 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 =>
'billg@hotmail.com
'

  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 Baruah
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/
Cdf378de2284d8acf137122e541caa28?d=identicon&s=25 Matt Wynne (mattwynne)
on 2008-10-21 17:52
(Received via mailing list)
On 21 Oct 2008, at 13:08, Rahoul Baruah 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
994e42bda994be2cd1d791f18ee6d561?d=identicon&s=25 Stephen Eley (Guest)
on 2008-10-21 20:58
(Received via mailing list)
On Tue, Oct 21, 2008 at 5:45 AM, Rob Lacey
<robl@mail.pigdestroyer.co.uk> 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 Eley (sfeley@gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org
Afe1e6b75aace67db4b3ac064256b0f1?d=identicon&s=25 Rahoul Baruah (Guest)
on 2008-10-21 21:59
(Received via mailing list)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On 21 Oct 2008, at 16:51, Matt Wynne 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 Baruah
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-----
994e42bda994be2cd1d791f18ee6d561?d=identicon&s=25 Stephen Eley (Guest)
on 2008-10-21 23:14
(Received via mailing list)
On Tue, Oct 21, 2008 at 3:57 PM, Rahoul Baruah <baz@madeofstone.net>
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-...

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 Eley (sfeley@gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org
Cdf378de2284d8acf137122e541caa28?d=identicon&s=25 Matt Wynne (mattwynne)
on 2008-10-22 09:46
(Received via mailing list)
On 21 Oct 2008, at 22:13, Stephen Eley 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 Fowler
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]http://martinfowler.com/eaaDev/ModelViewPresenter.html
42172acdf3c6046f84d644cb0b94642c?d=identicon&s=25 Pat Maddox (pergesu)
on 2008-10-22 22:37
(Received via mailing list)
Matt Wynne <matt@mattwynne.net> 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
387fb00ef9d6d523d43018d9c81ab36b?d=identicon&s=25 Jonathan Linowes (Guest)
on 2008-10-29 04:43
(Received via mailing list)
On Oct 22, 2008, at 4:32 PM, Pat Maddox 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...
42172acdf3c6046f84d644cb0b94642c?d=identicon&s=25 Pat Maddox (pergesu)
on 2008-10-30 04:37
(Received via mailing list)
Jonathan Linowes <jonathan@parkerhill.com> 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
This topic is locked and can not be replied to.