rspec-expectations icon indicating copy to clipboard operation
rspec-expectations copied to clipboard

Diff actual/expected captures from match.with_captures

Open jasonkarns opened this issue 10 years ago • 2 comments

Super stoked to see with_captures ship! Now that I'm able to use this feature in practice, I'm finding the first glaring oversight with our implementation. When a with_captures match fails, it only prints the diff of the expected/actual regex with the string. So if the captures are what cause the match to fail, it isn't made clear in the failure message what was actually captured.

At the very least, we'd like to see the actual captures printed with the failure message. The expected captures are already printed in the spec description, so they're less critical. I think the ideal would be:

  • if the match itself caused the spec to fail, just diff the regex/string as it does today
  • if the match succeeded and the captures cause the spec to fail, then print the diff of the expected/actual captures (not of the regex/string).

Because of the way diffable? works, relying on expected and actual, the naive solution would be to simply override expected and actual with the expected_captures and actual_captures. This feels dirty and may have other side effects. Does anyone have any other ideas or suggestions? I may attempt a pr this weekend as a POC.

jasonkarns avatar Nov 20 '15 21:11 jasonkarns

Does anyone have any other ideas or suggestions? I may attempt a pr this weekend as a POC.

This might be useful as a starting point: #713. In it, @waterlink implemented expecteds_for_multiple_diffs which might be useful for what you are doing.

myronmarston avatar Nov 20 '15 22:11 myronmarston

Quite! Thanks. Looks like the only missing piece is that expecteds_for_multiple_diffs deals with multiple expecteds, but only a single actual. Of course, in this particular case, we have a secondary actual to diff against. Since the with_captures check is only tested iff the string/regex already matched, there is no value in diffing the primary expected/actual, as they match. So an approach is to get ExpectedsForMultipleDiffs to handle multiple actuals. However, I've already hacked together a working POC where the primary expected/actual is simply replaced with the expected_captures/actual_captures. It feels so wrong, but there doesn't appear to be any further downstream effect.

I'll continue to play around and hopefully get a PR put together for more concrete discussion.

jasonkarns avatar Nov 21 '15 14:11 jasonkarns

Closing due to inactivity during the monorepo migration, but feel free to reopen there if someone works on it.

JonRowe avatar Nov 27 '24 21:11 JonRowe