Option for not printing full `expected` and `actual` in mismatch message

[philomates]

Currently when a mismatch happens (in clojure.test) we use the entire (match? the-expected the-actual) form as the Expected: field and the annotated mismatch diff as the Actual: field. This means that if the-expected or the-actual matcher/data-structures are large, we get really big output.

In some cases, like when you are matching and expected file contents with an actual file contents, it really suffices to only see the precise mismatch info. This is partially the motivation behind https://github.com/nubank/matcher-combinators/pull/157.

What would be useful to look into is a way to abbreviate or omit aspects of the output.

[philomates]

one suggestion by @gusbicalho is an annotation construct:

(match?
  (-> expected (reported-as "contents of foo.graphql"))
  (-> actual (reported-as "contents of tmp-file.graphql")))

[philomates]

another option would be to limit the size of the Expected: output at print time. Something like https://github.com/nubank/matcher-combinators/compare/expected-printing?expand=1# which would for

(is (match? [1 (m/not-matcher odd?) 3]
            [1 2 3]))

output

FAIL in (absent-pred) (test_test.clj:120)
expected: (match? [1 ...)
  actual: [1
 (mismatch
  "expected mismatch from: clojure.core$even_QMARK_@6a0a7d89"
  2)
 3]

[gabrielgiussi]

Couldn't we just traverse the result and redact/remove anything that is not a Mismatch? If we run this

(is (match? {:a {:b odd?}}
              {:a {:b 2
                   :c 3}}))

We have this result

{:matcher-combinators.result/type :mismatch,
 :matcher-combinators.result/value {:a {:b #matcher_combinators.model.Mismatch{:expected (pred
                                                                                          #object[matcher_combinators.matchers$pred
                                                                                                  0x46414468
                                                                                                  "matcher_combinators.matchers$pred@46414468"]),
                                                                               :actual 2},
                                        :c 3}},
 :matcher-combinators.result/weight 1}

I would like to see

  actual: {:a
 {:b
  (mismatch
   (expected
    (pred
          #object[matcher_combinators.matchers$pred 0x46414468 "matcher_combinators.matchers$pred@46414468"]))
   (actual 2))}}
; c was removed completely

For other collections where the order is important we could have something like

  actual: ['match (mismatch (expected 2) (actual 1)) 'match]

I didn't consider yet if this will play nicely with all the matchers (prefix, in-any-order, etc). This kind be done as part of the pprint logic, so we should run a filter over the mismatch result before calling to pprint, for example if a print-summary var is set to true.

[philomates]

I agree that omitting things like :c 3 would be a useful, especially if it could be turned off / on by the user. I think I explored implementing this at some point but got stuck. It would require us to trim the top node of a completely 'matched' sub-tree. Thinking about it now, I guess we could collect this data in the initial ascent from the recursive descent that computes mismatches and assoc it to the metadata of container datastructures (maps, vectors, etc) and then use metadata in a prewalk to omit any node that has a mismatching subnode.

[gabrielgiussi]

I guess we could collect this data in the initial ascent from the recursive descent that computes mismatches and assoc it to the metadata of container datastructures (maps, vectors, etc) and then use metadata in a prewalk to omit any node that has a mismatching subnode.

I can give it a try

[gabrielgiussi]

@philomates could you provide me some feedback on this approach before I move further with this implementation? 🙏

I'm not happy having to introduce the Match wrapper, besides making tests harder as it shows in this PR it might get in the middle of the validation itself which is something I've tried to avoid, but my initial attempt of using meta fell short for values that don't implement IObj which can be associated with metadata, in this category we have numbers, strings and keywords. However if this impl seems too intrusive we could take a best effort approach and only drop of redact data structures like maps, vectors, lists and sets which at the end are the ones which might be flooding the console due to its size. However at first, having an output like the following seems confusing

['matched 1 (mismatch (expected 2) (actual 3))]

Why the first was redacted to matched but the second was kept as 1?

[gabrielgiussi]

Actually this provides some value already because removes expected matched keys, it is not able to remove unmatched but unexpected keys, aka those keys that are in the actual value but which are not in the expected value, which is a pretty common case.

; the current impl won't be able to filter :a from the output, only :b because is wrapped in a Matched
(is (match? {:a 1 :b 2} {:b 2 :c 3}))

I played around with assuming everything is matched if is not one of the wrappers in model.clj but I had some troubles with this approach although I can't remember, I'll try again.

[gabrielgiussi]

assuming everything is matched if is not one of the wrappers in model.clj

I couldn't do that because for example a vector containing mismatched values is not itself wrapped in a mismatch.

(-> (matcher-combinators.core/match [1 2] [3 4])
      :matcher-combinators.result/value)
; => [#matcher_combinators.model.Mismatch{:expected 1, :actual 3} #matcher_combinators.model.Mismatch{:expected 2, :actual 4}]

It was enough with also distinguishing between an unwrapped value and a mismatch, and drop/redact the former and keep the later unmodified. I would like some feedback before adding tests, bc I'm not sure adding a Match wrapper is the way to go.

[philomates]

Here is a sketch of what I was thinking: https://github.com/nubank/matcher-combinators/compare/abbreviated-printing?expand=1

It seems to work in the limited cases that I tested

I guess either one works; feels like introducing the Match record is more explicit. But for the ellision/abbreviation implementation I think we only care about container datastructures (maps and sequentials), so it does feel like a lot of wrapping. I suppose the wrapping isn't necessarily bad unless it leaks into un-pretty-printed results.

[gabrielgiussi]

Yes, I've started exactly with that (although the mismatch-map is a cleaver way to not having to ask for map? in the prewalk, I wasn't using neither of those, nice) and explore another alternative just because not filtering scalar values seemed a confusing output, but I think I prefer the simplicity of your change at the cost of not filtering everything, because I also think that for the ellision/abbreviation implementation I think we only care about container datastructures. Besides, the user will be turning on this behavior explicitly so we can just document that scalar values aren't filtered out. For this wdyt about the dynamic var?

(binding [*print-summary* true]
    (is (match? {:name/first "Alfredo"
                 :f [1 2 3]}
                {:name/first  "Afredo"
                 :f [3 2 1]
                 :name/last   "da Rocha Viana"
                 :name/suffix "Jr."})))

[philomates]

The thing I'm a bit hung up on is the ergonomics of this. Ideally, if I want to use the feature, I shouldn't have to change source forms one by one. For example, I might really like it and want it on be default everywhere. We could prototype with a dynamic var, but I think it is important to figure out some sort of global config system that allows controlling this feature before expecting anyone to use it.

What do you think?

I'm not familiar with what patterns can be used for setting configurations in a clojure library. Is it possible via deps.edn somehow? Environment variables are an option, in my view they are quick but messy.

[philomates]

released an approach in 3.8.7 that can be enabled by calling (matcher-combinators.config/enable-abbreviation!)