Fork me on GitHub
#testing
<
2022-01-26
>
Ian Fernandez17:01:52

I'm having a problem on running tests with http-kit client, the test runs "sometimes", the bigger picture idea is:

(deftest my-e2e-test
  (let [system (start-system! config)]
    (let [token (->> token-req http/request deref treat-token-response)
          data (-> data-req (update :headers assoc "Authorization" token) http/request deref treat-data-response)]
          data))
   (async-stop-system! system)))

Ian Fernandez17:01:06

sometimes the test run

Ian Fernandez17:01:53

but there's a lot of times that async-stop-system! seems to finish before the response on data come

Ian Fernandez17:01:00

or something like this

Ian Fernandez17:01:37

you folks have some tips to debug this ?

Ian Fernandez17:01:01

the second let happens because I did try using promesa stuff / core.async stuff to try to make those requests blocking before async-stop-system!

fabrao18:01:33

Hello all, sometimes I have problem to understand some tests, like this:

(deftest insert-user!--test
  (let [user {:bio      "bio"
              :email    ""
              :image    "image"
              :password "password"
              :username "username"}
        _ (store/insert-user! user)
        res (store/find-by-email "")]
    (is (= (assoc user :id 1) res))))
what is the main use for this test? Testing if the database lib is ok or if store function is ok in calling the database lib or both?

seancorfield18:01:42

It's a bad test. It says -- based on its name -- that it is testing the insert-user! function but it is relying on find-by-email: a) that it works at all and b) that it returns a single row.

fabrao18:01:02

is this a bad test or useless?

fabrao18:01:02

@seancorfield what level of test do you consider good tests?

seancorfield19:01:47

@fabrao That question is far too open-ended to be answered in any useful way...

seancorfield19:01:26

Aside from any other concerns about the test, code that does (store/insert-user! user) without passing in any information about the database means it is relying on global state somewhere which makes testing harder and is something I would strongly discourage...

fabrao20:01:48

I´m trying to understand the ways some people do tests. I saw it in https://github.com/furkan3ayraktar/clojure-polylith-realworld-example-app

fabrao20:01:48

do you consider this kind of test as integration test?

seancorfield21:01:53

It is certainly testing more than one function and it is testing side-effects and relying on global data to do so. It is not a unit test. I don't consider it to be a very useful test and I don't consider it to be a well-written test. If the DB happens to already contain a match for that email and insert-user! does nothing, the test will still pass. It assumes no other records exist in the DB and the generated ID will be 1. It could pass for various types of incorrect code and it could also fail for correct code (if the DB env doesn't match its assumptions).

Colin P. Hill21:01:06

There's a case sometimes made for it being okay to test more than one function if they're closely related, as a select function and an insert function are, but the other assumptions there are kind of yikes.

Colin P. Hill21:01:49

Ah, it uses a fixture to clear the DB between tests. So the state assumptions are probably actually okay. The assumption about how the underlying id sequence works is still an overspecification.

seancorfield21:01:29

I didn't go digging for the specific file to look at the test in detail. The first few pieces of the app I looked in didn't have any tests at all(!). Having tests that rely on the database being cleared before each test can be... problematic... even having a namespace full of tests that expect the DB to be cleared once before they run is "less than ideal". But this is just a simple example app so it shouldn't be expected to be a shining light in terms of testing.

👍 1
seancorfield21:01:52

There are some arguments for testing your store/fetch operations if they do more than just call a low-level database library but there are better ways to do that don't require tearing down the DB before each test or each namespace, and you could also argue that separating the pure transform from the low-level database library call is worth doing so you can test the transform in isolation. But you really need to be getting specific about that, rather than trying to deduce "global rules about testing" from random examples in OSS repos.

seancorfield21:01:31

Where we have tests that specifically exercise a function that either inserts, updates, or deletes data, we're generally pretty careful to make sure that we can work with the DB in any state and that we return it to that same state as part of the test, where that makes sense to do so. To test an insert, you generally want to test "insert new data" and "attempt to insert existing data" to check how duplicate key insertion is handled, so that's at least two tests with different setup/teardown logic. Similarly, with updates and deletes -- there's a do nothing path and a do something path if you're really testing the functionality works (as I said, above and beyond the low-level database library which you really shouldn't be testing).