kaocha icon indicating copy to clipboard operation
kaocha copied to clipboard

Mark load errors during kaocha.watch reloads as load errors

Open plexus opened this issue 4 years ago • 5 comments

When kaocha.watch lets tools.namespace reloads files it can happen that tools.namespace fails and passes back an error, when one of the namespaces has a syntax error.

kaocha.watch stores this error for later handling, but it uses its own key for that at the top level, :kaocha.watch/error, instead of the what we do elsewhere, which is storing this on the ns testable as a :kaocha.testable/load-error.

In this case kaocha.watch also marks all test suites as to be skipped, since in the case of syntax error we assume that there's no use in trying to run tests until the syntax error is fixed. But this interacts badly with a check we do in kaocha.api, which now thinks we have a test run with zero tests, and bails out.

Instead the correct thing to do would be to mark the specific ns testable as having a load error, as well as adding other keys that are used by kaocha.testable/run. This would actually allow us to get rid of some of the custom error handling in watch pre-test. The main thing to look out for is that in this case this ns testable or its parent should not be marked as ::testable/skip.

   ::testable/load-error         exception
   ::testable/load-error-file    (str file)
   ::testable/load-error-line    1
   ::testable/load-error-message (str "Failed reading ns form in " file "\n"
                                      "Caused by: " (.getMessage ^Throwable exception))

plexus avatar May 31 '21 07:05 plexus

I'm still running into (what I assume is) this error. In case it's not clear how to repro:

Steps to repro

  1. kaocha --watch (one test file)
  2. see tests complete
  3. introduce error: require ns bla
  4. see tests fail

Expected

  • I would expect to see the actual error (can't find ns bla)

Actual

WARNING: No tests were found, make sure :test-paths and :ns-patterns are configured correctly in tests.edn.
[watch] Fatal error in test run #error {
 :cause throw+: #:kaocha{:early-exit 0}
 :data #:kaocha{:early-exit 0}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message throw+: #:kaocha{:early-exit 0}
   :data #:kaocha{:early-exit 0}
   :at [slingshot.support$stack_trace invoke support.clj 201]}]
 :trace
 [[slingshot.support$stack_trace invoke support.clj 201]
  [kaocha.api$run$fn__2826 invoke api.clj 107]
  [clojure.lang.AFn applyToHelper AFn.java 152]
  [clojure.lang.AFn applyTo AFn.java 144]
  [clojure.core$apply invokeStatic core.clj 667]
  [clojure.core$with_bindings_STAR_ invokeStatic core.clj 1977]
  [clojure.core$with_bindings_STAR_ doInvoke core.clj 1977]
  [clojure.lang.RestFn invoke RestFn.java 425]
  [kaocha.api$run invokeStatic api.clj 98]
  [kaocha.api$run invoke api.clj 85]
  [kaocha.watch$try_run$fn__4899 invoke watch.clj 51]
  [kaocha.watch$try_run invokeStatic watch.clj 50]
  [kaocha.watch$try_run invoke watch.clj 42]
  [kaocha.watch$run_loop invokeStatic watch.clj 198]
  [kaocha.watch$run_loop invoke watch.clj 192]
  [kaocha.watch$run_STAR_ invokeStatic watch.clj 300]
  [kaocha.watch$run_STAR_ invoke watch.clj 277]
  [kaocha.watch$run$fn__4977 invoke watch.clj 310]
  [clojure.lang.AFn applyToHelper AFn.java 152]
  [clojure.lang.AFn applyTo AFn.java 144]
  [clojure.core$apply invokeStatic core.clj 667]
  [clojure.core$with_bindings_STAR_ invokeStatic core.clj 1977]
  [clojure.core$with_bindings_STAR_ doInvoke core.clj 1977]
  [clojure.lang.RestFn invoke RestFn.java 425]
  [clojure.lang.AFn applyToHelper AFn.java 156]
  [clojure.lang.RestFn applyTo RestFn.java 132]
  [clojure.core$apply invokeStatic core.clj 671]
  [clojure.core$bound_fn_STAR_$fn__5767 doInvoke core.clj 2007]
  [clojure.lang.RestFn invoke RestFn.java 397]
  [kaocha.watch$run$fn__4979 invoke watch.clj 317]
  [clojure.core$binding_conveyor_fn$fn__5772 invoke core.clj 2034]
  [clojure.lang.AFn call AFn.java 18]
  [java.util.concurrent.FutureTask run FutureTask.java 264]
  [java.util.concurrent.ThreadPoolExecutor runWorker ThreadPoolExecutor.java 1130]
  [java.util.concurrent.ThreadPoolExecutor$Worker run ThreadPoolExecutor.java 630]
  [java.lang.Thread run Thread.java 831]]}

This makes watch hard to use with master - I downgraded to 1.0.732

pesterhazy avatar Jul 19 '21 19:07 pesterhazy

Note that if I restart the process, I do see the error - the error is swallowed only after reload

pesterhazy avatar Jul 19 '21 19:07 pesterhazy

Hey Paulus, thanks for the poke! This is indeed still an open issue. I've pushed a fix in #233, would be great if you could try that out and report back. It ended up being a fairly substantial change so some help testing this would be much appreciated.

plexus avatar Jul 20 '21 08:07 plexus

Good afternoon, and thanks for all your work on Kaocha! I'm still seeing this error when I run Kaocha with --watch, and then I introduce a syntax error in my codebase:

WARNING: No tests were found, make sure :test-paths and :ns-patterns are configured correctly in tests.edn.
[watch] Fatal error in test run #error {
 :cause throw+: #:kaocha{:early-exit 0}
 :data #:kaocha{:early-exit 0}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message throw+: #:kaocha{:early-exit 0}
   :data #:kaocha{:early-exit 0}
   :at [slingshot.support$stack_trace invoke support.clj 201]}]
...

Kaocha doesn't recover once I fix the codebase's syntax error. I have to stop the test watcher, and then start it again. I'm using Kaocha 1.60.945. Would you like me to open a new issue for this?

michael-reify avatar Nov 16 '21 22:11 michael-reify

Hi @plexus! A co-worker and and I are still seeing this issue. Would you be willing to re-open this ticket? Or may I create a new GitHub issue? I don't want to create a duplicate ticket if you'd prefer to re-open this one.

michael-reify avatar Feb 03 '22 13:02 michael-reify

Hi @michael-reify, sorry you never got a response. I haven't been able to replicate this issue. Can you post the tests.edn and the deps.edn? Does it happen for every syntax error, or just certain ones?

alysbrooks avatar Oct 28 '22 22:10 alysbrooks

Hi @alysbrooks! Thanks for following up. We have been trying to reproduce this, but we haven't been able to reproduce it reliably. I'm afraid I don't know what my tests.edn and deps.edn were when I was seeing the issue 😞 I also don't remember if this issue happened for all syntax errors, or just some of them. I know I'm not being much help! If I can get the issue to happen again repeatably, I'll try to remember to post more details. I understand if you'd prefer to close this issue again, though.

michael-reify avatar Oct 29 '22 21:10 michael-reify

Thanks for getting back to me! It seems to be an outstanding but very intermittent, so I'm inclined to leave it open, but prioritize bugs that are reproducible and more frequent.

alysbrooks avatar Oct 31 '22 19:10 alysbrooks

Unfortunately we cannot reproduce this issue and haven't heard anything in a while. I'm going to close this. Feel free to re-open this with more details.

alysbrooks avatar Jun 26 '23 22:06 alysbrooks