search

babashka / sci.configs

A collection of ready to be used SCI configs
https://babashka.org/sci.configs/
Other
19 stars 17 forks source link

Update testing macro to update-current-env #48

Closed jaidetree closed 1 week ago

jaidetree commented 1 week ago

Previously, the testing macro would update a testing-contexts dynamic var, but the APIs around for stringifying testing-context would read from the current-env.

Given the official cljs impl updates the current-env, this PR applies the same approach.

borkdude commented 1 week ago

Just to double-check, did you test this with nbb locally?

jaidetree commented 1 week ago

Just to double-check, did you test this with nbb locally?

I did not yet. What's that process look like? Clone nbb, update deps to point to local sci.configs, build nbb, and link to the artifact in my dev repo with the tests?

borkdude commented 1 week ago

yes

borkdude commented 1 week ago

what you could also do is make a fork of sci.configs and then make a candidate PR for nbb based on your fork of sci.configs which tests the change, and after that, we merge it into sci.configs

jaidetree commented 1 week ago

I'll give that a shot. Also looks like my formatter added more noise than desired. Going to try again first to mitigate that.

jaidetree commented 1 week ago

Created babashka/nbb#359 as my best attempt at what you suggested for testing. Also did test it locally:

Testing jaide.test

FAIL in (simple-test) (/Users/j/projects/nbb/test.cljs:10)
If you see this the testing macro is working
expected: (= (+ 1 1) 3)
  actual: (not (= 2 3))

Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
borkdude commented 1 week ago

I guess now you can update your nbb PR to use the newest sci.configs and then we'll merge that

jaidetree commented 1 week ago

I guess now you can update your nbb PR to use the newest sci.configs and then we'll merge that

Done. Thanks so much!