jeaye/orchestra

Allow to show input arguments when :ret spec fails.

jumarko opened this issue · 4 comments

Often, I'd find useful if orchestra could report input arguments for a function when the :ret spec fails.
That is not just the report the :ret spec failure (a return value) but also the input (which obviously passed the :args spec) from which the return value was computed.

Is this something that you think could be useful for others too?
Is there a reasonable way to have it implemented as an optional/configurable behavior?

jeaye commented

Hey! Thanks for making this issue. I agree that this would be a good thing to have. I'm not too interested in making it optional/configurable though, since nothing else we have is along those lines and I'd only want to add it IFF it's useful for, or at least not detrimental to, everyone.

I do wonder if this would break some functionality in libraries like expound. @bhb, what do you think? Furthermore, are you open to accommodating this additional info, if present, for expound's output?

bhb commented

I’m on vacation without my laptop so I can’t look at this in detail right now, but off the top of my head, I don’t think this would break expound as long as this new data is an expansion of the existing explain-data. If this info was present, I would display it in expound, I think.

The only wrinkle I see is that this could potentially print out a lot of output if the inputs to the function are large data structures. I can’t think of a sensible way to shrink the output but perhaps this isn’t a big deal.

bhb commented

I had forgotten that expound already prints out the function arguments if the :fn spec is not satisfied, e.g.:

-- Spec failed --------------------

Function arguments and return value

  {:ret 1, :args {:x 1, :y 0}}

should satisfy

  (fn
   [%]
   (> (:ret %) (-> % :args :x)))

I haven't received any complaints about this, so I'm inclined to think that it's not a problem in practice to print out the arguments.

So, if orchestra added this information to the explain-data, it should not be a big problem to print it in Expound.

@jeaye Just wanna check if this is something that could be implemented anytime soon.
I'd really love to have this feature.
Maybe I can give you hand but I'm not sure how to do this in a clean way.