Verify rewrite-cljc against zprint

[lread]

In a fork/branch, make sure that zprint continues to work when it is switched over to rewrite-cljc

[kkinnear]

I haven’t been following rewrite-cljc closely, but certainly hoping that you get it to work.

If you think that there is any chance that rewrite-cljc would support zprint, I’d be delighted to give it a try. In part because I would really like ##NaN and friends to work in zprint (which you have apparently done), but also because it would be great to have a single, maintained, parser that produces zippers!

I have plenty of tests for zprint (where plenty is 945 at the moment) so I expect I would wring out at least the features that zprint uses pretty well.

Is there anything special I have to do to try it with zprint? Do I just clone rewrite-clj-playground and install it, or is there something else I would need to know to try this?

Perhaps the larger question is: would this be a help to you?

[lread]

Hey @kkinnear, awesome to hear from you!

I started this project way back when working on an old abandoned issue for cljfmt gave me grief due to problems in rewrite-clj and rewrite-cljs. And then one thing led to another, and then I got distracted from Clojure entirely for several months.

But... I am dipping my toe back in and knocking the rust off the Clojure parts of my brain and getting back to it.

I will get this project out to production sometime!

@borkdude's carve is using a version of this project, last I heard, without issue.

As my use case for rewrite-clj/cljs was only cljfmt, I thought I should explore updating to rewrite-cljc in other projects to help me learn how folks are using these libraries.

Your interest has spurred me to try upgrading zprint to rewrite-cljc sooner than later. I'll do so today or tomorrow and get back to you.

[kkinnear]

That’s great! Impressive to learn that you have got it to the point that you can get cljfmt to work with it!

Just to be clear, I wasn’t trying to get you to try it, I was offering to give it a try myself if you thought there was a reasonable chance it might work.

[lread]

Yeah, I realize that, thanks 🙂. The opportunity for me to learn how other libs use rewrite-clj/rewrite-cljs is too valuable for me to pass up. So I'm gonna go for it.

I do think that there is a good chance that this will work in its current form for zprint.

One caveat is that rewrite-cljc is not yet up on clojars, and that prohibits libs that use it from publishing to clojars.

[kkinnear]

Yes, I was noticing that once you pointed out carve. If this works with zprint, I’m sure going to hope that you publish it to clojars in some way!

I don’t have nearly the documentation for zprint that you do for rewrite-cljc.

In particular, there is nothing about how to build or test zprint.

The tests are largely Clojure tests. You need to do lein with-profile expectations expectations to run the tests. That was a change in the 1.0.0 release that I will probably change back, but that’s the way it works now. The tests all run for me on macOS. If the tests all run, then the next level of testing is:

lein clean
lein uberjar 
./test_uberjar
# if that produces no output it worked
./test_config 1.0.0 uberjar 
# which will produce a line per test, and a lot more if any of them fail. It is not the best test in that it is fragile, but it does a lot. Needs python SimpleHttpServer to test —url switch. 

If it does all that, it is working in Clojure pretty well. I could do some by hand cljs testing if that all works.

I do build graalvm binaries, without modifying Clojure, interestingly enough. So I could try that too if the basics work.

Anyway, good luck. Sorry that there isn’t a doc on “contributing” yet!

[lread]

Hey no problemo, your docs seem very thorough on the usage side, and thanks for the tips.

I noticed you do have some notes on testing.

Before integrating rewrite-cljc, I wanted to see things pass so I did figure out some things.

I've found a way to run zprint tests under Clojure that worked for me:

lein with-profile expectations expectations

I ran your commands above, they seem to pass (?).

If you ^c out of this test, be sure and clean up ~/.zprintrc and ./.zprintrc!
...........  --url
...........  --url, with URL missing, see if cache works
...........  --url, with URL missing, see if expired cache works
...........  --url, with URL missing, and no cache
...........  --url, with URL not a valid Clojure map, and no cache
...........  --url, with URL not a valid zprint options map, and no cache
...........  --url-only, with valid URL and no cache
...........  --url-only, with valid URL, invalid file w/fn, and no cache
...........  basic config test.
...........  {:cwd-zprintrc? true} on command line test
...........  {:search-config? true} on command line test
...........  ~/.zprintrc being used test
...........  -d test -- ~/.zprintrc should not be used
...........  bad ~/.zprintrc, invalid Clojure map test
...........  invalid zprint options map in ~/.zprintrc
...........  -v test
...........  -h test
...........  -e test
...........  :coerce-to-false test
...........  fn in ~/.zprintrc file test (works for uberjar, fails for graalvm)
...........  fn is invalid in ./.zprintrc file test
...........  fn on command line test (works for uberjar, fails for graalvm)

I built a mac native image with Graal via:

./build.zprintm /Users/lee/.sdkman/candidates/java/20.1.0.r8-grl target/zprint-filter-1.0.0 zprintm-1.0.0

And then ran:

./test_config 1.0.0 graalvm-mac

Which returned identical output as ./test-config 1.0.0 uberjar, so that seems good.

[lread]

Some good results:
https://github.com/lread/zprint/commits/lread-rewrite-cljc-test

Noticed a couple of places where zprint was compensating for rewrite-cljs:
https://github.com/lread/zprint/blob/49dcf81d3d7f21797136076132d6c031c32c316b/src/zprint/zprint.cljc#L4969-L4971
https://github.com/lread/zprint/blob/49dcf81d3d7f21797136076132d6c031c32c316b/src/zprint/rewrite.cljc#L43-L45
I don't think they are necessary anymore and should be reviewed.
There might be other compensations, but those are the ones I saw.

I also would like to run the zprint test suite under cljs but don't know how to do so yet.

[kkinnear]

First, you figured out the tests amazingly well. The test_config did, indeed, work. You seem to have built the graalvm zprintm the correct way, and the graalvm-mac was the right way to test the graalvm binary. Go you!

The link to good results showed me some things to review. Which I will, as in both cases it seems like a win to just use the rewrite-cljc code.

Are you telling me that the expectations test pass? If so, that’s amazing!!

It is with chagrin that I tell you that I don’t have an easy way to run the expectations test in cljs. My cljs testing has been by-hand testing of planck, lumo, figwheel (not the latest version), and shadow-cljs. The level of exercise that each one gets has been minimal once I verified that rewrite-cljs largely worked. I mostly just make sure that each environment works and something hasn’t gone terribly wrong. Not least because rewrite-cljs doesn’t change much.

Perhaps I can look at how you have integrated cljs testing for rewrite-cljc to get some ideas how to automate it for zprint. Your command of the build environment is well beyond mine. But whatever I do in the future to automate cljs testing for zprint isn’t going to help you validate rewrite-cljc today.

I can try to build a rewrite-cljc-playground and then try it out myself with cljs, which might be some help. Or you could push an experimental library to clojars.

Which, if the zprint tests really pass, would seem like an obvious thing to do. I won’t release a zprint with rewrite-cljc until you say it is ready, though it sounds like it is pretty ready if cljfmt works for you and zprint works at least in Clojure. But it would be nice to have something to test with without having to get your (admittedly impressive) build tooling set up.

Anyway, I’m impressed with how well rewrite-cljc did with zprint, and with your facility in getting zprint built, tested, and working!

[lread]

Thanks @kkinnear, this has been both fun and educational for me.

It seems that maybe the expectations library can be used with cljs? Or maybe the clojure-test compatible version might be interesting to you? In any case you'd have to rework your tests to be cljc. Let me know if you want a hand with that or with getting your project setup on circleci.

Because rewrite-cljc's tests are passing and and cljfmt's tests (which include cljs) are passing, I'm not too concerned about zprint; zprint cljs tests would be more frosting on the confidence cake, but not having them is ok too.

I'm not sure what to do yet for clojars. This project is slated to be hosted on clj-commons. I'd like to get it over there before the first alpha release to avoid confusing upgrade paths. Even though I have many todos... maybe I am almost ready for an alpha release. I shall keep you posted.

[kkinnear]

Just a postscript here -- the pointer to the clojure.test compatible version of expectations was news to me, and looked interesting, so I ported the zprint clj tests to it expecting that it would of course support cljs. Imagine my chagrin when I found that it did not. But being stubborn, I then spent several weeks porting the testing framework to self-hosted cljs, modified my clj tests as necessary for cljs as well, and now I can run all of my zprint tests in both clj and cljs. Which did indeed find a few minor differences and bugs. So thanks for both the pointer and impetus to up my cljs testing game.

For what it is worth, the cljs compatible expectations/clojure-test framework is still in development, but should actually be released.

I'm still very interested in releasing zprint with rewrite-cljc when you get to that point!

[lread]

@kkinnear, I am glad you were stubborn - to the benefit of all! 🙂

I'll keep you posted as I slowly move forward. If you are Slack-inclined feel free to drop in for chats in the #rewrite-clj channel.