Skip to content

Commit 58cff26

Browse files
authored
Evalue (is (= exp act)) only once when failing the equality test (#713)
Hi, could you please review patch to have `(is (= exp act)` evaluate its argument only once when failing. It fixes #712. Previously, the arguments were evaluated once in the `when` close, and then once more in the `:actual` and `:expected` keys. I haven't introduced a test case, because this issue manifest itself only when the test fails, which inadvertently will fail the test suite. Thus I was expecting perhaps you could reason about its behavior for the review? (Though now that I think of it, it might be possible to write a test case for the `gen-assert` multimethod rather than the `is` case which will fail the test suite) Here are the result running the same test in #712 with the fix, it only prints `:5` once as expected this time: ``` clojure basilisp.user=> (require '[basilisp.test :refer [deftest is]]) nil basilisp.user=> (deftest issue (is (= 8 (println :5)))) #'basilisp.user/issue basilisp.user=> (issue) :5 {:failures [{:actual nil :message "Test failure: (= 8 (println :5))" :test-section nil :type :failure :expr (= 8 (println :5)) :expected 8 :test-name "issue" :line 2}]} ``` Thanks Co-authored-by: ikappaki <[email protected]>
1 parent 3351d3b commit 58cff26

File tree

2 files changed

+14
-11
lines changed

2 files changed

+14
-11
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
* Fix incompatibility with `(str nil)` returning "nil" (#706).
1111
* Fix `sort-by` support for maps and boolean comparator fns (#709).
1212
* Fix `sort` support for maps and boolean comparator fns (#711).
13+
* Fix `(is (= exp act))` should only evaluate its args once on failure (#712).
1314

1415
## [v0.1.0a2]
1516
### Added

src/basilisp/test.lpy

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,19 @@
7878

7979
(defmethod gen-assert '=
8080
[expr msg line-num]
81-
`(when-not ~expr
82-
(vswap! *test-failures*
83-
conj
84-
{:test-name *test-name*
85-
:test-section *test-section*
86-
:message ~msg
87-
:expr (quote ~expr)
88-
:actual ~(nth expr 2)
89-
:expected ~(second expr)
90-
:line ~line-num
91-
:type :failure})))
81+
`(let [actual# ~(nth expr 2)
82+
expected# ~(second expr)]
83+
(when-not (= expected# actual#)
84+
(vswap! *test-failures*
85+
conj
86+
{:test-name *test-name*
87+
:test-section *test-section*
88+
:message ~msg
89+
:expr (quote ~expr)
90+
:actual actual#
91+
:expected expected#
92+
:line ~line-num
93+
:type :failure}))))
9294

9395
(defmethod gen-assert 'thrown?
9496
[expr msg line-num]

0 commit comments

Comments
 (0)