-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
Enable running most of bbin's test suite on a JVM #92
base: main
Are you sure you want to change the base?
Conversation
This commit enables a JVM-based workflow. The tests that pass without modification on a JVM can now be run on a JVM. The tests that fail on JVM now only run on Babashka. In addition, test data has been updated to reflect the reality - per 2024-11-27, the latest version of org.babashka/http-server is 0.1.13.
.gitignore
Outdated
@@ -27,3 +27,5 @@ | |||
/checkouts | |||
/classes | |||
/target | |||
|
|||
/deps.local.edn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No trailing final newline is ugly!
dev/babashka/bbin/dev.clj
Outdated
[clojure.core.async :refer [<!] :as async] | ||
[clojure.core.async :as async :refer [<!]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My editor did this without me noticing, I'll revert it!
src/babashka/bbin/util.clj
Outdated
(defmacro ifbb [then else] | ||
(if (System/getProperty "babashka.version") | ||
then | ||
else)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ifbb
is not used, should be deleted.
In addition to the comments in the diff, I added Kaocha and Launchpad. I did that mostly out of habit, I like using the tools. Thoughts? Do you prefer staying "leaner" with regards to tooling? I added Launchpad to |
From @borkdude on Slack:
|
I generally prefer to keep maps like this sorted, I find things more easily then. If there was some previous structure here that I didn't pick up, I'll revert.
Kaocha has become heavily ingrained into my workflow through https://github.com/magnars/kaocha-runner.el. kaocha-runner requires that kaocha is on the classpath in order to work. Though that doesn't require that I put Kaocha into every project I work with. Clojure 1.12's add-lib to the rescue!
The way I'd added Launchpad in a previous commit in this PR would be bad for all bbin users: they would all be forced to download Launchpad. I don't want that. Launchpad is a development helper, it shouldn't carry weight that influences bbin users badly. So I'm removing Launchpad. We could possibly get around shipping Launchpad to bbin users by loading launchpad dynamically in the `bb launchpad` task, but that's not worth it for me for now.
I've removed Kaocha and Launchpad. I like using Kaocha from within my Emacs, but I can load Kaocha with As for Launchpad: Adding a Launchpad dep to I still have to set up a |
We didn't end up adding either Launchpad or Kaocha, so we might as well leave bb.edn unchanged.
(It's only used for testing)
@borkdude I think we're mostly there, but I'd like your thoughts on the tests that are failing in CI: https://github.com/teodorlu/bbin/actions/runs/12601558300/job/35122890533 My observations:
It would also be helpful to know you get the CI behavior locally (red) or as I do (green). |
Local development was the motivation for getting the test suite running on a JVM, so CI might not be needed for now. |
This PR enables a JVM-based workflow. The tests that pass without modification on a JVM can now be run on a JVM. The tests that fail on JVM only run on Babashka (for now).
In addition, test data has been updated to reflect the reality - per 2024-11-27, the latest version of org.babashka/http-server is 0.1.13.
Please answer the following questions and leave the below in as part of your PR.