Sort printed maps in test output

[sashton]

Fixes #916

This PR performs a clojure.walk to covert all maps to sorted-maps when printing an object in the test results. See #916 for example before and after screenshots of the diffed data.

---

Before submitting a PR make sure the following things have been done:

• The commits are consistent with our contribution guidelines
• You've added tests to cover your change(s)
• All tests are passing
• You've updated the README
• Middleware documentation is up to date
  • Please check out and modify the cider.nrepl ns which has all middleware documentation.
  • Run lein docs afterwards, and commit the results.

Note: If you're just starting out to hack on cider-nrepl you might find
nREPL's documentation and the
"Design" section of the README extremely useful.*

Thanks!

[bbatsov]

Is there a chance for this to throw an exception?

[sashton]

No, I suppose not. I had that in there when I was first testing it out locally and never removed it. But I don't see it being necessary. I'll remove it.

[frenchy64]

In general, this will throw if the keys of any nested map are not comparable.

user=> (deep-sorted-maps {{} 1})
Execution error (ClassCastException) at user/deep-sorted-maps$fn (REPL:7).
Default comparator requires nil, Number, or Comparable: {}
user=> (deep-sorted-maps {1 1 :a 1})
Execution error (ClassCastException) at user/deep-sorted-maps$fn (REPL:7).
class java.lang.Long cannot be cast to class clojure.lang.Keyword (java.lang.Long is in module java.base of loader 'bootstrap'; clojure.lang.Keyword is in unnamed module of loader 'app')
user=> (deep-sorted-maps {"a" 1 :a 1})
Execution error (ClassCastException) at user/deep-sorted-maps$fn (REPL:7).
class java.lang.String cannot be cast to class clojure.lang.Keyword (java.lang.String is in module java.base of loader 'bootstrap'; clojure.lang.Keyword is in unnamed module of loader 'app')

[sashton]

@frenchy64 That's a great catch! (no pun intended) I'll add a comment explaining the rationale for the try/catch here.

[bbatsov]

I'd probably add a note here why we're sorting maps.

[bbatsov]

Won't it be better to check around the call site if something's map?

[sashton]

Which call site do you mean? If you are referring to the call in print-object, we want to do a postwalk traversal to sort nested maps as well as top-level maps.

[sashton]

Here is an example nested map in the test:
https://github.com/clojure-emacs/cider-nrepl/pull/917/files#diff-d6825defeb66863e777ca37cd35612c08feadc23a12cab7d9b7d7791f00d407dR285

[sashton]

Alright, I'm seeing some unexpectedly deterministic (thought not sorted) results when using a released version of CIDER. Which, if true, would mean this PR is not needed. So I'm going to close this for the moment while I check my setup and make sure I know what is going on.

[sashton]

I determined that the deterministic behavior appears to be a factor of map size, and is probably some internal implementation of pprint. But that behavior does not help with printing test results, so this PR is still needed.

Details on the linked issue: #916 (comment)

[sashton]

@bbatsov I'm not convinced that the test failure is related to my change, since the tests passed in a6dd36b when I first had the try/catch there. From the test output, it looks like it was an nrepl timeout. Does that happen intermittently with the tests?