marick / Midje

Midje provides a migration path from clojure.test to a more flexible, readable, abstract, and gracious style of testing
MIT License
1.69k stars 129 forks source link

Mocking function calls in records is temperamental #422

Closed philomates closed 6 years ago

philomates commented 7 years ago

Mocking functions that are called within record instances seems to be very temperamental

For instance, given the following definitions:

(defprotocol Caller
  (call [caller x]))

(defrecord ACaller [func]
  Caller
  (call [_ x] (func x)))

(defn fake-func [arg] 'shouldnt-return)

The following tests that define the record instance inside of the fact pass:

(facts "Mock a func call inside an inline record"
  (against-background [(fake-func 1) => 100]
    (fact "against-background"
      (call (->ACaller fake-func) 1) => 100))

  (fact "provided"
    (call (->ACaller fake-func) 1) => 100
    (provided (fake-func 1) => 100)))

But if we define the record instance outside of the fact the provided/against-background overrides no longer work, causing the tests to faile

(def caller-instance (->ACaller fake-func))
(facts "Mock a func call inside a record reference"
  (against-background [(fake-func 1) => 100]
    (fact "against-background"
      (call caller-instance 1) => 100))

  (fact "provided"
    (call caller-instance 1) => 100
    (provided (fake-func 1) => 100)))

Failing tests on a dev branch: https://github.com/phillipm/Midje/blob/7388e48c9a78d8fea37e13867d8e7f7193612312/test/as_documentation/about_defrecord/using_refer__provided_tests/test.clj#L42-L50

marick commented 7 years ago

You've probably seen this page: https://github.com/marick/Midje/wiki/Prerequisites-and-protocols

If not, it's relevant.

philomates commented 6 years ago

Thanks for the pointer; I had forgotten about that doc. Strangely, if I update the tests above to use defrecord-openly they still fail in the same way.

I still haven't gotten a chance to dive into this or understand the constraints and ideas behind defrecord-openly. That said, I would expected this to work, regardless of being a defrecord or defrecord-openly, because I'm not mocking a protocol method but rather a function called within the method.

philomates commented 6 years ago

It turns out that the issue I posted isn't specific to records. It has more to do with binding variables to new variables before the redef logic has a chance to kick in.

For example, the following code also exhibits the same issue:

(defn hof [f]
  (fn [x] (f x)))
(defn my-incr [x] (+ 1 x))
(def obfuscated-incr (hof my-incr))

(fact "fails to pass because `my-incr` gets bound to `f` in `hof`"
  (obfuscated-incr 1) => 20
  (provided (my-incr 1) => 20))

(fact "passes because `my-incr` gets redefed"
  ((hof my-incr) 1) => 20
  (provided (my-incr 1) => 20))

As far as I can tell, creating logic to allow for mocking in these contexts would be quite complicated (control-flow static analysis or memory-level manipulations) and hence not worth it given the gains.