Skip to content

Instantly share code, notes, and snippets.

@mourjo
Forked from gfredericks/with-local-redefs.clj
Last active October 10, 2023 13:25
Show Gist options
  • Star 5 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save mourjo/c7fc03e59eb96f8a342dfcabd350a927 to your computer and use it in GitHub Desktop.
Save mourjo/c7fc03e59eb96f8a342dfcabd350a927 to your computer and use it in GitHub Desktop.
thread-local version of with-redefs
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Commentary ;;
;; ;;
;; The goal for writing this started with the idea to have tests run in ;;
;; parallel using the leiningen plugin eftest ;;
;; https://github.com/weavejester/eftest. ;;
;; ;;
;; With tests using with-redefs, it was not possible to run them in ;;
;; parallel if they were changing the root binding of the same ;;
;; vars. Here, we are binding the root of the var to one function that ;;
;; respects per-thread rebindings, if any exist. ;;
;; ;;
;; Known caveats: ;;
;; - This per-therad rebinding will only work with clojure concurrency ;;
;; primitives which copy per-thread bindings to newly spawned threads, ;;
;; eg, using clojure futures. But will not work for, say a ;;
;; java.lang.Thread. ;;
;; - As of now this only supports functions being bound and not other ;;
;; vars which store values, say (def x 19) for example. ;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(def ^:dynamic local-redefinitions {})
(defn current->original-definition
[v]
(when (var? v)
(get (meta v) ::original)))
(defn redefiniton-fn
[a-var]
(fn [& args]
(let [current-f (get local-redefinitions
a-var
(current->original-definition a-var))]
(apply current-f args))))
(defn dynamic-redefs
[vars func]
(let [un-redefs (remove #(::already-bound? (meta %)) vars)]
(doseq [a-var un-redefs]
(locking a-var
(when-not (::already-bound? (meta a-var))
(let [old-val (.getRawRoot ^clojure.lang.Var a-var)]
(.bindRoot ^clojure.lang.Var a-var
(redefiniton-fn a-var))
(alter-meta! a-var
(fn [m]
(assoc m
::already-bound? true
::original old-val))))))))
(func))
(defn xs->map
[xs]
(reduce (fn [acc [k v]] (assoc acc `(var ~k) v))
{}
(partition 2 xs)))
(defmacro with-dynamic-redefs
[bindings & body]
;; @TODO: Add support for non-functions
(let [map-bindings (xs->map bindings)]
`(let [old-rebindings# local-redefinitions]
(binding [local-redefinitions (merge old-rebindings# ~map-bindings)]
(dynamic-redefs ~(vec (keys map-bindings))
(fn [] ~@body))))))
(comment ;; for testing
(defn funk [& args] {:original-args args})
(dotimes [i 1000]
(let [f1 (future (with-dynamic-redefs [funk (constantly -100)]
(Thread/sleep (rand-int 10))
{:100 (funk) :t (.getName (Thread/currentThread))}))
f2 (future (with-dynamic-redefs [funk (constantly -200)]
(Thread/sleep (rand-int 1000))
{:200 (funk 9) :t (.getName (Thread/currentThread))}))
f3 (future (do
(Thread/sleep (rand-int 1000))
{:orig (funk 9) :t (.getName (Thread/currentThread))}))]
(when (or (not= (:100 @f1) -100)
(not= (:200 @f2) -200)
(not= (:orig @f3) {:original-args '(9)}))
(println "FAIL")
(prn @f1)
(prn @f2)
(println "----------------\n\n")))))
@rdivyanshu
Copy link

rdivyanshu commented Apr 1, 2019

Why not use local-vars ?
https://github.com/clojure/clojure/blob/ee3553362de9bc3bfd18d4b0b3381e3483c2a34c/src/clj/clojure/core.clj#L4340

Edit : Ok. Qualified names might not work with this.

@mourjo
Copy link
Author

mourjo commented Apr 1, 2019

@rdivyanshu Yes it is to be used for qualified names. The usage is similar to with-redefs.

@rdivyanshu
Copy link

rdivyanshu commented Apr 2, 2019

@mourjo Another way to do is

(defmacro with-dynamic-redefs
  [bindings & body]  
  (let [vars (map (fn [x] `(var ~x)) (take-nth 2 bindings))]
    (doall (map #(.setDynamic (eval %)) vars))
    `(binding  ~bindings ~@body)))

Approach here is binding does not work with function name/var as they are not dynamic. Set those vars as dynamic (Hoping it is ok for testing). This does not depend on global local-redefinitions. Variables and functions both should work.

@mourjo
Copy link
Author

mourjo commented Apr 2, 2019

@rdivyanshu Thanks! This is so much better 💯 ! I had tried this but did not work without the eval! I will use this! 👍

@mourjo
Copy link
Author

mourjo commented Apr 2, 2019

@rdivyanshu The above solution doesn't seem to be working all the time (when running in a test suite). Sometimes it reverts to the original definition. I am not sure why. I will investigate and come up with an exact test-case.

@rdivyanshu
Copy link

rdivyanshu commented Apr 2, 2019

Hmm. Only case I can think of, currently, why it will revert to original definition is when it will use java native thread.
Or some custom future without doing binding-conveyor-fn

@rdivyanshu
Copy link

rdivyanshu commented Apr 2, 2019

@mourjo

Other reason It might escape binding is due to laziness where body is lazy and body runs in different thread instead of thread which does binding or binding has been undone (see below). There is no solution to this problem either make code non lazy or use your solution (I am not sure if your solution avoids it though, at onset seems to me that it will faces same problem due to usage of local-redefinitions) .

For example

user=> (def ^:dynamic *x* 10)
#'user/*x*
user=> (def res (binding [*x* 20] (lazy-seq (repeat 10 *x*))))
#'user/res
user=> res
(10 10 10 10 10 10 10 10 10 10)

@mourjo
Copy link
Author

mourjo commented Apr 2, 2019

Lazy expressions wouldn't with my code either. For example:

(defn funk [& args] {:args args})

(with-dynamic-redefs [funk (constantly :redefined)]
  (map funk [1]))
=> ({:args (1)})

(with-dynamic-redefs [funk (constantly :redefined)]
  (doall (map funk [1])))
=> (:redefined)

Something else must be it, maybe some regression in my code. I'll get back to you on this in some time. I'd much rather prefer your solution.

@rdivyanshu
Copy link

rdivyanshu commented Apr 2, 2019

@mourjo It is not regression. It is due to laziness https://cemerick.com/2009/11/03/be-mindful-of-clojures-binding/

@mourjo
Copy link
Author

mourjo commented May 13, 2019

@rdivyanshu I tried your approach but it doesn't seem to work if the Vars have already been compiled:

;; this is the source
(ns ptesttrial.core)

(defn funk [a]
  a)
;; this is the test
(ns ptesttrial.core-test
  (:require [clojure.test :refer :all]
            [ptesttrial.core :as pc]))


(deftest b-test
  (is (= 99 (pc/funk 99)))
  (.setDynamic #'pc/funk true)
  (binding [pc/funk (constantly -1)]
    (is (= -1 (pc/funk 99)))))

This test case fails:

expected: (= -1 (pc/funk 99))
  actual: (not (= -1 99))

Clojure docs say the following https://clojure.org/reference/vars#interning :

If a def expression does not find an interned entry in the current namespace for the symbol being def-ed, it creates one, otherwise it uses the existing Var. This find-or-create process is called interning. This means that, unless they have been unmap-ed, Var objects are stable references and need not be looked up every time.

I also tried using ns-unmap and it does not seem to work.

(ns ptesttrial.core-test
  (:require [clojure.test :refer :all]
            [ptesttrial.core :as pc]))


(deftest b-test
  (is (= 99 (pc/funk 99)))
  (.setDynamic #'pc/funk true)
  (ns-unmap 'ptesttrial.core 'funk)
  (binding [pc/funk (constantly -1)]
    (is (= -1 (pc/funk 99)))))

;; FAIL in (b-test) (core_test.clj:11)
;; expected: (= -1 (pc/funk 99))
;;   actual: (not (= -1 99))

@rdivyanshu
Copy link

rdivyanshu commented May 16, 2019

This works fine.

(ns ptesttrial.core-test
  (:require [clojure.test :refer :all]
            [ptesttrial.core :as pc]))

(defmacro with-dynamic-redefs
  [bindings & body]  
  (let [vars (map (fn [x] `(var ~x)) (take-nth 2 bindings))]
    (doall (map #(.setDynamic (eval %)) vars))
    `(binding  ~bindings ~@body)))

(deftest b-test
  (is (= 99 (pc/funk 99)))
  (with-dynamic-redefs [pc/funk (constantly -1)]
    (is (= -1 (pc/funk 99)))))

Any reason for not doing this. Problem with your approach is setDynamic was not run during compilation so compiler emitted getRawRoot at call site (https://github.com/clojure/clojure/blob/b19b781b1f0f3f46aee5e951f415e0456a39cbcb/src/jvm/clojure/lang/Compiler.java#L5191). deftest put content of body in function which don't get invoked during compilation where as use of
defmacro make it run at compile.

@uwo
Copy link

uwo commented Jun 10, 2019

@mourjo Just had this issue pop up on us. Thanks for sharing!

@filipesilva
Copy link

@rdivyanshu was trying to use your macro as a drop-in replacement to with-redefs, but think I've hit a problem.

The direct invocation usage works great:

(defn get-thing []
  :none)

;; fails, usually
(deftest with-redefs-not-thread-safe
  (let [results (pmap (fn [i]
                        (with-redefs [get-thing (constantly i)]
                          (get-thing)))
                      (range 100))]
    (is (= (range 100) results))))

(defmacro with-dynamic-redefs
  [bindings & body]
  (let [vars (map (fn [x] `(var ~x)) (take-nth 2 bindings))]
    (doall (map #(.setDynamic (eval %)) vars))
    `(binding  ~bindings ~@body)))

;; doesn't seem to fail?
(deftest with-dynamic-redefs-is-thread-safe
  (let [results (pmap (fn [i]
                        (with-dynamic-redefs [get-thing (constantly i)]
                          (get-thing)))
                      (range 100))]
    (is (= (range 100) results))))

But if I add indirection, it won't work until I re-evaluate the caller:

(defn get-thing []
  :none)

(defn get-indirect-thing []
  (get-thing))

;; fails, usually
(deftest with-redefs-not-thread-safe
  (let [results (pmap (fn [i]
                        (with-redefs [get-thing (constantly i)]
                          (get-indirect-thing)))
                      (range 100))]
    (is (= (range 100) results))))

(defmacro with-dynamic-redefs
  [bindings & body]
  (let [vars (map (fn [x] `(var ~x)) (take-nth 2 bindings))]
    (doall (map #(.setDynamic (eval %)) vars))
    `(binding  ~bindings ~@body)))

;; fails too
(deftest with-dynamic-redefs-is-thread-safe
  (let [results (pmap (fn [i]
                        (with-dynamic-redefs [get-thing (constantly i)]
                          (get-indirect-thing)))
                      (range 100))]
    (is (= (range 100) results))))

;; evaluating the caller again will make the test work
(defn get-indirect-thing []
  (get-thing))

;; now it doesn't fail
(deftest with-dynamic-redefs-is-thread-safe
  (let [results (pmap (fn [i]
                        (with-dynamic-redefs [get-thing (constantly i)]
                          (get-indirect-thing)))
                      (range 100))]
    (is (= (range 100) results))))

It looks like the same class of problem as before: callers that were defined before the macro runs get a non-dynamic version of the var.

@filipesilva
Copy link

For whoever finds this, the author actually made a lib that works with my example above: https://github.com/mourjo/dynamic-redef

@rdivyanshu
Copy link

rdivyanshu commented Oct 10, 2023

@filipesilva It is happening due to there is no dynamic invocation of function get-thing. In get-indirect-thing you are invoking during definition. if you define get-indirect-thing according to below

(defn get-indirect-thing [f]
  (f))

And then call (get-indirect-thing get-thing) test passes. What you want in get-indirect-thing at run time is call current definition of get-thing but it is calling get-thing at time of definition. I am not sure if clojure provides such mechanism for functions with no arguments, it has been long since I wrote clojure code.

@filipesilva
Copy link

Hey thanks for getting back to me, wasn't expecting it at all :D

https://github.com/mourjo/dynamic-redef seems to handle the indirection case well so trying it out atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment