rspec users - Jan 2010 - Message expectation that verifies result of passed block

Hey speclers,

My spec-fu is failing me on a message expectation in which I would like to
verify that the block passed to a certain method yields the proper
value.  I would like to be able to say something like:

def bar
  # .. some code
  foo do
    ''bar'' # want to verify this value
  end
end

describe ''bar'' do
  it ''calls foo with proper block'' do
    should_receive(:foo).with(block_yielding(''bar''))
  end
end

The problem I''ve been banging my head against is the fact that I
can''t
seem to capture the passed block using this notation:

  should_receive(:foo) { |*args, &block|
    yield.should == ''bar''      # breaks
    block.call.should == ''bar'' # no dice
  }

I believe the lack of ability to use this notation comes down to a ruby
limitation, but I''m not sure.  If that''s the case, then we
would need a
specific argument expectation (along the lines of my suggestion) that
executes in a context in which it can call the block.

Am I missing something obvious here, or does rspec currently not allow
for this easily?  If not, let''s discuss how it should look and I can
work on adding feature.

Cheers,

Paul

On Jan 28, 2010, at 1:29 pm, Paul Hinze wrote:
> I believe the lack of ability to use this notation comes down to a ruby
> limitation, but I''m not sure.  If that''s the case, then
we would need a
> specific argument expectation (along the lines of my suggestion) that
> executes in a context in which it can call the block.
I can''t find a solution, I suspect Ruby 1.8 can''t do this, but
I''m guessing.

Can I ask why you want to do this though?  As another example, it would be
unusual to spec something like:

  @array = [1, 2, 3]
  @array.should_receive(:map).with(block_that_doubles_values)

You''d instead check that the array that came out was [2, 4, 6].

Ashley

-- 
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

Ashley Moran <ashley.moran at patchspace.co.uk> on 2010-01-28 at
13:28:
> 
> On Jan 28, 2010, at 1:29 pm, Paul Hinze wrote:
> 
> > I believe the lack of ability to use this notation comes down to a
ruby
> > limitation, but I''m not sure.  If that''s the case,
then we would need a
> > specific argument expectation (along the lines of my suggestion) that
> > executes in a context in which it can call the block.
> 
> I can''t find a solution, I suspect Ruby 1.8 can''t do
this, but I''m guessing.
> 
> Can I ask why you want to do this though?  As another example, it
> would be unusual to spec something like:
> 
>   @array = [1, 2, 3]
>   @array.should_receive(:map).with(block_that_doubles_values)
> 
> You''d instead check that the array that came out was [2, 4, 6].
I''m trying to spec a large set of what essentially come down to
decorator methods in a Rails FormBuilder extension plugin.  What this
boils down to is methods that wrap rails FormBuilder methods, so
`f.text(*args)` ends up calling `f.text_field(*args)` to generate an
<input> tag, but only after it does its own logic and wrapping, which
among a bunch of other things wraps the output in an <li>.

So the methods run the gamut in complexity from ''f.radio'' to
''f.dependent_collection'' to ''f.sigma'', but
much of the common code is
wrapped up in a method called ''f.question'', which does the
outer <li>
wrapping, required field detection, label and error display, and a few
other common things required by every control we use in our forms.

So most of our methods have this basic structure:

  class OurFormBuilder < ActionView::Helpers::FormBuilder
    def foo_text(method, options={})
      foo_option = options.delete(:foo_option)
      options[:value] ||= ''FOO''

      # some logic, using foo_option somewhere... 

      question(method, options) do |remaining_options|
        ''FOO -->'' + text_field(method, remaining_options) +
''<-- FOO''
      end
    end
  end

I started out with a nice spec for `question`''s behavior and made it
all
in a shared group, but because of the number of examples just for
question and the number of methods that call it (so both performance and
complexity), I''m thinking about switching to message expectations in
all
of my `foo_text`-style method specs:

  describe ''foo_text''
    it ''calls text field with the proper options'' do
      @builder.should_receive(:text_field).with(:some_method, :proper_args)
      @builder.foo_text(:some_method)
    end
    it ''yields a wrapped text_field into question'' do
      # dont test rails text_field
      text_field_return = "BOOGA"
      @builder.stub!(:text_field).and_return(text_field_return)

      expected = "FOO -->" + text_field_return + "<--
FOO"
     
@builder.should_receive(:question).with(:some_method).with_a_block_yielding(expected)

      @builder.foo_text(:some_method, :some => options)
    end
    it ''properly returns the result of the call to question''
do
      @builder.stub!(:question).and_return(''BOOGA'')
      xhtml = @builder.foo_text(:some_method)
      xhtml.should == ''BOOGA''
    end
  end

I''d appreciate any feedback that folks might be willing to give.
Particularly I realize the following:

 (a) This might be testing implementation too much (possible)
 (b) The architecture of the whole plugin needs a serious refactor to
     increase modularity and decouple the components (very likely); a
     dash of decent OO design could really help this whole situation,
     and it''s something I''m planning on tackling down the road
 
For now, I''m just trying to push things in the right direction, and I
_think_ the .with_a_block_yielding(value) or .with(block_yielding(value))
argument verification would help me do that.

Thanks for your time!

Paul

You''re definitely testing too much implementation and not enough
behavior.

Basically, what you want to spec, is that provided some options, when
you call a certain method of your form builder, you get a certain html
output. At least that''s how I would approach the problem.

So I would have something like this:

it "produces a correctly formatted FOO input" do
  html = @builder.foo_text(...)
  html.should == "<label
for=''foo''>FOO</label> <input
id=''foo''....>"
end

Since testing generated HTML like that sucks, I would proceed to use
something like http://github.com/fnando/rspec-hpricot-matchers, and
say

it "produces a correctly formatted FOO input" do
  html = @builder.foo_text(...)
  html.should have_tag("label", :for => "foo")
  html.should have_tag("input", :id => "foo")
end

Or something like that. But then if you stop using that
question(*args, &block) method, and refactor to a different
implementation, specs should continue to pass.

-foca

On Thu, Jan 28, 2010 at 7:14 PM, Paul Hinze <paul.t.hinze at gmail.com>
wrote:
> Ashley Moran <ashley.moran at patchspace.co.uk> on 2010-01-28 at
13:28:
>>
>> On Jan 28, 2010, at 1:29 pm, Paul Hinze wrote:
>>
>> > I believe the lack of ability to use this notation comes down to a
ruby
>> > limitation, but I''m not sure. ?If that''s the
case, then we would need a
>> > specific argument expectation (along the lines of my suggestion)
that
>> > executes in a context in which it can call the block.
>>
>> I can''t find a solution, I suspect Ruby 1.8 can''t do
this, but I''m guessing.
>>
>> Can I ask why you want to do this though? ?As another example, it
>> would be unusual to spec something like:
>>
>> ? @array = [1, 2, 3]
>> ? @array.should_receive(:map).with(block_that_doubles_values)
>>
>> You''d instead check that the array that came out was [2, 4,
6].
>
> I''m trying to spec a large set of what essentially come down to
> decorator methods in a Rails FormBuilder extension plugin. ?What this
> boils down to is methods that wrap rails FormBuilder methods, so
> `f.text(*args)` ends up calling `f.text_field(*args)` to generate an
> <input> tag, but only after it does its own logic and wrapping, which
> among a bunch of other things wraps the output in an <li>.
>
> So the methods run the gamut in complexity from ''f.radio''
to
> ''f.dependent_collection'' to ''f.sigma'',
but much of the common code is
> wrapped up in a method called ''f.question'', which does
the outer <li>
> wrapping, required field detection, label and error display, and a few
> other common things required by every control we use in our forms.
>
> So most of our methods have this basic structure:
>
> ?class OurFormBuilder < ActionView::Helpers::FormBuilder
> ? ?def foo_text(method, options={})
> ? ? ?foo_option = options.delete(:foo_option)
> ? ? ?options[:value] ||= ''FOO''
>
> ? ? ?# some logic, using foo_option somewhere...
>
> ? ? ?question(method, options) do |remaining_options|
> ? ? ? ?''FOO -->'' + text_field(method,
remaining_options) + ''<-- FOO''
> ? ? ?end
> ? ?end
> ?end
>
> I started out with a nice spec for `question`''s behavior and made
it all
> in a shared group, but because of the number of examples just for
> question and the number of methods that call it (so both performance and
> complexity), I''m thinking about switching to message expectations
in all
> of my `foo_text`-style method specs:
>
> ?describe ''foo_text''
> ? ?it ''calls text field with the proper options'' do
> ? ? ?@builder.should_receive(:text_field).with(:some_method, :proper_args)
> ? ? ?@builder.foo_text(:some_method)
> ? ?end
> ? ?it ''yields a wrapped text_field into question'' do
> ? ? ?# dont test rails text_field
> ? ? ?text_field_return = "BOOGA"
> ? ? ?@builder.stub!(:text_field).and_return(text_field_return)
>
> ? ? ?expected = "FOO -->" + text_field_return + "<--
FOO"
> ? ?
?@builder.should_receive(:question).with(:some_method).with_a_block_yielding(expected)
>
> ? ? ?@builder.foo_text(:some_method, :some => options)
> ? ?end
> ? ?it ''properly returns the result of the call to
question'' do
> ? ? ?@builder.stub!(:question).and_return(''BOOGA'')
> ? ? ?xhtml = @builder.foo_text(:some_method)
> ? ? ?xhtml.should == ''BOOGA''
> ? ?end
> ?end
>
> I''d appreciate any feedback that folks might be willing to give.
> Particularly I realize the following:
>
> ?(a) This might be testing implementation too much (possible)
> ?(b) The architecture of the whole plugin needs a serious refactor to
> ? ? increase modularity and decouple the components (very likely); a
> ? ? dash of decent OO design could really help this whole situation,
> ? ? and it''s something I''m planning on tackling down the
road
>
> For now, I''m just trying to push things in the right direction,
and I
> _think_ the .with_a_block_yielding(value) or .with(block_yielding(value))
> argument verification would help me do that.
>
> Thanks for your time!
>
> Paul
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On 28 Jan 2010, at 21:14, Paul Hinze wrote:
> Ashley Moran <ashley.moran at patchspace.co.uk> on 2010-01-28 at
13:28:
>>
>> On Jan 28, 2010, at 1:29 pm, Paul Hinze wrote:
>>
>>> I believe the lack of ability to use this notation comes down to a
>>> ruby
>>> limitation, but I''m not sure.  If that''s the
case, then we would
>>> need a
>>> specific argument expectation (along the lines of my suggestion)  
>>> that
>>> executes in a context in which it can call the block.
>>
>> I can''t find a solution, I suspect Ruby 1.8 can''t do
this, but I''m
>> guessing.
>>
>> Can I ask why you want to do this though?  As another example, it
>> would be unusual to spec something like:
>>
>>  @array = [1, 2, 3]
>>  @array.should_receive(:map).with(block_that_doubles_values)
>>
>> You''d instead check that the array that came out was [2, 4,
6].
>
> I''m trying to spec a large set of what essentially come down to
> decorator methods in a Rails FormBuilder extension plugin.  What this
> boils down to is methods that wrap rails FormBuilder methods, so
> `f.text(*args)` ends up calling `f.text_field(*args)` to generate an
> <input> tag, but only after it does its own logic and wrapping, which
> among a bunch of other things wraps the output in an <li>.
>
> So the methods run the gamut in complexity from ''f.radio''
to
> ''f.dependent_collection'' to ''f.sigma'',
but much of the common code is
> wrapped up in a method called ''f.question'', which does
the outer <li>
> wrapping, required field detection, label and error display, and a few
> other common things required by every control we use in our forms.
>
> So most of our methods have this basic structure:
>
>  class OurFormBuilder < ActionView::Helpers::FormBuilder
>    def foo_text(method, options={})
>      foo_option = options.delete(:foo_option)
>      options[:value] ||= ''FOO''
>
>      # some logic, using foo_option somewhere...
>
>      question(method, options) do |remaining_options|
>        ''FOO -->'' + text_field(method,
remaining_options) + ''<-- FOO''
>      end
>    end
>  end
>
> I started out with a nice spec for `question`''s behavior and made
it
> all
> in a shared group, but because of the number of examples just for
> question and the number of methods that call it (so both performance  
> and
> complexity), I''m thinking about switching to message expectations
in
> all
> of my `foo_text`-style method specs:
>
>  describe ''foo_text''
>    it ''calls text field with the proper options'' do
>       
> @builder.should_receive(:text_field).with(:some_method, :proper_args)
>      @builder.foo_text(:some_method)
>    end
>    it ''yields a wrapped text_field into question'' do
>      # dont test rails text_field
>      text_field_return = "BOOGA"
>      @builder.stub!(:text_field).and_return(text_field_return)
>
>      expected = "FOO -->" + text_field_return + "<--
FOO"
>       
> @builder 
> .should_receive 
> (:question).with(:some_method).with_a_block_yielding(expected)
>
>      @builder.foo_text(:some_method, :some => options)
>    end
>    it ''properly returns the result of the call to
question'' do
>      @builder.stub!(:question).and_return(''BOOGA'')
>      xhtml = @builder.foo_text(:some_method)
>      xhtml.should == ''BOOGA''
>    end
>  end
>
> I''d appreciate any feedback that folks might be willing to give.
> Particularly I realize the following:
>
> (a) This might be testing implementation too much (possible)
I''d say so. I would think the most stable seam around which to write  
tests for this is where you call the FormBuilder to make HTML. I would  
not start tinkering around inside it with mocks between inheritance  
layers like that - it''s a path that will make it very hard to do any  
refactoring in future.
> (b) The architecture of the whole plugin needs a serious refactor to
>     increase modularity and decouple the components (very likely); a
>     dash of decent OO design could really help this whole situation,
>     and it''s something I''m planning on tackling down the
road
>
> For now, I''m just trying to push things in the right direction,
and I
> _think_ the .with_a_block_yielding(value)  
> or .with(block_yielding(value))
> argument verification would help me do that.
>
> Thanks for your time!
>
> Paul
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
cheers,
Matt

http://mattwynne.net
+447974 430184

On 28 Jan 2010, at 21:43, Nicol?s Sanguinetti wrote:
> You''re definitely testing too much implementation and not enough  
> behavior.
>
> Basically, what you want to spec, is that provided some options, when
> you call a certain method of your form builder, you get a certain html
> output. At least that''s how I would approach the problem.
>
> So I would have something like this:
>
> it "produces a correctly formatted FOO input" do
>  html = @builder.foo_text(...)
>  html.should == "<label
for=''foo''>FOO</label> <input
id=''foo''....>"
> end
>
> Since testing generated HTML like that sucks, I would proceed to use
> something like http://github.com/fnando/rspec-hpricot-matchers, and
> say
>
> it "produces a correctly formatted FOO input" do
>  html = @builder.foo_text(...)
>  html.should have_tag("label", :for => "foo")
>  html.should have_tag("input", :id => "foo")
> end
>
> Or something like that. But then if you stop using that
> question(*args, &block) method, and refactor to a different
> implementation, specs should continue to pass.
What he said.

> -foca
>
> On Thu, Jan 28, 2010 at 7:14 PM, Paul Hinze <paul.t.hinze at
gmail.com>
> wrote:
>> Ashley Moran <ashley.moran at patchspace.co.uk> on 2010-01-28 at
13:28:
>>>
>>> On Jan 28, 2010, at 1:29 pm, Paul Hinze wrote:
>>>
>>>> I believe the lack of ability to use this notation comes down
to
>>>> a ruby
>>>> limitation, but I''m not sure.  If that''s the
case, then we would
>>>> need a
>>>> specific argument expectation (along the lines of my
suggestion)
>>>> that
>>>> executes in a context in which it can call the block.
>>>
>>> I can''t find a solution, I suspect Ruby 1.8 can''t
do this, but I''m
>>> guessing.
>>>
>>> Can I ask why you want to do this though?  As another example, it
>>> would be unusual to spec something like:
>>>
>>>   @array = [1, 2, 3]
>>>   @array.should_receive(:map).with(block_that_doubles_values)
>>>
>>> You''d instead check that the array that came out was [2,
4, 6].
>>
>> I''m trying to spec a large set of what essentially come down
to
>> decorator methods in a Rails FormBuilder extension plugin.  What this
>> boils down to is methods that wrap rails FormBuilder methods, so
>> `f.text(*args)` ends up calling `f.text_field(*args)` to generate an
>> <input> tag, but only after it does its own logic and wrapping,
which
>> among a bunch of other things wraps the output in an <li>.
>>
>> So the methods run the gamut in complexity from
''f.radio'' to
>> ''f.dependent_collection'' to
''f.sigma'', but much of the common code is
>> wrapped up in a method called ''f.question'', which
does the outer <li>
>> wrapping, required field detection, label and error display, and a  
>> few
>> other common things required by every control we use in our forms.
>>
>> So most of our methods have this basic structure:
>>
>>  class OurFormBuilder < ActionView::Helpers::FormBuilder
>>    def foo_text(method, options={})
>>      foo_option = options.delete(:foo_option)
>>      options[:value] ||= ''FOO''
>>
>>      # some logic, using foo_option somewhere...
>>
>>      question(method, options) do |remaining_options|
>>        ''FOO -->'' + text_field(method,
remaining_options) + ''<-- FOO''
>>      end
>>    end
>>  end
>>
>> I started out with a nice spec for `question`''s behavior and
made
>> it all
>> in a shared group, but because of the number of examples just for
>> question and the number of methods that call it (so both  
>> performance and
>> complexity), I''m thinking about switching to message
expectations
>> in all
>> of my `foo_text`-style method specs:
>>
>>  describe ''foo_text''
>>    it ''calls text field with the proper options'' do
>>       
>> @builder.should_receive(:text_field).with(:some_method, :proper_args)
>>      @builder.foo_text(:some_method)
>>    end
>>    it ''yields a wrapped text_field into question'' do
>>      # dont test rails text_field
>>      text_field_return = "BOOGA"
>>      @builder.stub!(:text_field).and_return(text_field_return)
>>
>>      expected = "FOO -->" + text_field_return +
"<-- FOO"
>>       
>> @builder 
>> .should_receive 
>> (:question).with(:some_method).with_a_block_yielding(expected)
>>
>>      @builder.foo_text(:some_method, :some => options)
>>    end
>>    it ''properly returns the result of the call to
question'' do
>>      @builder.stub!(:question).and_return(''BOOGA'')
>>      xhtml = @builder.foo_text(:some_method)
>>      xhtml.should == ''BOOGA''
>>    end
>>  end
>>
>> I''d appreciate any feedback that folks might be willing to
give.
>> Particularly I realize the following:
>>
>>  (a) This might be testing implementation too much (possible)
>>  (b) The architecture of the whole plugin needs a serious refactor to
>>     increase modularity and decouple the components (very likely); a
>>     dash of decent OO design could really help this whole situation,
>>     and it''s something I''m planning on tackling down
the road
>>
>> For now, I''m just trying to push things in the right
direction, and I
>> _think_ the .with_a_block_yielding(value)  
>> or .with(block_yielding(value))
>> argument verification would help me do that.
>>
>> Thanks for your time!
>>
>> Paul
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
cheers,
Matt

http://mattwynne.net
+447974 430184

A bit of a delayed reply, but I appreciate all the feedback everyone.
What a helpful list this is! :)

Nicol?s Sanguinetti <godfoca at gmail.com> on 2010-01-28 at
15:45:
> You''re definitely testing too much implementation and not enough
behavior.
This was the overwhelming opinion of the group, and I see what you are
all saying. 
> Basically, what you want to spec, is that provided some options, when
> you call a certain method of your form builder, you get a certain html
> output. At least that''s how I would approach the problem.
> 
> it "produces a correctly formatted FOO input" do
>   html = @builder.foo_text(...)
>   html.should have_tag("label", :for => "foo")
>   html.should have_tag("input", :id => "foo")
> end
This is more or less exactly the structure that current specs have.  And
the (mistakenly-designed, I now realize) refactor I was hoping to make
was to address the problem of _re-verifying_ all of the behavior of
`question` in every formbuilder method.

Here is how we''re currently doing it:

describe ''question'', :shared_behavior => true do
  it ''marks the question required'' do
    # stub question required in model
    xhtml = @builder.send(@current_method) # [A]
    xhtml.should have_tag(''li.required'')
  end
  # ... more examples describing question behavior [B]
end

describe ''#question'' do
  before { @current_method = ''question'' }
  it_should_behave_like ''question''
end

describe ''#text'' do
  before { @current_method = ''text'' }
  it_should_behave_like ''question''
  # ... examples describing text behavior
end

This worked great at first, until we got to the point of there being
more than 50 examples at [B] for `question`, and there being more than
30 other methods.  I also realized that some methods that share question
behavior require certain options to properly pass through the question
shared example group.  So I had to add another argument to [A] and
maintain this knowledge across the suite.

Not only that, but I realized that question is not the only shared
behavior.  Add that to the mix and you start to get structures like
this:

describe ''collection'', :shared => true do
  # ... examples describing collection behavior
end

describe ''#collection'' do
  before do
    @current_method = :collection
    @default_options = { :collection => [:foo, :bar, :baz] }
  end
  it_should_behave_like ''question''
  it_should_behave_like ''collection''
end

describe ''#dependent_collection'' do
  before do
    @current_method = :dependent_collection
    @default_options = { :collection => [:foo, :bar, :baz], :depends_on =>
:qux }
  end
  it_should_behave_like ''question''
  it_should_behave_like ''collection''
  # ... examples describing dependent_collection behavior
end

This gets unruly and difficult to read and maintain very quickly, and
because our group has several devs who touch this code, the pattern is
not always followed correctly.

So now I sit with these disadvantages:

 1) a relatively slow test suite for my formbuilder, since every detail
    of any shared behavior is verified over and over
 2) some mysterious instance variables i need to maintain across all
    example groups in order to keep shared groups testing the right
    methods
 3) strange and unintuitive coupling across the test suite

The advantage we''re getting, of course, is that behavior is properly
speced with this structure (when we pull it off properly at least).

I started this thread with the thought that I could simplify this by
basically flipping out ''it_should_behave_like'' with
''it_should_call'',
but you folks identified that as slipping down into verifying
implementation rather than behavior.

So the question that I pose to you, list, is this: is there any way to
change the way we''re dealing with this problem to minimize the
disadvantages I mentioned above or is the answer "yup, that''s
pretty
much how you would need to set this up"?

Thanks again for all your feedback and attention folks, it has been
incredibly helpful.

Cheers,

Paul