Add spec-assertion-thrown? test helper

[thumbnail]

Migrated from nedap/utils.test#2

[vemv]

Seems unlikely to conflict, but just in case I'll review after the bigger PR

[thumbnail]

but just in case I'll review after the bigger PR

If you're referencing to #45, this'll probably break compatibility because of the import for example. happy to update after #45 tho.

[vemv]

It was https://github.com/nedap/utils.spec/pull/54

Yours is the only PR now, feel free to rebase :)

[thumbnail]

I can't seem to get the cljs case working. for some reason the assert-expr-multimethod isn't defined at the time of loading the file so the compiler warns (and the impl crashes on undefined).

according to this page it should be possible; but I can't seem to get it to work.

@vemv would you care to take a look some time?

[vemv]

The defmethod should work indeed,
https://github.com/weavejester/cljfmt/blob/3c6fbde59cd931972feafe73917866064c9d7b08/cljfmt/test/cljfmt/test_util/cljs.cljc is a known-good example

Will review

[vemv]

The reason of your issue appears to be that you should only target the JVM:

https://github.com/weavejester/cljfmt/blob/3c6fbde59cd931972feafe73917866064c9d7b08/cljfmt/test/cljfmt/test_util/cljs.cljc

Note the #?(:clj wrapping at L5

It kind of makes sense: I don't expect something that uses syntax-quote, unquote-splicing, etc to work in vanilla cljs (similarly to how you don't have defmacro in vanilla cljs)

[thumbnail]

I did find a very similar example in cljs.test.cljc as well; so while I'm happy to just target the jvm for this helper, I figured it would just be possible.

[vemv]

Note that the cljs compiler can host itself, but we aren't targeting self-hosted cljs - only vanilla one.

So afaict that's an inaccurate example.

cljfmt can be run from JVM-compiled ("vanilla") clojurescript, so when I said "target the JVM", you are not excluding vanilla cljs.

Sorry for the confusion!

[thumbnail]

Had some trouble because of the 'expansion' of specs in speced/defn.

It'd require the user of spec-assertion-thrown? to know internals of speced/defn, because:

  (is (spec-assertion-thrown? 'number? (accepts-number "1234")))

Would fail ('number? != (clojure.spec.alpha/and number? (instance ....))).

To mitigate this, I check for either an exact match; or a derived match (using the same fn as defn does; parsing/infer-spec-from-symbol). This works quite well actually :)

[vemv]

shouldn't this test be a .cljc one and added to the runner ns?

[thumbnail]

I have had little luck getting any of this run on cljs. If i convert to cljc the jvm / cljs compiler throws the following error;

Syntax error (IllegalAccessError) compiling at (nedap/utils/spec/impl/spec_assertion.cljc:1:1).
infer-spec-from-symbol does not exist

[thumbnail]

I managed to narrow this behavior down to a local issue. This suggestion has been addressed.

note that the assertions only run in clj.

[vemv]

I have had little luck getting any of this run on cljs.

Can give it a shot after a last round of feedback

[vemv]

add a check! for these args

[thumbnail]

I can't use speced/defn because it'll be a circular dep; but neither does manually using check! seem to work; after importing like this:

[nedap.utils.spec.impl.check #?(:clj :refer :cljs :refer-macros) [check!]]

it throws;

Syntax error compiling at (nedap/utils/spec/impl/spec_assertion.cljc:9:1).
Can't take value of a macro: #'nedap.utils.spec.impl.check/check!

[vemv]

lgtm after final coverage. Thanks for this one!

I'm ok with having this feature as JVM only for now (which should be cleanly offered: no cljs version should be provided at all). A few other things aren't offered for cljs either currently.

Accordingly please create an issue for cljs compat, and include as part of that task adding the check! that was tricky