Surface var-less errors in JUnit output - #50
Conversation
Fixture-init throws (:once/:each) and namespace load/compile errors reach the JUnit reporter with no associated test var. clojure.test/eftest still counts them toward the run's error total, and mb.hawk.core derives the exit code from that total, so such an error fails the run -- but the namespace->var keyed writer had nowhere to put an error with no var, so it was dropped from JUnit entirely. The exit code and the JUnit output then disagreed, and any consumer reconstructing the failed set from JUnit (e.g. to compute a narrow rerun selector) would silently miss the error. Collect var-less errors during the run and write them to their own file (mb_hawk_var_less_errors.xml) at :summary. Each is emitted as a <testcase> with a non-empty name but deliberately no classname, so a consumer that attributes failures by resolving classname/name to ns/var treats it as unattributable rather than targeting a nonexistent var. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bronsa
left a comment
There was a problem hiding this comment.
looks solid to me, just a few implementation notes
| (write-element! | ||
| w "testcase" | ||
| ;; NOTE: intentionally no `classname` -- see `var-less-error-name`. | ||
| {:name (var-less-error-name result)} |
| (str (or (not-empty message) "Uncaught error with no associated test var") | ||
| (when where (format " (%s)" where))))) | ||
|
|
||
| (defn- throwable->string |
There was a problem hiding this comment.
could be (with-out-str (clojure.stacktrace/print-cause-trace t))
| ;;;; We collect them here and emit them into their own file at `:summary`. Each becomes a `<testcase>` with a | ||
| ;;;; non-empty `name` but deliberately NO `classname` -- see `var-less-error-name`. | ||
|
|
||
| (defonce ^:private var-less-errors (atom [])) |
There was a problem hiding this comment.
feels a bit weird to hold error states in this write namespace. I'd move it to the junit instead, and have the write functions take collected errors as args, e.g.
;; in mb.hawk.junit
(defonce ^:private var-less-errors (atom []))
(defmethod handle-event!* :begin-test-run
[_]
(write/clean-output-dir!)
(reset! var-less-errors [])
(write/create-thread-pool!))
(defmethod handle-event!* :summary
[_]
(write/write-var-less-errors! @var-less-errors)
(write/wait-for-writes-to-finish))There was a problem hiding this comment.
Moved to mb.hawk.junit in b8cf402 — write-var-less-errors! now takes the errors as an arg.
Replace the hand-rolled throwable->string helper with the standard `(with-out-str (clojure.stacktrace/print-cause-trace t))`, which also walks the cause chain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The name is built from the exception message, which may carry ANSI color codes and unprintable characters; run it through decolorize-and-escape like the other JUnit text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The write namespace is otherwise stateless XML-writing logic; the atom accumulating var-less errors fits better alongside the event handlers that produce them. Move it to mb.hawk.junit and have write-var-less-errors! take the collected errors as an argument. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
camsaul
left a comment
There was a problem hiding this comment.
I feel like there's too much duplication here with existing code but after looking at it for a bit I don't have a ton of concrete suggestions about how to minimize it so I'm going to go ahead an approve it for now.
Resolves DEV-2429
Premise
Fixture-init throws (:once/:each) and namespace load/compile errors reach the JUnit reporter with no associated test var. clojure.test/eftest still counts them toward the run's error total, and mb.hawk.core derives the exit code from that total, so such an error fails the run -- but the namespace->var keyed writer had nowhere to put an error with no var, so it was dropped from JUnit entirely. The exit code and the JUnit output then disagreed, and any consumer reconstructing the failed set from JUnit (e.g. to compute a narrow rerun selector) would silently miss the error.
Summary
Collect var-less errors during the run and write them to their own file (mb_hawk_var_less_errors.xml) at :summary. Each is emitted as a with a non-empty name but deliberately no classname, so a consumer that attributes failures by resolving classname/name to ns/var treats it as unattributable rather than targeting a nonexistent var.