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

Handle calls to failure_slash_error_line[s]. #554

Merged
merged 1 commit into from
Feb 9, 2016
Merged

Handle calls to failure_slash_error_line[s]. #554

merged 1 commit into from
Feb 9, 2016

Conversation

jzohrab
Copy link
Contributor

@jzohrab jzohrab commented Feb 8, 2016

This PR replaces #553

failure_slash_error_line is currently not defined in rspec-core
(ref https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/formatters/exception_presenter.rb).
This change uses it or failure_slash_error_lines if either of those are defined.

Ref this tweet: https://twitter.com/kantrn/status/664913752832126976

I have tried this locally on my cookbook development and the error is resolved. I don't know how to run the rspec tests for serverspec itself, though, so this PR is effectively untested.

failure_slash_error_line is currently not defined in rspec-core
(ref https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/formatters/exception_presenter.rb).
This change uses it or failure_slash_error_lines if defined.

This fix addresses a current breakage, but since the code currently
monkeypatches private methods, it may still be brittle.
@jzohrab
Copy link
Contributor Author

jzohrab commented Feb 8, 2016

We should still talk with the rspec-core team, I think they're open to expanding their API. I've opened up issue rspec/rspec-core#2169 there.

@jzohrab
Copy link
Contributor Author

jzohrab commented Feb 8, 2016

Hello @mizzy - I opened issue 2169 in rspec-core, and have been asked what you need to ensure you're not monkeypatching private methods and data. That issue has been closed, but if you drop in and add a note then you may be able to get them to make changes to get the data you need in a way that will be supported in the future. Thanks! jz

@mizzy
Copy link
Owner

mizzy commented Feb 9, 2016

Thanks @jzohrab. I'll consider it.

Anyway, I'll merge your PR.

Thanks a lot !

mizzy added a commit that referenced this pull request Feb 9, 2016
Handle calls to failure_slash_error_line[s].
@mizzy mizzy merged commit 66138a2 into mizzy:master Feb 9, 2016
@mizzy
Copy link
Owner

mizzy commented Feb 9, 2016

Released as v2.29.2.

@coderanger
Copy link
Contributor

Not sure why this was merged, this patch doesn't seem correct for any current case.

oholiab added a commit to oholiab/puppet-omnibus that referenced this pull request Jun 30, 2016
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.

3 participants