A Q about options. I see that the CLI has --output, so there's an :output key in the options map, but in the running entry points, :reporter is set to output and the lower-level machinery uses :reporter to control the "output".
As I'm working on integration with test-runner and the Polylith external test runner, I can map most of the options (`:include`/`:exclude` work the same, :namespace => :ns-filter, :var => :var-filter), but it looks like I'll have to add processing for :output, duplicating lazytest.cli's logic (which is fine), and then rename :output => :reporter for the low-level calls (which is also fine).
Is there anything in the low-level machinery in LazyTest that depends on both :output and*` :reporter, or can I safely just pass :reporter (with the cleaned up value of :output)? I think I can, looking at the code, but I'm curious why it is the way it is (since -r --reporter is *not a CLI option)?
great question
I went with --output because reporter felt too low level. Output could also mean "prints json", whereas reporter feels specifically like "displayed presentation"
$ clojure -M:test -m cognitect.test-runner --output dots -e :broken
Running tests in #{"test"}
(..)
Ran 2 test cases in 0.00461 seconds.
0 failures.
(excludes tests marked ^:broken, uses dots output/reporter)that's sick
I can expose or API-fy the internal functions if that would help you
And this runs both clojure.test and LazyTest tests marked ^:broken:
$ clojure -M:test -m cognitect.test-runner --output dots -i :broken
Running tests in #{"test"}
Testing lazy.lib.lazy.lib-test
FAIL in (old-test) (lib_test.clj:8)
FIXME, I fail.
expected: (= 0 1)
actual: (not (= 0 1))
Ran 1 tests containing 1 assertions.
1 failures, 0 errors.
=== FOCUSED TESTS ONLY ===
(F)
lazy.lib.lazy.lib-test
broken
needs fixing:
Expectation failed
Expected: (= 0 1)
Actual: false
Evaluated arguments:
* 0
* 1
Only in first argument:
0
Only in second argument:
1
in lazy/lib/lazy/lib_test.clj:12
Ran 1 test cases in 0.00328 seconds.
1 failure.(I was a bit surprised to see FOCUSED TESTS ONLY since I wasn't using :focus anywhere, but I gather it does that in other situations?)
that's very cool
you're narrowing with -i
Ah, got it...
$ clojure -M:test -m cognitect.test-runner --output dots -i :working
Running tests in #{"test"}
=== FOCUSED TESTS ONLY ===
(..)
Ran 2 test cases in 0.00353 seconds.
0 failures.i wanted it to be obvious that any narrowing is focusing on a subset, not the whole test suite
but i can be convinced away from that too lol
i don't remember why I went with :output converted to :reporter. i bet using the :id option in tools.cli would be a simple fix to make it all consistently :reporter or whatever
i'll need to look at the code again
I have the -X version of test-runner partly working too (the handling of :output is a bit wonky):
$ clojure -X:test cognitect.test-runner.api/test :output dots :includes '[:working]'
Running tests in #{"test"}
=== FOCUSED TESTS ONLY ===
(..)
Ran 2 test cases in 0.00369 seconds.
0 failures.(based on how the other -X options work, it probably should be :outputs but that's a bit unwieldy since you mostly just want one; pushed a reasonably fixed up version of it now)
I don't like -X so i completely forgot to support it lol. Thanks for the reminder to set something up
Well, test-runner already has it so I only needed to make :output work 🙂
Most of the time, you supply a single reporter that bundles smaller reporters together: "dots" is [focused dots* results summary], so it felt wrong to use outputs
No, I meant in the context of test-runner: all the -X options are plural and take collections, but the underlying -M / CLI options are all singular (but can be provided multiple times).
--include :a --include :b but :includes '[:a :b]'
oh interesting
i trust your judgment in this area
I think test-runner's approach is a bit weird but...
internally, --output returns a vector regardless of how many are provided
Yes, same in the -M options of test-runner. It's only the -X options that are plurals and require vectors...
(I don't think many folks use the options with -X, just "go run all the tests" 🙂 )
that's fine by me if that's how the other options are
i took direct inspiration from c.tr when building the lazytest cli, as you probably noticed
Made it consistent:
$ clojure -X:test cognitect.test-runner.api/test :includes '[:working]' :outputs '[dots]'
Running tests in #{"test"}
=== FOCUSED TESTS ONLY ===
(..)
Ran 2 test cases in 0.00382 seconds.
0 failures.I will say that the reporters don't seem to compose very well:
$ clojure -X:test cognitect.test-runner.api/test :includes '[:working]' :outputs '[dots clojure-test]'
Running tests in #{"test"}
=== FOCUSED TESTS ONLY ===
(
Testing lazy.lib.lazy.lib-test
..)
Ran 2 test cases in 0.00684 seconds.
0 failures.
Ran 1 tests containing 2 test cases.
0 failures, 0 errors.Ah, it works better if I swap the order...
And, I can use dots* to suppress the regular summary, so '[clojure-test dots*]' works pretty well.
(although maybe that wouldn't work with multiple nses?)
The reporters are... fairly simple
so there's nothing to prevent what you've seen
the precomposed reporters have pieces that don't step on each others toes
clojure-test isn't really composable like that because it's operating on all of levels instead just one level
i could split it into discrete pieces and make it composable too, just didn't think of it
dots* merely prints periods and Fs and parentheses, nothing else. so if you use it along with another reporter that operates on the same suite result types, then they'll clash as you saw
i'll write up some documentation/a spec for how the suite results and reporters works
there are some tracking issues so i don't forget any of this when i'm on my computer next week
if you've written modular code, you're welcome to open a PR to add -X or whatever other changes you've worked on for your test runner
The -X stuff was already in Cognitect's runner: cognitect.test-runner.api just wraps the main test function that takes an options map, that is also called from -main after CLI option processing.
All I did was add the mapping for :outputs to :output to pass through 🙂
okay, having looked at the code and thought about it a bit, i think my intention with :output vs :reporter was that :output is the raw "input" from the cli, and :reporter is the resolved&combined function(s)
I'm okay with the status quo at this point. I was just curious about the thinking behind it 🙂
let me know if that might break your code
As I said in the ticket related to that, if you change it in any way, it'll break both of my test runners 🙂
now ->config consumes :output and produces :reporter. i can open a PR to either/both
i don't mind doing that work
better to fix this inconsistency before more people rely on it
I get the desire for consistency -- either :output all the way through or :reporter all the way through. The current swapping is the problematic thing that each test runner has to handle -- so not having the swapping is a good thing.
Make the change, and I'll just update my runners...
"all the way through" as in, internal to lazytest?
The problem as it stands is that you have "internal" machinery that switches between :output -- which all test runners would have to handle -- and :reporter which your internal code uses, and which the test runners probably have to interact with?
"the test runners"? lazytest is the test runner. or do you mean something else?
i guess another library could (kaocha-style) write their own test running logic operating on the data structures that lazytest creates.
My fork of Cognitect's test-runner and my Polylith test runner.
For consistency with your public API, those need to accept :output or --output and interact with your lazytest.repl API
those call into lazytest, which performs the running and printing/reporting. ideally, they wouldn't have to know about :reporters, they would give lazytest a list of strings or symbols in :output , it would transform those strings/symbols into functions references and then use them to perform the reporting.
having discussed this, i see that the logic in --output is not ideal and should live somewhere else, as you've had to duplicate it in your test-runner
If lazytest.repl/* accept {:output ...} and all the processing is done internally, that cleans up stuff for any runners that call into you.
cool, I'll move that logic over so :output is merely a list of symbols
:output dots and :output lazytest.reporters/dots -- or passing a vector of symbols -- should both be acceptable. All the transformation to :reporter functions should happen internally, i.e., inside the lazytest.repl/* functions or any other public API pieces.
yep, agreed
But, like I say, the status quo "works" -- it's just more work for runners to call into your code. And changing it means breaking my two "prerelease" runners but that's okay. We're all in prerelease right now 🙂
a generous perspective!
Keep me posted...
I have a branch at work with all this speculative LazyTest work on it... I'm looking forward to merging that so I can start using LazyTest on master there 🙂
To get a sense of what I'm talking about, here's the Polylith runner, dealing with LazyTest options: https://github.com/seancorfield/polylith-external-test-runner/blob/main/bases/external-test-runner-cli/src/org/corfield/external_test_runner_cli/main.clj#L102-L125
And here's the Cognitect runner, doing the same thing: https://github.com/seancorfield/test-runner/blob/master/src/cognitect/test_runner.clj#L78-L91
This reflects your use of :id in the CLI and the output/reporter mapping.
lazy-is-test? is funny
You think it should be is-lazy-test? 🙂
All the vars are lazy-* 🙂
hah no, it's genuinely a funny turn of phrase
sounds like how it feels to speak another language
i didn't go as fast as i hoped so finishing this will have to wait until tomorrow