Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:via is empty when instrumentation fails on :ret spec #7

Closed
bhb opened this issue Aug 10, 2017 · 6 comments
Closed

:via is empty when instrumentation fails on :ret spec #7

bhb opened this issue Aug 10, 2017 · 6 comments

Comments

@bhb
Copy link

bhb commented Aug 10, 2017

Using [orchestra "2017.07.04-1"]

My environment is a lein REPL: lein repl.

(require '[orchestra.spec.test :as st1])

(s/def :example/age pos-int?)
(s/fdef foobar
        :args (s/cat :x :example/age)
        :ret :example/age)
(defn foobar [x] -10)
(st1/instrument `foobar)
;; Try an instrumentation failure for `:args`
(foobar -1)
;; Prints failure message including this `explain-data`:
;; {:clojure.spec.alpha/problems [{:path [:args :x], :pred clojure.core/pos-int?, :val -1, :via [:example/age :example/age], :in [0]}], :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0xac00133 "clojure.spec.alpha$regex_spec_impl$reify__1200@ac00133"], :clojure.spec.alpha/value (-1), :clojure.spec.alpha/args (-1), :clojure.spec.alpha/failure :instrument, :orchestra.spec.test/caller {:file "form-init5213886586620856240.clj", :line 256, :var-scope expound.alpha/eval36771}}
;; Note the 

;; Try an instrumentation failure for `:ret`
(foobar 10)
;; Prints failure message including this `explain-data`:
;; {:clojure.spec.alpha/problems [{:path [:ret], :pred clojure.core/pos-int?, :val -10, :via [], :in []}], :clojure.spec.alpha/spec #object[clojure.spec.alpha$spec_impl$reify__751 0x2c9a79fd "clojure.spec.alpha$spec_impl$reify__751@2c9a79fd"], :clojure.spec.alpha/value -10, :clojure.spec.alpha/ret -10, :clojure.spec.alpha/failure :instrument, :orchestra.spec.test/caller {:file "form-init5213886586620856240.clj", :line 259, :var-scope expound.alpha/eval36773}}

Expected: The :via value should be populated (namely, it should include [:example/age :example/age].

@jeaye
Copy link
Owner

jeaye commented Aug 10, 2017

Hm, I'm not sure what :via actually represents, but I think it's certainly an inconsistency between :args and the :ret and :fn which orchestra add; good find!

I'll get this fixed.

@jeaye
Copy link
Owner

jeaye commented Aug 10, 2017

When I look at the instrumentation source, there's no trace of :via, so this must be coming from higher up in clojure.spec, meaning outside the instrumentation code.

This is only after a cursor search, and I'm not yet certain of the meaning of :via, but it looks like either this needs to be addressed in clojure.spec or it needs to be subsequently accounted for in orchestra.

@bhb
Copy link
Author

bhb commented Aug 10, 2017

AIUI, :via is the set of specs that were followed to get to the failing predicate.

For example, note the following three spec failures. All three have the same predicate since 123 is not a string, but they arrive at that predicate via different specs:

(require '[clojure.spec.alpha :as s])

(s/def :example/name string?)
(s/def :example/person (s/keys :req-un [:example/name]))

(-> (s/explain-data string? 123) ::s/problems first (select-keys [:via :pred]))
;; {:via [], :pred :clojure.spec.alpha/unknown}
;; I'm actually not 100% sure why the above is unknown

(-> (s/explain-data :example/name 123) ::s/problems first (select-keys [:via :pred]))
;; {:via [:example/name], :pred clojure.core/string?}

(-> (s/explain-data :example/person {:name 123}) ::s/problems first (select-keys [:via :pred]))
;; {:via [:example/person :example/name], :pred clojure.core/string?}

@jeaye
Copy link
Owner

jeaye commented Aug 10, 2017

Thanks a great explanation of :via, thank you. I've found the cause of the empty :via in orchestra to be here: https://github.com/jeaye/orchestra/blob/master/src/clj/orchestra/spec/test.clj#L102 Looks like s/explain-data* is expecting [spec path via in x].

I'll work to get a fix for this in the next few days.

@jeaye jeaye closed this as completed in b6899ec Aug 13, 2017
@jeaye
Copy link
Owner

jeaye commented Aug 13, 2017

I've pushed a fix for this and I've tested this new orchestra version with your expound snapshot as well. The only thing missing is the "relevant specs" section on :ret failures, which should now be possible.

Please let me know if you have any more troubles with this. Thanks for the help!

@bhb
Copy link
Author

bhb commented Aug 15, 2017

@jeaye It works great! Thank for the quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants