cljr--find-source-ns-of-test-ns can choose the wrong namespace when there are multiple matches
The sut can choose the wrong namespace when there are multiple partial matches. For example, given a source tree with:
src/advent/day1.clj
src/advent/day10.clj
Creating a new buffer named test/advent/day10_test.clj results in:
(ns advent.day10-test
(:require [advent.day1 :as sut]
[clojure.test :as t]))
When we'd expect:
(ns advent.day10-test
(:require [advent.day10 :as sut]
[clojure.test :as t]))
I have a partial fix at https://github.com/orb/clj-refactor.el/commit/a8306c2ff0c81d5930e90afa4a304e2a495bed55. I did not submit a PR because I didn't meet all the contribution requirements yet. However, the basic idea was that instead of finding the first matching file, we'd find the longest matching file.
This solution works well when the sut class exists. If, in this example, src/advent/day10.clj did not exist yet, it would still choose advent.day1 as the best match. I don't like that, but I'm not sure how to best fix that and still maintain the flexibility the code is apparently trying to achieve.
Hi, thanks for taking an interest!
I'm not sure if we need two passes (candidates and then final selection). Perhaps the code would just do the right thing if we checked for equality instead?
Given a test buffer /home/expez/moneymaker/test/foo/bar/baz.cljall we'd have to do is replace test with src and we should have an exact match, right?
This solution works well when the
sutclass exists.
We definitely need to handle the other case too, because some people like to write the tests before the implementation.
If the sut class doesn't exist, ideally there would either be no sut import or it should import the "correct" non-existant class, resulting in an immediate error unless we generate the corresponding namespace too.
Take the test namespace, 'day10-test' in this case (with the ns being derived from the buffer name). It looks in the src/advent directory (tranforming test to src) and finds the first file that (after dropping the extension switching - and _) is either a prefix or suffix of day10-test. I assume the logic here is to be able to accommodate both test-* and *-test(s) patterns while failing gracefully (with no sut require) if it can't find a match. However, it current matches much more broadly than that and there may be other patterns we desire to match.
If we only care about the test-* and *-test(s) patterns then I would definitely suggest that instead of my solution we strip any test(s) prefix or suffixes from the test ns and add the sut require only if the corresponding source file exists. I can put together an alternate fix for that for comparison.
Looking at the tests, the feature is definitely intended to match aribtrary prefixes and suffixes.
Here's an addition to the test cases the demonstrate the problem:
diff --git a/features/auto-ns.feature b/features/auto-ns.feature
index e0d2078..cbf7954 100644
--- a/features/auto-ns.feature
+++ b/features/auto-ns.feature
@@ -3,6 +3,7 @@ Feature: Add namespace to blank .clj files
Background:
Given I have a project "cljr" in "tmp"
When I have a clojure-file "tmp/src/cljr/core.clj"
+ And I have a clojure-file "tmp/src/cljr/co.clj"
Scenario: Source file
When I open file "tmp/src/cljr/core.clj"
This causes multiple failures. Here's one:
Scenario: Test file
When I open file "tmp/test/cljr/core_test.clj"
Then I should see:
"""
(ns cljr.core-test
(:require [cljr.core :as sut]
[clojure.test :as t]))
"""
Expected
(ns cljr.core-test
(:require [cljr.core :as sut]
[clojure.test :as t]))
to be part of:
(ns cljr.core-test
(:require [cljr.co :as sut]
[clojure.test :as t]))