Merge pull request #371 from frenchy64/issue-370-ensure-unskipped-tes…

…table-for-load-error

Ensure unskipped testable for reporting load error

CHANGELOG.md

@@ -4,12 +4,13 @@
 
 ## Fixed
 
+- Ensure reloading errors are printed in watch mode when the first test suite is disabled.
 - Using a try-catch (without rethrowing) in an `:each` fixture could swallow
   thrown exceptions, resulting in a test being treated as passing when it should
   have been reported as an error. Fixed by changing how `:each` fixtures wrap
   the test function in execution. (thanks
   [@NoahTheDuke](https://github.com/NoahTheDuke))
-* Fix crash on Windows when using `--watch` with the default Beholder watcher.
+- Fix crash on Windows when using `--watch` with the default Beholder watcher.
 
 ## Changed
 

src/kaocha/watch.clj

@@ -244,15 +244,20 @@ errors as test errors."
                         ;; We don't really know which suite the load error
                         ;; belongs to, it could well be in a file shared by all
                         ;; suites, so we arbitrarily put the load error on the
-                        ;; first and skip the rest, so that it gets reported
-                        ;; properly.
-                        (into [(assoc (first suites)
-                                      ::testable/load-error error
-                                      ::testable/load-error-file (or file (util/ns-file error-ns))
-                                      ::testable/load-error-line line
-                                      ::testable/load-error-message (str "Failed reloading " error-ns ":"))]
-                              (map #(assoc % ::testable/skip true))
-                              (rest suites))))))
+                        ;; first non-skipped suite and skip all others, so that
+                        ;; it gets reported properly.
+                        (let [applied? (volatile! false)]
+                          (mapv (fn [suite]
+                                  (if (and (not @applied?)
+                                           (not (::testable/skip suite)))
+                                    (do (vreset! applied? true)
+                                        (assoc suite
+                                               ::testable/load-error error
+                                               ::testable/load-error-file (or file (util/ns-file error-ns))
+                                               ::testable/load-error-line line
+                                               ::testable/load-error-message (str "Failed reloading " error-ns ":")))
+                                    (assoc suite ::testable/skip true)))
+                                suites))))))
                 config))))
 
 (defn watch-paths [config]

test/shared/kaocha/integration_helpers.clj

@@ -1,7 +1,9 @@
 (ns kaocha.integration-helpers
   (:require [clojure.java.io :as io]
             [clojure.string :as str]
-            [kaocha.platform :as platform])
+            [clojure.test :refer [is]]
+            [kaocha.platform :as platform]
+            matcher-combinators.test)
   (:import java.io.File
            [java.nio.file Files OpenOption Path Paths]
            [java.nio.file.attribute FileAttribute PosixFilePermissions]))
@@ -126,6 +128,124 @@
 (defn spit-file [m path contents]
   (let [{:keys [dir] :as m} (test-dir-setup m)
         path (join dir path)]
-    (mkdir (.getParent path))
+    (mkdir (.getParent ^Path path))
     (spit path contents)
     m))
+
+(def ^:dynamic ^Process *process* nil)
+
+(defn interactive-process*
+  [{:keys [dir runner] :as _m} args f]
+  (let [p (-> (doto (ProcessBuilder. ^java.util.List (cons (str runner) args))
+                (.directory (io/file dir)))
+              .start)
+        kill (delay (.destroy p))
+        timeout-ms 30000]
+    (binding [*process* p]
+      (try
+        (let [input (.getOutputStream p)
+              output (.getInputStream p)]
+          (with-open [w (io/writer input)
+                      r (io/reader output)]
+            (future
+              ;; unblock read-line calls after 30 seconds and abandon test
+              (Thread/sleep timeout-ms)
+              @kill)
+            (binding [*in* r
+                      *out* w]
+              (f))))
+        (assert (.waitFor p 10 (java.util.concurrent.TimeUnit/SECONDS))
+                "Process failed to stop!")
+        (.exitValue p)
+        (finally
+          (is (not (realized? kill)) (format "Process was killed after %sms timeout" timeout-ms))
+          (try (.exitValue p)
+               (catch IllegalThreadStateException _
+                 (is nil "Process was killed after executing entire test suite")
+                 @kill)))))))
+
+(defmacro interactive-process
+  "Simulate a test run of a bin/kaocha integration test
+  using (read-line) and (println).
+
+  m is the output from `test-dir-setup`
+  args is a collection of arguments after ./bin/kaocha.
+  e.g., [\"--watch\" \"--focus\" \"foo\"]
+  
+  Kills the process after 30 seconds if it has not already
+  been exited. Ensures the process is killed immediately after
+  body is executed. Returns the exit code.
+  
+  Executes body in the following environment:
+  *process* is the running Process executing `./bin/kaocha ~@args`
+  *in* is bound to the output stream of this Process
+  - e.g., use (read-line) to read the output of ./bin/kaocha
+  *out* is bound to the input stream of this Process
+  - e.g., use (println) to send input to ./bin/kaocha
+  
+  Tips:
+  - avoid using `testing` or `is` in ways that can swallow exceptions
+  in `body`. The first exception that's thrown in your test should contain
+  all of the debugging information you need using the helpers in the
+  rest of this file. If exceptions are swallowed, then more read-line's
+  may happen after the test has already failed, making it very difficult
+  to understand test failures."
+  [m args & body]
+  `(interactive-process* ~m ~args #(do ~@body)))
+
+(comment
+  (zero?
+    (interactive-process {:dir "." :runner (io/file "echo")} ["hello"]
+                         (let [l1 (read-line)
+                               _ (assert (= "hello" l1) (pr-str l1))])))
+  )
+
+(defn read-line-or-throw
+  "Read a line from the current integration test and throw if the process has died."
+  []
+  (or (read-line)
+      (throw (ex-info "Process killed" {}))))
+
+(defn expect-lines
+  "Assert that lines, a vector of strings, matches the next lines from the integration process.
+
+  If not, slurps the rest of the output for debugging purposes and throws an exception."
+  [lines]
+  (mapv (fn [l]
+          (let [s (read-line-or-throw)]
+            (or (is (match? l s))
+                (throw (ex-info (format "Failed to match %s\nEntire expected: %s\nRest of stream:\n%s"
+                                        (pr-str l) lines (str/split-lines (slurp *in*)))
+                                {})))))
+        lines))
+
+(defn next-line-matches
+  "Checks that the next line from the integration process matches function f.
+  
+  If not, slurps the rest of the output for debugging purposes and throws an exception."
+  [f]
+  (let [s (try (read-line-or-throw)
+               (catch clojure.lang.ExceptionInfo e
+                 (is nil "Failed next-line-matches call: process ended")
+                 (throw e)))]
+    (or (f s)
+        (throw (ex-info (format "Failed next-line-matches call\nFound: %s\nRest of stream:\n%s"
+                                (pr-str s) (str/split-lines (slurp *in*)))
+                        {})))))
+
+(defn read-until
+  "Keep reading from integration process output until f is true.
+  
+  If never true, reports all the strings that failed."
+  [f]
+  ;; can't recur across try, use atom
+  (let [seen (atom [])]
+    (try
+      (loop []
+        (let [s (read-line-or-throw)]
+          (when-not (f s)
+            (swap! seen conj s)
+            (recur))))
+      (catch clojure.lang.ExceptionInfo e
+        (is false (format "Failed read-until\nSeen:\n%s" (str/join "\n" @seen)))
+        (throw e)))))

test/unit/kaocha/watch_test.clj

@@ -12,7 +12,8 @@
             [kaocha.config :as config]
             [clojure.test :as t]
             [clojure.string :as str]
-            [slingshot.slingshot :refer [try+]])
+            [slingshot.slingshot :refer [try+]]
+            matcher-combinators.test)
   (:import [java.io File]))
 
 (deftest make-queue-test
@@ -186,3 +187,61 @@
     (is (= "[(.)]\n1 tests, 1 assertions, 0 failures.\n\n[watch] watching stopped.\n"
            @out-str))
     (is (= 0 @exit-code))))
+
+;;TODO move to cucumber
+(deftest ^{:min-java-version "1.11"} watch-load-error-test
+  (let [{:keys [config-file test-dir] :as m} (integration/test-dir-setup {})
+        config (-> (config/load-config config-file)
+                   (assoc-in [:kaocha/cli-options :config-file] (str config-file))
+                   (update :kaocha/tests (fn [suites]
+                                           (let [base-suite (assoc (first suites)
+                                                                  :kaocha/source-paths []
+                                                                  :kaocha/test-paths [(str test-dir)])]
+                                             [(assoc base-suite :kaocha.testable/id :first-suite)
+                                              (assoc base-suite :kaocha.testable/id :second-suite)]))))
+        _ (spit (str config-file) (pr-str config))
+        spit-good-test #(integration/spit-file m "test/bar_test.clj" (str "(ns bar-test (:require [clojure.test :refer :all])) (deftest good-test (is true))"))
+        spit-bad-test #(integration/spit-file m "test/bar_test.clj" (str "(ns bar-test) (throw (Exception. \"Intentional compilation error\"))"))
+
+        dbg (bound-fn* prn)
+        _ (dbg "before")
+        _ (spit-good-test)
+        exit (integration/interactive-process m [":second-suite" "--watch"]
+               (try
+                 (dbg "first lines")
+                 (integration/expect-lines
+                   ["[(.)]"
+                    "1 tests, 1 assertions, 0 failures."
+                    ""])
+                 (spit-bad-test)
+                 (dbg "after bad test")
+                 (integration/expect-lines
+                   ["[watch] Reloading #{bar-test}"
+                    "[E]"
+                    ""
+                    "ERROR in second-suite (bar_test.clj:1)"
+                    "Failed reloading bar-test:"])
+                 (dbg "compiler exception")
+                 (integration/next-line-matches
+                   #{"Exception: clojure.lang.Compiler$CompilerException: Syntax error macroexpanding at (bar_test.clj:1:15)."
+                     "Exception: clojure.lang.Compiler$CompilerException: Syntax error compiling at (bar_test.clj:1:15)."
+                     "Exception: clojure.lang.Compiler$CompilerException: java.lang.Exception: Intentional compilation error, compiling:(bar_test.clj:1:15)"})
+                 (dbg "big trace")
+                 (integration/read-until #{"1 tests, 1 assertions, 1 errors, 0 failures."})
+                 (integration/expect-lines
+                   [""
+                    "[watch] Error reloading, all tests skipped."])
+                 ;; fix the compilation error...
+                 (spit-good-test)
+                 (dbg "after good-test")
+                 (integration/expect-lines
+                   ["[watch] Reloading #{bar-test}"
+                    "[(.)]"
+                    "1 tests, 1 assertions, 0 failures."])
+                 (finally
+                   ;; FIXME unsure how to exit process via (println) ... eg., how to enter ^C ?
+                   ;; Idea from Alys: What you have seems to work, but maybe if *process* were a map in an atom,
+                   ;; you could have a :process key containing an actual Process and :continue key telling it whether to keep going?
+                   ;; I think this might make *interactive-process* a little tidier, too.
+                   (.destroy integration/*process*))))]
+    (is (= 143 exit))))