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

Parts of test body can be evaluated multiple times after deftest is reevaluated #2233

Closed
xiongtx opened this issue Mar 8, 2018 · 2 comments · Fixed by clojure-emacs/cider-nrepl#520

Comments

@xiongtx
Copy link
Member

xiongtx commented Mar 8, 2018

Expected behavior

No part of test body is evaluated multiple times after deftest is re-evaluated.

Actual behavior

Parts of test body can be evaluated multiple times after deftest is re-evaluated.

Steps to reproduce the problem

Given the following:

(ns foo
  (:require [clojure.test :refer :all]))

(def a (atom 0))

(deftest foo-test
  (is (= (do (swap! a inc)
             (println "a: " @a)
             @a)
         (do (println "a 2: " @a)
             @a))))

Running foo-test passes:

a:  1
a 2:  1

;; again
a: 2
a 2: 2

Re-evaluating foo-test, however, either via the CIDER REPL or cider-eval-defun-at-point, causes the first arg to is to be evaluated multiple times and the test to fail:

;; fail
a 2:  2
a:  3
a:  4
a:  5
a:  6

This does not happen w/ lein repl.

Note: I came across this while investigating #1936. I suspect resolving this will go some way towards resolving that.

Environment & Version information

CIDER version information

;; CIDER 0.17.0snapshot (package: 20180131.913), nREPL 0.2.13
;; Clojure 1.8.0, Java 1.8.0_151

Lein/Boot version

Leiningen 2.8.1

Emacs version

GNU Emacs 26.0.91

Operating system

Darwin C02RW05YFVH6 15.6.0

@niklasfasching
Copy link

This is a bug in cider-nrepl test extenions =-body.
cider-nrepl extends clojure.test with custom behavior for (is (= ...)) expressions. In this extension the first form of (= x y) is evaluated multiple times.

Your examples can be made to pass by wrapping the is body inside a do so the clojure.test/assert-expr multimethod does not dispatch on = to =-body

;; is-body wrapped in do to prevent dispatch to =-body
(let [a (atom 0)]
  (is (do (= (do (swap! a inc)
                 (println "a: " @a)
                 @a)
             (do (println "a 2: " @a)
                 @a))))) ;; => true

Minimal reproduction

lein repl:

;; impure form first
user=> (let [x (atom 0)]
  #_=>   (is (= (swap! x inc) 1))
  #_=>   @x)
1

;; impure form second
user=> (let [x (atom 0)]
  #_=>   (is (= 1 (swap! x inc)))
  #_=>   @x)
1

cider repl:

;; impure form first
cider> (let [x (atom 0)]
         (is (= (swap! x inc) 1))
         @x)
2

;; impure form second
cider> (let [x (atom 0)]
         (is (= 1 (swap! x inc)))
         @x)
1

I'll submit a PR to cider-nrepl later on and link this issue

@xiongtx
Copy link
Member Author

xiongtx commented Apr 1, 2018

Ah, that makes sense. And thanks for putting up a PR to fix it!

xiongtx pushed a commit to clojure-emacs/cider-nrepl that referenced this issue Apr 1, 2018
(is (= ...)) tests executed in cider currently fail if the first
argument to = is not pure as it is evaluated multiple times inside
`=-body`. By caching the value, this can be prevented.

This fixes clojure-emacs/cider/issues/2233
arrdem pushed a commit to arrdem/cider-nrepl that referenced this issue Apr 3, 2018
(is (= ...)) tests executed in cider currently fail if the first
argument to = is not pure as it is evaluated multiple times inside
`=-body`. By caching the value, this can be prevented.

This fixes clojure-emacs/cider/issues/2233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants