lambdaisland / kaocha

Full featured next gen Clojure test runner
https://cljdoc.org/d/lambdaisland/kaocha/1.0.861/doc/1-introduction
Eclipse Public License 1.0
792 stars 81 forks source link

print-config cannot be enabled from exec-fn #401

Closed cap10morgan closed 1 year ago

cap10morgan commented 1 year ago

Seemingly related to #312, I can't seem to find a way to enable print-config from an exec-fn -X invocation. And this line being the only hit for print-config when searching the repo makes me think it likely isn't currently possible.

alysbrooks commented 1 year ago

I believe you are correct—exec-fn uses kaocha.api/run, instead of kaocha.runner/run. We'd typically recommend using our standard command line interface for things like this. Is there a particular reason the regular command-line interface doesn't work here? (Maybe just for consistency's sake?)

cap10morgan commented 1 year ago

Yeah, I guess you could call it consistency. We don't use ./bin/ stubs for invoking any of our other dev commands in the project in question, nor the tests in other projects that use other test runners. And it would be a lot of effort for negative gains to change everything over to that. So we want to use the exec-fn invoker here like we do w/ all of our other dev commands.

I'm happy to do some work in the code to improve this and issue a PR. But I've attempted to tackle the maximal "make everything share the things they should share" approach and got a little lost in there. So I might have to try to find a smaller chunk to bite off.

alysbrooks commented 1 year ago

Thinking about it more, perhaps a better way to go is to create a convenient API function for print-config specifically that you could specify for :exec-fn in an alias or invoke via -X. We could also add this to kaocha.repl (or document it as being useful for REPL use).

alysbrooks commented 1 year ago

Hi @cap10morgan, are you still interested in addressing this with a PR? My last comment offers a suggestion for doing it in a smaller chunk.

cap10morgan commented 1 year ago

@alysbrooks Sure, maybe. I'm not sure your suggestion would solve my use case, though. I'm trying to pass in other config params via the clojure -X:... invocation and I want print-config to show me what got picked up and what didn't. The logic for how the :exec-args get turned into kaocha config is mostly undocumented (or I haven't found it) and hard to trace through the code. So I was hoping this would be a way to debug it.

plexus commented 1 year ago

You can verify what comes out using a REPL

(require '[kaocha.config :as config])

(def exec-args {....})

(config/merge-config (config/load-config) (config/normalize exec-args))

This is essentially the same normalization that you achieve by putting #kaocha/v1 on top of your tests.edn.

plexus commented 1 year ago

Would something like this be enough? I haven't actually tested it, but I don't think there's harm in checking for these keys and having similar behavior as in the real test runner.

https://github.com/lambdaisland/kaocha/commit/2d10233aa273bb61c63a26783fc3f4ab92d8ff0b

@cap10morgan if you could test that and turn it into a proper PR with CHANGELOG and doc updates that would be very helpful!

cap10morgan commented 1 year ago

@plexus I'll give it a shot, thanks!

alysbrooks commented 1 year ago

@cap10morgan ah, right wasn't thinking of that usecase. Makes sense. We should perhaps have some documentation around exec-fn, too.

plexus commented 1 year ago

I think it would be helpful to have reference docs about all the (fully qualified) keys that can go into kaocha's config, and then separately document how config normalization works, which mainly involves translating short keywords into fully qualified ones. This would be all the documentation you need for exec-fn, except perhaps for other special keys we introduce like the proposed :print-config. But really exec-fn is just a way of passing in kaocha config directly, so good reference docs about what that data structure looks like would go a long way, and be helpful in general.

cap10morgan commented 1 year ago

Would something like this be enough? I haven't actually tested it, but I don't think there's harm in checking for these keys and having similar behavior as in the real test runner.

2d10233

@cap10morgan if you could test that and turn it into a proper PR with CHANGELOG and doc updates that would be very helpful!

This is working great and solves my original problem well! I'll work it up into a PR. Thanks!

imrekoszo commented 1 year ago

We don't use ./bin/ stubs for invoking any of our other dev commands

Linking my (closed as "as designed") issue #410 for the sake of documentation.