Skip to content

Conversation

vemv
Copy link
Member

@vemv vemv commented Sep 25, 2023

Fixes #2958
Fixes #3279

Rehash of #3480 with all my feedback directly applied.

Under this approach, cider-test-run-test will run the first valid test found by:

  • tests marked by text properties
  • deftests, and deftest-like forms (cider-test-defining-forms is no longer necessary)
  • :test metadata that may be present in vanilla defns
  • deftests, as found by cider-test-infer-test-ns
    • We search for a var named foo-test, falling back to foo (e.g. deftest foo - less idiomatic, but quite frequent)

All this lookup is very fast when cider-nrepl is present, since we use cider-resolve--get-in. It falls back to eval if we're on bare nrepl.

In a future, we can further search tests by leveraging xref functionality - but it's not as important now.

Special thanks to @tvirolai!

Fixes #2958
Fixes #3279

Co-authored-by: tvirolai <[email protected]>
@vemv vemv requested a review from bbatsov September 25, 2023 12:09
(found-ns (car found))
(found-var (cadr found)))
(if (not found-var)
(message "No test found at point")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be a (user-error instead?

cider-test.el Outdated
;; we're in a `cider-test-report-mode' buffer
;; or on a highlighted failed/erred test definition
(get-text-property (point) 'var)))
(found (or (when (and var-from-text-property
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this can be extracted to a private function.

(cider-resolve--get-in ns "interns" var "test")
(equal "true"
(nrepl-dict-get (cider-sync-tooling-eval
(format "(clojure.core/-> %s var clojure.core/meta (clojure.core/contains? :test))"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will do for now, but it'd be nice if we made something more portable at some point. I was also thinking it'd be handy to have a command that just dumps a var metadata in an overlay or a buffer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will do for now, but it'd be nice if we made something more portable at some point.

Speaking of, cljs functionality for our test-related functionality doesn't seem as hard as it might have been in the past.

Inspecting the cljs env, and evaling cljs code seem pretty vanilla things to do nowadays. We could look into it soon enough.

@vemv
Copy link
Member Author

vemv commented Sep 26, 2023

Thanks for the review!

@vemv vemv merged commit 52243cf into master Sep 26, 2023
@vemv vemv deleted the 3480 branch September 26, 2023 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot run tests defined with custom macro Support cider-test-run-test from the src NS

2 participants