kkinnear / zprint

Executables, uberjar, and library to beautifully format Clojure and Clojurescript source code and s-expressions.
MIT License
554 stars 47 forks source link

Add test workflow #249

Closed zharinov closed 1 year ago

zharinov commented 2 years ago

Closes #248

zharinov commented 2 years ago

Example: https://github.com/zharinov/zprint/runs/7554958077?check_suite_focus=true

UPD. A-a-and it stopped working once I've created this PR

zharinov commented 2 years ago

Works with separated CLJ (ubuntu) and CLJS (macos). I didn't manage to install Planck on ubuntu in reliable manner, so hopefully this separated approach would be a good start.

Example: https://github.com/zharinov/zprint/actions/runs/2752294939

UPD. ClojureScript testing is still quite slow, though

kkinnear commented 2 years ago

Wow, I'm impressed. I looked over the results from the runs you did. I haven't even tried Java 17, and you seem to have used it with no problems. That's cool on its own. You also did the clojure tests with the test-runner, which I've never done either. Overall, once you split the tests, it all looks like it worked pretty well. I've not tried any of this on Ubuntu, but I have no reason to think it wouldn't work, and you seem to have made it happen.

I expect your response to my comment in the Issue will elicit some additional information. I guess my question here is: Is this what you were intending to offer, and if I accept this PR, then you are finished? Or are you looking for a additional things that would be helpful, or what?

Thank you so much for doing this. I have already learned a lot. I knew nothing about Temurin Java, for instance. Thanks again!

I'm not going to accept this PR until we have additional dialog about what your plans are, but I expect that I will do so eventually.

zharinov commented 2 years ago

Is this what you were intending to offer, and if I accept this PR, then you are finished? Or are you looking for a additional things that would be helpful, or what?

My intention was to make the first step, because at the moment I don't know how complete solution would look like. The further course of action can be something like this:

  1. Automate your current release workflow, so you don't have to perform it manually
  2. With feedback from newly created CI pipeline, we can refactor more confidently some tests and the workflow itself
  3. Once I became familiar with code base, I would like to contribute some new features to zprint itself 🙂
zharinov commented 2 years ago

I knew nothing about Temurin Java, for instance.

I don't know much about current Java implementations either, but what I know is that temurin is kind of default, so GitHub Actions platform caches it while other distributions are being downloaded every run.

kkinnear commented 2 years ago

That sounds great.

I'm thinking that step one of getting some CI testing is almost there. I think that consists of:

  1. Getting some kind of Linux system working
  2. Getting Clojure working on it
  3. Running the zprint tests
  4. Running test_config with the uberjar
  5. Building zprintl-n.m.p
  6. Running test_config with zprintl-...

This is just my idea, I'm open to alternatives.

You've achieved 1-3. I'm thinking that #4 probably isn't too hard. Once you've build the uberjar, (which I think you have), you just:

./test_config 1.2.3 uberjar

Now, presently, test_config doesn't exit with a non-zero exit status if it fails. It just spits up vast quantities of junk which makes it pretty clear it failed, but that isn't going to be very useful in an automated CI environment. So I will have to figure out how to hack it to fail in a way that affects the exit status.

Doing #5 is ... interesting. I can show you how I do #5 "by hand", so to speak. I don't know how hard it will be to get that into a GitHub action. The short story on building zprintl... is that I use docker on an old Intel MacBook Air to do it. I haven't done it in several months (and so this might not work at all today). I don't have access to that laptop at the moment, so I can't verify that this works now. This might even work on my current M1 MacBook Air, but when last I tried it (which was maybe late 2020, early 2021) the linux docker image did work, but running graalVM to build zprint never terminated. It didn't fail with an error, it just never terminated. I gave it maybe 30 minutes, but nothing happened.

When building zprintl... on the old Intel laptop using docker, it takes 5 or 6 minutes to build. Never more than 10.

Anyway, the files are:

build.zprintl
zprintl.sh

This is the output on the screen when it builds:

Kims-2012-Air:zprint kkinnear$ ./build.zprintl java8-20.3.0 1.2.3
Downloading: Component catalog from www.graalvm.org
Processing Component: Native Image
Downloading: Component native-image: Native Image  from github.com
Installing new component: Native Image (org.graalvm.native-image, version 20.3.0
)
Refreshed alternative links in /usr/bin/
GraalVM Version 20.3.0 (Java Version 1.8.0_272-b10)
/app
[zprintl-1.2.3:106]    classlist:  12,501.82 ms,  5.82 GB
[zprintl-1.2.3:106]        (cap):   2,223.36 ms,  5.82 GB
[zprintl-1.2.3:106]        setup:   6,703.84 ms,  5.82 GB
[zprintl-1.2.3:106]     (clinit):   1,281.71 ms,  6.68 GB
[zprintl-1.2.3:106]   (typeflow):  76,022.62 ms,  6.68 GB
[zprintl-1.2.3:106]    (objects):  55,351.94 ms,  6.68 GB
[zprintl-1.2.3:106]   (features):   1,996.69 ms,  6.68 GB
[zprintl-1.2.3:106]     analysis: 138,804.21 ms,  6.68 GB
[zprintl-1.2.3:106]     universe:   4,398.94 ms,  6.68 GB
[zprintl-1.2.3:106]      (parse):  21,789.59 ms,  6.66 GB
[zprintl-1.2.3:106]     (inline):  16,512.05 ms,  6.41 GB
[zprintl-1.2.3:106]    (compile): 128,600.41 ms,  6.67 GB
[zprintl-1.2.3:106]      compile: 171,623.67 ms,  6.67 GB
[zprintl-1.2.3:106]        image:  18,345.02 ms,  6.59 GB
[zprintl-1.2.3:106]        write:  11,946.19 ms,  6.59 GB
[zprintl-1.2.3:106]      [total]: 365,157.80 ms,  6.59 GB

This is using an old version of graalvm, but whenever I upgrade something else breaks, and since this works fine, I haven't had any motivation for upgrading. I don't know if this version even exists anymore. I know that if you try to download graalvm, they don't even have a Jave8 version. Oh, that may be a problem. What I've shown here is how to build a Java8 version of zprintl. I think you will have a java17 uberjar, and so that probably won't work great with building a java8 version of zprintl. But maybe it doesn't matter.

I'm hoping that you can figure out how to get a working version of zprintl using GitHub actions. From that, you can then do the ./test_config 1.2.3 graalvm-linux thing and have it run the additional tests.

I'm also hoping that whatever you build in the GitHub action can be saved somewhere so that it can be used in the release, and I don't have to keep doing it by hand. I don't mind doing the MacOS one by hand, as I do it just for testing, but the linux one has been a pain since I upgraded my laptop in early 2020. I don't know how or where you can save results/output from a GitHub action, but that would be a goal. I'd just as soon not put it directly into a release, because it might be the release version. There is a whole thing about how to package up the release that we need to discuss, but that is probably not today's problem. I think today's problem is to have something to package up for a release, not so much how to do it.

Even without figuring out how to save a zprintl for a release, if you can get 1-5 working in your fork, I'll be delighted to take it.

zharinov commented 2 years ago

Now, presently, test_config doesn't exit with a non-zero exit status if it fails. It just spits up vast quantities of junk which makes it pretty clear it failed, but that isn't going to be very useful in an automated CI environment. So I will have to figure out how to hack it to fail in a way that affects the exit status.

If successful test outputs nothing, it may be as easy as:

- name: Test config
  run: test "$(./test_config 1.2.3 uberjar)" = ""

I'll read your comment more carefully tomorrow, it's night time in my location.

kkinnear commented 2 years ago

Good point about test_config and output. However this is what a successful run looks like:

√ projects/zprint % ./test_config 1.2.4 graalvm-mac
If you ^c out of this test, be sure and clean up ~/.zprintrc, ./.zprintrc and ./.zprint.edn!
...........  --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, file w/fn, and no cache
...........  basic config test.
...........  basic config with comment in ~/.zprintrc test.
...........  {:cwd-zprintrc? true} on command line test
...........  {:search-config? true} on command line test
...........  ~/.zprintrc being used 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
...........  --explain-all test
...........  --explain test
...........  :coerce-to-false test
...........  fn using (fn ...) in ~/.zprintrc file test
...........  fn using #(...) in ~/.zprintrc file test
...........  fn #(...) in .zprintrc, unused, no :cwd-zprintrc?/:search-config?
...........  fn #(...) in .zprintrc found since :cwd-zprintrc?
...........  local .zprintrc file that does not parse found with :search-config?
...........  local .zprintrc file that does not parse found with :cwd-zprintrc?
...........  local .zprintrc file with bad data found with :search-config?
...........  local .zprintrc file with bad data found with :cwd-zprintrc?
...........  fn is valid in ./.zprintrc file test
...........  fn using (fn [x y] ...) on command line test
...........  fn using #(...) on command line test
...........  -w test with one file
...........  -w test with two files
...........  -w test with three files, one of which doesn't exist
...........  -w test with two files, the first which is read-only
...........  -w test with two files, the first which fails to format: bad !zprint map
...........  -w test with two files, the first which fails to format: bad !zprint key
...........  local .zprintrc having a fn that is used
...........  command line access to guide option-fn
...........  html output

Which is not useless information, but makes the "no output, it must have worked" not be a useful approach.

I'll work on test_config to get it to do exit-status stuff. Which I assume will allow it to be noticed if it doesn't work in a GitHub action?

Thanks!

zharinov commented 2 years ago

Okay, I see now.

Another approach would be checking whether every non-first line of script output is starting from ........... or not. This one is more quick-and-dirty, but may be okay as the first phase of creating test pipeline. Up to you, though.

kkinnear commented 2 years ago

I just hacked test_config to exit with the number of tests that failed. I tested it by forcing a couple failures. I didn't try forcing every failure. I am in the end-stage of releasing 1.2.4, and after I build the zprintl with my old laptop Monday, I'll be releasing 1.2.4 on Tuesday so it will show up in the repo then. But I'll attach it to this comment now, in case it is helpful to. you in the meantime. I had to rename it to test_config.txt to get GitHub to let me attach it here. You will want to remove the .txt extension if you use it. test_config.txt .

zharinov commented 2 years ago

Status update: I managed to create workflow that is covering all six requirements mentioned earlier (see example).

However, it has some ugly parts (e.g., version string detection) and next week I'm about to refactoring initial solution.

kkinnear commented 2 years ago

That's beyond wonderful! Thanks for the example, it was great to see it all working! I'm truly impressed.

Two questions:

  1. Where does the uploaded zprintl-1.2.4 end up? I saw that it was uploaded, but it wasn't clear on first glance where it landed.
  2. It looks like this works on every push to the repo? Since I am using one branch to collaborate with someone testing some of the changes I am making, I'm thinking that it would be good to only run the workflow when a push to master is done. (Yes, I know I should change it to main.). The theory then would be that I'd push to master when I was about to do a release, and if all of the tests worked, then I'd tag it as a release and push the tags and then gather the files into the release.

I'm seeing that actions will do MacOS as well as linux, so downstream it might be valuable to have all of the released binaries built by actions, instead of the MacOS one on my laptop. Then maybe there would be a separate action/workflow to build the release when I tagged and pushed the tags to GitHub. But that isn't today's task.

I'm very impressed and excited about where this is all going. I'm looking forward to having this all work. It is going to make releasing zprint so much easier! Thank you so much for pursuing this work!

zharinov commented 2 years ago
  1. If I understand it correctly, it is uploading to the artifact storage internal to the GitHub actions. This is storage to share common data between jobs. What we want at the end is to put it to some downloadable space, so that it will be available in the release page. I don't know much about it yet.
  2. In further work, I'll separate this whole workflow into the smaller chunks depending on the feedback speed they're providing. For example, Babashka tests are ultra-fast, so we can require them to pass in the first place PR-level checks are also a good place for clj-kondo, etc. Once changes are in trunk, ideally it should be in the releasable state, so that you can quickly release changes. This can be build for the whole lineage of platforms, or maybe just linux binary build+test (via ./test_config), while macos and windows versions are waiting for release.

Today I've created new GitHub action for Clojure tooling that combines anther 3 actions (setup-java, setup-graalvm and setup-clojure) and makes them start a way faster thanks to caching. Unlike original actions, this one won't download Java distribution such as GraalVM (+native-image component) every run and you won't need to wait one extra minute just for starting something useful. I want to use it here.

zharinov commented 2 years ago

One problematic thing I see now is that build automation for M1 processor. GitHub doesn't provide runners on this chip, so we need to build it somewhere else. I tend to think it should be CirrusCI, which is the place where the Babashka M1 binary is being built.

kkinnear commented 2 years ago

So, I'm not currently providing M1 pre-built binaries. I have found the MacOS binaries built for Intel by graalVM to be quite fine when running on an M1 MacBook Air. I suppose that M1 specific binaries might be even better, but so far that hasn't made it near to the top of my list for things to do for zprint.

zharinov commented 1 year ago

Seems like this one can be closed now