Cider: Better output of matcher-combinators on tests

[ericdallo]

Is your feature request related to a problem? Please describe.
Currently running tests using the matcher-combinators library via terminal, it pretty prints the mismatch very well:

But when I run the same test on cider, this is the output:

Describe the solution you'd like
Improve the output of cider to behave similarly to the terminal, if possible with some color.
I checked that running the test via cursive works like the terminal.

Describe alternatives you've considered
I checked the matcher-combinators code here but it doesn't seems wrong, let me know if this should be fixed there or here in cider.

Additional context
I checked this but found nothing that could help.

this is the sample test used.

[bbatsov]

I guess this will require some changes to the clojure.test functionality that lives in cider-nrepl. I'm not familiar with the library in question, so it's hard to say how easy it'd be to do those.

[ericdallo]

I see @bbatsov, if you need any help let me know, I'm familiar with elisp, maybe I just need some points to cider code to what to change

[ericdallo]

@bbatsov could you please be more specific where we should try to improve this behavior in cider-nrepl?
This is a library that @nubank uses for every clojure service, so it'd be nice if we manage to improve that somehow :)

[bbatsov]

That's the test runner middleware https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/test.clj#L19 that builds the report you see in CIDER.

[FelipeCortez]

The problem seems to be matcher-combinators has its own printing logic:
https://github.com/nubank/matcher-combinators/blob/master/src/cljc/matcher_combinators/printer.cljc
https://github.com/nubank/matcher-combinators/blob/master/src/clj/matcher_combinators/clj_test.clj#L155.

cider-nrepl always calls pprint on actual, which ignores the print-method and outputs a map: https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/test.clj#L75

Here's a similar scenario:

(defmethod clojure.core/print-method ::pretty [{:keys [msg]} out]
  (binding [*out* out]
    (.write out (str "◕ᴥ◕: " msg))))

(with-out-str
  (println (with-meta {:msg "hello there"} {:type ::pretty})))
;; => "◕ᴥ◕: hello there\n"

(with-out-str
 (clojure.pprint/pprint (with-meta {:msg "hello there"} {:type ::pretty})))
;; => "{:msg \"hello there\"}\n" 

a possible workaround is doing

pprint-str #(with-out-str (if (not= (get-method print-method (-> % meta :type))
                                    (get-method print-method :default))
                            (pp/pprint %)
                            (print %)))

but maybe I'm missing something. I saw there's a defmethod report :matcher-combinators/mismatch in cider-nrepl but I have no idea how it's called.

[ericdallo]

Cool! :clap:
@bbatsov this seems to fix the print but not the coloring yet, do you know what we can do to fix the coloring too?

[FelipeCortez]

Found out why report ::mismatch is not being called: https://github.com/nubank/matcher-combinators/commit/7635d040fb81ad44b3587644d75bba8961269dc7

[FelipeCortez]

This has been fixed in part by https://github.com/clojure-emacs/cider-nrepl/pull/683

matcher-combinators uses ANSI escape sequences not caught by cider-ansi-color-string-p, and there are tests enforcing this behavior: https://github.com/clojure-emacs/cider/blob/master/test/cider-util-tests.el#L299

Given the tests, I don't know if we should change cider-ansi-color-string-p. In any case, I "fixed" it locally by adding to this to my init.el

(use-package cider
  :ensure t
  :config
  (cider-add-to-alist 'cider-jack-in-lein-plugins "cider/cider-nrepl" "0.25.5")
  (advice-add 'cider-ansi-color-string-p :override
              (lambda (string) (string-match "\\[" string)))
  (advice-add 'cider-font-lock-as
              :before
              (lambda (&rest r)
                (advice-add 'substring-no-properties :override #'identity)))
  (advice-add 'cider-font-lock-as
              :after
              (lambda (&rest r)
                (advice-remove 'substring-no-properties #'identity))))

[ericdallo]

Thanks @FelipeCortez, @bbatsov do you see any way where we can fix that on cider to avoid the need of adding this code :point_up: ?

[bbatsov]

(cider-add-to-alist 'cider-jack-in-lein-plugins "cider/cider-nrepl" "0.25.5")

That's no longer needed on master.

Thanks @FelipeCortez, @bbatsov do you see any way where we can fix that on cider to avoid the need of adding this code ☝️ ?

As I said before I guess the ansi detection code needs to be tweaked or made more flexible, but I still don't get why this test library outputs non-standard ansi sequences.