gfredericks / test.chuck

A utility library for test.check
Eclipse Public License 1.0
215 stars 26 forks source link

Add ClojureScript support #14

Closed nberger closed 9 years ago

nberger commented 9 years ago

Port com.gfrederick.test.chuck namespaces to ClojureScript, including tests. Add build profiles to project.clj to test against Node.js and browsers with :none & :advanced settings.

What's pending?

NOTE: The test.check port to cljs was taken as an example to make this, so the build profiles, runner and other stuff are similar to those in test.check, except for the directory layout that was not changed yet, waiting for review from the project maintainers.

nberger commented 9 years ago

@gfredericks,

I hope this makes sense :)

The tests are passing in the four targets (node dev/adv, browser dev/adv). I still haven't tried to use it as a dependency in a cljs project.

I took the easier path with regard to directory layout: keep the clj directories untouched, adding new cljs base dirs (src-cljs, test-cljs and src-target). I'd like to hear from you as to how the final layout should be. The same as test.check perhaps?

The commits are pretty raw, will need to be squashed.

gfredericks commented 9 years ago

I hope this won't require you to redo much, but is there any reason not to use the new .cljc file format instead of having a copypasted codebase?

gfredericks commented 9 years ago

Also thanks a lot for caring about this, it's definitely something I've wanted for a while.

nberger commented 9 years ago

I hope this won't require you to redo much, but is there any reason not to use the new .cljc file format instead of having a copypasted codebase?

I first started using reader conditionals, but switched to the copypaste because of two main reasons:

  1. Reader conditionals depend on clojure 1.7.0, so new versions of the library would only be used on clojure 1.7.0 (both from clojure & clojurescript). I didn't want to take that decision, so I went by this route. I also found test.check took that route to support cljs, I'm not sure if it was because 1.7.0 was not released yet at that time or if it was to not bump dependencies. Also, test.check is a contrib library so it has to be even more conservative, but had the impression that test.check is very on par with it.
  2. After some iterations on the .cljc route (and also given my non-experience migrating a codebase to .cljc) I had the feeling that there was going to be too many differences between clj & cljs. Now that it's working, I don't think there are so many differences, but at that time, given point 1, it was a no brainer to continue the copypaste route.

I thought that the natural route is to go the .cljc route in the end, but perhaps not right now. Nonetheless, I'll check the status of my cljc branch to see if there's something of use there.

nberger commented 9 years ago

Also thanks a lot for caring about this, it's definitely something I've wanted for a while.

I'm glad to hear that :)

gfredericks commented 9 years ago

I can see you've thought about the cljc issue at least as much as I have.

As a maintainer of test.check, I've definitely been hesitant to consolidate the codebases to cljc there for exactly the reasons you describe. However several days ago Stuart Sierra expressed a willingness to to exactly that to his contrib libraries, under the reasoning that nobody is being forced to upgrade to 1.7 since they can still use old versions of the contrib libs.

So we might make the jump in test.check sooner rather than later. In any case the dual codebases have been a decent headache for me already, and I'm not eager to take on another case of that. Since test.chuck isn't even contrib, I don't mind making the jump now.

gfredericks commented 9 years ago

Response to specific details of your original message (I still haven't actually looked at the code, and will probably wait for cljc to do that so there are decent diffs):

nberger commented 9 years ago

Oh, I missed that message of yours in that thread. Probably, I would have tried the .cljc path a bit harder otherwise :)

Perfect, I think it's a good decision to embrace Clojure 1.7.0 moving forward. I'll start working on the .cljc version, as you said the diff should be small.

About porting times, I agree it's interesting to have it, but might it be better not to hurry and leave it separate from this PR? There are a couple of alternatives to explore for configuring it, and it really is something isolated from the rest: it doesn't depend on anything else, nothing else from test.chuck depends on it.

gfredericks commented 9 years ago

Yep, leaving out times is fine with me

nberger commented 9 years ago

Continued in #15