Implement dedicated clojure.spec/assert stacktrace handling #440
Conversation
vspinu
force-pushed
the
spec-stacktrace
branch
from
f7ee5cc
to
1156092
Compare
| (defn prepare-spec-data [ed pprint-fn] | ||
| (let [pp-str #(with-out-str (pprint-fn %)) | ||
| problems (sort-by #(count (:path %)) (:clojure.spec.alpha/problems ed))] | ||
| ;; (prn {:ed ed |
There was a problem hiding this comment.
Some debug statements leftover here.
|
I restarted the tests because the coverage was failing, but I'm not sure how they're generally green on 1.7 where spec can't even be required. To solve that, in a previous pr, I remember a new test-selector was added: (deftest ^{:min-clj-version "1.9.0-alpha16"} spec-list-integration-test
(let [filter-regex "clojure"
filtered-spec-list (:spec-list (session/message {:op "spec-list"
:filter-regex filter-regex}))]
(testing "Filtered spec list retrieving nothing extra"
(is (every? #(re-find (re-pattern (str ":?" filter-regex)) %)
filtered-spec-list)))
(testing "Filtering with simple words regex"
(is (= (count filtered-spec-list)
(count (:spec-list (session/message {:op "spec-list"
:filter-regex (str filter-regex ".+")}))))))))This looks like another great addition btw! 👍 |
|
One or two tests in a matrix usually fail for unrelated reasons. It's mostly 7th, but could be some other. I guess it's because travis chokes on resources on parallel builds or something like that. You should restart the failing test only.
Spec tests are not run on 1.7. Those are in dedicated "test/spec" directory which is added to tests-src in 1.9 build only. It's harder to do otherwise because one would need to require |
vspinu
force-pushed
the
spec-stacktrace
branch
from
1156092
to
23330e9
Compare
| @@ -187,6 +188,37 @@ | |||
| (when-let [data (ex-data e)] | |||
| (into {} (filter (comp (complement ex-data-blacklist) key) data)))) | |||
|
|
|||
|
|
|||
| (def spec-abbrev (delay | |||
| (if (try (require 'clojure.spec.alpha) true | |||
There was a problem hiding this comment.
Seems to me you should check for the non-alpha version of the ns as well, as it's eventually going to be released.
| (resolve 'clojure.spec.alpha/abbrev) | ||
| #'identity))) | ||
|
|
||
| (defn prepare-spec-data [ed pprint-fn] |
There was a problem hiding this comment.
Guess this could use a docstring.
|
|
||
| (defn prepare-spec-data [ed pprint-fn] | ||
| (let [pp-str #(with-out-str (pprint-fn %)) | ||
| problems (sort-by #(count (:path %)) (:clojure.spec.alpha/problems ed))] |
There was a problem hiding this comment.
I'm a bit worried about the hardcoded alpha namespaces.
There was a problem hiding this comment.
One way to fix this would be to create a correct namespace in try-catch from above and create an alias there. Will think of something.
| :predicate (pr-str (@spec-abbrev pred)) | ||
| :reason reason ; <- always nil or string | ||
| :spec (when-not (empty? via) (pr-str (last via))) | ||
| :at (when-not (empty? path) (pr-str path)) |
There was a problem hiding this comment.
Seems you added an extra space here.
| (pp-str extras)))} | ||
| (filter clojure.core/val) | ||
| (into {})))})) | ||
|
|
||
| (defn analyze-cause | ||
| "Return a map describing the exception cause. If `ex-data` exists, a `:data` |
There was a problem hiding this comment.
Guess you can mention this now does some spec-related stuff as well.
|
Apart from my small remarks - everything looks good to me. |
vspinu
force-pushed
the
spec-stacktrace
branch
from
23330e9
to
b460b84
Compare
Accompanies clojure-emacs/cider#2082.