lambdaisland / kaocha-cljs

ClojureScript support for Kaocha
Eclipse Public License 1.0
40 stars 9 forks source link

Fork cljs REPL implementations #44

Open alysbrooks opened 3 years ago

alysbrooks commented 3 years ago

In my experience, tests using kaocha-cljs sometimes fail because it can't connect successfully to the ClojureScript client. This is particularly problematic on CI, as it causes PR test runs to fail for reasons unrelated to the PR itself.

A typical error is as follows:

Exception: clojure.lang.ExceptionInfo: Kaocha ClojureScript client failed connecting back.

I don't know if it's reasonable to solve all of the unreliability, especially since I think we want to focus efforts on kaocha-cljs2 (among other projects). However, finding a way to mitigate the issue for CI and documenting it seems like a doable fix.

plexus commented 3 years ago

Note that this is largely due to the repl implementations not being reliable, or prone to race conditions. We can only fix this by forking then from Clojurescript.

On Mon, Sep 20, 2021, 17:44 A Brooks @.***> wrote:

In my experience, tests using kaocha-cljs sometimes fail because it can't connect successfully to the ClojureScript client. This is particularly problematic on CI, as it causes PR test runs to fail for reasons unrelated to the PR itself.

A typical error is as follows:

Exception: clojure.lang.ExceptionInfo: Kaocha ClojureScript client failed connecting back.

I don't know if it's reasonable to solve all of the unreliability, especially since I think we want to focus efforts on kaocha-cljs2 (among other projects). However, finding a way to mitigate the issue for CI and documenting it seems like a doable fix.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lambdaisland/kaocha-cljs/issues/44, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH3VAFMGYYI5DAWS7KO3TUC5JHTANCNFSM5EMH7A4Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

alysbrooks commented 2 years ago

Leaning toward closing this, as solving/mitigating this in kaocha-cljs2 makes more sense.

plexus commented 2 years ago

I wouldn't dismiss this just yet. We still use kaocha-cljs on several projects, and we plan to continue supporting kaocha-cljs. It's a different approach than kaocha-cljs2, which in some scenarios makes more sense. I think in particular for libraries that kaocha-cljs is often a better fit than kaocha-cljs2.

kaocha-cljs has two main drawbacks, there are a number of places where it can't be used, e.g. when using shadow-cljs or when wanting to test advanced compiled code. For those cases we made kaocha-cljs2. The other drawback is that it's hard to debug when things go wrong, and this I think we can fix by taking ownership of the REPL implementations, and adding more instrumentation to them.

It's not a high priority issue, but it's something I'm still interested in exploring.

plexus commented 1 year ago

Renaming this to point out what the actionable task is.

alysbrooks commented 1 year ago

This seems like an issue we would like to address but it's never quite reached the top of our TODO list. I'm inclined to leave it open for the time being.