lambdaisland / kaocha

Full featured next gen Clojure test runner
https://cljdoc.org/d/lambdaisland/kaocha/
Eclipse Public License 1.0
803 stars 84 forks source link

Clojure 1.9 Compatibility Seems Broken #433

Closed lread closed 6 months ago

lread commented 7 months ago

After upgrading to kaocha 1.88.1376, I noticed that tests that I run against Clojure v1.9 fail.

Test Environment

OS: Linux PopOS! JDK: Tried under 8 and 21, same result.

Minimal Repro

In an empty folder create deps.edn:

{:deps {org.clojure/clojure {:mvn/version "1.9.0"} }
 :aliases
 {:test {:extra-deps {lambdaisland/kaocha {:mvn/version "1.88.1376"}}
         :main-opts ["-m" "kaocha.runner"]}}}

Then run:

clojure -M:test

Expected Behaviour

I should see the normal warnings from kaocha, like:

WARNING: Did not load a configuration file and using the defaults.
This is fine for experimenting, but for long-term use, we recommend creating a configuration file to avoid changes in behavior between releases.
To create a configuration file using the current defaults and configuration file location, create a file named tests.edn that contains '#kaocha/v1 {}'.
WARNING: In :test-paths, no such file or directory: test
WARNING: No tests were found. This may be an issue in your Kaocha test configuration. To investigate, check the :test-paths and :ns-patterns keys in tests.edn.

Actual Behaviour

An exception is printed to the terminal:

Exception in thread "main" java.lang.ClassCastException: class clojure.lang.Var cannot be cast to class java.lang.String (clojure.lang.Var is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap'), compiling:(clojure/core/specs/alpha.clj:4:1), compiling:(kaocha/version_check.clj:1:1)
    at clojure.lang.Compiler.checkSpecs(Compiler.java:6891)
    at clojure.lang.Compiler.macroexpand1(Compiler.java:6907)
    at clojure.lang.Compiler.macroexpand(Compiler.java:6972)
    at clojure.lang.Compiler.eval(Compiler.java:7046)
    at clojure.lang.Compiler.load(Compiler.java:7514)
    at clojure.lang.RT.loadResourceScript(RT.java:379)
    at clojure.lang.RT.loadResourceScript(RT.java:370)
    at clojure.lang.RT.load(RT.java:460)
    at clojure.lang.RT.load(RT.java:426)
    at clojure.core$load$fn__6548.invoke(core.clj:6046)
    at clojure.core$load.invokeStatic(core.clj:6045)
    at clojure.core$load.doInvoke(core.clj:6029)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.core$load_one.invokeStatic(core.clj:5848)
    at clojure.core$load_one.invoke(core.clj:5843)
    at clojure.core$load_lib$fn__6493.invoke(core.clj:5888)
    at clojure.core$load_lib.invokeStatic(core.clj:5887)
    at clojure.core$load_lib.doInvoke(core.clj:5868)
    at clojure.lang.RestFn.applyTo(RestFn.java:142)
    at clojure.core$apply.invokeStatic(core.clj:659)
    at clojure.core$load_libs.invokeStatic(core.clj:5925)
    at clojure.core$load_libs.doInvoke(core.clj:5909)
    at clojure.lang.RestFn.applyTo(RestFn.java:137)
    at clojure.core$apply.invokeStatic(core.clj:659)
    at clojure.core$require.invokeStatic(core.clj:5947)
    at clojure.core$require.doInvoke(core.clj:5947)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at user$eval11.invokeStatic(runner.clj:1)
    at user$eval11.invoke(runner.clj:1)
    at clojure.lang.Compiler.eval(Compiler.java:7062)
    at clojure.lang.Compiler.load(Compiler.java:7514)
    at clojure.lang.RT.loadResourceScript(RT.java:379)
    at clojure.lang.RT.loadResourceScript(RT.java:370)
    at clojure.lang.RT.load(RT.java:460)
    at clojure.lang.RT.load(RT.java:426)
    at clojure.core$load$fn__6548.invoke(core.clj:6046)
    at clojure.core$load.invokeStatic(core.clj:6045)
    at clojure.core$load.doInvoke(core.clj:6029)
    at clojure.lang.RestFn.invoke(RestFn.java:408)
    at clojure.core$load_one.invokeStatic(core.clj:5848)
    at clojure.core$load_one.invoke(core.clj:5843)
    at clojure.core$load_lib$fn__6493.invoke(core.clj:5888)
    at clojure.core$load_lib.invokeStatic(core.clj:5887)
    at clojure.core$load_lib.doInvoke(core.clj:5868)
    at clojure.lang.RestFn.applyTo(RestFn.java:142)
    at clojure.core$apply.invokeStatic(core.clj:659)
    at clojure.core$load_libs.invokeStatic(core.clj:5925)
    at clojure.core$load_libs.doInvoke(core.clj:5909)
    at clojure.lang.RestFn.applyTo(RestFn.java:137)
    at clojure.core$apply.invokeStatic(core.clj:659)
    at clojure.core$require.invokeStatic(core.clj:5947)
    at clojure.main$main_opt.invokeStatic(main.clj:317)
    at clojure.main$main_opt.invoke(main.clj:313)
    at clojure.main$main.invokeStatic(main.clj:424)
    at clojure.main$main.doInvoke(main.clj:387)
    at clojure.lang.RestFn.applyTo(RestFn.java:137)
    at clojure.lang.Var.applyTo(Var.java:702)
    at clojure.main.main(main.java:37)
Caused by: java.lang.ClassCastException: class clojure.lang.Var cannot be cast to class java.lang.String (clojure.lang.Var is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap'), compiling:(clojure/core/specs/alpha.clj:4:1)
    at clojure.lang.Compiler.load(Compiler.java:7526)
    at clojure.lang.RT.loadResourceScript(RT.java:379)
    at clojure.lang.RT.loadResourceScript(RT.java:370)
    at clojure.lang.RT.load(RT.java:460)
    at clojure.lang.RT.load(RT.java:426)
    at clojure.lang.Compiler.ensureMacroCheck(Compiler.java:6877)
    at clojure.lang.Compiler.checkSpecs(Compiler.java:6889)
    ... 57 more
Caused by: java.lang.ClassCastException: class clojure.lang.Var cannot be cast to class java.lang.String (clojure.lang.Var is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')
    at clojure.core$symbol.invokeStatic(core.clj:579)
    at clojure.core$symbol.invoke(core.clj:574)
    at clojure.spec.alpha$__GT_sym.invokeStatic(alpha.clj:309)
    at clojure.spec.alpha$__GT_sym.invoke(alpha.clj:305)
    at clojure.spec.alpha$res.invokeStatic(alpha.clj:323)
    at clojure.spec.alpha$res.invoke(alpha.clj:320)
    at clojure.spec.alpha$res$fn__1883.invoke(alpha.clj:324)
    at clojure.walk$walk.invokeStatic(walk.clj:50)
    at clojure.walk$postwalk.invokeStatic(walk.clj:52)
    at clojure.walk$postwalk.invoke(walk.clj:52)
    at clojure.core$partial$fn__5561.invoke(core.clj:2616)
    at clojure.core$map$fn__5587.invoke(core.clj:2747)
    at clojure.lang.LazySeq.sval(LazySeq.java:40)
    at clojure.lang.LazySeq.seq(LazySeq.java:49)
    at clojure.lang.RT.seq(RT.java:528)
    at clojure.core$seq__5124.invokeStatic(core.clj:137)
    at clojure.core$apply.invokeStatic(core.clj:652)
    at clojure.walk$walk.invokeStatic(walk.clj:44)
    at clojure.walk$postwalk.invokeStatic(walk.clj:52)
    at clojure.walk$postwalk.invoke(walk.clj:52)
    at clojure.spec.alpha$res.invokeStatic(alpha.clj:324)
    at clojure.spec.alpha$res.invoke(alpha.clj:320)
    at clojure.spec.alpha$def.invokeStatic(alpha.clj:354)
    at clojure.spec.alpha$def.invoke(alpha.clj:347)
    at clojure.lang.AFn.applyToHelper(AFn.java:165)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.lang.Var.applyTo(Var.java:702)
    at clojure.lang.Compiler.macroexpand1(Compiler.java:6912)
    at clojure.lang.Compiler.macroexpand(Compiler.java:6972)
    at clojure.lang.Compiler.eval(Compiler.java:7046)
    at clojure.lang.Compiler.load(Compiler.java:7514)
    ... 63 more

Notes

If I bump deps.edn clojure version to 1.10.3 same test gives expected behaviour.

Leaving clojure version at 1.9.0 and downgrading kaocha to the previous release of 1.87.1366 also results in expected behaviour.

lread commented 7 months ago

Diagnosis

Ok, did a little poking. It seems that the issue is that the org.clojure/spec.alpha v0.4.233 min Clojure version is 1.10. I think we are running into the effect of this change.

symbol got smarter in Clojure 1.10 beta 5:

symbol can now be passed vars or keywords to obtain the corresponding symbol

Let's do a little test in Clojure 1.10:

$ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.10.3"}}}'
Clojure 1.10.3
user=> (def foo 42)
#'user/foo
user=> (symbol #'foo)
user/foo

And if we repeat this in Cloure 1.9:

$ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.9.0"}}}'
Clojure 1.9.0
user=> (def foo 42)
#'user/foo
user=> (symbol #'foo)
ClassCastException class clojure.lang.Var cannot be cast to class java.lang.String (clojure.lang.Var is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')  clojure.core/symbol (core.clj:579)

Options

  1. Revert kaocha's dep on org.clojure/spec.alpha to 0.3.218.
  2. Bump min version of kaocha to Clojure 1.10
  3. Drop explicit reference from kaocha to spec.alpha and let clojure bring in its natural dep (I expect tho, that there is a reason kaocha is explicitly bringing in a specific version of spec.alpha)

I think option 1 seems reasonable for now. There's (not yet) a version of Clojure that brings in the newest version of spec.alpha (we can assume 1.12 will do so eventually):

$ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.9.0"}}}' -Stree
org.clojure/clojure 1.9.0
  . org.clojure/spec.alpha 0.1.143
  . org.clojure/core.specs.alpha 0.1.24

$ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.11.2"}}}' -Stree
org.clojure/clojure 1.11.2
  . org.clojure/spec.alpha 0.3.218
  . org.clojure/core.specs.alpha 0.2.62

$ clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.12.0-alpha9"}}}' -Stree
org.clojure/clojure 1.12.0-alpha9
  . org.clojure/spec.alpha 0.3.218
  . org.clojure/core.specs.alpha 0.2.62
plexus commented 7 months ago

What happens if we remove clojure and spec from deps.edn? Presumably these are always provided already by clojure cli, or declared by the project. And the project's Clojure dependency would then determine which spec version gets included.

puredanger commented 7 months ago

In my opinion, you should definitely remove the spec.alpha dependency and rely on getting that transitively via whatever Clojure version you use. Whether you remove Clojure itself as a dep is harder to say for certain, depends whether you have some Clojure version needs.

lread commented 7 months ago

My assumption that kaocha depends on spec.alpha for some good reason is probably wrong. I had a wee peek at the kaocha deps.edn history. It looks like the spec.alpha dep has been explicitly there almost from the beginning, probably for no good reason.

If memory serves, a wise man once told me that a lib can express its minimum supported Clojure version by its clojure dep. So if kaocha were to include a dep on clojure, it should probably be 1.9.0.

puredanger commented 7 months ago

That seems reasonable

plexus commented 6 months ago

I dropped both dependencies, we can put the clojure dependency at 1.9, but then we have to manually hold it back each time we run antq. That's extra work and chance of forgetting for little benefit. The README clearly states the minimum supported version.

lread commented 6 months ago

Thanks @plexus!