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.

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

road to bb: http client lite & bb tests #438

Merged
merged 6 commits into from
May 24, 2022

Conversation

lread
Copy link
Collaborator

@lread lread commented May 23, 2022

When running babashka, we now use the compatible http-client-lite.

Reader conditionals select the existing http-client when running on from
the JVM.

Reader conditionals mean .cljc, which often means Clojure and
ClojureScript, but in our case it means JVM Clojure and Babashka
Clojure. Update configs for cljdoc and clj-kondo to let them know
that this is not a ClojureScript project.

Facility to test our bb implementation added via a new --platform option
on our bb test task. I'm not sure of the best way to run babashka
tests, but my first stab generates a runner.clj with appropriate
namespaces and classpath then runs that.

Etaoin does have some debug logging, so Babashka's log level is adjusted
from the default of debug to info in the generated test runner.

Finally adjusted our CI matrix to include the bb platform.

Contributes to #380

lread added 2 commits May 23, 2022 17:20
When running babashka, we now use the compatible http-client-lite.
- Thanks borkdude, your etaoin pod work was a very useful reference!
- I worked around an issue in http-client-lite when running on Windows.
See clj-commons/clj-http-lite#18.

Reader conditionals select the existing http-client when running on from
the JVM.

Reader conditionals mean .cljc, which often means Clojure and
ClojureScript, but in our case it means JVM Clojure and Babashka
Clojure. Update configs for cljdoc and clj-kondo to let them know
that this is not a ClojureScript project.

Facility to test our bb implementation added via a new --platform option
on our `bb test` task. I'm not sure of the best way to run babashka
tests, but my first stab generates a runner.clj with appropriate
namespaces and classpath then runs that.

Etaoin does have some debug logging, so Babashka's log level is adjusted
from the default of debug to info in the generated test runner.

Finally adjusted our CI matrix to include the bb platform.

Contributes to clj-commons#380
Other browsers seem to cope with a missing slash after file: but firefox
cannot in this scenario, at least on Windows.
@lread lread requested review from borkdude and slipset as code owners May 23, 2022 23:33
@lread lread changed the title Lread bb http client lite2 bb http client lite May 23, 2022
@lread lread changed the title bb http client lite road to bb: http client lite & bb tests May 23, 2022
@lread
Copy link
Collaborator Author

lread commented May 24, 2022

@borkdude, I'm sure some things can be done differently, but what do you think of this approach? Is generating a bb test runner a decent approach? I only noticed I maybe could be using test-runner after the fact.

@lread lread removed the request for review from slipset May 24, 2022 00:36
@borkdude
Copy link
Contributor

@lread Ah, I now read the whole PR. Yeah, using the cognitect test runner may unify the tests for clojure and bb more? I'm surprised how little change it was to the actual library code :)

@lread
Copy link
Collaborator Author

lread commented May 24, 2022

@borkdude, thanks for reviewing!

I will take a crack at using the cognitect test-runner before merging.
Always excited to simplify an delete code!

And yes, I agree there wasn't a ton of change required to make this library bb compatible. I'll likely summarize in the closing PR which will include docs. I think most of my time was spent on Windows mysteries.

One thing that I found a little odd for this PR was this io/resource Windows dance. Not sure if it is coming from Babashka but suspect maybe a Graalism? I can try to distill the symptom/work-around separately for Babashka if you like.

@borkdude
Copy link
Contributor

The io/resource thing may be a bug. Feel free to provide a issue + repro.

@borkdude
Copy link
Contributor

@lread Don't know if it was clear already, but you can use the cognitect test runner using this snippet in this README:

https://github.com/babashka/tools.namespace

lread added 4 commits May 24, 2022 09:02
* master:
  docs: README: use optimized bb compatible badge
  docs: changelog [skip ci]
  docs: split out user guide from readme [skip ci]
  docs: add bb compatible badge
Left in a little wrapper to adjust bb log level from debug to info.
Still driving bb test deps from deps.edn because I'd rather not repeat deps
in bb.edn.
Test reporter is now also reporting platform (jvm or bb) for that extra
confidence that we're actually running what we want to be running.
This seems to satisfy our needs.
For now.
We'll see.
I'm not optimistic.
About shelling on Windows.
@lread
Copy link
Collaborator Author

lread commented May 24, 2022

Ok, @borkdude the cognitect test-runner worked great! Gonna merge this puppy.

@lread lread merged commit a89c9a3 into clj-commons:master May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants