bguthrie / shrubbery Goto Github PK

Very nice library. A function-based mocking utility that relies on polymorphism via protocols (rather than magic var rewriting macros a la Midje) was exactly what I'd been looking for 👍

One useful feature of using test fakes is that it's usually very easy to test all the various failure cases that a component can produce. I notice that there is currently no way using shrubbery to add a stubbed function that throws an exception.

Do you have any thoughts on if it would be possible to include this? Happy to submit a PR but I thought I'd ask about what you feel is the best approach first.

My first thought was to check whether it was possible to supply a function that would be called to provide the result of a stubbed call, but I see this is explicitly not supported, and I guess this is because of the non-macro approach here which makes delayed evaluation impossible. Do you think that this makes an exception throwing stub impossible with the current approach too?

I'm a bit confused by this:

$ clj -Srepro -Sdeps '{:deps {com.gearswithingears/shrubbery {:mvn/version "0.4.1"}}}'

user=> (use 'shrubbery.core)
nil
user=> (defprotocol A (x [this]))
A
user=> (def fake-A (stub A {:x (java.util.Date.)}))
#'user/fake-A
user=> (x fake-A)
#inst "2018-07-06T10:44:01.025-00:00"

Why does (x fake-A) return a Clojure instant instead of the original java.util.Date object? Is there any way around it other than using reify directly to create the stub?

Disclaimer: I might very well be doing it wrong.

When trying to spy on a protocol I usually only require the protocol and not :all from the namespace to avoid pollution of the tests namespace. However, with the current implementation of the spy macro I cannot do this. To clarify, this is how the problem is reproducible:

user> (ns test1)
nil
test1> (defprotocol TestProto (a [this arg]))
TestProto
test1> (ns test2)
nil
test2> (require '[shrubbery.core :as s])
nil
test2> (require 'test1)
nil
test2> (def fake-impl (reify test1/TestProto (a [_ _] :fake)))
#'test2/fake-impl
test2> (s/spy test1/TestProto fake-impl)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: a in this context, compiling:(/tmp/form-init8310557090395405498.clj:1:1)

If I require everything from the test1 namespace it works:

test2> (require '[test1 :refer :all])
nil
test2> (s/spy TestProto fake-impl)
#object[test2$eval35584$reify__35585 0x48c2232f "test2$eval35584$reify__35585@48c2232f"]

Is there a way to make it work? I.e. by taking the namespace into account or prefixing the methods with a dot when generating the proxy calls in the macro?

In my service I create components, and then they have config assoc'd onto them before they are started. The components adhere to the component Lifecycle, and also implement protocols for their behaviour.

Shrubbery is great for implementing the protocols, but when my system tries to assoc config into them, it fails because they are not records. Would it be possible to make an option where stubs behave like records?

Is there a way to use shrubbery's matchers in a way that is equivalent to Mockito's refeq() matcher? This can be useful when testing Clojure code that must use Java interop to create object instances.

For example, in my function I have to construct a java.util.ArrayList to pass to an underlying Java library. I want to assert that the contents of the ArrayList created inline in the function are equivalent (but not object-identical) to the contents of the vec I use to call it.

(defn subscribe! [kafka-consumer topic]
  (let [topics (doto (new ArrayList) (.add topic))]
    (log/info "Subscribing on topic" topic)
    (.subscribe kafka-consumer topics)))

;; test
(deftest test-subscribe!
  (testing "Calling subscribe! calls the #subscribe method on the underlying consumer"
    (let [kafka-consumer (spy mock-consumer)
          topics-vec ["a-topic"]
          topics-list (doto (new ArrayList) (.add "a-topic"))]
      (module/subscribe! kafka-consumer topics-vec)
      (is (received? kafka-consumer subscribe (ref-eq topics-list))))))

I'm trying to use this library, but I get the following error:

java.lang.IllegalArgumentException: db-spec xchange.data.user_test$eval27076$reify__27077@6ea22666 is missing a required parameter
 at clojure.java.jdbc$get_connection.invokeStatic (jdbc.clj:379)
    clojure.java.jdbc$get_connection.invoke (jdbc.clj:226)
    clojure.java.jdbc$insert_rows_BANG_.invokeStatic (jdbc.clj:1301)

... 
  (:require [my.utils.db :refer [insert]])

(defn create-user
  [conn user]
  (insert conn :users user))

(defprotocol DbQueryClient
  (insert [conn table user]))

(def db-stub
  (stub DbQueryClient
        {:insert :ok}))

(deftest user
  (testing "stub"
    (is (= :ok (create-user db-stub {:name "itaied"})))))

Am I missing something?

This kind of thing fails:

(defprotocol AProtocol
  (qux [this] [this that]))

...

  (testing "with multiple arities"
    (let [subject (stub AProtocol {:qux 1})]
      (is (= 1 (qux subject)))
      (is (= 1 (qux subject :that)))))

because stub implementations are only added for the first arglist in the signature.

From #9 (comment):

Another thing I've been thinking about recently, which is somewhat related (although this may go too far in changing the spirit of shrubbery) is whether we could change the signature of stub entirely to allow arg values to be given when setting up a stubbed function. When the arg values match, the return value would be returned.

Obviously it's possible to use received? afterwards to match/verify args given, but it would be more useful to supply these when subbing IMO.

When we stub a function we typically do care what arg values are given, in some cases we want to return different values based on those arg values. We can verify arg values with received? but it would be much more convenient (and more powerful) to provide these when stubbing.

Some options for the syntax:

;; 1. 
;; Replace the key in the config map with a vector, where the first value
;; denotes the function being stubbed and the subsequent values represent 
;; args to be matched.
(stub AProtocol {[:select-user "id1"] user1
                 [:select-user "id2"] user2})
;; Backwards compatible since we can inspect the key and check if it's a vector.


;; 2.
;; Allow a stubbed function to be added to an existing stub. stub-fn takes the 
;; stub as its first arg, the fn to stub as its second, the remaining args are args 
;; to match but the last arg is the return value 
(-> (stub AProtocol)
    (stub-fn :select-user "id1" user1)
    (stub-fn :select-user "id2" user2))


;; 3.
;; Replace the current config map with a vector, so that fn names are no longer 
;; unique keys. The first value in each vector is the fn to stub, the last is the 
;; return value and the values between are the args to match.
(stub AProtocol [[:select-user "id1" user1]
                 [:select-user "id2" user2]]
;; Backwards compatible since we can inspect the config and check if it's a vector

;; 4.
;; Similar to above, but with a more explicitly descriptive structure
(stub AProtocol [{:fn :select-user :args ["id1"] :return user1}
                 {:fn :select-user :args ["id2"] :return user2}]
;; Backwards compatible since we can inspect the config and check if it's a vector

I'm not in love with any of these, but I like how terse option 3 is. Feel free to critique these and/or suggest something completely different :)

For stub, spy, etc. Real records often implement several protocols.