nubank / matcher-combinators

Library for creating matcher combinator to compare nested data structures
Other
467 stars 23 forks source link

add flare string diffing to output #157

Closed philomates closed 2 years ago

philomates commented 3 years ago

Midje, when reporting test failures where the expected and actual are strings, uses https://github.com/duckyuck/flare to do string diff'ing.

for example you might see output like

Expected:
"hello world!"
Actual:
"hello world"
Diffs: strings have 1 difference (91% similarity)
                expected: "... world(!)"
                actual:   "... world(-)"

This PR is a proposal to add this to matcher-combinators.

For something like this

(fact 
  ["abc" "ayz"] => (match (matchers/in-any-order ["ay*" "ab*"])))

Before:

FAIL at (scratch.clj:50)
Actual result did not agree with the checking function.
Actual result:
["abc" "ayz"]
Checking function: (match (matchers/in-any-order ["ay*" "ab*"]))
    The checker said this about the reason:
        [:mismatch
 ((mismatch "ab*" "abc")
  (mismatch "ay*" "ayz"))]

after

FAIL at (scratch.clj:50)
Actual result did not agree with the checking function.
Actual result:
["abc" "ayz"]
Checking function: (match (matchers/in-any-order ["ay*" "ab*"]))
    The checker said this about the reason:
        [:mismatch
 ((mismatch "ab(*)" "ab(c)")
  (mismatch "ay(*)" "ay(z)"))]

or for

(deftest bar
  (is (match? "hello o brave new world!"
              "hello world!")))

before:

FAIL in (bar) (test_test.clj:120)
expected: (match? "hello o brave new world!" "hello world!")
  actual: (mismatch
 "hello o brave new world!"
 "hello world!")

after

FAIL in (bar) (test_test.clj:120)
expected: (match? "hello o brave new world!" "hello world!")
  actual: (mismatch
 "hello (o brave new )world!"
 "hello (------------)world!")

Any suggestions on making it clearer what is expected, actual, and the diff between them?

gabrielgiussi commented 3 years ago
  1. It applies when using regex as well?
  2. Could we add a way to turn it off? I'm wondering if may affect performance when dealing with big maps with lot's of strings. wdyt?
philomates commented 3 years ago
  1. It applies when using regex as well?

It should only apply when both the expected and actual are strings

  1. Could we add a way to turn it off? I'm wondering if may affect performance when dealing with big maps with lot's of strings. wdyt?

Good point. So the diff code only gets run after a mismatch is detected, so it will only slow things down if the expected/actual strings are large. I tried it on a sorta large string and it took 2 seconds to print, which isn't great

To be honest I don't really know if this change is worth the effort. I find the output kind of confusing. Maybe leaving it as off by default and allowing people to turn it on if they need it? The user that asked for this wanted to compare large string files, which I don't think is so common

gabrielgiussi commented 3 years ago

leaving it as off by default and allowing people to turn it on if they need it

I like this idea if is not very complex to add.

Or could we even think in a way to plug-in matchers? So this particular engineer could add the dependency to https://github.com/duckyuck/flare but it won't be included in matcher-combinators. This may be harder though, I really don't know much about the internals of matcher-combinators so it is hard for me to estimate the effort such thing requires.