-
-
Save mourjo/c7fc03e59eb96f8a342dfcabd350a927 to your computer and use it in GitHub Desktop.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; | |
;; 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"))))) |
Developed at Metabase, but since we stopped needing it there I've preserved it in my github account.
Out of curiosity, what did you end up using?
We just removed it and did not do parallel tests. They were a bit tricky to get right, and weren't worth the complexity. If doing it again, I'd probably try sharding the test suite at the process level instead.
Got it, thank you!
This gist is a bit unfortunate since both of you (@filipesilva, @mourjo) have a way to fix a lot of complexity we are finding on the Clojure services created out there. The fact that with-redefs
is not thread-safe influenced (for wrong reasons) the creation of a lot of indirections. I wish the Clojure community would embrace what you have created.
@KingMob wrote something great about it: https://modulolotus.net/blog/2022-06-22-tidd/
I wish all of this had a bit more visibility.
I wish all of this had a bit more visibility.
Me too 😄
Hey all! Sorry it took me so long to get to this - it would be really great to combine the two implementations
@filipesilva would you be okay if I blatantly copy how you are handling metadata in your implementation? Or if you prefer a pull request, that'd be great too.
I wish we would be end up with a single with-dynamic-redefs library.
@mourjo go for it :D
@filipesilva : Wait... you are pointing to your implementation which apparently covers what I would like to do [1]. Let me give it a shot.
[1] : https://github.com/filipesilva/with-dynamic-redefs/blob/d121f3b2d92d2b9a4d8538033dbf3a051f42d496/src/with_dynamic_redefs.clj#L13-L15
EDITED: Ha, perfect, this is exactly the issue I'm encountering. https://github.com/filipesilva/with-dynamic-redefs?tab=readme-ov-file#with-dynamic-redefs. I wish we would be end up with a single
with-dynamic-redefs
library.