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

It would be nice to add .to receive(...).with_block #1065

Open
ioquatix opened this issue Jul 2, 2018 · 15 comments
Open

It would be nice to add .to receive(...).with_block #1065

ioquatix opened this issue Jul 2, 2018 · 15 comments

Comments

@ioquatix
Copy link

ioquatix commented Jul 2, 2018

There is no easy way to verify a method would be called with a block.

It would be nice to have

expect(thing).to receive(:method).with_block
@benoittgt
Copy link
Member

benoittgt commented Jul 2, 2018

Hello @ioquatix

This is probably not what you are asking but have you seen? https://github.com/rspec/rspec-expectations/blob/v3.7.0/features/custom_matchers/define_block_matcher.feature

Also, it's funny because factory_bot have a matcher for block with the name you mentioned.
https://github.com/thoughtbot/factory_bot/blob/40c8708c473279d3f812a825ff979f2c93451e05/spec/support/matchers/trait.rb

Used like this:
https://github.com/thoughtbot/factory_bot/blob/ad107bea15aa75b60decbbebbc86980fd3e85d34/spec/factory_bot/definition_proxy_spec.rb#L127-L130

Bye

@myronmarston
Copy link
Member

@benoittgt I think @ioquatix is talking about receive(...) from rspec-mocks, so that you can expect to receive a message with a block. You can currently deal with this by passing a block to receive:

expect(thing).to receive(:method) do |&block|
  expect(block).not_to be_nil
end

If we wanted to add more explicit support for this, I'd rather not add a new with_block API; instead maybe something like:

expect(thing).to receive(:method).with(a_block)

Where a_block is a "special" argument matcher like any_args or no_args that doesn't match on a positional arg but instead rspec-mocks handles internally.

@ioquatix does the approach I suggested above work for you?

@benoittgt
Copy link
Member

Thanks, @myronmarston for clarification. I agree with your proposal.

@ioquatix
Copy link
Author

ioquatix commented Jul 2, 2018

Yep it would be fine. Does it work with arguments too?

@myronmarston
Copy link
Member

Yep it would be fine. Does it work with arguments too?

I'm not sure what you mean by "work with arguments". Can you provide an example of what you mean?

@ioquatix
Copy link
Author

ioquatix commented Jul 2, 2018

expect(thing).to receive(:method).with(1, 2, 3, a_block)

@ioquatix
Copy link
Author

ioquatix commented Jul 2, 2018

it's actually not that important to my use case, just thought it might be useful in the general case.

@myronmarston
Copy link
Member

Hopefully, yes, we could make that work.

Do you want to take a stab at adding it. Improving RSpec is largely a self-service operation unless one of the core team members wants to take this work on (which often doesn't happen; we all have lives and jobs!).

@ioquatix
Copy link
Author

ioquatix commented Jul 2, 2018

Sure, that makes sense.

If I have time, I will mention here I am starting work. Otherwise, this is free for anyone who is interested to implement it. If someone else picks it up, please mention it here so we don't duplicate effort :)

@kaiwren
Copy link

kaiwren commented Sep 16, 2018

@myronmarston I've tried taking a stab at this in kaiwren/1065-with-a-block - am able to get this working at the cucumber feature level, but I'm missing something on the sad path spec which I'm unable to get to pass (even though it works fine in the cuke). I'm unable to figure out why I'm not able to catch the exception; probably missing something really obvious...

Would be grateful if I could get a quick review and some feedback. If my approach isn't flawed, will complete this and raise a PR.

@benoittgt
Copy link
Member

Maybe @ioquatix can provide a review too?

kaiwren/rspec-mocks@524637d

@ioquatix
Copy link
Author

It looks pretty good.

@JonRowe
Copy link
Member

JonRowe commented Sep 17, 2018

I've expanded on @kaiwren's work over on rspec/rspec-mocks#1237, it's worth noting we already had the and_yield expectation, and I think there are actually a few issues with a_block, what happens when used positionally? and does it always mean the supplied &block argument? After all object.msg(proc {}, proc {}) is valid Ruby, should we expect on this with .with(a_block, a_block) and how do we differentiated that from msg(proc{}) { }.

@alexpapworth
Copy link

Did this ever get merged in? Looking at it 4 years later, and still think it would be useful.

@ioquatix
Copy link
Author

I don't think so!

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

No branches or pull requests

6 participants