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

Bug fix for have_failed with RSpec 3.0.0 #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ronen
Copy link
Contributor

@ronen ronen commented Jun 5, 2014

With Mr. Weirich sadly no longer with us, I don't know who if anyone wil be maintaing this, but I figure I may as well at least post the issue/PR in case anybody else comes across the same issue.

With the release of RSpec 3.0.0, the have_failed mechanism no longer works. Attempting:

  When(:result) { stack.pop }
  Then { expect(result).to have_failed(UnderflowError, /empty/) }

Results in an error like:

 Failure/Error: Then { expect(credentials).to have_failed UnderflowError }
    expected UnderFlowError but was not given a block

This PR should fix that.

(The specific RSpec change that caused this to break was rspec/rspec-expections@79582c2, which cleaned up the delineation between blocks and values for expect { } vs expect() as per rspec/rspec-expectations#526 ; unfortunately rspec-given was relying on the previous looseness to be able to use a value that quacked like a Proc.)

As of a recent [commit](https://github.com/rspec/rspec-expectations/blame/79582c2c160ac7b3a58d3d128012b022c63d6174/lib/rspec/matchers/built_in/raise_error.rb#L38) to rspec-expectations, Rspec::Matchers::BuiltIn::RaiseError strictly requires a Proc rather than something that quacks like one.
benizi added a commit to forever-inc/netsweet that referenced this pull request Jun 9, 2014
@searls
Copy link

searls commented Jan 9, 2015

Hey ronen — I'm going to take a look at this branch today. Do you think it's ready to merge into rspec-given's new repo? https://github.com/rspec-given/rspec-given

@searls searls mentioned this pull request Jan 9, 2015
3 tasks
@ronen
Copy link
Contributor Author

ronen commented Jan 9, 2015

hey justin -- glad to see you've taken on this gem!

Do you think it's ready to merge into rspec-given's new repo? https://github.com/rspec-given/rspec-given

hmm.... no idea... i haven't thought about this issue or looked at any of the repos since i submitted the PR...

@searls
Copy link

searls commented Jan 9, 2015

No worries. Fortunately there were tests broken by your issue and this patch fixes several of them. I'm digging into it now and I think I understand the RSpec API change that caused the issue. I've already merged your patch into my PR and you'll get the credit :D

@ronen
Copy link
Contributor Author

ronen commented Jan 9, 2015

Cool, glad you're on top of it.

@searls searls mentioned this pull request Jan 9, 2015
7 tasks
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

Successfully merging this pull request may close these issues.

2 participants