slagyr / speclj

pronounced "speckle": a TDD/BDD framework for Clojure.
MIT License
459 stars 58 forks source link

Inconsistent and confusing behavior with with-redefs #126

Open eigenhombre opened 9 years ago

eigenhombre commented 9 years ago

lein spec and lein spec -a treat the following differently; the test fails in the former, and passes in the latter.

(ns myapp.test
  (:require [speclj.core :refer :all]))

(def db "database-a")

(with-redefs [db "database-b"]
  (describe "db"
    (it "should be called database-b"
      (should= db "database-b"))))

In general, we find that running lein spec requires with-redefs to be specified within it blocks (and lein spec -a doesn't), making it harder to mock out e.g. a database connection for a whole set of tests. Can this be improved somehow?

trptcolin commented 9 years ago

Usually I've used around for this, something like

(ns myapp.test
  (:require [speclj.core :refer :all]))

(def db "database-a")

(describe "db"
  (around [it] (with-redefs [db "database-b"] (it)))

  (it "should be called database-b"
    (should= db "database-b")))

Not sure why the difference between lein spec -a and lein spec, or if this gets rid of the problem completely, but that's at least a workaround in case you're not familiar with around.

thesoftwarephilosopher commented 9 years ago

I wonder if this could have to do with Speclj internally requiring a different version of Clojure when run with the vigilant runner than with the standard, so that, when lein spec is run, certain forms do nothing when rebound with with-redefs and require binding instead.

eigenhombre commented 9 years ago

@trptcolin thanks for the suggestion. That works; however, the following does not:

(ns myapp.test
  (:require [myapp.test :refer :all]
            [speclj.core :refer :all]))

(def db "database-a")

(defmacro describe-with-db [text & body]
  `(describe ~text
     (around [it] (with-redefs [db "database-b"] (it)))
     ~@body))

(describe-with-db "db"
  (it "should be called database-b"
    (should= db "database-b")))

Throws

1) java.lang.RuntimeException: Can't use qualified name as parameter: speclj.core/it, compiling:(myapp/test.clj:20:1) [...]

-- this more closely matches what we want to do (dry out DB mocking to use an in-memory test DB rather than a "real" one).

Any suggestions (I know you know one or two things about macros AND speclj :-) )? Thanks in advance!

thesoftwarephilosopher commented 9 years ago

@eigenhombre I believe this is solved by changing db in your with-redefs with #'db (which is short-hand for (var-get db)).

eigenhombre commented 9 years ago

@sdegutis It's complaining about it, not db ... it looks like one of the macros is expanding into (let [it ...] ...) which wants a gensym inside the (remaining) macro.

trptcolin commented 9 years ago

@eigenhombre Yep! it -> it# (auto-gensym), as below:

(ns myapp.test
  (:require [myapp.test :refer :all]
            [speclj.core :refer :all]))

(def db "database-a")

(defmacro describe-with-db [text & body]
  `(describe ~text
     (around [it#] (with-redefs [db "database-b"] (it#)))
     ~@body))

(describe-with-db "db"
  (it "should be called database-b"
    (should= db "database-b")))

Does that do the trick for you?

eigenhombre commented 9 years ago

@trptcolin works for our minimal test case -- trying it in our app. I also called it foo# instead of it# and that also worked. So I guess the name is just a temporarily alias for all the forms in the body?

trptcolin commented 9 years ago

Yep, inside around, it (or whatever you name it) is just a function that represents all the tests to be executed inside that describe/context.

eigenhombre commented 9 years ago

@trptcolin We're unblocked -- that worked for us. I guess there's still the open question of why this is only needed with lein spec and not lein spec -a -- anyways, thanks for the help!

TheTedHogan commented 8 years ago

I'm running into the same issue. It seems to be a difference between the standard runner and the vigilant runner. Using around works, but I'm also curious why the runners are treating with-redefs differently.