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

Test runner not finding tests with + in middle of the name #1383

Closed
rjerue opened this issue Nov 16, 2021 · 7 comments
Closed

Test runner not finding tests with + in middle of the name #1383

rjerue opened this issue Nov 16, 2021 · 7 comments
Labels
bug Something isn't working pr-welcome test-runner

Comments

@rjerue
Copy link
Contributor

rjerue commented Nov 16, 2021

Given the following file:

(ns app.fun-test
  (:require [clojure.test :refer [deftest is]]))

(deftest name-with+in-it
  (is true))

(deftest name-with-plus-end+
  (is true))

(deftest name-with-plus-in-it
  (is true))

(deftest +name-with-plus-start
  (is true))

this is the output

; Evaluating file: fun_test.clj
#'app.fun-test/name-with-plus-in-it
; Running test: name-with+in-it…
; No tests found. 😱, ns: 0, vars: 0
clj꞉app.fun-test꞉> 
; Evaluating file: fun_test.clj
#'app.fun-test/name-with-plus-in-it
; Running test: name-with-plus-end+…
; 1 tests finished, all passing 👍, ns: 1, vars: 1
clj꞉app.fun-test꞉> 
; Evaluating file: fun_test.clj
#'app.fun-test/name-with-plus-in-it
; Running test: name-with-plus-in-it…
; 1 tests finished, all passing 👍, ns: 1, vars: 1
clj꞉app.fun-test꞉> 
; Evaluating file: fun_test.clj
#'app.fun-test/+name-with-plus-start
; Running test: +name-with-plus-start…

Note, the last one hangs with the following error in the console:

Exception in thread "nREPL-session-f877bb99-3510-4125-81e9-6647caa1a1dd" 
java.util.regex.PatternSyntaxException: Dangling meta character '+' near index 0
+name-with-plus-end

The last error + regex escaping fun a coworker had while searching for the test lead me to believe that there's a regex escaping issue somewhere with the + character.

Expected behavior is that these tests would be detected with the Run Current Test command.

@bpringe bpringe added test-runner bug Something isn't working labels Nov 17, 2021
@bpringe
Copy link
Member

bpringe commented Nov 17, 2021

Thanks for reporting! We should see if this might be an issue with nrepl/cider-nrepl. Calva pretty much just uses cider-nrepl to run the tests - see here: https://github.com/BetterThanTomorrow/calva/blob/published/src/testRunner.ts.

Do you have the same / similar issues when running the command to run all tests and the command to run all tests for the namespace?

@bpringe
Copy link
Member

bpringe commented Nov 17, 2021

@bbatsov Have you seen something like this before?

@rjerue
Copy link
Contributor Author

rjerue commented Nov 21, 2021

Using the same file as above, if I use the run all tests or run all tests in current namespace, things seem to work fine. It's only the individual "run current test" where things break.

@bbatsov
Copy link
Contributor

bbatsov commented Nov 22, 2021

@bpringe I haven't seen this problem, but it's likely on cider-nrepl's side. Feel free to file a ticket there, so we don't forget to look into this. I guess it should be an easy fix.

@bpringe
Copy link
Member

bpringe commented Nov 27, 2021

It seems we can/should fix this issue in Calva. Details are here: clojure-emacs/cider-nrepl#730 (comment).

If you pass a + it should be escaped (presumably client-side: we shouldn't second-guess what a client meant).

@marcomorain
Copy link
Contributor

I might take a look at this this week.

marcomorain added a commit to marcomorain/calva that referenced this issue Nov 30, 2021
Per clojure-emacs/cider-nrepl#730, clients
should make sure that queries are escaped. We can use the
`escape-string-regexp` package, which is already in the source-tree for
this.

Also, change the call to `test-var-query` to narrow the search to just
`test?` true, and remove `search-property`, since `name` is the default
per the Cider docs.

Fixes BetterThanTomorrow#1383
@marcomorain
Copy link
Contributor

I've opened a PR that addresses this.

@bpringe bpringe closed this as completed in ac6c35a Dec 1, 2021
@PEZ PEZ mentioned this issue Dec 2, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pr-welcome test-runner
Projects
None yet
Development

No branches or pull requests

4 participants