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 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. IThe "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 at 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/
On 21 Oct 2008, at 13:08, Rahoul Baruah wrote:> > 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 > > 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.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 Lacey <robl at 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 at gmail.com) 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 Wynne wrote:> 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.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-----
On Tue, Oct 21, 2008 at 3:57 PM, Rahoul Baruah <baz at 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 modelsThere 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 Eley (sfeley at gmail.com) ESCAPE POD - The Science Fiction Podcast Magazine http://www.escapepod.org
On 21 Oct 2008, at 22:13, Stephen Eley wrote:> On Tue, Oct 21, 2008 at 3:57 PM, Rahoul Baruah <baz at 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.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
Matt Wynne <matt at 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
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...
Jonathan Linowes <jonathan at parkerhill.com> writes:> 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...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