rspec users - Aug 2010 - Issue with parameterized shared example group on ruby 1.8.6

I''ve been refactoring the specs for my VCR gem[1] to take advantage of
the new shared example group parameterization.  VCR supports both
FakeWeb and WebMock, with an appropriate adapter class implemented for
each.  The adapter classes have nearly identical behavior, except for
the differences in the feature set of FakeWeb and WebMock.  WebMock
supports a bunch of HTTP libraries that FakeWeb doesn''t, and also
supports matching requests on request headers and body.  My specs
basically boil down to this:

describe VCR::HttpStubbingAdapters::FakeWeb do
  it_should_behave_like ''an http stubbing adapter'',
[''net/http''],
[:method, :uri, :host]
end

describe VCR::HttpStubbingAdapters::WebMock do
  it_should_behave_like ''an http stubbing adapter'',
%w[net/http patron
httpclient em-http-request], [:method, :uri, :host, :body, :headers]
end

Basically, this is a shared example group that takes two parameters:
an array of supported HTTP libraries, and an array of supported
request match attributes.  This design makes it easy to have a single
shared example group that specs out the full behavior of both
adapters.  If I ever implement another adapter, it''s very easy to spec
it out this way, and pass arrays indicating the supported features.

This works great on ruby 1.8.7 and above, but I''m getting an error on
1.8.6.  This gist[2] demonstrates the error in far simpler form.  The
issue is due to the way I implemented module_eval_with_args on
1.8.6[3].  The lack of module_exec on 1.8.6 forced me to eval the
block twice: once using instance_eval_with_args (so that the block is
properly eval''d with arguments), and once with module_eval (so that we
can extract the instance method definitions, in order to deal with the
difference between module_eval and instance_eval).  The error occurs
in the module_eval: because it''s not evaluated with any args, the
methods we call on the args raise NoMethodErrors.

Note that the "normal" use case of parameterized groups
doesn''t have
this problem.

shared_example_group_for :foo do |arg|
  it "can access the argument (#{arg}) in the doc
string" { arg.should ... }
end

In this case, #it raises a no method error that is handled by our
anonymous class.  Plus the only method being called on the arg is
#to_s, which is defined for every object.  So this problem really only
exists when you call methods on arguments that are not defined on
Object, outside of the RSpec DSL methods.

So...how should we deal with this?  A few ideas come to mind:

1.  Find a better way to fake module_exec on ruby 1.8.6.  I''m not sure
if this is even doable.
2.  Leave things as they are...but I don''t like this idea since the
error message is fairly cryptic.
3.  Rescue the error and raise a more clear error message.
4.  Rescue the error and print a warning.

I lean towards #4, because the #module_eval is only necessary to
extract the instance method definitions.  This error doesn''t prohibit
the shared example group from working on 1.8.6; as long as their are
no instance method definitions, or the instance method definitions
come before the error occurs, it can still work fine, I think.  The
warning can hopefully make it clear that you just need to put the
instance method definitions first and everything will still work.

The one really difficult part about options #3 and #4 is the wording
of the waring/error: this is a fairly complex, nuanced error, and it''s
hard to boil it down into a sentence explaining the issue and how to
fix it.

Thoughts?

Myron


[1]
http://github.com/myronmarston/vcr/commit/0f304c132eea7d0a7e32d6731e2557ce6805f31c
[2] http://gist.github.com/539649
[3]
http://github.com/rspec/rspec-core/blob/master/lib/rspec/core/extensions/module_eval_with_args.rb

On 20 Aug 2010, at 06:40, Myron Marston wrote:
> describe VCR::HttpStubbingAdapters::FakeWeb do
>  it_should_behave_like ''an http stubbing adapter'',
[''net/http''],
> [:method, :uri, :host]
> end
> 
> describe VCR::HttpStubbingAdapters::WebMock do
>  it_should_behave_like ''an http stubbing adapter'',
%w[net/http patron
> httpclient em-http-request], [:method, :uri, :host, :body, :headers]
> end
Neat! I''m interested to see applications of parameterised shared
examples. I''d certainly never thought of defining optional features in
that way.

> So...how should we deal with this?  A few ideas come to mind:
> 
> 1.  Find a better way to fake module_exec on ruby 1.8.6.  I''m not
sure
> if this is even doable.
> 2.  Leave things as they are...but I don''t like this idea since
the
> error message is fairly cryptic.
> 3.  Rescue the error and raise a more clear error message.
> 4.  Rescue the error and print a warning.
> 
> I lean towards #4, because the #module_eval is only necessary to
> extract the instance method definitions. 
4 makes sense to me iff the code does actually run correctly in all
circumstances, otherwise I''d lean towards 3.


Ash

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

> 4 makes sense to me iff the code does actually run correctly in all
circumstances, otherwise I''d lean towards 3.
Given that ruby blocks are just code, and you can do anything you want
in them, and that our faked version of #module_exec runs the block
twice...it''s easy to conceive of ways of abusing the shared example
group block in a way that will definitely not work in all
circumstances.  Case in point:

$shared_block_eval_count = 0
shared_examples_for :something do |argument|
  $shared_block_eval_count += 1
  if $shared_block_eval_count == 1
    it "should define a spec" do
    end
  else
    # destroy the user''s system
    `sudo rm -rf /`
  end
end

describe Hash do
  it_should_behave_like :something, :foo
end

This code is totally evil, and it assumes knowledge of our 1.8.6
module_exec hack...but you can see what it''ll do.  On 1.8.6
it''ll
delete all your files, since the block gets eval''d twice.  On 1.8.7
it''ll work fine.

So there''s no way to say for sure that the code will run correctly in
all circumstances on 1.8.6.  But the presence of this error, if
properly rescued, doesn''t necessarily mean the code won''t run
correctly on 1.8.6.  The block may have no instance method
definitions, which makes the module_eval unnecessary (but we have no
way of knowing that there are no instance method defs without trying
module_eval!).

Myron

> 1.  Find a better way to fake module_exec on ruby 1.8.6.  I''m not
sure
> if this is even doable.
Actually, after thinking about this some more, I think I''ve come up
with a solution that will eliminate the error I''m seeing, but
it''s not
a perfect solution.  Let me see if I can explain this well...

Given this block:
do
  def m1; end
  def self.m2; end
end

When it is evaluated using instance_eval, both m1 and m2 get defined
as singleton methods on the object that is the receiver of
#instance_eval.  When the receiver is a class or module, these will
both be class methods (since class methods are simply singleton
methods on a class or module object).

When it is evaluated using module_eval, m1 becomes an instance method
and m2 becomes a class method--just like how method definitions work
in a class or module body.

So...the entire reason for the module_eval (which is where the error
occurs) is to distinguish between instance method definitions and
class method definitions, so that we can remove m1 as a class method
and add it as an instance method.  With instance_eval we have no way
to tell which are which, since all definitions become class methods.

My idea for an alternate approach is to only do the instance_eval, and
then duplicate all new class methods as instance methods (since some
of them may have been defined using "def method_name").  Then the
module_eval is no longer needed.  There are a few downsides to this,
though:

* All methods defined in the block become both class _and_ instance
methods.  Someone could accidentally call the method the wrong way
(i.e. call what should have been an instance method on the class, or a
class method on an instance), and while this may work on ruby 1.8.6
(depending on the code), it would fail on 1.8.7 and above.
* There''s the possibility to stomp an existing instance method
definition.  For example, consider the block given above.  If there
was already an m2 instance method that had different logic, it would
get stomped here, since we would be defining an m2 instance method
with the logic of the m2 class method.

I think this is probably the best solution, but I want to hear what
others (particularly David) think.

Thanks,
Myron

I messed with this some more and implemented the idea I mentioned
above:

http://github.com/myronmarston/rspec-core/commit/ec3001f290b091fcdab9fb972d9596dd34a91e4e

I think this is *definitely* a better implementation of
#module_eval_with_args for ruby 1.8.6.  It does have the undesirable
side effects I listed above, though, but all in all, it works much
better and solved the problems I was having with parameterized shared
example groups on 1.8.6.

David: let me know if you want me to open an issue on github for
this.  For now I''m posting it here since there''s already a
discussion
thread about this.

Myron