codewars / runner

Issue tracker for Code Runner
34 stars 8 forks source link

Confusing output of Clojure tests #241

Open hobovsky opened 1 year ago

hobovsky commented 1 year ago

Found when working on an old Clojure translation:

image

Is there something what could be fixed in CW reporter for Clojure tests, or it has to be this way for some technical reason?

kazk commented 1 year ago

See https://gist.github.com/kazk/70e6dbcf1e74383f984fcce1c1d122ec for the reporter.

The submitted code is concatenated like this:

; test.clj
; <preloaded>
; <solution>
; <tests>

; run tests in "current" namespace which should be the one defined in tests
(require '[qualified.codewars-reporter :as codewars-reporter])
(codewars-reporter/run-tests)

Executed with:

java -cp /usr/share/java/leiningen-2.7.1-standalone.jar:/runner/codewars-reporter.jar clojure.main ./test.clj

Clojure is one of the oldest languages. I've been wanting to fix the implementation when adding a new version (#6).

falcowinkler commented 4 months ago

"Expected false but got false" is not an issue and it will be reported like this also in standard clojure.test. of course it's confusing but it's only a "problem" if and only if your assertion is exactly one of the illogical (is false) or (is nil).

hobovsky commented 4 months ago

Do you mean that it's a problem only when the argument to is is an explicit literal, and it will not result in confusing messages if the is is passed in a more complex expression?

Still, I am not convinced that it's not an issue in general. It's not a problem with CW reporter, which is good, but it definitely does not look healthy (at least to my cojure-noobish eyes), might rise some concerns about general correctness of related code of the library, and maybe would be worth reporting and fixing in the clojure.test?

falcowinkler commented 3 months ago

it will only display like this for falsey values, which is just nil or false. So as i said this output will only occur in the two illogical cases (is false) or (is nil) that no one should ever write. And if you think about it the output makes sense in these cases. Or what else should the test framework output in these two cases?clojure.test is fine.

falcowinkler commented 3 months ago

I looked into the implementation of the current reporter and indeed, an <IT:> block is reported for every test within a testing expression. After some research i didn't find an appropriate hook from clojure.test to report these correctly. The solution could be a mutable state that keeps track of if the context changed, and prints the block every time that happens.

https://github.com/falcowinkler/clojure-runner/blob/main/workspace/test/main.clj