search

replikativ / konserve

A clojuresque key-value/document store protocol with core.async.
Eclipse Public License 1.0
298 stars 25 forks source link

test: use kaocha test-runner for cljs #90

Closed jsmassa closed 1 year ago

jsmassa commented 1 year ago

We are already using kaocha for our clojure tests, so it simplifies things if we are moving to kaocha for clojurescript as well.

Fixes #89.

jsmassa commented 1 year ago

Unfortunately kaocha's node test runner fails for this project with an error after all tests have been executed. Until the problem has been fixed I am using the olical cljs test runner here, which also requires only a setup in the deps.edn and no further config or runner files.

We can also try simply updating the karma packages. Vulnerabilities still shown on my system might be just a local problem.

TimoKramer commented 1 year ago

Does not work right now because of #96

jsmassa commented 1 year ago

The problem is fixed now with support for salt as in the clojure version still missing as stated in this comment.

TimoKramer commented 1 year ago

Just uncomment the cljstest from main and it should be working

whilo commented 1 year ago

You can either merge now or wait for my cljs fix in the next days.

jsmassa commented 1 year ago

A couple more days won't make much of a difference I guess.

whilo commented 1 year ago

This now implements the corresponding salt concatenation in ClojureScript, but I am certain that the cljs tests are not properly executed. Can you check that you can really execute the tests and then we need to figure out why the cljstest here passed earlier while the unit test correctly failed in https://circleci.com/gh/replikativ/konserve/788 ? On my machine I get macro import compile errors (which also might be my setup) and cannot run the tests or start a REPL atm.

jsmassa commented 1 year ago

Is there still a problem? Like on the server, all of the tests are passing on my system.

I get a few warnings, but cleanups of this repo will be the goal of PR #88.

jsmassa commented 1 year ago

Fixed the lz4 warning though as this is a seems to be a difference in cljs

jsmassa commented 1 year ago

What about the line here? Isn't that supposed to be the unsupported compressor as well in case of an exception in the clojure code?

whilo commented 1 year ago

AES is now fixed in cljs and has the same cold storage format as in Clojure (i.e. you can read the same store from Clojure and ClojureScript), thanks to the help of @pkpkpk :tada: . A problem was that the cljs compliance test was not run asynchronously and I think this is why it just passed before it failed.

whilo commented 1 year ago

What about the line here? Isn't that supposed to be the unsupported compressor as well in case of an exception in the clojure code?

@jsmassa I think this try-catch can be removed (it is redundant), it accidentally slipped in there while I was getting the native build to work.