marick/Midje

Unexpected exceptions cause load failure and test run is not completed

Opened this issue · 3 comments

If a test code calls code that throws an exception but the exception is not expected in the test (so it's a situation that should not happen), Midje does not run further tests. Instead there is a load failure loading the namespace where the exception originated and rest of the test run is not run and there are no results.

I think in case of an unexpected exception, it should be logged and the particular test should fail. The test run as a whole should continue and there should be results at the end of it.

Basic test namespace to reproduce this:

(ns load-failure.core-test
  (:require [midje.sweet :refer :all]))

(fact "testi"
  (throw (Exception. "exception"))
  (+ 1 1) => 2)

(fact " testi 2"
  (+ 1 1) => 2)

Here the exception is thrown directly in the test code, but it could also originate from your source code.

Running this with lein-midje ends up with the following:

LOAD FAILURE for load-failure.core-test
clojure.lang.Compiler$CompilerException: Syntax error compiling at (load_failure/core_test.clj:4:1).
data: #:clojure.error{:phase :compile-syntax-check,
:line 4,
:column 1,
:source "load_failure/core_test.clj"}
java.lang.Exception: exception
load-failure.core-test/eval10638/fn/fn core_test.clj: 4
midje.util.thread-safe-var-nesting/with-altered-roots* thread_safe_var_nesting.clj: 32
load-failure.core-test/eval10638/fn core_test.clj: 4
...
midje.checking.facts/check-one/fn facts.clj: 32
midje.checking.facts/check-one facts.clj: 31
midje.checking.facts/creation-time-check facts.clj: 36
load-failure.core-test/eval10638 core_test.clj: 4
...
clojure.core/load/fn core.clj: 6126
clojure.core/load core.clj: 6125
clojure.core/load core.clj: 6109
...
clojure.core/load-one core.clj: 5908
clojure.core/load-lib/fn core.clj: 5948
clojure.core/load-lib core.clj: 5947
clojure.core/load-lib core.clj: 5928
...
clojure.core/apply core.clj: 667
clojure.core/load-libs core.clj: 5985
clojure.core/load-libs core.clj: 5969
...
clojure.core/apply core.clj: 667
clojure.core/require core.clj: 6007 (repeats 2 times)
...
midje.repl/load-facts/fn/fn repl.clj: 208
midje.repl/load-facts/fn repl.clj: 208
midje.repl/load-facts repl.clj: 194 (repeats 2 times)
...
user/eval10625 REPL Input
...
clojure.main/load-script main.clj: 452
clojure.main/init-opt main.clj: 454
clojure.main/initialize main.clj: 485
clojure.main/null-opt main.clj: 519
clojure.main/main main.clj: 598
clojure.main/main main.clj: 561
...
clojure.main.main main.java: 37

Hello, thanks for logging this issue and the reproduction steps.

I definitely agree that the suggested behavior would lead to a better workflow and I tried to solve this already in #441 (this same issue has been brought up in #302 and #336).

I ended up getting stuck with my attempt to fix the issue and abandoning it as "too hard / tricky". I wish I had written more details in the PR because I can't remember exactly was tricky about it.

That said, it will probably take me a while to revisit this issue, if ever because I'm more focused on other testing tools and I already failed once at a fix :)

I remembered (vaguely) what was so hard about this fix:
I found it difficult to make reporting work with nested facts when you catch exceptions thrown in non-check expressions of a fact.

I haven't looked at the source code that closely, but I presume it wouldn't then be possible to just wrap the body in the fact macro with a try-catch and then fail the test something is caught? I'd guess that the exceptions that are being checked in a test are caught by the checker?