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

Fix pretty printing of quotes in mocha_inspect #215

Closed
wants to merge 1 commit into from

Conversation

neonichu
Copy link

The pretty-printing of quotes from inspect was messing up actual quotes a bit, consider this example:

#!/usr/bin/env ruby

require 'bacon'
require 'mocha-on-bacon'
require 'open4'

Open4.expects(:spawn).with('"').once
Open4.spawn('\'')

before this PR:

./quotes.rb:8:in `<main>': unexpected invocation: Open4.spawn(''') (Mocha::ExpectationError)
unsatisfied expectations:
- expected exactly once, not yet invoked: Open4.spawn('\'')

after this PR:

./quotes.rb:8:in `<main>': unexpected invocation: Open4.spawn('\'') (Mocha::ExpectationError)
unsatisfied expectations:
- expected exactly once, not yet invoked: Open4.spawn('"')

So one now no longer gets confused by replaced double-quotes or incorrectly escaped single-quotes in the messages.

@floehopper
Copy link
Member

@neonichu: Thanks for taking the time to submit this - it does look as if you've identified a genuine problem. I'll try to have a proper look as soon as I can. At first glance, the new implementation is hard to follow. Also it's not obvious that the two new unit tests are sufficient to require all of the logic in the new implementation. Writing more exhaustive unit tests might help explain the implementation better.

floehopper added a commit that referenced this pull request Mar 25, 2015
This is an alternative attempt at fixing the problem identified
and addressed in #215 regarding the "pretty printing" of single- and
double-quotes.

In this attempt, I asked myself why we *ever* needed to do the `gsub`
replacement of escaped double-quote characters with single-quote characters.
I couldn't come up with a reason other than to make the tests more readable.

Thus in this commit I've removed the custom implementation of
`String#mocha_inspect` so it falls back to the default `String#inspect`
implementation. As far as I can see the error messages are still sensibly
formatted and by using the `%{}` string literal construction, I think the
tests are still perfectly readable.

I hope that this is a simpler and more robust way to solve the problems
that @neonichu was seeing.
@floehopper
Copy link
Member

@neonichu: I'm so sorry not to have looked at this for so long. I've had a bit of a think and come up with a possible alternative solution in #223. Can you see whether that would address your issue? Thanks.

@neonichu
Copy link
Author

Superseded by #223

@neonichu neonichu closed this Jul 21, 2015
chrisroos pushed a commit that referenced this pull request Oct 18, 2016
This is an alternative attempt at fixing the problem identified
and addressed in #215 regarding the "pretty printing" of single- and
double-quotes.

In this attempt, I asked myself why we *ever* needed to do the `gsub`
replacement of escaped double-quote characters with single-quote characters.
I couldn't come up with a reason other than to make the tests more readable.

Thus in this commit I've removed the custom implementation of
`String#mocha_inspect` so it falls back to the default `String#inspect`
implementation. As far as I can see the error messages are still sensibly
formatted and by using the `%{}` string literal construction, I think the
tests are still perfectly readable.

I hope that this is a simpler and more robust way to solve the problems
that @neonichu was seeing.
chrisroos pushed a commit that referenced this pull request Oct 24, 2016
This is an alternative attempt at fixing the problem identified
and addressed in #215 regarding the "pretty printing" of single- and
double-quotes.

In this attempt, I asked myself why we *ever* needed to do the `gsub`
replacement of escaped double-quote characters with single-quote characters.
I couldn't come up with a reason other than to make the tests more readable.

Thus in this commit I've removed the custom implementation of
`String#mocha_inspect` so it falls back to the default `String#inspect`
implementation. As far as I can see the error messages are still sensibly
formatted and by using the `%{}` string literal construction, I think the
tests are still perfectly readable.

I hope that this is a simpler and more robust way to solve the problems
that @neonichu was seeing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants