Skip to content

Commit

Permalink
improve failure reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
seancorfield committed Sep 30, 2023
1 parent bda6978 commit a1766b8
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 95 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
Only accretive/fixative changes will be made from now on.

* 2.0.next in progress
* Improved failure reporting: most failures now provide an additional message describing the failure as well as improving how the expected and actual values are displayed (primarily hiding `=?` and showing a more accurate/intuitive test form).
* Update `deps.edn` to use `:main-args` (instead of `:main-opts`) for parameterized tasks in `build.clj` -- see [Running Tasks based on Aliases](https://clojure-doc.org/articles/cookbooks/cli_build_projects/)
* Update dependencies to latest stable versions.

Expand Down
155 changes: 81 additions & 74 deletions src/expectations/clojure/test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
(:require [clojure.data :as data]
[clojure.string :as str]
#?(:cljs [planck.core])
#?(:clj [clojure.test :as t]
#?(:clj [clojure.test :as t]
:cljs [cljs.test :include-macros true :as t])
#?(:clj [clojure.spec.alpha :as s])
#?(:cljs [cljs.spec.alpha :as s])
#?@(:cljs [pjstadig.humane-test-output
pjstadig.print
Expand All @@ -37,11 +38,12 @@
; this framework and running the :negative tests.
true)))

(defn ^:no-doc illegal-argument [s]
(#?(:clj IllegalArgumentException. :cljs js/Error.) s))

;; stub functions for :refer compatibility:
(defn- bad-usage [s]
`(throw (#?(:clj IllegalArgumentException.
:cljs js/Error.)
(str ~s " should only be used inside expect"))))
`(throw (illegal-argument (str ~s " should only be used inside expect"))))

(defmacro =?
"Internal fuzzy-equality test (clojure.test/assert-expr)."
Expand Down Expand Up @@ -124,12 +126,7 @@
(defn ^:no-doc spec?
"Detects whether an expected expression seems to be a Spec."
[e]
(and (keyword? e)
#?(:clj (try (require 'clojure.spec.alpha)
(when-let [get-spec (resolve 'clojure.spec.alpha/get-spec)]
(boolean (get-spec e)))
(catch Throwable _))
:cljs (boolean (s/get-spec e)))))
(and (keyword? e) (s/get-spec e)))

(defn ^:no-doc str-match
"Returns the match off of the beginning of two strings."
Expand All @@ -155,7 +152,8 @@
"\n>>> expected diverges: " (pr-str
(clojure.string/replace a in-both ""))
"\n>>> actual diverges: " (pr-str
(clojure.string/replace b in-both ""))))
(clojure.string/replace b in-both ""))
"\n"))

;; smart equality extension to clojure.test assertion -- if the expected form
;; is a predicate (function) then the assertion is equivalent to (is (e a))
Expand All @@ -167,66 +165,81 @@
#?(:clj [msg form] :cljs [menv msg form])
;; (is (=? val-or-pred expr))
(let [[_ e a form'] form
conform? (spec? e)]
conform? (boolean (spec? e))]
`(let [e# ~e
a# ~a
valid?# (when ~conform? #?(:clj (resolve 'clojure.spec.alpha/valid?)
:cljs s/valid?))
explain-str?# (when ~conform?
#?(:clj (resolve 'clojure.spec.alpha/explain-str)
:cljs s/explain-str))
r# (cond ~conform?
(valid?# e# a#)
(fn? e#)
(e# a#)
(isa? (type e#)
#?(:clj java.util.regex.Pattern
:cljs (type #"regex")))
(some? (re-find e# a#))
#?(:clj (and (class? e#) (class? a#))
:cljs false) ; maybe figure this out later
(isa? a# e#) ; (expect parent child)
#?(:clj (class? e#)
:cljs false) ; maybe figure this out later
(instance? e# a#) ; (expect klazz object)
:else
(= e# a#))
f# ~form'
valid?# (when ~conform? s/valid?)
explain-str?# (when ~conform? s/explain-str)
[r# m# ef# af#]
(cond ~conform?
[(valid?# e# a#)
(explain-str?# e# a#)
(list '~'s/valid? '~e '~a)
(list '~'not (list '~'s/valid? '~e a#))]
(fn? e#)
[(e# a#)
(str '~a " did not satisfy " '~e "\n")
(list '~e '~a)
(list '~'not (list '~e a#))]
(isa? (type e#)
#?(:clj java.util.regex.Pattern
:cljs (type #"regex")))
[(some? (re-find e# a#))
(str (pr-str a#) " did not match " (pr-str e#) "\n")
(list '~'re-find '~e '~a)
(list '~'not (list '~'re-find e# a#))]
#?(:clj (and (class? e#) (class? a#))
:cljs false) ; maybe figure this out later
[(isa? a# e#) ; (expect parent child)
(str a# " is not derived from " e# "\n")
(list '~'isa? '~a '~e)
(list '~'not (list '~'isa? a# e#))]
#?(:clj (class? e#)
:cljs false) ; maybe figure this out later
[(instance? e# a#) ; (expect klazz object)
(str a# " (" (class a#) ") is not an instance of " e# "\n")
(list '~'instance? '~e '~a)
(class a#)]
:else
[(= e# a#)
(when (and (string? e#) (string? a#) (not= e# a#))
(let [[_# _# in-both#] (str-diff e# a#)]
(str-msg e# a# in-both#)))
(list '~'= '~e '~a)
(list '~'not= e# a#)])
humane?# (and humane-test-output? (not (fn? e#)) (not ~conform?))]
(if r#
(t/do-report {:type :pass, :message ~msg,
:expected '~form, :actual (if (fn? e#)
(list '~e a#)
a#)})
(t/do-report
(let [[_# _# in-both# :as diff-vec#]
(when (and (string? e#) (string? a#)) (str-diff e# a#))]
{:type :fail,
:message (if ~conform?
(if ~msg
(str ~msg "\n" (explain-str?# e# a#))
(explain-str?# e# a#))
(if diff-vec#
; Both e# an a# are strings, put a nice
; comparison in the msg.
(str ~msg "\n" (str-msg e# a# in-both#) "\n")
~msg)),
:diffs (if humane?#
[[a# (take 2 (data/diff e# a#))]]
[])
:expected (cond humane?#
e#
~form'
~form'
:else
'~form)
:actual (cond (fn? e#)
(list '~'not (list '~e a#))
humane?#
[a#]
{:type :fail,
:message (if m# (if ~msg (str ~msg "\n" m#) m#) ~msg)
:diffs (if humane?#
[[a# (take 2 (data/diff e# a#))]]
[])
:expected (cond humane?#
e#
f#
f#
ef#
ef#
:else
(list '~'not (list '~'=? e# a#)))})))
'~form)
:actual (cond af#
af#
humane?#
[a#]
:else
(list '~'not (list '~'=? e# a#)))}))
r#)))

(comment
(data/diff "foo" ["bar"])
)

(defmacro ^:no-doc ?
"Wrapper for forms that might throw an exception so exception class names
can be used as predicates. This is only needed for `more->` so that you can
Expand Down Expand Up @@ -351,13 +364,9 @@
(if (map? e#)
(let [submap# (select-keys a# (keys e#))]
(t/is (~'=? e# submap# '~form) ~msg'))
(throw (#?(:clj IllegalArgumentException.
:cljs js/Error.)
"'in' requires map or sequence")))
(throw (illegal-argument "'in' requires map or sequence")))
:else
(throw (#?(:clj IllegalArgumentException.
:cljs js/Error.)
"'in' requires map or sequence")))))
(throw (illegal-argument "'in' requires map or sequence")))))

(and (sequential? e) (= 'more (first e)))
(let [sa (gensym)
Expand Down Expand Up @@ -393,15 +402,15 @@
"#object[Function]")))))
#?(:clj (if (isa? (resolve e) Throwable)
`(t/is (~'thrown? ~e ~a) ~msg')
`(t/is (~'instance? ~e ~a) ~msg'))
`(t/is (~'=? ~e ~a) ~msg'))
:cljs (if (= 'js/Error e)
`(t/is (~'thrown? ~e ~a) ~msg')
`(t/is (~'instance? ~e ~a) ~msg')))
`(t/is (~'=? ~e ~a) ~msg')))

(isa? (type e)
#?(:clj java.util.regex.Pattern
:cljs (type #"regex")))
`(t/is (re-find ~e ~a) ~msg')
;; (isa? (type e)
;; #?(:clj java.util.regex.Pattern
;; :cljs (type #"regex")))
;; `(t/is (re-find ~e ~a) ~msg')

:else
`(t/is (~'=? ~e ~a) ~msg')))))
Expand Down Expand Up @@ -452,9 +461,7 @@
by name will return `nil`."
[fn-vec & forms]
(when-not (vector? fn-vec)
(throw (#?(:clj IllegalArgumentException.
:cljs js/Error.)
"side-effects requires a vector as its first argument")))
(throw (illegal-argument "side-effects requires a vector as its first argument")))
(let [mocks (reduce (fn [m f-spec]
(if (vector? f-spec)
(assoc m (first f-spec) (second f-spec))
Expand Down
42 changes: 21 additions & 21 deletions test/expectations/clojure/test_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
(is (sut/expect "foo" (str "f" "oo"))))

(deftest ^:negative not-equality-test
(is-not' (sut/expect 2 (* 1 1)) (not (=? 2 1)))
(is-not' (sut/expect "fool" (str "f" "oo")) (not (=? "fool" "foo"))))
(is-not' (sut/expect 2 (* 1 1)) (not= 2 1))
(is-not' (sut/expect "fool" (str "f" "oo")) (not= "fool" "foo")))

(deftest ^:negative message-test
(is-not' (sut/expect even? (+ 1 1 1) "It's uneven!")
Expand All @@ -48,10 +48,10 @@
(not (empty? [1]))
#"full")
(is-not' (sut/expect 2 (* 1 1) "One times one isn't two?")
(not (=? 2 1))
(not= 2 1)
#"isn't two")
(is-not' (sut/expect "fool" (str "f" "oo") "No fooling around!")
(not (=? "fool" "foo"))
(not= "fool" "foo")
#"fooling"))

(deftest regex-test
Expand Down Expand Up @@ -103,7 +103,7 @@
(s/def :small/value (s/and pos-int? #(< % 100)))
(deftest spec-test
(is (sut/expect :small/value (* 13 4)))
(is-not' (sut/expect :small/value (* 13 40)) (not (=? :small/value 520))))))
(is-not' (sut/expect :small/value (* 13 40)) (not (s/valid? :small/value 520))))))
(catch Throwable _
(println "\nOmitting Spec tests for Clojure" (clojure-version)))))

Expand All @@ -122,22 +122,22 @@
(is (nil? (sut/expect :foo (in [:bar :foo])))))

(deftest ^:negative not-collection-test
(is-not' (sut/expect {:foo 1} (in {:foo 2 :cat 4})) (not (=? {:foo 1} {:foo 2}))))
(is-not' (sut/expect {:foo 1} (in {:foo 2 :cat 4})) (not= {:foo 1} {:foo 2}))

;; TODO: need better tests here
(deftest grouping-more-more-of-from-each
(sut/expecting "numeric behavior"
(sut/expect (more-of {:keys [a b]}
even? a
odd? b)
{:a (* 2 13) :b (* 3 13)})
(sut/expect pos? (* -3 -5)))
(sut/expecting "string behavior"
(sut/expect (more #"foo" "foobar" #(str/starts-with? % "f"))
(str "f" "oobar"))
(sut/expect #"foo"
(from-each [s ["l" "d" "bar"]]
(str "foo" s)))))
(deftest grouping-more-more-of-from-each
(sut/expecting "numeric behavior"
(sut/expect (more-of {:keys [a b]}
even? a
odd? b)
{:a (* 2 13) :b (* 3 13)})
(sut/expect pos? (* -3 -5)))
(sut/expecting "string behavior"
(sut/expect (more #"foo" "foobar" #(str/starts-with? % "f"))
(str "f" "oobar"))
(sut/expect #"foo"
(from-each [s ["l" "d" "bar"]]
(str "foo" s))))))

(deftest more-evals-once
(let [counter (atom 1)]
Expand Down Expand Up @@ -204,7 +204,7 @@
(deftest string-test
(is (= "abc" (sut/str-match "abcdef" "abcefg")))
(is (= ["def" "efg" "abc"] (sut/str-diff "abcdef" "abcefg")))
(is (= "matches: \"abc\"\n>>> expected diverges: \"def\"\n>>> actual diverges: \"efg\""
(is (= "matches: \"abc\"\n>>> expected diverges: \"def\"\n>>> actual diverges: \"efg\"\n"
(sut/str-msg "abcdef" "abcefg" "abc"))))

; Test use of string compare routines as well as actual form in message
Expand All @@ -213,7 +213,7 @@

#?(:clj (deftest ^:negative string-compare-failure-test
(is-not' (sut/expect "abcdef" (str "abc" "efg"))
(not (=? "abcdef" "abcefg"))
(not= "abcdef" "abcefg")
#"(?is)(str \"abc\" \"efg\").*matches: \"abc\""))
:cljs (deftest string-compare-failure-test
(is-not' (sut/expect "abcdef" (str "abc" "efg"))
Expand Down

0 comments on commit a1766b8

Please sign in to comment.