Merge pull request #16 from shopsmart/pre-1.0

Exhaustively validate retry-with-timeout parameter values

Author: coconutpalm

.gitignore

@@ -9,3 +9,4 @@ pom.xml.asc
 /.nrepl-port
 .hgignore
 .hg/
+.nightlight.edn

project.clj

@@ -1,4 +1,4 @@
-(defproject com.github.shopsmart/clj-foundation "0.9.16"
+(defproject com.github.shopsmart/clj-foundation "0.9.17"
   :description "Common patterns enabling simpler to be easier and harder to be possibler."
   :url "https://github.com/shopsmart/clj-foundation"
 

src/clj_foundation/errors.clj

@@ -167,7 +167,7 @@
   (fn [f & args] (apply retry tries pause-millis f args)))
 
 
-;; Retry calling fn with a specified timeout; abort early if abort? returns truthy --
+;; Retry calling fn with a specified timeout; abort early if abort?-fn returns truthy
 
 
 (def TIMEOUT-ERROR
@@ -232,7 +232,7 @@
      abort?-fn      :- (=> s/Bool [[Throwable]])])
 
 
-(s/defn retry-with-timeout :- s/Any
+(s/defn ^:always-validate retry-with-timeout :- s/Any
   "Retry (apply f args) up to tries times with pause-millis time in between invocation and a
   timeout value of timeout-millis.  On failure, abort?-fn is called with a vector containing the
   unwrapped exception stack.
@@ -254,6 +254,8 @@
    f              :- (=> s/Any [s/Any])
    & args         :- [s/Any]]
 
+  (s/validate RetrySettings settings)
+
   (let [tries          (:tries settings)
         timeout-millis (:timeout-millis settings)
         pause-millis   (:pause-millis settings)

test/clj_foundation/errors_test.clj

@@ -7,7 +7,8 @@
             [clj-foundation.data :refer [any?]]
             [clj-foundation.patterns :as p]
             [clj-foundation.millis :as millis])
-  (:import [java.util Date]))
+  (:import [java.util Date]
+           [clojure.lang ExceptionInfo]))
 
 
 (common/register-fixtures)
@@ -157,41 +158,101 @@
 
 
 (deftest retry-with-timeout-test
-  (testing "Success -> Results!!!"
-    (is (= "Results!!!"
-           (retry-with-timeout
-            "Success"
-            (->RetrySettings 1 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
-            (constantly "Results!!!")))))
-
-  (testing "Taking too much time fails!"
-    (is (instance? RuntimeException
-                   (try*
-                    (retry-with-timeout
-                     "Sloooow"
-                     (->RetrySettings 3 (millis/<-seconds 1) 50 (constantly false))
-                     #(Thread/sleep (millis/<-seconds 2)))))))
-
-  (testing "Fatal errors abort retrying"
-    (is (instance? RuntimeException
-                   (try*
-                    (retry-with-timeout
-                     "Ooops..."
-                     (->RetrySettings 3 (millis/<-seconds 1) (millis/<-seconds 5) (constantly true))
-                     #(throw (RuntimeException.)))))))
-
-  (testing "If at first you don't succeed, try, try again..."
-    (let [attempts (atom 0)
-          job-fn (fn []
-                   (swap! attempts inc)
-                   (when (< @attempts 2) (throw (RuntimeException. "Not this time")))
-                   "Finally--success!")]
-      (is (= "Finally--success!"
+  (testing "nil arguments throw ExceptionInfo"
+    (testing "nil in main argument list"
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    nil
+                    (->RetrySettings 1 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
+                    (constantly "nil not allowed!"))))
+
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    nil
+                    (constantly "nil not allowed!"))))
+
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    (->RetrySettings 1 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
+                    nil))))
+
+    (testing "nil in RetrySettings"
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    (->RetrySettings nil (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
+                    (constantly "nil not allowed!"))))
+
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    (->RetrySettings 1 nil (millis/<-seconds 5) (constantly false))
+                    (constantly "nil not allowed!"))))
+
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    (->RetrySettings 1 (millis/<-seconds 1) nil (constantly false))
+                    (constantly "nil not allowed!"))))
+
+      (is (thrown? ExceptionInfo
+                   (retry-with-timeout
+                    "nil throws!"
+                    (->RetrySettings 1 (millis/<-seconds 1) (millis/<-seconds 5) nil)
+                    (constantly "nil not allowed!"))))))
+
+  (testing "Happy paths: "
+    (testing "Success -> Results!!!"
+      (is (= "Results!!!"
              (retry-with-timeout
-              "Persistance pays off"
-              (->RetrySettings 3 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
-              job-fn)))
-      (is (= 2 @attempts)))))
+              "Success"
+              (->RetrySettings 1 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
+              (constantly "Results!!!")))))
+
+    (testing "If at first you don't succeed, try, try again..."
+      (let [attempts (atom 0)
+            job-fn (fn []
+                     (swap! attempts inc)
+                     (when (< @attempts 2) (throw (RuntimeException. "Not this time")))
+                     "Finally--success!")]
+        (is (= "Finally--success!"
+               (retry-with-timeout
+                "Persistance pays off"
+                (->RetrySettings 3 (millis/<-seconds 1) (millis/<-seconds 5) (constantly false))
+                job-fn)))
+        (is (= 2 @attempts)))))
+
+  (testing "Sad paths: "
+    (testing "Taking too much time fails when abort?-fn is (constantly false)!"
+      (is (thrown? RuntimeException
+                   (retry-with-timeout
+                    "Sloooow"
+                    (->RetrySettings 3 (millis/<-seconds 1) 50 (constantly false))
+                    #(Thread/sleep (millis/<-seconds 2))))))
+
+    (testing "Taking too much time fails--even with retries! (Note: timeouts are not exceptions so abort?-fn is not relevant)"
+      (let [total-tries (atom 0)]
+        (is (thrown? RuntimeException
+                     (retry-with-timeout
+                      "Sloooow"
+                      (->RetrySettings 3 (millis/<-seconds 1) 50 (constantly true))
+                      (fn []
+                        (swap! total-tries inc)
+                        (Thread/sleep (millis/<-seconds 2))))))
+        (is (= 4 @total-tries))))
+
+    (testing "Fatal errors abort retrying"
+      (let [total-tries (atom 0)]
+        (is (thrown? RuntimeException
+                     (retry-with-timeout
+                      "Ooops..."
+                      (->RetrySettings 3 (millis/<-seconds 1) (millis/<-seconds 5) (constantly true))
+                      (fn []
+                        (swap! total-tries inc)
+                        (throw (RuntimeException.))))))
+        (is (= 1 @total-tries))))))
 
 
 ;; (dis)allowed values ------------------------------------------------------------------
@@ -220,4 +281,4 @@
 
 
 
-(run-tests)
+;(run-tests)