Hi guys, I have a weird failing spec, for which I just cannot figure out the reason of failure. I''m now rewriting my controller specs based on the advice of David and Ashley, and I got stuck on this (see: [rspec- users] specing rescue, ensure and else blocks of an Exception). http://pastie.caboo.se/111221 I''ve tried everything, like stubbing out :update_attributes! , even tested it in real-life and it works like a charm: updates / creates new records nicely and everything else is working properly. Any ideas would be greatly appreciated, have a nice day and thanks, Andr?s -- Tarsoly Andr?s tarsolya at gmail.com -------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/rspec-users/attachments/20071026/76478bcc/attachment.html
On Oct 26, 2007, at 5:39 pm, Tarsoly Andr?s wrote:> http://pastie.caboo.se/111221 > > I''ve tried everything, like stubbing out :update_attributes! , even > tested it in real-life and it works like a charm: updates / creates > new records nicely and everything else is working properly.First, like David said earlier, you''re putting too much into one spec: it "should retrieve the Option instance identified by ID" do CalciumOption.should_receive(:find).with("1").and_return(@option) @option.should_receive(:update_attributes!).with (invalid_option_attributes).and_raise(ActiveRecord::RecordInvalid) post :update, :id => 1, :option => invalid_option_attributes end should really be it "should retrieve the Option instance identified by ID" do CalciumOption.should_receive(:find).with("1").and_return(@option) post :update, :id => 1, :option => invalid_option_attributes end it "should update the Option instance''s attributes" do @option.should_receive(:update_attributes!).with (invalid_option_attributes).and_raise(ActiveRecord::RecordInvalid) post :update, :id => 1, :option => invalid_option_attributes end Also I don''t think you need ''@option = '' in front of @option.update_attributes!, as that method call modifies the object in-place. But that''s a Rails issue. One other thing - you aren''t stubbing CalcumOption.find in ''Admin::OptionsController, "#update (POST) with valid params"''. Are you sure this isn''t loading real models? As for the actual problem you asked about... sorry but I am lost. I''ve looked over it about 5 times and everything I thought was wrong was me not paying attention. I can''t see ANY reason it should fail. Is it still not working? Ashley -- blog @ http://aviewfromafar.net/ linked-in @ http://www.linkedin.com/in/ashleymoran currently @ home
On 2007.10.26., at 21:28, Ashley Moran wrote:> > First, like David said earlier, you''re putting too much into one spec: > > Also I don''t think you need ''@option = '' in front of > @option.update_attributes!, as that method call modifies the object > in-place. But that''s a Rails issue. > > One other thing - you aren''t stubbing CalcumOption.find in > ''Admin::OptionsController, "#update (POST) with valid params"''. Are > you sure this isn''t loading real models?Yes, I was aware of this as I was just messing around, trying to figure out where could this failed specs could come from, I pastied the real stuff with a rake spec --trace output here: http://pastie.caboo.se/111379> > As for the actual problem you asked about... sorry but I am lost. > I''ve looked over it about 5 times and everything I thought was wrong > was me not paying attention. I can''t see ANY reason it should fail. > Is it still not working?In the link above, you can see, that the specs still fail. Looks like it has to do something with exception handling as only those examples fail which should raise errors (eg. doing :create! or :update_attributes! with invalid and/or missing attributes and specing :and_raise()) The test.log sample in the pastie shows unfinished requests in all the :update! specs, where the :update_attributes! is stubbed on the mock. However, one example at #create raises an Exception properly, which can be seen in the log too, only where I''m using :and_raise(). I believe I stubbed everything out properly in the above example. I''m using edge rspec with the latest rspec_on_rails and using autotest for convenient coding. The problem is, I''m rather new to rspec / ruby / rails, so I can''t just start to track this down, however I went into the rspec source already, but got lost pretty much :) Thanks, Andr?s -- Tarsoly Andr?s tarsolya at gmail.com
On 10/26/07, Tarsoly Andr?s <tarsolya at gmail.com> wrote:> > On 2007.10.26., at 21:28, Ashley Moran wrote: > > > > > First, like David said earlier, you''re putting too much into one spec: > > > > Also I don''t think you need ''@option = '' in front of > > @option.update_attributes!, as that method call modifies the object > > in-place. But that''s a Rails issue. > > > > One other thing - you aren''t stubbing CalcumOption.find in > > ''Admin::OptionsController, "#update (POST) with valid params"''. Are > > you sure this isn''t loading real models? > > Yes, I was aware of this as I was just messing around, trying to > figure out where could this failed specs could come from, I pastied > the real stuff with a rake spec --trace output here: > > http://pastie.caboo.se/111379 > > > > > As for the actual problem you asked about... sorry but I am lost. > > I''ve looked over it about 5 times and everything I thought was wrong > > was me not paying attention. I can''t see ANY reason it should fail. > > Is it still not working? > > In the link above, you can see, that the specs still fail. Looks like > it has to do something with exception handling as only those examples > fail which should raise errors (eg. doing :create! > or :update_attributes! with invalid and/or missing attributes and > specing :and_raise()) > > The test.log sample in the pastie shows unfinished requests in all > the :update! specs, where the :update_attributes! is stubbed on the > mock. > > However, one example at #create raises an Exception properly, which > can be seen in the log too, only where I''m using :and_raise(). > > I believe I stubbed everything out properly in the above example. I''m > using edge rspec with the latest rspec_on_rails and using autotest > for convenient coding. > > The problem is, I''m rather new to rspec / ruby / rails, so I can''t > just start to track this down, however I went into the rspec source > already, but got lost pretty much :)wrong number of arguments (0 for 1) That''s coming from ActiveRecord::RecordInvalid, which takes an argument in its constructor. I think I left this out in my example to you earlier. If you pass raise_error a class, it instantiates in order to throw an instance of it. To solve your problem, you just need to create an instance and pass it the mock_model: and_raise(ActiveRecord::RecordInvalid.new(@option)) Sorry if I led you astray on that one. Cheers, David
On 2007.10.27., at 7:18, David Chelimsky wrote:> and_raise(ActiveRecord::RecordInvalid.new(@option)) > > Sorry if I led you astray on that one. >No problems, actually it makes perfect sense, since you want @option to have all the proper error messages and such. After adjusting the specs like this one, it was all working fine and dandy as soon as I stubbed the .errors.full_messages() call. This raises up a question in me about mocking or using real-life models, because in some of my specs the setups getting extremely bulky. But this is for another topic :) Cheers, Andr?s -- Tarsoly Andr?s tarsolya at gmail.com
On Oct 27, 2007, at 9:45 am, Tarsoly Andr?s wrote:> This raises up a question in me about mocking or using real-life > models, because in some of my specs the setups getting extremely > bulky. But this is for another topic :)Well while you have everyone''s attention :) Often you can factor out the setup code - if you search through the archives of this list there''s plenty of examples. But don''t worry about having long setups - I''ve written some setups that aren''t much shorter than the specs that follow. Write whatever setup you need to enable you to write solid specs, then worry after about making it look nice. I would say you should ALWAYS mock your models in your controllers specs. Otherwise you will further complicate your setups, make your specs run unacceptably slow, and you risk coupling your controllers and models too tightly. Many people in the BDD community (and at a guess I am assuming David is among them) would frown on using database access in your *model* code, because persistence is a separate issue to business logic. What you want is a set of specs for each of model (ideally, database persistence and business logic separately), controller, and view, and then an integration test, such as an RSpec story, that tests the full stack. Without this integration step, you will miss errors when you change the interface of a model. What you do not want is the Rails way of testing, where each layer of testing re-tests everything below it. (I recently saw a blog post where someone asked for comments on how people test views in isolation - it never occurred to me that some Rails developers still see that as an issue!) Having thought about it, you could argue Rail has no *unit* testing support built in at all. Ashley -- blog @ http://aviewfromafar.net/ linked-in @ http://www.linkedin.com/in/ashleymoran currently @ home
On 10/27/07, Ashley Moran <work at ashleymoran.me.uk> wrote:> On Oct 27, 2007, at 9:45 am, Tarsoly Andr?s wrote: > > > This raises up a question in me about mocking or using real-life > > models, because in some of my specs the setups getting extremely > > bulky. But this is for another topic :)Bulky setup is a design smell. That doesn''t mean it never happens, but when it does you should ask yourself what you can do to improve the design. If you''re not familiar with the Law of Demeter, search around for it and give it some thought.> Well while you have everyone''s attention :) > > Often you can factor out the setup code - if you search through the > archives of this list there''s plenty of examples. But don''t worry > about having long setups - I''ve written some setups that aren''t much > shorter than the specs that follow. Write whatever setup you need to > enable you to write solid specs, then worry after about making it > look nice.In spite of what I said earlier, this is true. For specs, coming from TDD, "the way" is to get from red to green and then refactor. The refactoring step should be considered every time you get to green. Where is duplication? How can I improve the design to eliminate it? You won''t always actually do anything, but the thought process is very important. It''s like a 24/7 design review.> I would say you should ALWAYS mock your models in your controllers > specs. Otherwise you will further complicate your setups, make your > specs run unacceptably slow, and you risk coupling your controllers > and models too tightly. Many people in the BDD community (and at a > guess I am assuming David is among them) would frown on using > database access in your *model* code, because persistence is a > separate issue to business logic.If I were writing the framework, this would be true. But Rails provides efficiencies in exchange for blending these concepts in to one. It''s a trade off, but I''ve come to appreciate the efficiencies enough where I live with it.> What you want is a set of specs for each of model (ideally, database > persistence and business logic separatelyDo you mean like this? spec/models-persistence spec/models-business_logic Or do you just mean separating the concerns within a single spec document?> ), controller, and view, and > then an integration test, such as an RSpec story, that tests the full > stack. Without this integration step, you will miss errors when you > change the interface of a model. What you do not want is the Rails > way of testing, where each layer of testing re-tests everything below > it. (I recently saw a blog post where someone asked for comments on > how people test views in isolation - it never occurred to me that > some Rails developers still see that as an issue!) > > Having thought about it, you could argue Rail has no *unit* testing > support built in at all.Absolutely agree. You have to go some distance to do any real unit testing in Rails. But, on the flip side, you get so much for free that you get from other more highly-decoupled framework that it might be worth the trade-off. There''s a balance in there somewhere ... let me know when you find it :) Cheers, David
On Oct 27, 2007, at 3:36 pm, David Chelimsky wrote:> Bulky setup is a design smell. That doesn''t mean it never happens, but > when it does you should ask yourself what you can do to improve the > design. If you''re not familiar with the Law of Demeter, search around > for it and give it some thought.I don''t think I have any LoD violations in my long setups. It''s just the sheer number of things that some pages need to load. It''s actually a while since I looked over the code I am referring to. It may actually just be view specs I am thinking of, where bulky setups are just a sign of the number of different things being displayed. But either way, I''ll be vigilant when I return to them.> In spite of what I said earlier, this is true. For specs, coming from > TDD, "the way" is to get from red to green and then refactor. The > refactoring step should be considered every time you get to green. > Where is duplication? How can I improve the design to eliminate it? > You won''t always actually do anything, but the thought process is very > important. It''s like a 24/7 design review.I just watched this today: <http://www.infoq.com/presentations/ applying-agile-to-ruby> Nothing revolutionary if you''ve been doing BDD in Ruby a while, but one of the things he drives home is the idea that in Ruby (and dynamic languages in general), you must have the shortest possible cycles, and must always work to make sure quality of the code is maintained as high as you can manage. It''s puzzling looking at developers who don''t do BDD, who think that refactoring is something you do when adding a new feature would cause the collapse of half the application. I don''t know how they live with (a) the stress of leaving a mess behind them and (b) the impending nervous breakdown when they actually have to fix it (with nothing to tell them it still works!) I dread to look at some of the code that is developed with none of the principles you describe above. Hell, mine is bad enough sometimes and I make the effort.>> Many people in the BDD community (and at a >> guess I am assuming David is among them) would frown on using >> database access in your *model* code, because persistence is a >> separate issue to business logic. > > If I were writing the framework, this would be true. But Rails > provides efficiencies in exchange for blending these concepts in to > one. It''s a trade off, but I''ve come to appreciate the efficiencies > enough where I live with it.You''ve become unusually forgiving lately :)>> What you want is a set of specs for each of model (ideally, database >> persistence and business logic separately > > Do you mean like this? > > spec/models-persistence > spec/models-business_logic > > Or do you just mean separating the concerns within a single spec > document?I''d never thought about splitting them into directories like that. Currently I just make sure that associations have a separate spec block to the others, something like describe MyModel do # ... end describe MyModel, "associations" do # ... end There''s probably not so much to do with ActiveRecord, because your models HAVE to map directly to tables. But I still test separately the persistence of any custom methods that build object graphs.> Absolutely agree. You have to go some distance to do any real unit > testing in Rails. But, on the flip side, you get so much for free that > you get from other more highly-decoupled framework that it might be > worth the trade-off. There''s a balance in there somewhere ... let me > know when you find it :)I still believe that Ruby is powerful enough that we can build highly- decoupled frameworks that collapse down to the simple cases by default. Something that has Rails'' beauty on the inside as well as the outside. I''ll let you know when I find *that*! Ashley -- blog @ http://aviewfromafar.net/ linked-in @ http://www.linkedin.com/in/ashleymoran currently @ home