Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom matchers defined using the define DSL cannot receive a block #321

Closed
myronmarston opened this issue Sep 6, 2013 · 5 comments
Closed

Comments

@myronmarston
Copy link
Member

This fails:

RSpec::Matchers.define :receive_a_block do |&block|
  match { block }
end

describe "A custom matcher that receives a block" do
  it 'works' do
    expect(nil).to receive_a_block { }
  end
end
@xaviershay
Copy link
Member

To save the curious a step, here is the current behaviour:

Failures:

  1) A custom matcher that receives a block works
     Failure/Error: expect(nil).to receive_a_block { }
       expected nil to receive a block
     # ./repro.rb:7:in `block (2 levels) in <top (required)>'

@myronmarston
Copy link
Member Author

I'm currently working on a rewrite of the matcher DSL that solves #272 among other improvements. I've been looking into this a bit.

The problem is that to provide the custom matcher DSL, the user's block has to be eval'd in the context of another object that defines the DSL methods...so it has to use class_exec or instance_exec. Ruby only allows a method to receive one block, and the block we pass to the _exec method is the define block -- so there's no way to pass along a second block.

Thinking outside the box a bit, I do think there is a not-too-bad alternative:

  • Make the block param available via a block_param attribute.
  • When the user tries to define a matcher using a line like RSpec::Matchers.define :receive_a_block do |&block|, print a warning telling the use we can't give them the block that way and that they should use block_param instead.

It's not ideal but it's the best solution I've thought of. Anyone have a better idea?

@jemc
Copy link

jemc commented Jan 23, 2014

I'd appreciate this improvement (and I think making it available via a block_param attribute would be a good solution). I ran into this issue today when working on my own custom matcher.

I was discussing this on irc, and I was linked to thie issue, and it was suggested that I post here and give some information about my use case.

Without going into too much detail, the matcher asserts that one or more events have happened that match the arguments given to the matcher. I want to extend this behaviour to allow the user to give a block to the matcher, which would be used as a type of select to further eliminate some events from being considered matching by using arbitrary logic in the block on each event and returning a true or false to indicate if that one event is acceptable. After the results have been narrowed, if any remain, the matcher succeeds.

The thing that scares me about doing this is the following ruby syntax distinction:

expect(nil).to receive_a_block { } #=> the block is a &block_arg to the #receive_a_block method
expect(nil).to receive_a_block do end  #=> the block is a &block_arg to the #to method

Now, I realize this is a ruby parsing thing and not an rspec thing, but it makes my use case a little scary.

In my use case, where the block is optional, receive_a_block could still succeed without the block, so the user won't necessarily know that their interchanging of { } for do end caused the block to not be received at all, and thrown away by the to/should method.

I'm not sure what the proper way to address this issue would be without being obtrusive, but it's something to bring up and think about. It's probably not something rspec could help me solve, unless to/should would keep the block somehow and make it available to the matcher. But yeah, that seems a little obtrusive to the current codebase.

In general, it may be best if I and others hoping to use blocks passed to matchers avoid the optional use of a block if this pattern becomes possible.

@kcdragon
Copy link
Contributor

kcdragon commented Sep 8, 2014

PR for this issue #645

@myronmarston
Copy link
Member Author

Closed by #645.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants