search

splitwise / super_diff

A more helpful way to view differences between complex data structures in RSpec.
https://splitwise.github.io/super_diff/
MIT License
977 stars 50 forks source link

raise_error + an exception object produces wrong failure message #94

Open mcmire opened 4 years ago

mcmire commented 4 years ago

This may be fixed already, but the gem raises an error on this line when you attempt to use raise_error with a single exception object and the matcher fails:

https://github.com/mcmire/super_diff/blob/master/lib/super_diff/rspec/monkey_patches.rb#L664

(undefined method name for SomeExceptionClass)

pavolzbell commented 3 years ago

Hello @mcmire, maybe I bumped into this one (or a very similar one) today too:

expect { raise 'x' }.to raise_error(start_with('x')) #=> true
expect { raise 'x' }.to raise_error(start_with('y'))

NoMethodError: undefined method `name' for #<RSpec::Matchers::BuiltIn::StartWith:0x63306e58>
from .../gems/super_diff-0.5.2/lib/super_diff/rspec/monkey_patches.rb:668:in `expected_for_matcher_text'

How can I help fixing it?

mcmire commented 3 years ago

I think that might be technically a different issue, but I guess it's the same message so perhaps it's related. That is happening because super_diff doesn't support all of the matcher objects yet. It sounds like in this case your matcher failed so you would need to debug/resolve it manually, but obviously it would be great if there were a useful error message here. :) But yeah, super_diff needs to know how to handle start_with and end_with.

pavolzbell commented 3 years ago

Ok, so I'm thinking, would it be better to add support for start_with and end_with matchers or fallback to default RSpec matcher's failure_message if we encounter an error in super_diff monkey patches? The latter will probably solve similar cases for various custom matchers as well (not sure how much are custom matchers supported), what do you think?

mcmire commented 3 years ago

Yeah, it might be a good idea to fall back if there is an error. There's an issue I made in #44 around this, so I've been thinking about doing this anyway (probably should have done it all this time). I don't have a ton of time these days so I'm not sure if I could get to either one soon, but falling back is probably a good strategy overall regardless.