Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test workflow #249

Closed
wants to merge 5 commits into from
Closed

Add test workflow #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
88cc462
Add test workflow
zharinov Jul 27, 2022
83f8fa2
Bump `lein-zprint` to 1.2.4.1
zharinov Aug 4, 2022
4f7b9bf
Add integration test
zharinov Aug 4, 2022
2abdd42
Skip bb test for now
zharinov Aug 4, 2022
cac2df4
Change event triggers
zharinov Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
on:
push:
branches: [ master ]
pull_request:
branches: [ master ]

jobs:
test-clj:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3.0.2

- name: Setup Java
uses: actions/setup-java@v3.4.0
with:
distribution: temurin
java-version: 17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using a matrix syntax? Running it on all the LTS JDK versions would seem reasonable.


- name: Setup Clojure
uses: DeLaGuardo/setup-clojure@9.3
with:
cli: latest
lein: latest
bb: latest

- name: Cache
uses: actions/cache@v3.0.5
with:
path: |
~/.m2/repository
~/.gitlibs
~/.deps.clj
key: cljdeps-${{ hashFiles('project.clj', 'deps.edn', 'bb.edn') }}
restore-keys: cljdeps-

- name: Uberjar
run: lein uberjar

# - name: Babashka tests
# run: bb test:bb

- name: Clojure tests
run: clojure -M:cljtest:humane:runner

- name: Integration tests
run: ./test_config $(awk -F "\"" '{print $2}' project.clj | head -n 1) uberjar

test-cljs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to at least draft a lint job. https://github.com/clj-kondo/lein-clj-kondo needs particularly little setup (no graalvm installation, etc)

runs-on: macos-latest
steps:
- name: Checkout
uses: actions/checkout@v3.0.2

- name: Setup Java
uses: actions/setup-java@v3.4.0
with:
distribution: temurin
java-version: 17

- name: Setup Clojure
uses: DeLaGuardo/setup-clojure@9.3
with:
cli: latest
lein: latest
bb: latest
Comment on lines +64 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using fixed versions? It's often quite annoying to have CI builds break for seemingly no reason

Copy link
Contributor Author

@zharinov zharinov Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and actually I would like to propose using dependency updating the tool I'm actively contributing to: https://github.com/renovatebot/renovate

The app is free for GitHub, and it handles project.clj as well as deps.edn.

Using it, we would have both of the two worlds: fresh dependencies (thanks to Renovate) and merge confidence (thanks to CI tests)

Copy link
Contributor Author

@zharinov zharinov Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my demo of what is possible with automatic dependency updates:
https://github.com/zharinov/zprint/pulls

@kkinnear What do you think?

(I need to fix Leiningen commented deps handling, though) — done

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's amazing. As you may have noticed, I rarely change the versions of the things on which zprint depends, as I find it a pain to debug things like that when they break. Not that they break all that often, other than graalVM, which I noticed isn't something that renovate found. The thought of having major dependencies automatically updated (or at least automatically PR'ed for update) is kind of scary, but probably a good idea. As long as we really believe that the automated tests will run.

I didn't read too closely, but is it the case that if it automatically accepts the renovate PR's, and the tests don't run correctly, then it will back them out or something? Or is that a manual operation?

In any case, I'm up for giving renovate a try, but I think that first we need to get the tests to work in GitHub actions, including test_config, and at least the Linux pre-built binary (zprintl-1.2.n). It would be nice to do the Mac version as well, but I expect that is not possible with GitHub actions. Anyway, once the tests work, then adding renovate to the mix would seem like a good, if kind of unsettling, idea. Thanks for proposing it!

Copy link
Contributor Author

@zharinov zharinov Aug 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can create PRs for you to review and merge, but also can be configured to automatically merge non-major releases once all CI checks are passing, leaving major updates up to you. Also it can create Dependency Dashboard issue for managing dependencies from the single central place.

Actually, it's very configurable. For example, I think it's possible to make it upgrade clojure dependencies that are documented in the markdown files (using regex).

So again, recommend you to give it a try once we're done here with build/test pipeline.

Copy link
Contributor Author

@zharinov zharinov Aug 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of GraalVM, it's also possible to upgrade it automatically (and test on CI), as soon as we refer to https://github.com/graalvm/graalvm-ce-builds releases somewhere in the code


- name: Cache
uses: actions/cache@v3.0.5
with:
path: |
~/.m2/repository
~/.gitlibs
~/.deps.clj
key: cljdeps-${{ hashFiles('project.clj', 'deps.edn', 'bb.edn') }}
restore-keys: cljdeps-

- name: Uberjar
run: lein uberjar

- name: Install Planck
run: brew install planck

- name: ClojureScript tests
run: clojure -M:cljs-runner
2 changes: 1 addition & 1 deletion doc/using/files.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ the `:plugins` key in `project.clj`:
:url "http://example.com/FIXME"
:license {:name "EPL-2.0 OR GPL-2.0-or-later WITH Classpath-exception-2.0"
:url "https://www.eclipse.org/legal/epl-2.0/"}
:plugins [[lein-zprint "1.2.4"]]
:plugins [[lein-zprint "1.2.4.1"]]
:dependencies [[org.clojure/clojure "1.10.0"]]
:repl-options {:init-ns zpuse.core})
```
Expand Down
2 changes: 1 addition & 1 deletion project.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
:url "https://opensource.org/licenses/MIT",
:key "mit",
:year 2015}
:plugins [[lein-doo "0.1.10"] [lein-codox "0.10.3"] [lein-zprint "1.2.4"]]
:plugins [[lein-doo "0.1.10"] [lein-codox "0.10.3"] [lein-zprint "1.2.4.1"]]
:profiles {:repl {:dependencies [#_[com.taoensso/tufte "1.1.1"]
[better-cond "1.0.1"]
[olical/cljs-test-runner "3.7.0"]
Expand Down