Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forms which macroexpand into deftest aren't recognised by cider-test-run-test #1776

Closed
samroberton opened this issue Jun 4, 2016 · 1 comment
Labels

Comments

@samroberton
Copy link

Expected behavior

cider-test-run-test should run the test at point, even if it is not directly declared in the Emacs buffer with deftest.

You might use a convenience macro which macroexpands into a deftest form as a means of following Stuart Sierra's advice to avoid clojure.test's fixtures mechanism, for example.

Actual behavior

"No test at point"

Steps to reproduce the problem

(ns fancy-test
  (:require [clojure.test :refer :all]))

(defn- do-setup []
  (println "Expensive resource setup"))

(defmacro deffancytest [sym & body]
  `(deftest ~sym
     (do-setup)
     ~@body))

(deffancytest fancytest
  (is (= 1 1)))

Invoke cider-test-run-test with the point in fancytest and you'll get "No test at point" in the minibuffer.

Environment & Version information

CIDER version information

;; CIDER 0.12.0 (Seattle), nREPL 0.2.12
;; Clojure 1.8.0, Java 1.8.0_40

Lein/Boot version

boot 2.5.5

Emacs version

24.4.1

Operating system

OS X 10.11

@dpsutton
Copy link
Contributor

dpsutton commented Jun 4, 2016

Seems like the line (if (and ns (member (car def) '("deftest" "defspec"))) in cider-test-run-test would be the offender. I suppose we could make it a case and then check the macro-expansion with (when-let ((expansion (cider-sync-request:macroexpand expander expr))). I can do this if someone confirms this is the right track to proceed on.

@bbatsov bbatsov added the bug label Jul 10, 2016
ncalexan added a commit to ncalexan/cider that referenced this issue Jan 3, 2017
…efining-names`.

This fix is strictly simpler than that suggested in
clojure-emacs#1776 (comment).  It
has the advantage of being much more robust.

I partially implemented the suggested fix, and witnessed the following two
issues.  First, full "macroexpand" does not leave a recognizable `deftest` form;
it generally leaves a `(def test-... (fn [] ...))` sexp.  That implies that a
recursive macroexpansion would be required, with each step of the recursion
checking for a recognized form.  Second, even recognizing such a form is tricky,
because the expansion may refer to deftest in an aliased namespace (e.g.,
`t/deftest` or `clojure.test/deftest` rather than bare `deftest`).  One could
then try to identify possible namespaces using the current environment, but this
is getting complicated.

Therefore, I think it better to have the user configure their environment to
help them solve this problem.  (And, of course, future work can implement the
full macroexpansion approach.)
ncalexan added a commit to ncalexan/cider that referenced this issue Jan 4, 2017
…efining-forms`.

This fix is strictly simpler than that suggested in
clojure-emacs#1776 (comment).  It
has the advantage of being much more robust.

I partially implemented the suggested fix, and witnessed the following two
issues.  First, full "macroexpand" does not leave a recognizable `deftest` form;
it generally leaves a `(def test-... (fn [] ...))` sexp.  That implies that a
recursive macroexpansion would be required, with each step of the recursion
checking for a recognized form.  Second, even recognizing such a form is tricky,
because the expansion may refer to deftest in an aliased namespace (e.g.,
`t/deftest` or `clojure.test/deftest` rather than bare `deftest`).  One could
then try to identify possible namespaces using the current environment, but this
is getting complicated.

Therefore, I think it better to have the user configure their environment to
help them solve this problem.  (And, of course, future work can implement the
full macroexpansion approach.)
@bbatsov bbatsov closed this as completed in a4d85b6 Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants