cider.nrepl.middleware.test does not bind clojure.test/*report-counters*

vemv commented 3 years ago

Brief

I have the impression that test runners are expected to bind clojure.test/*report-counters* during test execution. It looks like it should be initialized to clojure.test/*initial-report-counters*.

cider.nrepl.middleware.test doesn't appear to have this logic.

Why this is a problem

I found at least one instance where inc-report-counter is invoked: https://github.com/clojure-emacs/cider-nrepl/blob/ac0625fb095330e80ddcbcad9167655924799211/src/cider/nrepl/middleware/test.clj#L212
- problem being, without the binding that is a no-op: https://github.com/clojure/clojure/blob/57ac70c904e8250f0bd4e084272659aeeb8749c6/src/clj/clojure/test.clj#L314-L320
- i.e. there's effectively dead code in cider-nrepl, which is surely undesirable
Also, the lack of this binding in a popular middleware precludes authors from designing testing macros that inspect clojure.test/*report-counters*
- e.g., one could author a deftest-like macro that ensured that *report-counters* was incremented after execution of that deftest.

cider-nrepl version

This repo @ master

bbatsov commented 3 years ago

Likely an oversight on our part. PR welcome!

bbatsov commented 3 years ago

@jeffvalk wrote the test middleware, so he might have more to say on the subject.

saskenuba commented 3 months ago

Any thoughts on this? This is exactly an issue that I have on some projects. I have some tests macros that juggle around report-counters and expects it to be valid, and when running tests with cider they don't work.

vemv commented 3 months ago

I appreciate that you found the right issue!

I could give it a try myself but can't strongly promise anything as time is quite scarce for me.

Might be a good "first PR".

clojure-emacs / cider-nrepl