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

Add test support for exec #131

Merged
merged 1 commit into from
May 27, 2023
Merged

Conversation

lread
Copy link
Contributor

@lread lread commented May 22, 2023

New bb tasks:

  • clean - delete build work
  • test:bb - runs all tests under babashka
  • test:native - runs exec tests against native image of run_exec.clj
  • test:jvm - (renamed from test) runs non-exec tests under jvm clojure

CI:

  • GitHub Actions updated to run native image tests
    • jobs renamed to match CircleCI naming convention
  • CircleCI was doing a subset of GitHub Actions work, it now is a full reflection.
    • added jdk install support in script/circle_ci.clj
    • bootstrapping CI to be able to run bb and then scripting in bb is much more pleasant/maintainable than scripting in bash+Powershell
    • used jdk install support even on linux, instead of trying to to select jdk via docker image. This made config cleaner, allowed for consitent jdk selection across OSes and had surprisingly little performance hit especially after caching jdks.
    • installing jdks via bb hugely more performant than doing so via brew/scoop/chocolatey
    • jdk download urls, and shas discovered by disco API. See: https://github.com/foojayio/discoapi
  • Both CIs are temporarily using a SHAPSHOT release of bb for babashka.process/exec reload support. We'll turf this after next release of babashka.

Add run_exec.clj - a wee program to exercise exec

  • source and build support under ./test-native
  • includes script-adjacent bb.edn to suport running as a script and reloading sources instead of using babashka built-in babashka.process
  • specify babashka.process.test.reload system property to signal that run_exec.clj should reload babashka.process from sources when running from bb.
  • note: test:native task does not natively compile tests, it only natively compiles run_exec.clj. It launches natively compiled run_exec.clj via jvm clojure and observes results for correctness.

Test utilities moved to babashka.process.test-utils so that they can be used by existing babashka.process-test and new:

  • babashka.process-exec-test
  • bb build scripts (specifically: test-native/bb.edn)

Add :ps-me to wee dummy wd.clj to support exec arg0 testing for macOS and Linux.

Add doc/dev.md with some relevant notes/pointers.

Other changes:

  • deleted script/test bash script, it is replaced by bb tasks
  • we now look for bb first in current dir, then on path. This seems the safer way to choose bb when running babashka.process tests from babashka
  • tests now printing bb version to give confidence we are using the bb we want to be using

Bugs found and fixed as a result of testing:

  • tests using java now understand that java executable path can contain spaces.
  • test using javac seem to be happier running javac from same dir as source
  • exec now converting :env and :extra-env keywords the same way process function do, see exec does not convert :env and :extra-env keywords #123

Closes #117, Fixes #123

New bb tasks:
- clean - delete build work
- test:bb - runs all tests under babashka
- test:native - runs exec tests against native image of run_exec.clj
- test:jvm - (renamed from test) runs non-exec tests under jvm clojure

CI:
- GitHub Actions updated to run native image tests
  - jobs renamed to match CircleCI naming convention
- CircleCI was doing a subset of GitHub Actions work, it now is a full reflection.
  - added jdk install support in script/circle_ci.clj
  - bootstrapping CI to be able to run bb and then scripting in bb is
  much more pleasant/maintainable than scripting in bash+Powershell
  - used jdk install support even on linux, instead of trying to
  to select jdk via docker image. This made config cleaner, allowed for
  consitent jdk selection across OSes and had surprisingly little
  performance hit especially after caching jdks.
  - installing jdks via bb hugely more performant than doing so via
    brew/scoop/chocolatey
  - jdk download urls, and shas discovered by disco API. See:
  https://github.com/foojayio/discoapi
- Both CIs are temporarily using a SHAPSHOT release of bb for
`babashka.process/exec` reload support. We'll turf this after next
release of babashka.

Add `run_exec.clj` - a wee program to exercise `exec`
- source and build support under ./test-native
- includes script-adjacent `bb.edn` to suport running as a script and
reloading sources instead of using babashka built-in babashka.process
- specify `babashka.process.test.reload` system property to signal that
`run_exec.clj` should reload `babashka.process` from sources when
running from bb.
- note: `test:native` task does not natively compile tests, it only
natively compiles `run_exec.clj`. It launches natively compiled
`run_exec.clj` via jvm clojure and observes results for correctness.

Test utilities moved to `babashka.process.test-utils` so that they can
be used by existing `babashka.process-test` and new:
- `babashka.process-exec-test`
- bb build scripts (specifically: test-native/bb.edn)

Add :ps-me to wee dummy wd.clj to support `exec` arg0 testing for macOS
and Linux.

Add doc/dev.md with some relevant notes/pointers.

Other changes:
- deleted `script/test` bash script, it is replaced by bb tasks
- we now look for bb first in current dir, then on path. This seems the
safer way to choose bb when running babashka.process tests from babashka
- tests now printing bb version to give confidence we are using the
bb we want to be using

Bugs found and fixed as a result of testing:
- tests using java now understand that java executable path can contain
spaces.
- test using `javac` seem to be happier running `javac` from same dir as source
- `exec` now converting `:env` and `:extra-env` keywords the
same way `process` function do, see babashka#123

Closes babashka#117, Fixes babashka#123
@lread
Copy link
Contributor Author

lread commented May 22, 2023

Another biggie. Lemme know watcha think.
As always, happy to collab and adapt.

After merge into babashka, we can adjust this:

;;;; babashka.process
;; test built-in babashka.process
(test-namespaces 'babashka.process-test)

;; test babashka.process from source
#_{:clj-kondo/ignore [:duplicate-require]}
(require '[babashka.process] :reload)
(test-namespaces 'babashka.process-test)

To maybe something like this:

;;;; babashka.process
;; test built-in babashka.process
(test-namespaces 'babashka.process-test)

(when (= "native" (System/getenv "BABASHKA_TEST_ENV"))
  ;; test babashka.process from source
  #_{:clj-kondo/ignore [:duplicate-require]}
  (require '[babashka.process] :reload)
  (System/setProperty "babashka.process.test.reload" "true")
  (test-namespaces 'babashka.process-test 'babashka.process-exec-test))

@borkdude
Copy link
Contributor

borkdude commented May 24, 2023 via email

@lread
Copy link
Contributor Author

lread commented May 25, 2023

I'll look at this soon, but I'm on a trip so it'll take a few days

Thanks for the ping! Enjoy your trip!

@borkdude borkdude merged commit 204b0bf into babashka:master May 27, 2023
@borkdude
Copy link
Contributor

Thanks!

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.

exec does not convert :env and :extra-env keywords Explore adding test coverage for exec
2 participants