[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

Show:
import
January 16, 2018, 2:13 PM

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.

Michael Glaesemann
January 18, 2018, 3:19 AM

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.

gfredericks
January 18, 2018, 12:23 PM

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.

Michael Glaesemann
January 18, 2018, 9:40 PM

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?

gfredericks
January 27, 2018, 2:05 PM

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

Unassigned

Reporter

import

Labels

Approval

Triaged

Patch

None

Affects versions

Priority

Major
Configure