dev cleanup: consider reviewing commented tests

Currently

There are a number of tests that are commented out.
They are historical and were brought over from the hiredman fork.

In my experience code that is commented out is too mysterious and can lead to more of the same.

Propsoal

Review these disabled tests and do one of:

uncomment - if they work
uncomment and fix - if they should work and the fix is easy enough
quietly delete - if they are not relevant
delete and create a git issue - if we can determine that they represent some sort of TODO

For examples:

clj-http.test.core/delete-with-body is commented with "HUC can't do this" (HUC = HttpURLConnection). This seems to be not the case today. Uncomment. Our README states "No proxy-ing DELETEs with body", not sure what the "proxying" means here. Anybody?

clj-http-lite/test/clj_http/test/core.clj

Lines 147 to 152 in da0e634

    
           ;; HUC can't do this 
        
           ;; (deftest ^{:integration true} delete-with-body 
        
           ;;   (run-server) 
        
           ;;   (let [resp (request {:request-method :delete :uri "/delete-with-body" 
        
           ;;                        :body (.getBytes "foo bar")})] 
        
           ;;     (is (= 200 (:status resp)))))

clj-http.test.core/parse-headers - is commented it with no explanation. Delete? Or Fix?

clj-http-lite/test/clj_http/test/core.clj

Lines 181 to 206 in da0e634

    
           ;; (deftest parse-headers 
        
           ;;   (are [headers expected] 
        
           ;;        (let [iterator (BasicHeaderIterator. 
        
           ;;                        (into-array BasicHeader 
        
           ;;                                    (map (fn [[name value]] 
        
           ;;                                           (BasicHeader. name value)) 
        
           ;;                                         headers)) 
        
           ;;                        nil)] 
        
           ;;          (is (= (core/parse-headers iterator) 
        
           ;;                 expected))) 
        
           ;;        [] 
        
           ;;        {} 
        
           ;;        [["Set-Cookie" "one"]] 
        
           ;;        {"set-cookie" "one"} 
        
           ;;        [["Set-Cookie" "one"] 
        
           ;;         ["set-COOKIE" "two"]] 
        
           ;;        {"set-cookie" ["one" "two"]} 
        
           ;;        [["Set-Cookie" "one"] 
        
           ;;         ["serVer"     "some-server"] 
        
           ;;         ["set-cookie" "two"]] 
        
           ;;        {"set-cookie" ["one" "two"] 
        
           ;;         "server"     "some-server"}))

clj-http.test.cookies - every test in this namespace is commented out. Our README states "No cookie support". Delete.

clj-http-lite/test/clj_http/test/cookies.clj

Lines 1 to 209 in da0e634

    
           (ns clj-http.test.cookies 
        
             (:use [clj-http.lite.util] 
        
                   [clojure.test])) 
        
           ;; (defn refer-private [ns] 
        
           ;;   (doseq [[symbol var] (ns-interns ns)] 
        
           ;;     (when (:private (meta var)) 
        
           ;;       (intern *ns* symbol var)))) 
        
           ;; (refer-private 'clj-http.cookies) 
        
           ;; (def session (str "ltQGXSNp7cgNeFG6rPE06qzriaI+R8W7zJKFu4UOlX4=-" 
        
           ;;                   "-lWgojFmZlDqSBnYJlUmwhqXL4OgBTkra5WXzi74v+nE=")) 
        
           ;; (deftest test-compact-map 
        
           ;;   (are [map expected] 
        
           ;;        (is (= expected (compact-map map))) 
        
           ;;        {:a nil :b 2 :c 3 :d nil} 
        
           ;;        {:b 2 :c 3} 
        
           ;;        {:comment nil :domain "example.com" :path "/" :ports [80 8080] :value 1} 
        
           ;;        {:domain "example.com" :path "/" :ports [80 8080] :value 1})) 
        
           ;; (deftest test-decode-cookie 
        
           ;;   (are [set-cookie-str expected] 
        
           ;;        (is (= expected (decode-cookie set-cookie-str))) 
        
           ;;        nil nil 
        
           ;;        "" nil 
        
           ;;        "example-cookie=example-value;Path=/" 
        
           ;;        ["example-cookie" 
        
           ;;         {:discard true :path "/" :value "example-value" :version 0}] 
        
           ;;        "example-cookie=example-value;Domain=.example.com;Path=/" 
        
           ;;        ["example-cookie" 
        
           ;;         {:discard true :domain ".example.com" :path "/" 
        
           ;;          :value "example-value" :version 0}])) 
        
           ;; (deftest test-decode-cookies-with-seq 
        
           ;;   (let [cookies (decode-cookies [(str "ring-session=" (url-encode session))])] 
        
           ;;     (is (map? cookies)) 
        
           ;;     (is (= 1 (count cookies))) 
        
           ;;     (let [cookie (get cookies "ring-session")] 
        
           ;;       (is (= true (:discard cookie))) 
        
           ;;       (is (nil? (:domain cookie))) 
        
           ;;       (is (= "/" (:path cookie))) 
        
           ;;       (is (= session (:value cookie))) 
        
           ;;       (is (= 0 (:version cookie)))))) 
        
           ;; (deftest test-decode-cookies-with-string 
        
           ;;   (let [cookies (decode-cookies 
        
           ;;                  (str "ring-session=" (url-encode session) ";Path=/"))] 
        
           ;;     (is (map? cookies)) 
        
           ;;     (is (= 1 (count cookies))) 
        
           ;;     (let [cookie (get cookies "ring-session")] 
        
           ;;       (is (= true (:discard cookie))) 
        
           ;;       (is (nil? (:domain cookie))) 
        
           ;;       (is (= "/" (:path cookie))) 
        
           ;;       (is (= session (:value cookie))) 
        
           ;;       (is (= 0 (:version cookie)))))) 
        
           ;; (deftest test-decode-cookie-header 
        
           ;;   (are [response expected] 
        
           ;;        (is (= expected (decode-cookie-header response))) 
        
           ;;        {:headers {"set-cookie" "a=1"}} 
        
           ;;        {:cookies {"a" {:discard true :path "/" 
        
           ;;                        :value "1" :version 0}} :headers {}} 
        
           ;;        {:headers {"set-cookie" 
        
           ;;                   (str "ring-session=" (url-encode session) ";Path=/")}} 
        
           ;;        {:cookies {"ring-session" 
        
           ;;                   {:discard true :path "/" 
        
           ;;                    :value session :version 0}} :headers {}})) 
        
           ;; (deftest test-encode-cookie 
        
           ;;   (are [cookie expected] 
        
           ;;        (is (= expected (encode-cookie cookie))) 
        
           ;;        [:a {:value "b"}] "a=b" 
        
           ;;        ["a" {:value "b"}] "a=b" 
        
           ;;        ["example-cookie" 
        
           ;;         {:domain ".example.com" :path "/" :value "example-value"}] 
        
           ;;        "example-cookie=example-value" 
        
           ;;        ["ring-session" {:value session}] 
        
           ;;        (str "ring-session=" (url-encode session)))) 
        
           ;; (deftest test-encode-cookies 
        
           ;;   (are [cookie expected] 
        
           ;;        (is (= expected (encode-cookies cookie))) 
        
           ;;        {:a {:value "b"} :c {:value "d"} :e {:value "f"}} 
        
           ;;        "a=b;c=d;e=f" 
        
           ;;        {"a" {:value "b"} "c" {:value "d"} "e" {:value "f"}} 
        
           ;;        "a=b;c=d;e=f" 
        
           ;;        {"example-cookie" 
        
           ;;         {:domain ".example.com" :path "/" :value "example-value"}} 
        
           ;;        "example-cookie=example-value" 
        
           ;;        {"example-cookie" 
        
           ;;         {:domain ".example.com" :path "/" :value "example-value" 
        
           ;;          :discard true :version 0}} 
        
           ;;        "example-cookie=example-value" 
        
           ;;        {"ring-session" {:value session}} 
        
           ;;        (str "ring-session=" (url-encode session)))) 
        
           ;; (deftest test-encode-cookie-header 
        
           ;;   (are [request expected] 
        
           ;;        (is (= expected (encode-cookie-header request))) 
        
           ;;        {:cookies {"a" {:value "1"}}} 
        
           ;;        {:headers {"Cookie" "a=1"}} 
        
           ;;        {:cookies 
        
           ;;         {"example-cookie" {:domain ".example.com" :path "/" 
        
           ;;                            :value "example-value"}}} 
        
           ;;        {:headers {"Cookie" "example-cookie=example-value"}})) 
        
           ;; (deftest test-to-basic-client-cookie-with-simple-cookie 
        
           ;;   (let [cookie (to-basic-client-cookie 
        
           ;;                 ["ring-session" 
        
           ;;                  {:value session 
        
           ;;                   :path "/" 
        
           ;;                   :domain "example.com"}])] 
        
           ;;     (is (= "ring-session" (.getName cookie))) 
        
           ;;     (is (= (url-encode session) (.getValue cookie))) 
        
           ;;     (is (= "/" (.getPath cookie))) 
        
           ;;     (is (= "example.com" (.getDomain cookie))) 
        
           ;;     (is (nil? (.getComment cookie))) 
        
           ;;     (is (nil? (.getCommentURL cookie))) 
        
           ;;     (is (not (.isPersistent cookie))) 
        
           ;;     (is (nil? (.getExpiryDate cookie))) 
        
           ;;     (is (nil? (seq (.getPorts cookie)))) 
        
           ;;     (is (not (.isSecure cookie))) 
        
           ;;     (is (= 0 (.getVersion cookie))))) 
        
           ;; (deftest test-to-basic-client-cookie-with-full-cookie 
        
           ;;   (let [cookie (to-basic-client-cookie 
        
           ;;                 ["ring-session" 
        
           ;;                  {:value session 
        
           ;;                   :path "/" 
        
           ;;                   :domain "example.com" 
        
           ;;                   :comment "Example Comment" 
        
           ;;                   :comment-url "http://example.com/cookies" 
        
           ;;                   :discard true 
        
           ;;                   :expires (java.util.Date. (long 0)) 
        
           ;;                   :ports [80 8080] 
        
           ;;                   :secure true 
        
           ;;                   :version 0}])] 
        
           ;;     (is (= "ring-session" (.getName cookie))) 
        
           ;;     (is (= (url-encode session) (.getValue cookie))) 
        
           ;;     (is (= "/" (.getPath cookie))) 
        
           ;;     (is (= "example.com" (.getDomain cookie))) 
        
           ;;     (is (= "Example Comment" (.getComment cookie))) 
        
           ;;     (is (= "http://example.com/cookies" (.getCommentURL cookie))) 
        
           ;;     (is (not (.isPersistent cookie))) 
        
           ;;     (is (= (java.util.Date. (long 0)) (.getExpiryDate cookie))) 
        
           ;;     (is (= [80 8080] (seq (.getPorts cookie)))) 
        
           ;;     (is (.isSecure cookie)) 
        
           ;;     (is (= 0 (.getVersion cookie))))) 
        
           ;; (deftest test-to-basic-client-cookie-with-symbol-as-name 
        
           ;;   (let [cookie (to-basic-client-cookie 
        
           ;;                 [:ring-session {:value session :path "/" 
        
           ;;                                 :domain "example.com"}])] 
        
           ;;     (is (= "ring-session" (.getName cookie))))) 
        
           ;; (deftest test-to-cookie-with-simple-cookie 
        
           ;;   (let [[name content] 
        
           ;;         (to-cookie 
        
           ;;          (doto (BasicClientCookie. "example-cookie" "example-value") 
        
           ;;            (.setDomain "example.com") 
        
           ;;            (.setPath "/")))] 
        
           ;;     (is (= "example-cookie" name)) 
        
           ;;     (is (nil? (:comment content))) 
        
           ;;     (is (nil? (:comment-url content))) 
        
           ;;     (is (:discard content)) 
        
           ;;     (is (= "example.com" (:domain content))) 
        
           ;;     (is (nil? (:expires content))) 
        
           ;;     (is (nil? (:ports content))) 
        
           ;;     (is (not (:secure content))) 
        
           ;;     (is (= 0 (:version content))) 
        
           ;;     (is (= "example-value" (:value content))))) 
        
           ;; (deftest test-to-cookie-with-full-cookie 
        
           ;;   (let [[name content] 
        
           ;;         (to-cookie 
        
           ;;          (doto (BasicClientCookie2. "example-cookie" "example-value") 
        
           ;;            (.setComment "Example Comment") 
        
           ;;            (.setCommentURL "http://example.com/cookies") 
        
           ;;            (.setDiscard true) 
        
           ;;            (.setDomain "example.com") 
        
           ;;            (.setExpiryDate (java.util.Date. (long 0))) 
        
           ;;            (.setPath "/") 
        
           ;;            (.setPorts (int-array [80 8080])) 
        
           ;;            (.setSecure true) 
        
           ;;            (.setVersion 1)))] 
        
           ;;     (is (= "example-cookie" name)) 
        
           ;;     (is (= "Example Comment" (:comment content))) 
        
           ;;     (is (= "http://example.com/cookies" (:comment-url content))) 
        
           ;;     (is (= true (:discard content))) 
        
           ;;     (is (= "example.com" (:domain content))) 
        
           ;;     (is (= (java.util.Date. (long 0)) (:expires content))) 
        
           ;;     (is (= [80 8080] (:ports content))) 
        
           ;;     (is (= true (:secure content))) 
        
           ;;     (is (= 1 (:version content))) 
        
           ;;     (is (= "example-value" (:value content))))) 
        
           ;; (deftest test-wrap-cookies 
        
           ;;   (is (= {:cookies {"example-cookie" {:discard true :domain ".example.com" 
        
           ;;                                       :path "/" :value "example-value" 
        
           ;;                                       :version 0}} :headers {}} 
        
           ;;          ((wrap-cookies 
        
           ;;            (fn [request] 
        
           ;;              (is (= (get (:headers request) "Cookie") "a=1;b=2")) 
        
           ;;              {:headers 
        
           ;;               {"set-cookie" 
        
           ;;                "example-cookie=example-value;Domain=.example.com;Path=/"}})) 
        
           ;;           {:cookies {:a {:value "1"} :b {:value "2"}}}))))

I'd likely also move clj-http.test.client/rountrip to clj-http.test.core.
The roundtrip test is the only integration test in the client ns (all others are in core), and it introduces a server setup fixture that is not used by any other test in the client namespace.

Next Steps

If we agree these are helpful things to cleanup, I shall proceed with a PR.

A little sleuthing on delete with body:

hiredman forked clj-http-lite from darkron/clj-http in 2012
At that time JDK7 was the current release
I think that delete with body was fixed in JDK8
Our min supported version is JDK8, so we can uncomment test and update README

	;; HUC can't do this
	;; (deftest ^{:integration true} delete-with-body
	;; (run-server)
	;; (let [resp (request {:request-method :delete :uri "/delete-with-body"
	;; :body (.getBytes "foo bar")})]
	;; (is (= 200 (:status resp)))))

	;; (deftest parse-headers
	;; (are [headers expected]
	;; (let [iterator (BasicHeaderIterator.
	;; (into-array BasicHeader
	;; (map (fn [[name value]]
	;; (BasicHeader. name value))
	;; headers))
	;; nil)]
	;; (is (= (core/parse-headers iterator)
	;; expected)))

	;; []
	;; {}

	;; [["Set-Cookie" "one"]]
	;; {"set-cookie" "one"}

	;; [["Set-Cookie" "one"]
	;; ["set-COOKIE" "two"]]
	;; {"set-cookie" ["one" "two"]}

	;; [["Set-Cookie" "one"]
	;; ["serVer" "some-server"]
	;; ["set-cookie" "two"]]
	;; {"set-cookie" ["one" "two"]
	;; "server" "some-server"}))

	(ns clj-http.test.cookies
	(:use [clj-http.lite.util]
	[clojure.test]))

	;; (defn refer-private [ns]
	;; (doseq [[symbol var] (ns-interns ns)]
	;; (when (:private (meta var))
	;; (intern ns symbol var))))

	;; (refer-private 'clj-http.cookies)

	;; (def session (str "ltQGXSNp7cgNeFG6rPE06qzriaI+R8W7zJKFu4UOlX4=-"
	;; "-lWgojFmZlDqSBnYJlUmwhqXL4OgBTkra5WXzi74v+nE="))

	;; (deftest test-compact-map
	;; (are [map expected]
	;; (is (= expected (compact-map map)))
	;; {:a nil :b 2 :c 3 :d nil}
	;; {:b 2 :c 3}
	;; {:comment nil :domain "example.com" :path "/" :ports [80 8080] :value 1}
	;; {:domain "example.com" :path "/" :ports [80 8080] :value 1}))

	;; (deftest test-decode-cookie
	;; (are [set-cookie-str expected]
	;; (is (= expected (decode-cookie set-cookie-str)))
	;; nil nil
	;; "" nil
	;; "example-cookie=example-value;Path=/"
	;; ["example-cookie"
	;; {:discard true :path "/" :value "example-value" :version 0}]
	;; "example-cookie=example-value;Domain=.example.com;Path=/"
	;; ["example-cookie"
	;; {:discard true :domain ".example.com" :path "/"
	;; :value "example-value" :version 0}]))

	;; (deftest test-decode-cookies-with-seq
	;; (let [cookies (decode-cookies [(str "ring-session=" (url-encode session))])]
	;; (is (map? cookies))
	;; (is (= 1 (count cookies)))
	;; (let [cookie (get cookies "ring-session")]
	;; (is (= true (:discard cookie)))
	;; (is (nil? (:domain cookie)))
	;; (is (= "/" (:path cookie)))
	;; (is (= session (:value cookie)))
	;; (is (= 0 (:version cookie))))))

	;; (deftest test-decode-cookies-with-string
	;; (let [cookies (decode-cookies
	;; (str "ring-session=" (url-encode session) ";Path=/"))]
	;; (is (map? cookies))
	;; (is (= 1 (count cookies)))
	;; (let [cookie (get cookies "ring-session")]
	;; (is (= true (:discard cookie)))
	;; (is (nil? (:domain cookie)))
	;; (is (= "/" (:path cookie)))
	;; (is (= session (:value cookie)))
	;; (is (= 0 (:version cookie))))))

	;; (deftest test-decode-cookie-header
	;; (are [response expected]
	;; (is (= expected (decode-cookie-header response)))
	;; {:headers {"set-cookie" "a=1"}}
	;; {:cookies {"a" {:discard true :path "/"
	;; :value "1" :version 0}} :headers {}}
	;; {:headers {"set-cookie"
	;; (str "ring-session=" (url-encode session) ";Path=/")}}
	;; {:cookies {"ring-session"
	;; {:discard true :path "/"
	;; :value session :version 0}} :headers {}}))

	;; (deftest test-encode-cookie
	;; (are [cookie expected]
	;; (is (= expected (encode-cookie cookie)))
	;; [:a {:value "b"}] "a=b"
	;; ["a" {:value "b"}] "a=b"
	;; ["example-cookie"
	;; {:domain ".example.com" :path "/" :value "example-value"}]
	;; "example-cookie=example-value"
	;; ["ring-session" {:value session}]
	;; (str "ring-session=" (url-encode session))))

	;; (deftest test-encode-cookies
	;; (are [cookie expected]
	;; (is (= expected (encode-cookies cookie)))
	;; {:a {:value "b"} :c {:value "d"} :e {:value "f"}}
	;; "a=b;c=d;e=f"
	;; {"a" {:value "b"} "c" {:value "d"} "e" {:value "f"}}
	;; "a=b;c=d;e=f"
	;; {"example-cookie"
	;; {:domain ".example.com" :path "/" :value "example-value"}}
	;; "example-cookie=example-value"
	;; {"example-cookie"
	;; {:domain ".example.com" :path "/" :value "example-value"
	;; :discard true :version 0}}
	;; "example-cookie=example-value"
	;; {"ring-session" {:value session}}
	;; (str "ring-session=" (url-encode session))))

	;; (deftest test-encode-cookie-header
	;; (are [request expected]
	;; (is (= expected (encode-cookie-header request)))
	;; {:cookies {"a" {:value "1"}}}
	;; {:headers {"Cookie" "a=1"}}
	;; {:cookies
	;; {"example-cookie" {:domain ".example.com" :path "/"
	;; :value "example-value"}}}
	;; {:headers {"Cookie" "example-cookie=example-value"}}))

	;; (deftest test-to-basic-client-cookie-with-simple-cookie
	;; (let [cookie (to-basic-client-cookie
	;; ["ring-session"
	;; {:value session
	;; :path "/"
	;; :domain "example.com"}])]
	;; (is (= "ring-session" (.getName cookie)))
	;; (is (= (url-encode session) (.getValue cookie)))
	;; (is (= "/" (.getPath cookie)))
	;; (is (= "example.com" (.getDomain cookie)))
	;; (is (nil? (.getComment cookie)))
	;; (is (nil? (.getCommentURL cookie)))
	;; (is (not (.isPersistent cookie)))
	;; (is (nil? (.getExpiryDate cookie)))
	;; (is (nil? (seq (.getPorts cookie))))
	;; (is (not (.isSecure cookie)))
	;; (is (= 0 (.getVersion cookie)))))

	;; (deftest test-to-basic-client-cookie-with-full-cookie
	;; (let [cookie (to-basic-client-cookie
	;; ["ring-session"
	;; {:value session
	;; :path "/"
	;; :domain "example.com"
	;; :comment "Example Comment"
	;; :comment-url "http://example.com/cookies"
	;; :discard true
	;; :expires (java.util.Date. (long 0))
	;; :ports [80 8080]
	;; :secure true
	;; :version 0}])]
	;; (is (= "ring-session" (.getName cookie)))
	;; (is (= (url-encode session) (.getValue cookie)))
	;; (is (= "/" (.getPath cookie)))
	;; (is (= "example.com" (.getDomain cookie)))
	;; (is (= "Example Comment" (.getComment cookie)))
	;; (is (= "http://example.com/cookies" (.getCommentURL cookie)))
	;; (is (not (.isPersistent cookie)))
	;; (is (= (java.util.Date. (long 0)) (.getExpiryDate cookie)))
	;; (is (= [80 8080] (seq (.getPorts cookie))))
	;; (is (.isSecure cookie))
	;; (is (= 0 (.getVersion cookie)))))

	;; (deftest test-to-basic-client-cookie-with-symbol-as-name
	;; (let [cookie (to-basic-client-cookie
	;; [:ring-session {:value session :path "/"
	;; :domain "example.com"}])]
	;; (is (= "ring-session" (.getName cookie)))))

	;; (deftest test-to-cookie-with-simple-cookie
	;; (let [[name content]
	;; (to-cookie
	;; (doto (BasicClientCookie. "example-cookie" "example-value")
	;; (.setDomain "example.com")
	;; (.setPath "/")))]
	;; (is (= "example-cookie" name))
	;; (is (nil? (:comment content)))
	;; (is (nil? (:comment-url content)))
	;; (is (:discard content))
	;; (is (= "example.com" (:domain content)))
	;; (is (nil? (:expires content)))
	;; (is (nil? (:ports content)))
	;; (is (not (:secure content)))
	;; (is (= 0 (:version content)))
	;; (is (= "example-value" (:value content)))))

	;; (deftest test-to-cookie-with-full-cookie
	;; (let [[name content]
	;; (to-cookie
	;; (doto (BasicClientCookie2. "example-cookie" "example-value")
	;; (.setComment "Example Comment")
	;; (.setCommentURL "http://example.com/cookies")
	;; (.setDiscard true)
	;; (.setDomain "example.com")
	;; (.setExpiryDate (java.util.Date. (long 0)))
	;; (.setPath "/")
	;; (.setPorts (int-array [80 8080]))
	;; (.setSecure true)
	;; (.setVersion 1)))]
	;; (is (= "example-cookie" name))
	;; (is (= "Example Comment" (:comment content)))
	;; (is (= "http://example.com/cookies" (:comment-url content)))
	;; (is (= true (:discard content)))
	;; (is (= "example.com" (:domain content)))
	;; (is (= (java.util.Date. (long 0)) (:expires content)))
	;; (is (= [80 8080] (:ports content)))
	;; (is (= true (:secure content)))
	;; (is (= 1 (:version content)))
	;; (is (= "example-value" (:value content)))))

	;; (deftest test-wrap-cookies
	;; (is (= {:cookies {"example-cookie" {:discard true :domain ".example.com"
	;; :path "/" :value "example-value"
	;; :version 0}} :headers {}}
	;; ((wrap-cookies
	;; (fn [request]
	;; (is (= (get (:headers request) "Cookie") "a=1;b=2"))
	;; {:headers
	;; {"set-cookie"
	;; "example-cookie=example-value;Domain=.example.com;Path=/"}}))
	;; {:cookies {:a {:value "1"} :b {:value "2"}}}))))