cider-test-run-ns-tests not respecting test-ns-hook

[jarohen]

Hey @bbatsov et al, hope you're all keeping well :)

Let me know if more information would be helpful, think this is my first CIDER bug report :)

Cheers,

James

Use the template below when reporting bugs. Please, make sure that you're
running the latest stable release or the latest snapshot/pre-release of
cider-nrepl and that the problem you're reporting hasn't been reported (and
potentially fixed) already.

Expected behavior

If the namespace contains a test-ns-hook function, cider-test-run-ns-tests should call this instead of directly calling whatever test functions the namespace may contain

Actual behavior

It doesn't - in my case (because the namespace has no concrete tests) it fails with 'No assertions (or tests) were run. Did you forget to use is in your tests?'

Steps to reproduce the problem

(ns foo.bar_test
  (:require [clojure.test :as t])

(defn test-ns-hook []
  (t/is (= 1 1)))

Calling (clojure.test/test-ns 'foo.bar-test) directly does check the (= 1 1) assertion.

Environment & Version information

Spacemacs develop branch

;; CIDER 1.0.0snapshot (package: 20200916.1152), nREPL 0.7.0
;; Clojure 1.10.1, Java 1.8.0_265

[cider/cider-nrepl "0.25.3"]

Operating system

Arch Linux

[jarohen]

Looking into this in more detail, this message seems to be going via handle-test-var-query-op and then test-var-query - (query/vars var-query) is returning an empty seq (correctly, because there's no concrete test vars in that namespace). IIUC, this code-path is missing a test-ns-hook check? Not sure where it should live, though!

[jarohen]

Oh - cider.nrepl.middleware.test/test-ns (directly above) would check for test-ns-hook, but it never gets called, because the doseq in test-var-query is running over an empty seq.

HTH!

[bbatsov]

Thanks for the detailed report! If you'd like to take a stab at solving this yourself that'd be great. Seems the solution would be pretty-straightforward.

[jarohen]

Hey @bbatsov, thanks for getting back to me :)

Have had a bit of a bash at this today, struggling a little with the semantics. I was aiming for the same semantics as clojure.test - i.e. when you call c.t/test-ns (cider-test-run-ns-tests) it checks for the hook; when you call c.t/test-var (cider-test-run-test) it doesn't - but this doesn't seem to be how CIDER behaves currently? If so, would you prefer to preserve CIDER's current semantics, or would you be happy to make this consistent?

I might be focusing in the wrong area, but I also seem to be running into Orchard's query namespace, working against the has-tests? and test? flags on the vars function - would a change to Orchard be palatable if need be?

Cheers,

James

[bbatsov]

If so, would you prefer to preserve CIDER's current semantics, or would you be happy to make this consistent?

I'd be happy to make this consistent.

I might be focusing in the wrong area, but I also seem to be running into Orchard's query namespace, working against the has-tests? and test? flags on the vars function - would a change to Orchard be palatable if need be?

Yeah, absolutely.

[jarohen]

Hey @bbatsov, cheers! I'm afraid it might be a while until I get back around to this one - we've recently had a baby so don't have a lot of spare brain cycles at the moment 😄 Thankfully the workaround in the commit above is fairly straightforward for anybody else stumbling on this issue, and the fix doesn't seem too involved - maybe someone else reading this might be able to pick up the baton :)

James

[bbatsov]

I'm afraid it might be a while until I get back around to this one

No rush!

we've recently had a baby so don't have a lot of spare brain cycles at the moment

Congratulations! 🎉 🎈 🍾

[jarohen]

thanks! 🙏

[hodgiwabi]

Since I'm working with cider-test-run-ns-tests in cider#2958 I can also try to look into this. It looks a bit more involved, and may take me some extra time because I'm new, but I would be happy to take up the torch.