chore(tests): New match-strict? cljs.test directive (#20825)

Equality checks in tests using = give a bad experience by default on test
failures containing nested data structures. We use the cljs.test directive
match? from matcher-combinators library to help compare nested structures. The
problem with match? is that its default matcher for maps (embeds) can be too
permissive, and this causes surprises.

Here we upgrade matcher-combinators to latest, where a new matcher called
nested-equals is available. This matcher won't allow extra keys in maps. This
matcher eliminates the need for manually adding nested equals matchers as we
have to do currently.

- Upgrades matcher-combinators from 3.8.8 to 3.9.1 (latest as of 2024-07-19)

What changes?

When asserting in tests, we now have the option to use match-strict? or match?.
Both directives are available by integrating with cljs.test. The code
implementing the new match-strict? directive was 100% copied from the library
matcher-combinators because we need to wrap the expected value ourselves with
matcher-combinators.matchers/nested-equals. It's ugly code, but it's how we can
integrate with cljs.test/assert-expr.
This commit is contained in:
Icaro Motta 2024-07-24 23:06:41 -03:00 committed by GitHub
parent 4989c92780
commit 60ad7c8a29
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 143 additions and 78 deletions

View File

@ -74,7 +74,8 @@
;; https://github.com/borkdude/clj-kondo/issues/867 ;; https://github.com/borkdude/clj-kondo/issues/867
:unresolved-symbol {:exclude [PersistentPriorityMap.EMPTY :unresolved-symbol {:exclude [PersistentPriorityMap.EMPTY
number number
legacy.status-im.test-helpers/restore-app-db]} legacy.status-im.test-helpers/restore-app-db
(cljs.test/is [match-strict?])]}
:unresolved-var {:level :error} :unresolved-var {:level :error}
:unsorted-required-namespaces {:level :error} :unsorted-required-namespaces {:level :error}
:unused-alias {:level :warning} :unused-alias {:level :warning}

View File

@ -711,11 +711,11 @@
}, },
{ {
"path": "nubank/matcher-combinators/3.8.8/matcher-combinators-3.8.8", "path": "nubank/matcher-combinators/3.9.1/matcher-combinators-3.9.1",
"host": "https://repo.clojars.org", "host": "https://repo.clojars.org",
"jar": { "jar": {
"sha1": "4c94bd510f0c18a20191e46dd6becedebc640bbd", "sha1": "f0830a112cae8ee931a90d9f39214a0ed4d44150",
"sha256": "0wpla2hx0s4mda58ndyd8938zmnz1gyhgfr6pzfphy27xfbvsssl" "sha256": "1rjcgqhms84xmnhs7sqcd3b2dpa37mmzgz2yz33yz2rdb9skl96g"
} }
}, },

View File

@ -77,7 +77,7 @@ net/cgrand/macrovich/0.2.1/macrovich-0.2.1.jar
net/java/dev/jna/jna/5.12.1/jna-5.12.1.jar net/java/dev/jna/jna/5.12.1/jna-5.12.1.jar
nrepl/bencode/1.1.0/bencode-1.1.0.jar nrepl/bencode/1.1.0/bencode-1.1.0.jar
nrepl/nrepl/1.0.0/nrepl-1.0.0.jar nrepl/nrepl/1.0.0/nrepl-1.0.0.jar
nubank/matcher-combinators/3.8.8/matcher-combinators-3.8.8.jar nubank/matcher-combinators/3.9.1/matcher-combinators-3.9.1.jar
org/apache/ant/ant/1.10.11/ant-1.10.11.jar org/apache/ant/ant/1.10.11/ant-1.10.11.jar
org/apache/ant/ant-launcher/1.10.11/ant-launcher-1.10.11.jar org/apache/ant/ant-launcher/1.10.11/ant-launcher-1.10.11.jar
org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar

View File

@ -19,7 +19,7 @@
[cider/piggieback "0.4.1"] [cider/piggieback "0.4.1"]
[org.slf4j/slf4j-nop "2.0.9"] [org.slf4j/slf4j-nop "2.0.9"]
[re-frisk-remote "1.6.0"] [re-frisk-remote "1.6.0"]
[nubank/matcher-combinators "3.8.8"] [nubank/matcher-combinators "3.9.1"]
;; Use the same version specified in the Nix dependency. ;; Use the same version specified in the Nix dependency.
[clj-kondo/clj-kondo "2024.03.13"] [clj-kondo/clj-kondo "2024.03.13"]

View File

@ -1,7 +1,6 @@
(ns status-im.contexts.wallet.data-store-test (ns status-im.contexts.wallet.data-store-test
(:require (:require
[cljs.test :refer-macros [deftest is testing]] [cljs.test :refer-macros [deftest is testing]]
[matcher-combinators.matchers :as matchers]
matcher-combinators.test matcher-combinators.test
[status-im.contexts.wallet.data-store :as sut])) [status-im.contexts.wallet.data-store :as sut]))
@ -162,82 +161,73 @@
(deftest reconcile-keypairs-test (deftest reconcile-keypairs-test
(testing "reconcile-keypairs represents updated key pairs and accounts" (testing "reconcile-keypairs represents updated key pairs and accounts"
(is (is
(match? (match-strict?
(matchers/match-with {:removed-keypair-ids #{}
[set? matchers/set-equals :removed-account-addresses #{}
map? matchers/equals] :updated-accounts-by-address {"1x123" (merge account
{:removed-keypair-ids #{} {:key-uid "0x123"
:removed-account-addresses #{} :address "1x123"})
:updated-accounts-by-address {"1x123" (merge account "1x456" (merge account
{:key-uid "0x123" {:key-uid "0x456"
:address "1x123"}) :address "1x456"
"1x456" (merge account :operable? false
{:key-uid "0x456" :operable :no})}
:address "1x456" :updated-keypairs-by-id {"0x123" {:key-uid "0x123"
:operable? false :type :seed
:operable :no})} :lowest-operability :fully
:updated-keypairs-by-id {"0x123" {:key-uid "0x123" :accounts [(merge account
:type :seed {:key-uid "0x123"
:lowest-operability :fully :address "1x123"})]}
:accounts [(merge account "0x456" {:key-uid "0x456"
{:key-uid "0x123" :type :key
:address "1x123"})]} :lowest-operability :no
"0x456" {:key-uid "0x456" :accounts [(merge account
:type :key {:key-uid "0x456"
:lowest-operability :no :address "1x456"
:accounts [(merge account :operable? false
{:key-uid "0x456" :operable :no})]}}}
:address "1x456"
:operable? false
:operable :no})]}}})
(sut/reconcile-keypairs [raw-keypair-seed-phrase (sut/reconcile-keypairs [raw-keypair-seed-phrase
raw-keypair-private-key])))) raw-keypair-private-key]))))
(testing "reconcile-keypairs represents removed key pairs and accounts" (testing "reconcile-keypairs represents removed key pairs and accounts"
(is (is
(match? (match-strict?
(matchers/match-with {:removed-keypair-ids #{"0x456"}
[set? matchers/set-equals :removed-account-addresses #{"1x456"}
map? matchers/equals] :updated-accounts-by-address {"1x123" (merge account
{:removed-keypair-ids #{"0x456"} {:key-uid "0x123"
:removed-account-addresses #{"1x456"} :address "1x123"})}
:updated-accounts-by-address {"1x123" (merge account :updated-keypairs-by-id {"0x123" {:key-uid "0x123"
{:key-uid "0x123" :type :seed
:address "1x123"})} :lowest-operability :fully
:updated-keypairs-by-id {"0x123" {:key-uid "0x123" :accounts [(merge account
:type :seed {:key-uid "0x123"
:lowest-operability :fully :address "1x123"})]}}}
:accounts [(merge account
{:key-uid "0x123"
:address "1x123"})]}}})
(sut/reconcile-keypairs [raw-keypair-seed-phrase (sut/reconcile-keypairs [raw-keypair-seed-phrase
(assoc raw-keypair-private-key :removed true)])))) (assoc raw-keypair-private-key :removed true)]))))
(testing "reconcile-keypairs ignores chat accounts inside updated accounts" (testing "reconcile-keypairs ignores chat accounts inside updated accounts"
(is (is
(match? (match-strict?
(matchers/match-with {:removed-keypair-ids #{}
[set? matchers/set-equals :removed-account-addresses #{}
map? matchers/equals] :updated-accounts-by-address {"2x000" (merge account
{:removed-keypair-ids #{} {:key-uid "0x000"
:removed-account-addresses #{} :address "2x000"
:updated-accounts-by-address {"2x000" (merge account :chat false
{:key-uid "0x000" :wallet true
:address "2x000" :default-account? true})}
:chat false :updated-keypairs-by-id {"0x000" {:key-uid "0x000"
:wallet true :type :profile
:default-account? true})} :lowest-operability :fully
:updated-keypairs-by-id {"0x000" {:key-uid "0x000" :accounts [(merge account
:type :profile {:key-uid "0x000"
:lowest-operability :fully :address "1x000"
:accounts [(merge account :chat true
{:key-uid "0x000" :wallet false
:address "1x000" :default-account? false})
:chat true (merge account
:wallet false {:key-uid "0x000"
:default-account? false}) :address "2x000"
(merge account :chat false
{:key-uid "0x000" :wallet true
:address "2x000" :default-account? true})]}}}
:chat false
:wallet true
:default-account? true})]}}})
(sut/reconcile-keypairs [raw-keypair-profile]))))) (sut/reconcile-keypairs [raw-keypair-profile])))))

View File

@ -0,0 +1,47 @@
(ns test-helpers.matchers
"Internal use. Don't require it directly."
(:require
[cljs.test :as test]
[matcher-combinators.core :as core]
[matcher-combinators.matchers :as matchers]
[matcher-combinators.parser]
[matcher-combinators.result :as result]))
;; This implementation is identical to `match?`, but wraps the expected value
;; with `nested-equals`. This differs from the default `embeds` matcher on maps,
;; where extra map keys are considered valid.
(defmethod test/assert-expr 'match-strict?
[_ msg form]
`(let [args# (list ~@(rest form))
[matcher# actual#] args#]
(cond
(not (= 2 (count args#)))
(test/do-report
{:type :fail
:message ~msg
:expected (symbol "`match-strict?` expects 2 arguments: a `matcher` and the `actual`")
:actual (symbol (str (count args#) " were provided: " '~form))})
(core/matcher? matcher#)
(let [result# (core/match (matchers/nested-equals matcher#) actual#)]
(test/do-report
(if (core/indicates-match? result#)
{:type :pass
:message ~msg
:expected '~form
:actual (list 'match? matcher# actual#)}
(with-file+line-info
{:type :fail
:message ~msg
:expected '~form
:actual (tagged-for-pretty-printing (list '~'not (list 'match? matcher# actual#))
result#)
:markup (::result/value result#)}))))
:else
(test/do-report
{:type :fail
:message ~msg
:expected (str "The first argument of `match-strict?` "
"needs to be a matcher (implement the match protocol)")
:actual '~form}))))

View File

@ -0,0 +1,24 @@
(ns test-helpers.matchers
"Some vars in this namespace solely exist to support the matchers.clj file."
(:require-macros test-helpers.matchers)
(:require
[cljs.test :as t]
[matcher-combinators.parser]
[matcher-combinators.printer :as printer]
[matcher-combinators.result :as result]))
(defrecord Mismatch [summary match-result])
(defn tagged-for-pretty-printing
[actual-summary result]
(->Mismatch actual-summary result))
(extend-protocol IPrintWithWriter
Mismatch
(-pr-writer [this writer _]
(-write writer (printer/as-string (-> this :match-result ::result/value)))))
(defn with-file+line-info
[report]
(merge (t/file-and-line (js/Error.) 4)
report))

View File

@ -12,7 +12,10 @@
[re-frame.events :as rf-events] [re-frame.events :as rf-events]
[re-frame.registrar :as rf-registrar] [re-frame.registrar :as rf-registrar]
[re-frame.subs :as rf-subs] [re-frame.subs :as rf-subs]
[taoensso.timbre :as log])) [taoensso.timbre :as log]
;; We must require this namespace to register the custom cljs.test directive `match-strict?`.
test-helpers.matchers))
(defn db (defn db
"A simple wrapper to get the latest value from the app db." "A simple wrapper to get the latest value from the app db."