nubank / matcher-combinators

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

match-with matcher #134

Closed dchelimsky closed 4 years ago

dchelimsky commented 4 years ago

This PR, which addresses issue #129 introduces a match-with matcher, which has a similar API to the match-with? assert expression, but works directly on expected values. You can use it anywhere in an arbitrarily deeply nested structure, and overrides can be, themselves, overridden in deeper scopes with nested match-with matchers, e.g.

(deftest example
  (is (match? {:a (m/match-with
                   {clojure.lang.IPersistentMap m/equals}
                   {:b
                    (m/match-with
                     {clojure.lang.IPersistentMap m/embeds
                      clojure.lang.IPersistentVector m/embeds}
                     {:c [odd? even?]})})}
              {:a {:b {:c [1 2 3]
                       :d :e}}
               :f :g})))

This also implements the midje match-with checker and the clojure.test match-with? assert-expr in terms of the new match-with matcher, allowing us to bind types directly to their matchers without the dispatch mechanism and eliminate with-redefs.

dchelimsky commented 4 years ago

One thing that bugs me about this addition is that it introduces more with-redefs. Just a note that I'm exploring an alternate implementation of dispatch that would eliminate the need for with-redefs. I'll follow up if anything useful comes of it.

markomafs commented 4 years ago

One thing that bugs me about this addition is that it introduces more with-redefs. Just a note that I'm exploring an alternate implementation of dispatch that would eliminate the need for with-redefs. I'll follow up if anything useful comes of it.

Do you think i would work with meta data? Checking for the meta data to define the “dispatchers” .

Another option would be dynamic var.

Wdyt?

dchelimsky commented 4 years ago

One thing that bugs me about this addition is that it introduces more with-redefs. Just a note that I'm exploring an alternate implementation of dispatch that would eliminate the need for with-redefs. I'll follow up if anything useful comes of it.

Do you think i would work with meta data? Checking for the meta data to define the “dispatchers” .

Another option would be dynamic var.

Wdyt?

Metadata of which Var?

I know of at least one person interested in this repo who will object to dynamic vars :)

I'm working on solving this with an additional function on the Matcher protocol that you can hand the type->matcher map directly. No redefs. No metadata. No dynamic vars. Should have something early next week.

dchelimsky commented 4 years ago

@markomafs I reimplemented this without any redefs, and was able to eliminate the dispatch layer in the process.

/cc @nubank/eng-prod-test @philomates @sovelten

fernando-nubank commented 4 years ago

Is this test supposed to pass?

(is (match?
        (m/match-with [map?    m/equals
                       vector? m/embeds]
          {:a (m/match-with [map? m/embeds]
                {:vec [1 2]
                 :a/X :a/X1})})
        {:a {:a/X :a/X1
             :a/Y :a/Y1
             :vec [1 2 3]}}))

this declares a matcher for vector? that is not used in the inner matcher.

dchelimsky commented 4 years ago

Is this test supposed to pass?

(is (match?
        (m/match-with [map?    m/equals
                       vector? m/embeds]
          {:a (m/match-with [map? m/embeds]
                {:vec [1 2]
                 :a/X :a/X1})})
        {:a {:a/X :a/X1
             :a/Y :a/Y1
             :vec [1 2 3]}}))

this declares a matcher for vector? that is not used in the inner matcher.

@fernando-nubank No. The implementation doesn't support merging of match-with scopes. I documented this in the docstring for match-with.