search

edn-query-language / eql

EQL is a declarative way to make hierarchical (and possibly nested) selections of information about data requirements. This repository contains the base specs and definitions for EQL parsing, AST, etc.
http://edn-query-language.org
MIT License
381 stars 18 forks source link

Remove test.check from runtime deps by adding gen-helpers namespace #6

Closed kennyjwilli closed 4 years ago

abhi18av commented 5 years ago

Hi @kennyjwilli , I'm just curious here - does this provide other benefits with spec as opposed to plainly using test.check ?

kennyjwilli commented 5 years ago

@abhi18av No - it uses test.check internally. This just removes the need to have test.check on the classpath at runtime - you typically don't need to generate things in production.

abhi18av commented 5 years ago

Oh, I see. Thanks for the explanation :)

So, I gather that basically through this PR we're mostly optimizing the Classpath deps.

kennyjwilli commented 5 years ago

That is correct.

awkay commented 5 years ago

Hey there...this is actually on my TODO list.

Did you test that this approach works for CLJS, and that advanced builds don't end up with cruft in them? I.e. via https://shadow-cljs.github.io/docs/UsersGuide.html#_build_report

kennyjwilli commented 5 years ago

@awkay I did not. Sounds like a good thing to do. I don't have the time to do that at the moment though.

wilkerlucio commented 4 years ago

@kennyjwilli hello, sorry the immense delay to respond on this, I just tried it, but I saw a few problems, when I try to run the tests I get:


Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.AssertionError: Assert failed: Args to tuple must be generators
(every? generator? generators)```

Also, CLJS doesn't compile for the same reason, I don't have the time to look into it, do you know what is happening here?
kennyjwilli commented 4 years ago

@kennyjwilli hello, sorry the immense delay to respond on this, I just tried it, but I saw a few problems, when I try to run the tests I get:

I'd need the full stack trace to figure out what's happening. It sounds like a bad call to gen/tuple. How would I repro this?

kennyjwilli commented 4 years ago

I see that the current version in master no longer requires the edn-query-language.gen which allowed you to remove the hard dep on test.check. This PR would allow you to require edn-query-language.gen in the core ns if desired. If that is no longer desired then this PR is not needed.

wilkerlucio commented 4 years ago

Yeah, I intended to keep it separated for now, the pattern around specs may be revisited in the future.