Fork me on GitHub
#kaocha
<
2019-01-18
>
plexus02:01:47

hmmm.... not sure we have a hook for that šŸ™‚

plexus02:01:21

I guess what you want is a kind of decorator on the reporter, so write a plugin with a config hook that takes the resolved reporter, and wraps it in a function that rewrites certain clojure.test messages

plexus02:01:19

you mean inside an (is (= ...)) right?

rgm04:01:47

hi yā€™all ā€¦ I whipped up a terminal-notifier/libnotify post-run hook, in case thatā€™s useful to anyone: https://gist.github.com/rgm/7cc2aaf2a131f637940ffd1d4f0bce05

rgm04:01:20

seems to work but critique/ideas welcome, in particular this hacky bit of whatever for which I presume a prettier function already exists, but that I wasnā€™t able to find: https://gist.github.com/rgm/7cc2aaf2a131f637940ffd1d4f0bce05#file-kaocha_notifier-clj-L28-L37

plexus06:01:21

Hi @rgm, this is great! You should be able to use kaocha.result/testable-totals and kaocha.result/failed?.

plexus06:01:36

How would you feel about turning this into a plugin? You're almost there, just need to change your last notification-hook into a (defplugin ... (post-run ...))

plexus06:01:18

For anything that's worth distributing I recommend making it a plugin, hooks are really for end users that want to locally patch/hack something in.

plexus07:01:54

;;deps.edn
{:deps {rgm/kaocha-notifier {:git/url ":e5befecc5b3f950d67944dd63401c580.git", :sha "1aa88253c2065568198478e9cd80e80e54210d63"}}}

plexus07:01:10

;;tests.edn
{:plugins [:rgm/kaocha-notifier]}

magnars07:01:36

@plexus yes, I mean to reverse the meaning of (is (= expected actual)) to (is (= actual expected)) ... and I hope to do it without writing a custom test reporter. šŸ™‚ I would think I could either hook into some place to rewrite the test definitions. In any case, it would be good to do so before the diffs are done in the reporter. Any ideas?

plexus07:01:31

I think you need to look at the clojure.test/assert-expr multimethod. This defaults to calling clojure.test/assert-predicate. Kaocha adds an implementation that does the single-arity-= check, then delegates to assert-predicate

plexus07:01:44

(defmethod t/assert-expr '= [msg form]
  (if (= 2 (count form))
    `(t/do-report {:type ::one-arg-eql
                   :message "Equality assertion expects 2 or more values to compare, but only 1 arguments given."
                   :expected '~(concat form '(arg2))
                   :actual '~form})
    (t/assert-predicate msg form)))

plexus07:01:11

You could overwrite this with your own version that changes form before passing it on.

plexus07:01:41

Not super clean, but nothing about customizing clojure.test is clean šŸ˜›

magnars07:01:12

Aha, I see. I presume I would then overwrite your (excellent) feature then? Would an alternative be to add a hook with the data generated by clojure.test, just prior to the reporter getting it?

plexus07:01:02

you can copy those two lines above to retain that excellent feature šŸ™‚ The alternative would indeed be to manipulate the map that gets passed to clojure.test/report, which you could do by wrapping the reporter as I suggested earlier. It'll take a bit of poking around though, you'll have to change the :expected and :actual keys in that map, but you can't just swap them.

plexus07:01:44

This is the function that deals with printing failing = assertions

plexus07:01:51

(defmethod print-expr '= [m]
  (let [printer (output/printer)]
    ;; :actual is of the form (not (= ...))

    (if (and (not= (:type m) ::one-arg-eql)
             (seq? (second (:actual m)))
             (> (count (second (:actual m))) 2))

      (let [[_ expected & actuals] (-> m :actual second)]
        (output/print-doc
         [:span
          "Expected:" :line
          [:nest (output/format-doc expected printer)]
          :break
          "Actual:" :line
          (into [:nest]
                (interpose :break)
                (for [actual actuals]
                  (output/format-doc ((jit lambdaisland.deep-diff/diff) expected actual)
                                     printer)))]))
      (output/print-doc
       [:span
        "Expected:" :line
        [:nest (output/format-doc (:expected m) printer)]
        :break
        "Actual:" :line
        [:nest (output/format-doc (:actual m) printer)]]))))

magnars07:01:03

Aha, okay, I thought the report was the one writing output to the terminal - so wrapping it would be too late.

plexus07:01:22

Notice this part: (let [[_ expected & actuals] (-> m :actual second)

plexus07:01:00

so you'll have to take the value of :actual, which will look like (not (= ...)), and update it

magnars07:01:28

I see - very good šŸ™‚ I'll give that a shot. Thanks so much for the kickstart!

plexus07:01:20

Good luck! It wouldn't be a bad idea to add a report hook (or pre-report) so people can mess with those messages before they reach the reporter

magnars07:01:59

I could take a look at a PR for that, if you would like.

plexus07:01:13

we could easily do that in kaocha.monkey-patch (see my earlier comment about clojure.test šŸ˜‰ )

plexus07:01:27

that would be great!

magnars07:01:23

I'll give that a shot then šŸ™‚

plexus07:01:22

Adding a m (plugin/run-hook :pre-report m) on line 34 of monkey-patch could be enough

magnars07:01:07

If it's a very easy change for you to do, then that's okay - otherwise I'm happy to do the work and submit a PR šŸ™‚

magnars07:01:35

I see that kaocha is testing itself, that's a cool milestone to reach šŸ˜„

plexus07:01:30

I'd appreciate a PR with a test, in these final weeks of the funding I really hope to encourage people to start contributing, so it'll be good for people to see I'm not the only dev

magnars07:01:41

Agreed. šŸ‘šŸ™‚

plexus07:01:44

Kaocha has been testing itself from the very beginning šŸ’Ŗ

šŸ™Œ 5
plexus07:01:15

that's how I found most of the bugs as well šŸ˜‰

magnars07:01:50

@plexus running just the kaocha unit tests with --watch enabled seems to fail. Running without watch is šŸ‘

plexus07:01:33

oops... let me check

plexus07:01:53

haven't used --watch in a while, I mostly use kaocha.repl

plexus07:01:45

this is the challenging thing with the thing testing itself, sometimes hard to keep configuration and state isolated

magnars07:01:07

aye, I was thinking the same thing. A couple friends of mine made an excellent testing library for JS (Buster.JS) which has much the same architecture as yours (very extensible with plugins and hooks) - and they had the misfortune of testing this huge thing without using their own testing library. šŸ˜›

plexus07:01:29

and a good reminder that we should find a way to minimize the results of deep-diff šŸ™ˆ

magnars07:01:29

because of these issues šŸ™‚

magnars07:01:38

haha, indeed

magnars07:01:11

there might be some work to that end done in flare

magnars07:01:48

no, seems like it doesn't really do deep diffing

plexus07:01:59

seems it's not getting a file/line for the error event... This detecting of file/line is tricky. Not sure what's going on here, I'm just gonna loosen the test a bit for now.

magnars07:01:21

yeah, had a lot of issues with that myself when we were writing Prone

plexus07:01:54

at some point I hope to better study the prior art on this stuff and maybe rewrite that whole bit.

magnars07:01:03

it seems to me that testing the monkey-patch would need a feature test. The hooks_plugin.feature only has one test, testing the hooks infrastructure via the pre-test hook. Do you have any thoughts on where I could add a test for :kaocha.hooks/pre-report ?

plexus07:01:07

hmm good question šŸ™‚

plexus07:01:49

I guess you did pick a tricky thing to test. I'd still add a monkey_patch_test.clj, even though it's tested indirectly.

magnars07:01:25

Okay, so should I mock out t/report and try to make a unit-test that way?

plexus07:01:21

something like this

(defplugin kaocha.monkey-patch-test/test-plugin
  (pre-test [m]
    (assoc m :been-here true)))

(binding [plugin/*current-chain* (plugin/load-all [:kaocha.monkey-patch-test/test-plugin])]
  (with-redefs [clojure.test/report (fn [m] (is (:been-here m)))]
    (clojure.test/do-report {:type :pass})))

plexus07:01:40

actually you can drop some of that boilerplate

plexus07:01:02

(binding [plugin/*current-chain* [{:pre-test (fn [m] (assoc m :been-here true))}]]
  (with-redefs [clojure.test/report (fn [m] (is (:been-here m)))]
    (clojure.test/do-report {:type :pass})))

plexus07:01:08

(untested pseudocode)

magnars07:01:18

nice šŸ‘ŒšŸ™‚

plexus07:01:48

uh not pre-test, pre-report of course

magnars08:01:44

@plexus what are these reductions in kaocha.plugin.hooks/defplugin? I'm unsure how to handle pre-report here. Something like (reduce #(%2 %1) results (:kaocha.hooks/pre-report results)) (or just m instead of results?)

plexus08:01:56

These reduce calls are just there to pass the value through every plugin that implements the given hook

plexus08:01:52

Plugin chain is a vector of maps, mapping from hook name to a function

plexus08:01:44

That function returns a possibly updated version of its first argument

plexus08:01:27

When you call run-hook you give it a hook name, a starting value, and possibly extra arguments. In this case you don't need extra arguments, just m

plexus08:01:36

Does that make sense @magnars?

magnars08:01:32

hmm, yes, possibly šŸ™‚ the general workings of the plugin chain and hooks are clear to me, but I'm a bit confused about where the hooks are to be found. I think I got it right, tho. I'll open a PR, and you can take a look.

plexus10:01:23

yeah I'm sorry I was in a cab, I didn't realize you were talking about the hooks plugin. Hooks are configured in tests.edn so they are on the config (or test-plan). Anyway I commented on the PR.

rickmoynihan15:01:11

Are there any plans to add support for running clojure.spec.test.alpha/check ā€œtestsā€ with reporting etc into kaocha?

plexus06:01:31

it hasn't been explored yet. Feel free to write a ticket with a proposal with how you think this should behave.

rickmoynihan10:01:12

Ok Iā€™ve written up a brief proposal here: https://github.com/lambdaisland/kaocha/issues/45

borkdude12:01:47

@U06HHF230 FWIW, I have a small lib here that provides a little help around testing fdefs: https://cljdoc.org/d/respeced/respeced/0.0.1/doc/readme

borkdude12:01:41

It doesnā€™t do anything intelligent around report except relying on the behavior of clojure.test

rickmoynihan12:01:20

yes Iā€™m using it after you mentioned last week šŸ™‚ I guess that is the other option, is use that with clojure.testā€¦ though I think youā€™re very limited with the error reporting if youā€™re wrapping check with clojure.test/is. If there was a native runner that ran specs independently of clojure.test wrappers youā€™d get better reporting, and I think for less boilerplate.

borkdude12:01:47

sorry, forgot I mentioned it already šŸ™‚

rickmoynihan12:01:25

hehe easily doneā€¦ I did mean to mention it on the ticketā€¦ as I thought your check wrapper did a little more than it actually does.

borkdude12:01:50

I guess such a runner could also be part of a more focused lib like respeced or a plugin for koacha

šŸ‘ 5
borkdude12:01:34

if itā€™s more general than koacha people could use it in their everyday clojure.tests as well

rickmoynihan12:01:50

Iā€™d hope it would ship out of the box with kaocha ā€” as spec is effectively ā€œa coreā€ library (though not literally) like clojure.test

rickmoynihan12:01:55

We may also want a richer st/summarise-results

borkdude13:01:04

for my personal use Iā€™m fine with the behavior of respeced right now. when thereā€™s an issue, if I have enough information to fix it. but then again, I never had these high demands of pretty error reporting šŸ˜‰

borkdude13:01:23

yeah, I havenā€™t even looked at that. I probably should

rickmoynihan13:01:04

As I was saying the other day though - I do wonder if thereā€™s a usecase for a conform testing feature. You can use an fdef for thatā€¦ but if you are specing a datastructure and not a function, you may want to test thatā€¦ via s/valid? and explain-*

borkdude13:01:27

yeah, similar issue

borkdude13:01:19

what I would probably do is use s/explain-data directly in the test and not call s/valid at all

borkdude13:01:35

and then do an assert on that, maybe through a macro

rickmoynihan13:01:13

yeahā€¦ again it becomes hard to get a decent error out though ā€” hence why I think thereā€™s a need for a custom runner. I may just be doing it wrong though. šŸ™‚

borkdude13:01:04

well, in the end youā€™re comparing some seq of maps to what you expect it to be and then itā€™s up to the testing framework to figure out how to print the difference in case it fails

borkdude13:01:01

I havenā€™t thought about this as much as you have

rickmoynihan13:01:18

Iā€™ve not thought about it too much ā€” and could easily have just been doing it wrong but the problem is that explain data is quite detailedā€¦ so you want it prettified somewhat. Itā€™s ok at a repl when you get ex-data pretty printed in cider for youā€¦ but in CI you get a lot of noise printed. Then clojure.test assert wants to print the errors in a particular way; so the spec errors arenā€™t part of the clojure.test error. They end up just being a println or something. In the past Iā€™ve resorted to hacks like (is (= "Success!\n" (s/explain-str ::spec data))) just to get a half decent error out.

rgm17:01:50

much better, and I didnā€™t actually realize tools.deps can take a gist.

rgm17:01:42

(though now that I think about it for a sec given that a gist is a repo under the hood it seems logical)

rgm17:01:25

I think Iā€™d like to clojar it up but might not have the time for a while; even if distribution via gist feels ephemeral at least itā€™s a way to put stuff out for other people as an example.

plexus06:01:31

it hasn't been explored yet. Feel free to write a ticket with a proposal with how you think this should behave.