Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deep-diff not working for expectations.clojure.test #435

Closed
AlexChalk opened this issue May 14, 2024 · 6 comments · Fixed by #437
Closed

deep-diff not working for expectations.clojure.test #435

AlexChalk opened this issue May 14, 2024 · 6 comments · Fixed by #437

Comments

@AlexChalk
Copy link
Contributor

I'm getting the

Expected:
  (=
   {:huge "map"}
   result)
Actual:
  (not=
   {:huge "map"}
   {:slightly "different" :huge "map"}
...

style output when I use kaocha with https://github.com/clojure-expectations/expectations.

Versions:

org.clojure/clojure {:mvn/version "1.11.3"}
org.clojure/tools.cli {:mvn/version "1.1.230"}
org.clojure/tools.logging {:mvn/version "1.3.0"}
com.github.seancorfield/expectations {:mvn/version "2.1.201"}
lambdaisland/kaocha {:mvn/version "1.89.1380"}

With regular clojure.test assertions:

Test file:

(ns my.test.ns
  (:require
   [clojure.test :refer [deftest is]]))

(deftest test
  (is (= {:foo "bar"
          :baz "quux"}
         {:foo "bar"
          :baz "quuux"})))

Output:

./bin/kaocha
[(F)]
Randomized with --seed 943555300

FAIL in my.test.ns (ns_test.clj:6)
{:foo "bar", :baz "quuux"}

Expected:
  {:baz "quux", :foo "bar"}
Actual:
  {:baz -"quux" +"quuux", :foo "bar"}
1 tests, 1 assertions, 1 failures.

With clojure-expectations:

Test file:

(ns my.test.ns
  (:require
   [expectations.clojure.test :refer [defexpect expect]]))

(defexpect my-test
  (expect {:foo "bar"
           :baz "quux"}
          {:foo "bar"
           :baz "quuux"}))

Output:

./bin/kaocha
[(F)]
Randomized with --seed 943555300

FAIL in my.test.ns (ns_test.clj:6)
{:foo "bar", :baz "quuux"}

Expected:
  (= {:baz "quux", :foo "bar"} {:baz "quuux", :foo "bar"})
Actual:
  (not= {:baz "quux", :foo "bar"} {:baz "quuux", :foo "bar"})
1 tests, 1 assertions, 1 failures.

I'm opening an issue as the docs suggest clojure-expectations is supported: https://cljdoc.org/d/lambdaisland/kaocha/1.0.861/doc/1-introduction#1-introduction.

Let me know if there's anything I can do to help debug. Thanks!

@plexus
Copy link
Member

plexus commented May 15, 2024

Kaocha is "compatible" with expectations because expectations is compatible with clojure.test. It seems in this case they deviate from the clojure.test behavior.

I would try running with the debug reporter and comparing the output.

@plexus
Copy link
Member

plexus commented May 15, 2024

I think it might actually be an easy fix, try adding this to report.clj (see lines 276-280)

(defmethod print-expr 'not= [m]
  (print-expression m))

@AlexChalk
Copy link
Contributor Author

AlexChalk commented May 15, 2024

Thanks for the quick reply. I've taken a look at report.clj and done some println debugging:

Expectations diverges from clojure-test here:

(defn print-expression [m]
(let [printer (output/printer)]
;; :actual is of the form (not (= ...))
(if (and (not= (:type m) ::one-arg-eql)
(seq? (second (:actual m)))
(> (count (second (:actual m))) 2))
(let [[_ expected & actuals] (-> m :actual second)]

The if passes for clojure-test but not for expectations, which fails to satisfy (seq? (second (:actual m))).

  • clojure test outputs (not (=...., so (=... satisfies seq?.
  • expectations outputs (not= val, which doesn't.

As a solution, what do you think about adding a new multimethod like this?

(defmulti get-actual
  (fn [m] (first (:actual m))))

;; e.g. (not= ...)
(defmethod get-actual 'not= [m]
  (-> m :actual))

;; e.g. (not (= ...))
(defmethod get-actual :default [m]
  (-> m :actual second))

(defn print-expression [m]
  (let [printer (output/printer)]
    (if (and (not= (:type m) ::one-arg-eql)
             (seq? (get-actual m))
             (> (count (get-actual m)) 2))

      (let [[_ expected & actuals] (get-actual m)]
        (output/print-doc

I'd be happy to contribute this PR—I'll have some time next week. If that sounds good, can you point me to any development docs you have?

@plexus
Copy link
Member

plexus commented May 16, 2024

Go for it!

@AlexChalk
Copy link
Contributor Author

This was pretty short so I just submitted the PR. Tests pass locally, but it's failing one that seems unrelated in CI.

@AlexChalk
Copy link
Contributor Author

Any advice on the failing workflow?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants