[rspec-users] Data-wise context combination for controller speccing

Wincent Colaiuta win at wincent.com
Mon Jul 19 06:38:41 EDT 2010


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





More information about the rspec-users mailing list