[spec] spec.test/check returns the wrong value of :failure for failing tests
Description
When trying to manually wire clojure.test and spec for fdef'ed functions, I realized that spec.test/check returns a map with [:failure false] for failing tests. I'm (almost) sure it should return true, because spec.test/abbrev-result receives as argument the return of spec.test/check and tests the value of :failure. I couldn't produce a case where spec.test/check returned [:failure true] for failing tests. For information it returns a map without the key :failure for passing test.
Here's a simple reproduction case in the REPL :
(defn foo-fn [x] "bar")
(s/fdef foo-fn
:args (s/cat :x any?)
:ret string?)
(stest/check `foo-fn) ;; => no error, normal small map printed, no :failure entry
(defn foo-fn [x] 1)
(stest/check `foo-fn) ;; => full error map, with :failure equals false. :failure shown below
(-> (stest/check `foo-fn) first :failure) ;; => false
Environment
[org.clojure/clojure "1.9.0-beta1"]
[org.clojure/spec.alpha "0.1.123"]
[org.clojure/test.check "0.10.0-alpha2" :scope "test"]
Activity
Comment made by: djebbz
Thanks a lot Michael. In the end I went with a small variation, where I don't `(dissoc x ::stc/ret)` but keep the whole check result because it includes the stack trace, the spec/explain-data map, the shrunk failing case etc.
Looks like abbrev-result should use result-type to test whether the check passed. Attaching a patch which does this. If you'd like tests to accompany it, I'm happy to resubmit.
Edit: Nope: that was too hasty. Will investigate and resubmit. Sorry for the noise.
Edit 2: Second guessing myself incorrectly. I'm pretty sure I was correct the first time. I think the patch is good.
note that there are unreleased changes on the master branch of test.check that may influence the best thing to do here. this stuff in particular.
Thanks, . The Result protocol is something to keep in mind. I think using result-type is the right abstraction at the level of abbrev-result. I would think Result/passing? would be used when decorating the quick-check results in make-check-result in lieu of the (true? result) call. Does that make sense to you?
Yes I think so. But now that I've reviewed the spec code and see how many incidental details about the return value are being relied on (which is probably true for other test.check users, and shouldn't surprise me), I think I will be saving the Result protocol details for a quick-check-2 ([https://dev.clojure.org/jira/browse/TCHECK-142|TCHECK-142]).
Assignee
Reporter
Labels
Approval
Patch
Affects versions
Priority
