El 19/07/2010, a las 10:58, Matt Wynne escribió:

> On 18 Jul 2010, at 00:10, David Chelimsky wrote:
> 
>> On Jul 17, 2010, at 1:18 PM, Costa Shapiro wrote:
>> 
>>> Hello,
>>> 
>>> I've been thinking of how to express my idea in code, but since I've never 
>>> been involved in RSpec development, I'd better have some feedback here 
>>> first.
>>> The feature suggestion below applies to any controller-like code under 
>>> spec, i.e. a stateless module producing output or just altering its data 
>>> store (it doesn't necessarily have to be a C of the MVC, but I suppose 
>>> merb/rails developers will particularly appreciate it).
>>> 
>>> Here is a skimmed sample to illustrate the pain:
>>> 
>>>    describe BookController do
>>> 
>>>      context "registering a book" do
>>> 
>>>        specify "from a new author on a new subject" do
>>>          auth = mock(:name => 'John Doe')
>>>          Author.should_receive(:find_
>>> by_name).and_return(nil)
>>>          Author.should_receive(:new).and_return(auth)
>>>          auth.should_receive(:save).and_return(true)
>>> 
>>>          subj = mock(:short => 'Mockery')
>>>          Subject.should_receive(:find_by_short).and_return(nil)
>>>          Subject.should_receive(:new).and_return(subj)
>>>          subj.should_receive(:save).and_return(true)
>>> 
>>>          title = 'Specs on Steroids'
>>> 
>>>          book = mock
>>>          Book.should_receive(:new).and_return(book)
>>>          book.should_receive(:save).and_return(true)
>>> 
>>>          post :register :author => auth.name, :title => title, :subject => 
>>> subj.short
>>>          response.should be_success
>>>        end
>>> 
>>>        specify "from a known author on a new subject" do
>>>          auth = mock(:name => 'Joe Dohn')
>>>          Author.should_receive(:find_by_name).and_return(auth)
>>> 
>>>          subj = mock(:short => 'Mockery')
>>>          Subject.should_receive(:find_by_short).and_return(nil)
>>>          Subject.should_receive(:new).and_return(subj)
>>>          subj.should_receive(:save).and_return(true)
>>> 
>>>          title = 'Specs on Steroids II'
>>> 
>>>          book = mock
>>>          Book.should_receive(:new).and_return(book)
>>>          book.should_receive(:save).and_return(true)
>>> 
>>>          post :register :author => auth.name, :title => title, :subject => 
>>> subj.short
>>>          response.should be_success
>>>        end
>>> 
>>>        specify "from a known author on a known subject" do
>>>          auth = mock(:name => 'Joe Dohn')
>>>          Author.should_receive(:find_by_name).and_return(auth)
>>> 
>>>          subj = mock(:short => 'Forgery')
>>>          Subject.should_receive(:find_by_short).and_return(subj)
>>> 
>>>          #...
>>>        end
>>> 
>>>        specify "from a new author on a known subject" do
>>>          #...
>>>        end
>>>      end
>>>    end
>>> 
>>> 
>>> And this is what I have in mind for doing exactly the same job:
>>> 
>>>    describe BookController do
>>> 
>>>      context "registering a book" do
>>> 
>>>        before :any, "from a new author", :author do
>>>          @auth = mock(:name => 'John Doe')
>>>          Author.should_receive(:find_by_name).and_return(nil)
>>>          Author.should_receive(:new).and_return(@auth)
>>>          @auth.should_receive(:save).and_return(true)
>>>        end
>>> 
>>>        before :any, "from a known author", :author do
>>>          @auth = mock(:name => 'Joe Dohn')
>>>          Author.should_receive(:find_by_name).and_return(@auth)
>>>        end
>>> 
>>>        before :any, "on a new subject", :subject do
>>>          @subj = mock(:short => 'Mockery')
>>>          Subject.should_receive(:find_by_short).and_return(nil)
>>>          Subject.should_receive(:new).and_return(@subj)
>>>          @subj.should_receive(:save).and_return(true)
>>>        end
>>> 
>>>        before :any, "on a known subject", :subject do
>>>            @subj = mock(:name => 'Joe Dohn')
>>>            Subject.should_receive(:find_by_name).and_return(@subj)
>>>        end
>>> 
>>>        it "should succeed", :with => [:author, :subject] do
>>>          title = 'Specs on Steroids X'
>>> 
>>>          post :register :author => @auth.name, :title => title, :subject => 
>>> @subj.short
>>>          response.should be_success
>>>        end
>>>      end
>>>    end
>>> 
>>> A run of such specs will effectively multiply the tests — automatically — 
>>> choosing before and after blocks as specified.
>>> I'm sorry, I haven't thought the DSL through, but I hope the main idea can 
>>> be seen: contexts do not have to be hierarchical.
>>> In my opinion, adding some sort of context selection+combination 
>>> capabilities (AOP-style) will contribute greatly to the expressiveness of 
>>> the spec language.
>> 
>> I think the idea of mixing/matching sub-contexts is very interesting, but it 
>> definitely needs from fleshing out. It would have to be easy to 
>> read/understand in the spec file as well as the output.
>> 
>> Also, this only works if every combination should behave the same way. I 
>> think we'd need a means of saying "given these combinations of data, expect 
>> these outcomes".
>> 
>> Anybody else have thoughts on this?
> 
> It's a nice idea.
> 
> I'm not sure whether I'd use it though. I think this idea comes from the 
> desire to write specs that are *complete*, which I can perfectly understand 
> but I don't think I subscribe to anymore. I prefer to really craft the 
> examples so there's 'just enough' tests but no more than that. I'd be worried 
> this might offer a temptation to think less about why you're writing each 
> example, and I'd be worried how that would help me to do TDD.
> 
> It should be possible to do something like this using macros now, right? Can 
> I suggest that the OP has a go at refactoring his code using macros and we 
> can see how it looks?

I know that the posted code may be a contrived toy example for the purposes of 
illustration, but when I see a spec like that alarm bells start to ring. So 
much mocking, so many assertions in each example block etc. And it's not at all 
clear what the pertinent behavior is that you want to test here, because each 
example looks exactly like a one-to-one rewrite of the original implementation 
that uses mocks instead of real objects.

And when the alarm bells start to ring, before I think about changing my 
testing framework to make things easier, I look at the code under test to see 
if it could be changed to be more testable.

So we basically have a controller action that accepts three parameters (author, 
title, and subject), and it has a conditional code path for two of those:

  if thing.exists
    great
  else
    create it
  end

And in your spec you're wanting to test for all the different permutations of 
new author/existing author and new subject/existing subject.

First thing you could do to eliminate a lot of mocking is use something like 
"find_or_create_by_name" and "find_or_create_by_short". Then you only have to 
mock three calls (one for each parameter) and forget about the permutations 
entirely.

This is an example of pushing logic down into the model in order to make 
controllers simpler and more testable. If "find_or_create_by_*" doesn't do what 
you need it to, then create a model method which does.

You could go even further and create a "register" method on your Book class 
which accepted the three parameters of author, title, and subject, and did 
everything which you are currently doing in your controller in the model layer 
instead. Then your controller spec becomes ridiculously simple, can be tested 
with a single mock, and the rest of the logic now resides in a model, which is 
easily testable.

So whether or not the example was a toy example, the need for the any automatic 
permutation and spec generation in RSpec has disappeared. Let's imagine, 
however, that the need was still there. Would adding this kind of code to RSpec 
itself be a good idea?

I don't necessarily think so. Matt says you can probably do this right now by 
using macros. I don't actually know what he means by that, but I do know that 
there are cases where I sometimes want a bunch of nearly identical specs, and I 
generate them in code using enumeration or some other means; ie. dumb example:

   [:foo, :bar, :baz].do |thing|
     describe "#{thing} dimensions" do
       it 'has length' do
         thing.to_s.length.should > 0
       end
     end
   end

So like I said, if your tests are painful, I think the first port of call 
should be to look at how the code under test could be change. Good code isn't 
just code that works. It's also code that is readable, maintainable, any among 
many other things, testable. Adding support to RSpec to make it easier to test 
bad code doesn't seem the right thing to do.

Maybe you have another example that could illustrate how what you propose would 
be useful, but right now I don't really see the need for this kind of thing.

Cheers,
Wincent



_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

Reply via email to