[#537] A second stab at fixing the main/init problem (#539)

My fix yesterday was not properly tested - I started nREPL from the command line, but I didn't attempt to exercise it with messages. As a result, I missed the fact I was setting passing the middleware as the handler, which kept things just as broken.

This PR contains a fix for that problem, plus a new test ns cider.nrepl.main-test that exercises the start-nrepl function by passing it a variety of valid and invalid option maps. The tests go as far as checking that two :op "clone" messages can be exchanged before declaring success.

Author: hagmonk

src/cider_nrepl/main.clj

@@ -28,17 +28,35 @@
   [middleware-var-strs]
   (into [] mw-xf middleware-var-strs))
 
+(defn- build-handler
+  [middleware]
+  (apply nrepl.server/default-handler (->mw-list middleware)))
+
 (defn start-nrepl
-  [opts]
-  (let [{:keys [handler middleware bind port] :or {port 0 bind "::"}} opts
+  "Starts a socket-based nREPL server. Accepts a map with the following keys:
+ 
+   * :port — defaults to 0, which autoselects an open port
+
+   * :bind — bind address, by default \"::\" (falling back to \"localhost\" if
+     \"::\" isn't resolved by the underlying network stack)
+
+   * :handler — the nREPL message handler to use for each incoming connection;
+     defaults to the result of `(nrepl.server/default-handler)`
 
-        handler (cond-> (or handler nrepl.server/default-handler)
-                  middleware (apply (->mw-list middleware)))
+   * :middleware - a sequence of vars or string which can be resolved to vars,
+     representing middleware you wish to mix in to the nREPL handler. Vars can
+     resolve to a sequence of vars, in which case they'll be flattened into the
+     list of middleware."
+  [{:keys [handler middleware bind port] :as opts}]
+  (let [handler
+        (if handler
+          (handler)
+          (build-handler middleware))
 
         {:keys [server-socket port] :as server}
         (nrepl.server/start-server :handler handler
-                                   :bind bind
-                                   :port port)
+                                   :bind (or bind "localhost")
+                                   :port (or port 0))
 
         bind
         (-> server-socket (.getInetAddress) (.getHostName))]
@@ -50,8 +68,8 @@
 
 (defn init
   ([]
-   (init nil))
-  ([handlers]
-   (start-nrepl {:handler handlers})
+   (start-nrepl {}))
+  ([middleware]
+   (start-nrepl {:middleware middleware})
    ;; Return nil so the value doesn't print
    nil))

test/clj/cider/nrepl/main_test.clj

@@ -0,0 +1,43 @@
+(ns cider.nrepl.main-test
+  (:require [cider.nrepl :refer [wrap-debug cider-middleware]]
+            [cider-nrepl.main :as m]
+            [clojure.test :refer :all]
+            [clojure.tools.nrepl :as nrepl]
+            [clojure.tools.nrepl.server :as nrepl.server]
+            [clojure.tools.nrepl.transport :as transport]))
+
+(defn start-stop-nrepl-session [opts]
+  (with-open [server    (#'m/start-nrepl opts)
+              transport (nrepl/connect :port (:port server))]
+    (transport/send transport {:op "clone" :id 1})
+    (let [session-id (:new-session (transport/recv transport 1000))]
+      (assert session-id)
+      (transport/send transport {:session session-id
+                                 :id      2
+                                 :op      "clone"})
+      (is (= (:status (transport/recv transport 1000)) ["done"])))))
+
+(deftest start-nrepl-test
+  (testing "passing a specific handler should work"
+    (let [opts {:handler nrepl.server/default-handler}]
+      (start-stop-nrepl-session opts)))
+
+  (testing "passing a sequence instead of a map shouldn't crash"
+    (let [opts ["cider.nrepl/cider-middleware"]]
+      (start-stop-nrepl-session opts)))
+
+  (testing "passing nil shouldn't crash"
+    (let [opts nil]
+      (start-stop-nrepl-session opts)))
+
+  (testing "passing valid middleware should work"
+    (let [opts {:middleware ["cider.nrepl/cider-middleware"]}]
+      (start-stop-nrepl-session opts)))
+
+  (testing "passing options as given by boot task middleware should work"
+    (let [opts {:middleware '(cider.nrepl.middleware.version/wrap-version
+                              cider.nrepl.middleware.apropos/wrap-apropos)
+                :port nil
+                :bind nil}]
+      (start-stop-nrepl-session opts))))
+