Clojurians Log v2

@tolitius The question was really about the function signatures of notify, query & send because those are the functions i would write tests for. I'm not sure what do u suggest because u went from having • a Database protocol with one query method with an implementation accessible as (:db customers) • an EmailSender protocol with one send method with an implementation accessible as (:email customers) to • an app.db state doing the same as (query (:db customers)) • an app.email state doing the same as (send (:email customers))

So let me ask more specifically. Given this implementation:

(in-ns 'app.db)
(defstate db
  :start (db-lib/connect "db-uri")
  :stop (db-lib/disconnect db))

(defn query [name]
  (db-lib/query db some-query name))

; ----------------------------------------

(in-ns 'app.emailer)
(defstate emailer ...)
(defn send [addr msg]
  (email-lib/send emailer addr msg))

; ----------------------------------------

(in-ns 'app.customers)
(require 'app.db)
(require 'app.emailer)

(defn notify [name message]
  (app.emailer/send 
    (app.db/query [name])
    message))

which could be tested as:

(in-ns 'app.customers-test)
(require 'app.customers)
; The test should be aware what other states does the System Under Test depends on
; which would be nice to keep as an implementation detail to make tests less rigid
(require 'app.db)
(require 'app.emailer)

(defn mount-start-stop [test]
  (mount/stop)
  (mount/start-with
    {#'app.db/db           (comment "some mock db implementation acceptable to db-lib/query")
     #'app.emailer/emailer (comment "some mock emailer implementation acceptable to email-lib/send")})
  (try (test)
    (finally (mount/stop))))

(use-fixtures :each mount-start-stop)

(deftest notify-test
  ; GIVEN
  (db-lib/insert app.db/db {:name "Joe" :address "<mailto:joe@address.com|joe@address.com>"})
  ; OR using some domain function instead?
  (app.db/add-person "Joe" "<mailto:joe@address.com|joe@address.com>")

  ; WHEN
  (let [notify-result (app.customers/notify "Joe" "Hello Joe")]

    ; THEN function return value
    (is (= :ok notify-result))

    ; AND other state changes are
    (let [email (first-sent-email app.emailer/emailer)]
      (is (= "<mailto:joe@address.com|joe@address.com>" (:to email)))
      (is (= "Hello Joe" (:body email))))))

(deftest other-notify-test
  ; GIVEN
  "some other db content"
  ...)

I think it's important to show a complete realistic test to really evaluate the consequences of an approach.

but based on your example, I think what u wanted to say was

(in-ns 'app.customers-test)
(require 'app.customers)
; Still need to know the dependent states
(require 'app.db)
(require 'app.emailer)

(mount/start-with
  {#'app.db/query     (fn [name] (println "querying DB with" name))
   #'app.emailer/send (fn [addr msg] (println "sending email to" addr "body:" msg))})

If that's correct then a full example would look like this:

(in-ns 'app.customers-test)
(require 'app.customers)
; Still need to know the dependent states
(require 'app.db)
(require 'app.emailer)

(defn ensure-mount-stop [test]
  (mount/stop)
  (try (test)
    (finally (mount/stop))))

(use-fixtures :each ensure-mount-stop)

(deftest notify-test
  ; GIVEN
  (let [emails (atom [])]
    (mount/start-with
      {#'app.db/query     (fn [name] 
                            (if (= "Joe" name)
                                "<mailto:joe@address.com|joe@address.com>" 
                                (str "&lt;address for '" name "''&gt;")))
       #'app.emailer/send (fn [addr msg]
                            (swap! emails conj [addr msg]))})

    ; WHEN
    (let [notify-result (app.customer/notify "Joe" "Hello Joe")]

      ; THEN function return value
      (is (= :ok notify-result))

      ; AND other state changes are
      (is (= 1 (count @emails)))
      (is (= ["<mailto:joe@address.com|joe@address.com>" "Hello Joe"]
             (first @emails))))))

(deftest other-notify-test
  ; GIVEN all over again just slightly differently?
  (let [emails (atom [])]
    (mount/start-with
      {#'app.db/query     (fn [name] 
                            (if (= "Mary" name)
                                "<mailto:mary@address.com|mary@address.com>" 
                                (str "&lt;address for '" name "''&gt;")))
       #'app.emailer/send (fn [addr msg]
                            (swap! emails conj [addr msg]))})
  ...

What i miss here is some standardized way of setting up the substituted states. some kind of a unified way to describing preconditions to the test. I also feel like there should be a (reset! emails []) in there somewhere but since it's in just local scope now, not inside a state, it works fine this way too. I've also noticed that the two approaches match the 2 major approaches of testing 1. using mocks (when states are some kind data/objects) 2. using stubs (when states are functions)

For the sake of completeness lets also see how the implementation would look like for the test above:

(in-ns 'app.db)
(defstate db
  :start (db-lib/connect "db-uri")
  :stop (db-lib/disconnect db))

(defstate query
  :start (fn query [name]
           (db-lib/query db some-query name))
  ; :stop is not necessary because this state is a pure function which is stateless...
  ; which is kinda confusing to define it as a state just because it depends on a state...
  )

; ----------------------------------------

(in-ns 'app.emailer)
(defstate emailer ...)
(defstate send 
  :start (fn send [addr msg]
           (email-lib/send emailer addr msg))
  ; :stop is not necessary for the same reasons as explained above
  )

; ----------------------------------------

(in-ns 'app.customers)
(require 'app.db)
(require 'app.emailer)

(defn notify [name message]
  (app.emailer/send 
    (app.db/query [name])
    message))

; OR wait a second!!!
; the functions it uses are defined as states so this function also depends on states,
; hence this should be defined as state too, no?
; 
; otherwise it might see older functions referring to stale, already stopped state?...
; okay, here is where im not sure and find it slightly magical :/

(defstate notify
  :start (fn notify [name message]
          (app.emailer/send 
            (app.db/query [name])
            message))

If I must define notify using defstate in order to have a consistent test structure, where I don't have to handle "function states" differently from "value/object/mutable states", then it smells a bit like a whole-app buyin to me...

So my question is Q1. Which approach do you promote and why? 1st approach is wrapping objects into states which are substituted in tests 2nd approach is packaging not just the objects but the functions (which are the APIs of our modules) into states so such API can be substituted during tests (in both cases the functions implicitly reference the states holding objects which are required by their underlying libraries. there is actually an approach where we make these dependencies of our API functions explicit by requiring them as arguments, just like how most underlying libraries do so too)

Q2. Am I correct to say that the difference between the 2 approaches whether the tests for them must be written in a mockist or stubbist style?

Q3. If you recommend the second approach, then should notify be a function or a state?

I think "it can be both" is not an acceptable answer, because it affects how tests should be written for them and that introduces such inconsistency which i feel is not inherent to the problem (of combining states and state using functions)

interesting example. I find myself doing the following when using Mount: * states are things which have a clearly defined lifecycle - a DB connection, a stream of data, an atom of things * states are hidden in the declaring namespace. The API of the namespace are the functions operating on states. * tests test the API directly by mocking the functions

(testing ...
  (let [!results (atom [])]
     (with-redefs [my.notifications/notify! #(swap! !results conj %)]
       (my.email/send-with-notifications "IMPORTANT")
       (is (= ["IMPORTANT"] @!results))))

(this is a degenerate test that I would not consider writing in real life)

start-with has more uses when the namespace API is a shallow wrapper or the test uses an actual testable implementation of the resource captured in the state, like a mem datomic connection.

@onetom ^

@dm3 thanks. interesting. i thought a major point of using mount is to avoid with-redefs so im even more puzzled now 🙂

why would you try to avoid with-redefs?

if that’s your API?

@dm3 https://youtu.be/13cmHf_kt-Q?t=1626 watch it until 29:16 he explains quite precisely what kind of issues will u have if you have any async code within your tested system and some other considerations

I know all of that

never had to deal with issues like that in tests though

maybe because I’m already aware of them

I can see how that could lead to problems if running tests in parallel though. Haven’t had the need to do that yet

you’d have to use something like Boot’s pods

using pods would be even more ceremony and distraction from the core logic of what you are trying to achieve...

and i don't think the main issue with with-redefs is parallel test running but more like having any kind of asynchrony mixed into the program. an rather realistic example is sending an email out. would you wait for an amazon ses api call to complete during a user registration step before you respond to the user regarding his registration being successful?

if yes, it slows the response down, if no, then how would u know from your test when has that call finished? if u provide a mock ses component which is synchronous, then you are cheating because you are testing a system with different semantics...

hmm, are you talking about testing code like

(defn register [email name]
  (db/save! {:email email, :name name})
  (email/send! {:to email, :text (str "hi " name)}))

if email/send! returns a promise then the mocked function would also return a promise

yes, email/send! might just return a promise and that might not even be the return value of the register function, so from a test you would need to be able to wait for the email to arrive

I guess I don’t understand why you need to wait for an email to arrive if you mock it out

imagine you are writing a route handler or a castra endpoint. those functions should return a http response (roughly)

so you wont have access to the promise from the tests

are we talking about a unit test for a e.g. route handler where you mock out all dependencies?

okay, so i guess that's the source of the misunderstanding. what is a unit test? to me a unit in this case is the register function and i want to test that...

preferably with the least amount of modification compared to the production setup

ok, so a unit test would mock out everything and just test the unit logic.

so most of my code should run behind it

you mean like stuff inside db/save! and email/send!?

yes

ok, that’s not a unit test in my book anymore 🙂

because if i mock those out then im totally coupling my test to the implementation details

yep

which is rather pretty fucking useless in my book 😜

that’s why you wouldn’t write a unit test for that fn in real world

then we agree 🙂

yeah, but I guess we were working towards some example where with-redefs doesn’t work

I use it to mock out APIs, like email/send! or db/save!

so how would you call such a test where you still let code behind db/save! and email/send! run?

I’d have to with-states the actual things inside defstate with testable implementations

we are going off-topic though... maybe #testing would be a better channel for such discussion 🙂

i agree with that statement too

however the "cheating" enters the equation when we start to use defstate on the API functions around the stateful constructs of our program...

i think the problem is that we don't have a clear, consistent, shared vocabulary around the topic

in my world you either mock the API function for a unit test or use a testable stateful thing for an integration test

but I must confess that I do less testing than I probably should

dm3: i would actually call the "testable stateful thing" case mocking and the fake function case stubbing

and those two styles was i trying to show in my examples earlier

hmm, I always thought of stubbing - returns a default value/does nothing; mocking - responds to complicated interactions, remembers how many times it was called with what arguments, etc

the “testable stateful thing” is more like an in-memory db instead of a file-based one

well, you heard about mock objects but not stub objects right?

does my definition disagree with what stub objects are? Wiki says > A Stub Object is one whose methods are stubs (or “mocks”?); that is, they do no useful work but prevent #doesNotUnderstand errors and return plausible values so that the computation can continue.

haha >(or “mocks”?)

URL?

because currently it's just you who are saying it 😜

It's like as if I'm saying: Wiki says > A Stub Object is something which @dm3 should use more often ;D

I think this is the best explanation: https://martinfowler.com/articles/mocksArentStubs.html#TheDifferenceBetweenMocksAndStubs. The one I quoted is from c2 wiki

yeah, thats what i consider the best source too and thats why i said earlier > we don't have a clear, consistent, shared vocabulary which the article expresses as: > The vocabulary for talking about this soon gets messy - all sorts of words are used: stub, mock, fake, dummy.

btw, that Gerard Meszaros he is referring to was also a Hungarian guy, like myself 😉

and mészáros means butcher 😉

ok 🙂

Did any of your questions get answered though?

so maybe we should agree then on not using any of these dummy, fake, stub, spy, mock terms, just test double instead

since we are not even talking about object oriented code strictly

our stateful constructs though are still objects in "reality" and they should be differentiated from the functions working with them. there are two distinct ways how functions can use such objects: • explicitly depending on them, meaning they receive them as parameters • implicitly depending on them, meaning they have a reference wired into them at compile time

and no, i don't think we got any clearer, since i asked 3 quite specific questions and i didn't get 3 corresponding specific answers

regarding with-redefs im on the same view as expressed here: https://www.reddit.com/r/Clojure/comments/4wrjw5/withredefs_doesnt_play_well_with_coreasync/ > So IMO, the solution is simple. Don't use with-redefs. Any time you see "with-redefs" think "nasty hack" in your mind.

well, my answers are: Q1 - 1st approach for integration tests, mocking API functions for unit tests Q2/Q3 - haven’t seen anyone use states as functions so no comments

and I like my redefs 🙂

I also don’t use core.async

the master of mount just gave an example of using functions as states: https://clojurians.slack.com/archives/C0H7M5HFE/p1499782555817759

and his article he referenced also gives such an example in form of the #'app.sms/send-sms state https://clojurians.slack.com/archives/C0H7M5HFE/p1499782582834684

oh, right 🙂 well, I haven’t had the need to do that

this one: https://github.com/tolitius/stater/blob/master/smsio/src/app/sms.clj#L6-L13

so I won’t be able to help

that's fine, but you can't say anymore that you haven't seen anyone using that approach 😉

yep

@onetom: If I understood your questions correctly I believe the confusion is around the actual implementation vs. testing.

In case states extend protocols, their stubs can be reified in start-with to a new behavior that would be useful for testing

In case states are functions they can be simply replaced with testing functions in start-with. EmailSender is such an example. I would have a state to be a function that is able to send emails (similar to an "sms sender" example).

In case states are IO, resources, etc.. objects (such as a database "conn"ection) and you'd like to test the code that uses them, I would only access them via functions that take them as arguments. For example, I would not do this:

(defn query [query params]
  (jdbc/fetch conn query params))

but would rather do this:

(defn query [conn query params]
  (jdbc/fetch conn query params))

in which case you can either stub or mock a conn state. i.e. usually an in memory db or you could mock conn object as well of course.

> which approach do you promote and why?

I don't like to promote an approach 🙂 I believe there is no one approach that works for all. I would personally avoid creating states for higher order constructs such as a query function above, and would only have a handful of states that are as low level as possible: sockets, db connections, thread pools, etc..

from your example above I would keep notify, query, send and other functions as just functions and would only have two states: email-sender and conn. Functions that rely on states would take them as arguments.

@tolitius thanks!

so as a consequence, what would be the signature of the notify function? 1. (notify [db emailer addr msg]) 2. (notify [emailer db addr msg]) 3. (notify [[db emailer] addr msg]) 4. (notify [{:keys [db emailer]} addr msg])

would really depend on a way you'd prefer to structure your application / functions. from your example above it seems notify does two things: * find an email address in a database by a user id * sends a message to the email address so something like:

(-> (select conn query)
    (send-email msg))

(defn find-email [conn id]
  (jdbc/select conn ... 'id' ))

(defn notify [send-email conn id msg]
  (-> (find-email conn id)
      (send-email msg)))