reiddraper / simple-check

QuickCheck for Clojure
http://reiddraper.github.io/simple-check/
286 stars 18 forks source link

Make API portable to ClojureScript #11

Closed cemerick closed 10 years ago

cemerick commented 11 years ago

You expressed interest in doing this before. I wanted to ask what your preference is for accomplishing it; there are three options AFAIK, which I describe assuming you haven't done much ClojureScript (apologies if that's incorrect):

  1. Maintain separate codebases
  2. Ensure that the Clojure codebase is fully portable, and let lein-cljsbuild crossovers copy things over to .cljs files when users consume simple-check.
  3. Use cljx.
(1) is crazy. (2) is doable; the biggest downside there that every user needs to manually add all of the Clojure namespaces they plan on using from ClojureScript in their `project.clj`, i.e. lein-cljsbuild doesn't figure out namespace-level dependencies or anything. I am partial to (3), having used it for some time, and recently mostly-rewrote it. You can see an example of its use over in [pprng](https://github.com/cemerick/pprng).
reiddraper commented 11 years ago

which I describe assuming you haven't done much ClojureScript

Correct.

I'm inclined to trust your opinion on this. I need to do some reading on cljx, but that seems totally reasonable. Offhand, do you know if javascript has a close-enough pseudo-random number generating object? The main thing we need is for you to be able to instantiate it with a seed and have it be deterministic from there.

cemerick commented 11 years ago

JavaScript itself, no. seedrandom.js is the best option I've found (not wanting to bother with the tall grass of ostensibly cryptographically-secure RNGs in JavaScript). I folded it into my portable data.generators fork here, which will likely end up shipping with it. It's not Google Closure Advanced Compilation-safe yet, but I'll make sure it is, eventually.

Since it looks like simple-check generators are Just Functions™ (yay!), perhaps now would be a good time to suggest again that simple-check not be in the generator business at all? That would greatly simplify making it Clojure[Script]-portable.

cemerick commented 11 years ago

Oh, there's one caveat to cljx that I should mention: directly using e.g. require at the REPL won't pick up new changes (since they're in .cljx files). The other side of that is, if you're using any nREPL client, there's cljx middleware that allows you to evaluate expressions, load files, and so on straight from the editor. Also, lein cljx auto will watch your .cljx sources, and update the generated Clojure[Script] sources as necessary; this is the best way to eliminate the require problem if you're not using nREPL.

cemerick commented 10 years ago

Getting serious about this now, see above commit. for-all is the only problematic bit I've hit, due to the :require-clojure :exclude in the generators ns and how cljs reuses Clojure's namespaces for cljs defs. Hopefully we can find a sane place for it (currently in a "sugar" ns now, which surely makes little sense).

reiddraper commented 10 years ago

Woot, awesome. What can I do to help? Do I understand correctly that the issue is something regarding clojurescript not having (:refer-clojure :exclude [...])?

cemerick commented 10 years ago

No, cljs has :refer-clojure :exclude. The problem is that if for-all stays in the properties ns, the generators ns is loaded in both ClojureScript (to be used) and in Clojure (as a side-effect of loading the properties ns to use the macro). This ends up being a problem because each language aliases a different namespace to lang in generators (cljs.core and clojure.core, respectively). That isn't conceptually a problem, but the ClojureScript compiler (the analyzer, actually) uses Clojure namespaces to represent ClojureScript namespaces, so the mismatch produces an error.

This will always happen if you're using simple-check in both Clojure and ClojureScript within the same JVM process (something I'd be doing all the time, ideally), so I'm pretty motivated to find a more general solution. Just haven't come up with one, yet.

cemerick commented 10 years ago

The fundamental problem re: namespaces has been described here. It needs some enhancements upstream in tools.reader, so it'll be a while.

I really don't want to provoke an API change (more out of principle than anything else). What would you think of capturing the core fns used in generators locally (e.g. (def ^:private coremap #+clj clojure.core/map #+cljs cljs.core/map))? Gnarly as hell, but that would keep the body of the namespace clean syntactically, and allow me to drop the conflicting alias. Once a fix for CLJS-632 lands, we can go back to using the lang alias and eliminate the shite ns-local coremap definitions, etc.

cemerick commented 10 years ago

For those following along, the commit above is now rebased on master, and is working well with the latest ClojureScript release, as well as the updated nREPL/piggieback/austin ClojureScript REPL toolchain. Looking darn close… :-)

ghoseb commented 10 years ago

:thumbsup:

cemerick commented 10 years ago

Rebased again, now with a bunch of tests that were being missed (I had botched what defspec was emitting for ClojureScript!) now running. Part of the result of that is CLJS-677…so, if you run lein cleantest, and the ClojureScript tests fail because of the edn-roundtrip spec, that's why. We'll have to wait for a fix upstream for that to succeed reliably. (I'm rather scared of what we'll find in the CLJS reader once this is properly usable.)

Another change is that the cljx middleware will now apply transformations on files loaded either via direct require/use/etc, or transitively-loaded namespaces that are the result of a direct .cljx file load in the REPL. The tl;dr is that you don't need to run lein cljx once before using the REPL as you usually would. This is enabled for Clojure only at the moment; ClojureScript support is coming.

cemerick commented 10 years ago

Oh, also, I've discovered something that is currently preventing simple-check from working under advanced GClosure optimization, thus the :simple for now. Working on it. :-)

cemerick commented 10 years ago

My cljx branch now tests well on Clojure and ClojureScript, the latter with :advanced optimizations. Continuing to apply it to some of my own projects. Getting close to a PR.

reiddraper commented 10 years ago

Closing this for now since it may be some time since simple-check proper gets CLJS support. In the meantime, cemerick/double-check is the way forward.