lambdaisland / kaocha

Full featured next gen Clojure test runner
https://cljdoc.org/d/lambdaisland/kaocha/
Eclipse Public License 1.0
799 stars 84 forks source link

Fixture with try-catch around f swallows failed tests #367

Closed NoahTheDuke closed 1 year ago

NoahTheDuke commented 1 year ago

In clojure.test, the function test-vars calls each-fixture-fn, wrapping calling test-var on each test function being run. test-var wraps the test function in a try-catch, and if there's an error in the test, it reports an error directly. In other words, :once and :each fixtures don't see any exceptions thrown by the test because test-var catches all exceptions and returns nil.

kaocha, on the other hand, wraps the test function in any :each fixtures in kaocha.type.var with the reduce call. Then the wrapped test is executed within the try-catch block, which means that the :each fixtures see any thrown exceptions before the kaocha.type.var try-catch can catch and report them.

The result is that if an :each fixture catches and doesn't rethrow the exception, then no error is reported and the test is treated as successful.

Reproduction below. If you use kaocha to run nested-test, you'll see that it outputs "once before", "each before", "each caught", and "once after", and shows 1 passing test with 1 assertion.

(ns example-test
  (:require [clojure.test :refer [deftest is use-fixtures]]))

(defn once-fix
  [f]
  (try (prn "once before")
       (f)
       (prn "once after")
       (catch Throwable _
         (prn "once caught"))))

(defn each-fix
  [f]
  (try (prn "each before")
       (f)
       (prn "each after")
       (catch Throwable _
         (prn "each caught"))))

(use-fixtures :once #'once-fix)
(use-fixtures :each #'each-fix)

(deftest nested-test
  (is (= 1 1))
  (throw (Exception. "fake exception"))
  (is (= 2 1)))
alysbrooks commented 1 year ago

Thanks for the issue report! One theoretically useful thing about our behavior is that you could use a fixture to throw a more meaningful exception. I'm not sure this theoretical use case is worth diverging from clojure.test, though. Defining your own function or macro to wrap the problematic part probably makes more sense (or getting the bad fixture fixed).

(FYI for people stumbling upon this, you should use throws? for tests that deliberately throw an exception.)

alysbrooks commented 1 year ago

For future reference, here are the relevant bits of test.clojure:

NoahTheDuke commented 1 year ago

Thanks for linking to the clojure.test code, I should have done that to begin with!