Skip to content

Commit

Permalink
chore(tests): New match-strict? cljs.test directive (#20825)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ilmotta
ilmotta committed Jul 28, 2024
1 parent 5d4e12d commit 4b3dfc6
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 78 deletions.
3 changes: 2 additions & 1 deletion .clj-kondo/config.edn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@
;; https://github.com/borkdude/clj-kondo/issues/867
:unresolved-symbol {:exclude [PersistentPriorityMap.EMPTY
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}
:unsorted-required-namespaces {:level :error}
:unused-alias {:level :warning}
Expand Down
6 changes: 3 additions & 3 deletions nix/deps/clojure/deps.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"jar": {
"sha1": "4c94bd510f0c18a20191e46dd6becedebc640bbd",
"sha256": "0wpla2hx0s4mda58ndyd8938zmnz1gyhgfr6pzfphy27xfbvsssl"
"sha1": "f0830a112cae8ee931a90d9f39214a0ed4d44150",
"sha256": "1rjcgqhms84xmnhs7sqcd3b2dpa37mmzgz2yz33yz2rdb9skl96g"
}
},

Expand Down
2 changes: 1 addition & 1 deletion nix/deps/clojure/deps.list
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
nrepl/bencode/1.1.0/bencode-1.1.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-launcher/1.10.11/ant-launcher-1.10.11.jar
org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar
Expand Down
2 changes: 1 addition & 1 deletion shadow-cljs.edn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
[cider/piggieback "0.4.1"]
[org.slf4j/slf4j-nop "2.0.9"]
[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.
[clj-kondo/clj-kondo "2024.03.13"]
Expand Down
132 changes: 61 additions & 71 deletions src/status_im/contexts/wallet/data_store_test.cljs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
(ns status-im.contexts.wallet.data-store-test
(:require
[cljs.test :refer-macros [deftest is testing]]
[matcher-combinators.matchers :as matchers]
matcher-combinators.test
[status-im.contexts.wallet.data-store :as sut]))

Expand Down Expand Up @@ -162,82 +161,73 @@
(deftest reconcile-keypairs-test
(testing "reconcile-keypairs represents updated key pairs and accounts"
(is
(match?
(matchers/match-with
[set? matchers/set-equals
map? matchers/equals]
{:removed-keypair-ids #{}
:removed-account-addresses #{}
:updated-accounts-by-address {"1x123" (merge account
{:key-uid "0x123"
:address "1x123"})
"1x456" (merge account
{:key-uid "0x456"
:address "1x456"
:operable? false
:operable :no})}
:updated-keypairs-by-id {"0x123" {:key-uid "0x123"
:type :seed
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x123"
:address "1x123"})]}
"0x456" {:key-uid "0x456"
:type :key
:lowest-operability :no
:accounts [(merge account
{:key-uid "0x456"
:address "1x456"
:operable? false
:operable :no})]}}})
(match-strict?
{:removed-keypair-ids #{}
:removed-account-addresses #{}
:updated-accounts-by-address {"1x123" (merge account
{:key-uid "0x123"
:address "1x123"})
"1x456" (merge account
{:key-uid "0x456"
:address "1x456"
:operable? false
:operable :no})}
:updated-keypairs-by-id {"0x123" {:key-uid "0x123"
:type :seed
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x123"
:address "1x123"})]}
"0x456" {:key-uid "0x456"
:type :key
:lowest-operability :no
:accounts [(merge account
{:key-uid "0x456"
:address "1x456"
:operable? false
:operable :no})]}}}
(sut/reconcile-keypairs [raw-keypair-seed-phrase
raw-keypair-private-key]))))
(testing "reconcile-keypairs represents removed key pairs and accounts"
(is
(match?
(matchers/match-with
[set? matchers/set-equals
map? matchers/equals]
{:removed-keypair-ids #{"0x456"}
:removed-account-addresses #{"1x456"}
:updated-accounts-by-address {"1x123" (merge account
{:key-uid "0x123"
:address "1x123"})}
:updated-keypairs-by-id {"0x123" {:key-uid "0x123"
:type :seed
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x123"
:address "1x123"})]}}})
(match-strict?
{:removed-keypair-ids #{"0x456"}
:removed-account-addresses #{"1x456"}
:updated-accounts-by-address {"1x123" (merge account
{:key-uid "0x123"
:address "1x123"})}
:updated-keypairs-by-id {"0x123" {:key-uid "0x123"
:type :seed
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x123"
:address "1x123"})]}}}
(sut/reconcile-keypairs [raw-keypair-seed-phrase
(assoc raw-keypair-private-key :removed true)]))))
(testing "reconcile-keypairs ignores chat accounts inside updated accounts"
(is
(match?
(matchers/match-with
[set? matchers/set-equals
map? matchers/equals]
{:removed-keypair-ids #{}
:removed-account-addresses #{}
:updated-accounts-by-address {"2x000" (merge account
{:key-uid "0x000"
:address "2x000"
:chat false
:wallet true
:default-account? true})}
:updated-keypairs-by-id {"0x000" {:key-uid "0x000"
:type :profile
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x000"
:address "1x000"
:chat true
:wallet false
:default-account? false})
(merge account
{:key-uid "0x000"
:address "2x000"
:chat false
:wallet true
:default-account? true})]}}})
(match-strict?
{:removed-keypair-ids #{}
:removed-account-addresses #{}
:updated-accounts-by-address {"2x000" (merge account
{:key-uid "0x000"
:address "2x000"
:chat false
:wallet true
:default-account? true})}
:updated-keypairs-by-id {"0x000" {:key-uid "0x000"
:type :profile
:lowest-operability :fully
:accounts [(merge account
{:key-uid "0x000"
:address "1x000"
:chat true
:wallet false
:default-account? false})
(merge account
{:key-uid "0x000"
:address "2x000"
:chat false
:wallet true
:default-account? true})]}}}
(sut/reconcile-keypairs [raw-keypair-profile])))))
47 changes: 47 additions & 0 deletions src/test_helpers/matchers.clj
View file
Original file line number Diff line number Diff line change
@@ -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}))))
24 changes: 24 additions & 0 deletions src/test_helpers/matchers.cljs
View file
Original file line number Diff line number Diff line change
@@ -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))
5 changes: 4 additions & 1 deletion src/test_helpers/unit.cljs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
[re-frame.events :as rf-events]
[re-frame.registrar :as rf-registrar]
[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
"A simple wrapper to get the latest value from the app db."
Expand Down

0 comments on commit 4b3dfc6

Please sign in to comment.