weavejester / eftest

Fast and pretty Clojure test runner
425 stars 40 forks source link

Add synchronized metadata for multithread testing #2

Closed ayato-p closed 7 years ago

ayato-p commented 7 years ago

Hi.

I'm fixing a multithread issue in this PR. This PR makes test vars in multithread testing accept :serialize meta data Like this.

(deftest ^ :eftest/synchronized p-test1
 (with-redefs [c/inner-fn (constantly 0)]
   (is (= 0 (c/heavy-fn)))))

I'll explain the problem that I encountered.

I have the folloing functions:

(ns demo.core)

(defn- inner-fn [])

(defn heavy-fn []
 (Thread/sleep 100)
 (inner-fn))

Which for testing I would like to redefine inner-fn for testing. with-redefs is a popular solution for redefinition/mocking in Clojure, I guess.

(ns demo.parallel-test
 (:require  [clojure.test :as t :refer :all]
            [demo.core :as c]))

(deftest p-test1
 (with-redefs [c/inner-fn (constantly 0)]
   (is (= 0 (c/heavy-fn)))))

(deftest p-test2
 (with-redefs [c/inner-fn (constantly 1)]
   (is (= 1 (c/heavy-fn)))))

(deftest p-test3
 (is (= nil (c/heavy-fn))))

(deftest p-test4
 (is (= nil (c/heavy-fn))))

(deftest p-test5
 (is (= nil (c/heavy-fn))))

Running tests with clojure.test/run-tests is working.

(time
(t/run-tests 'demo.parallel-test))
;; Testing demo.parallel-test

;; Ran 5 tests containing 5 assertions.
;; 0 failures, 0 errors.
;; "Elapsed time: 510.025336 msecs"

However, running tests with eftest.runner/run-tests does not work. (Ofcourse, with :multithred? false option it works, but not fast)

(require '[eftest.runner :as r])
;; nil
(r/run-tests (r/find-tests "test"))

;; 0/5     0% [                                                  ]  ETA: --:--

;; FAIL in demo.parallel-test/p-test3 (parallel_test.clj:14)
;; expected: nil
;;   actual: 1

;; ...

;; 4/5    80% [========================================          ]  ETA: 00:00
;; 5/5   100% [==================================================]  ETA: 00:00
;; 5/5   100% [==================================================]  ETA: 00:00

;; Ran 5 tests in 0.145 seconds
;; 5 assertions, 4 failures, 0 errors.
;; nil

My PR version's eftest.runner/run-tests is works and is still faster then clojure.test/run-tests.

(r/run-tests (r/find-tests "test"))

;; 0/5     0% [                                                  ]  ETA: --:--
;; 0/5     0% [                                                  ]  ETA: --:--
;; 1/5    20% [==========                                        ]  ETA: 00:00
;; 1/5    20% [==========                                        ]  ETA: 00:00
;; 2/5    40% [====================                              ]  ETA: 00:00
;; 2/5    40% [====================                              ]  ETA: 00:00
;; 3/5    60% [==============================                    ]  ETA: 00:00
;; 3/5    60% [==============================                    ]  ETA: 00:00
;; 4/5    80% [========================================          ]  ETA: 00:00
;; 4/5    80% [========================================          ]  ETA: 00:00
;; 5/5   100% [==================================================]  ETA: 00:00
;; 5/5   100% [==================================================]  ETA: 00:00

;; Ran 5 tests in 0.342 seconds
;; 5 assertions, 0 failures, 0 errors.
;; nil
weavejester commented 7 years ago

I like the patch, but rather than :serialize, perhaps :eftest/synchronized would be a better keyword to use.

ayato-p commented 7 years ago

Thank you for your feedback ! PR updated :)

ayato-p commented 7 years ago

Updated. Thank you 🙇

weavejester commented 7 years ago

Released as version 0.1.2. Thanks for the patch!