Clojurians
# test-check

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

mfikes 19:49:22

There is an evidently breaking change in the 0.10.0-alpha1 release, somehow related to the introduction of the result protocol (https://github.com/clojure/test.check/commit/95112167af9636a48d3174ddd03409e8ac752179). Was it intentional that this be a breaking change (does it require clients to update?)
Here is a related ticket in the ClojureScript compiler regarding it: https://dev.clojure.org/jira/browse/CLJS-2110

gfredericks 19:52:45

@mfikes the issue is spec returning an exception object?

mfikes 19:53:11

To be honest, I haven’t sorted out why things are regressing.

gfredericks 19:53:53

So spec used a hacky workaround that the result protocol was meant to obviate

mfikes 19:53:58

I only know empirically right now based on bisecting. I’ve read the code to at least attempt to grok why, but haven’t gained comprehension yet.

gfredericks 19:54:36

So ideally spec should change to the better version, but I don't think I meant it to break. Though I might have

mfikes 19:55:06

(I also don’t know if this affects Clojure—my experience with it is with JVM and self-hosted ClojureScript, where it breaks in the same way for both.)

gfredericks 19:55:08

I don't have a sense for what parts are changing at what speed :confused:

gfredericks 19:55:49

And whether it would be bad for spec to require tc alpha

mfikes 19:55:49

Cool. If there was no clear intent to be a breaking change, I’ll dig further to see if can suss out what is going on.

gfredericks 19:57:20

I expect making it nonbreaking would consist of having t.c continue special-casing return values that are Exceptions

mfikes 19:59:56

That helps… I’ll dig deeper—t.c. 0.10.0 is still alpha, after all.

alexmiller 20:05:34

I would say it’s possible that spec.alpha could depend on an alpha

alexmiller 20:05:46

but that regardless of that, t.c shouldn’t introduce a breaking change

alexmiller 20:06:09

and if the semantics need to change, it should have a new name

mfikes 20:36:22

FWIW, t.c 0.10.0-alpha1 has a breaking change for Clojure 1.9.0-alpha17 for this case as well:
``` $ lein repl nREPL server started on port 55971 on host 127.0.0.1 - nrepl://127.0.0.1:55971
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.9.0-alpha17
Java HotSpot(TM) 64-Bit Server VM 1.8.0_131-b11
Docs: (doc function-name-here) (find-doc "part-of-name-here") Source: (source function-name-here) Javadoc: (javadoc java-object-or-class-here) Exit: Control+D or (exit) or (quit) Results: Stored in vars 1, 2, 3, an exception in e

user=> (require '[clojure.spec.alpha :as s]
#_=> '[clojure.spec.test.alpha :as st]) nil
user=> (defn ranged-rand [start end] 0)

'user/ranged-rand

user=> (s/fdef ranged-rand
#=> :args (s/and (s/cat :start int? :end int?) #=> #(< (:start %) (:end %))) #=> :ret int? #=> :fn (s/and #(>= (:ret %) (-> % :args :start)) #_=> #(< (:ret %) (-> % :args :end)))) user/ranged-rand
user=> (st/check `ranged-rand)
({:spec #object[clojure.spec.alpha$fspec_impl$reify1215 0x2af92bb9 "clojure.spec.alpha$fspec_impl$reify1215@2af92bb9"], :clojure.spec.test.check/ret {:result true, :num-tests 1000, :seed 1497980057359}, :sym user/ranged-rand}) ```

alexmiller 20:38:41

what’s the change?

alexmiller 20:39:22

at a glance I don’t see anything amiss there

mfikes 20:40:11

Right, it might not be a breaking change, but an internal bug. Here is what it used to do…

mfikes 20:41:10

Here is a gist… it is too big to really paste: https://gist.github.com/mfikes/1caa83751019495cfdda83e79ad7e3ed

mfikes 20:43:17

Internal bug because: It doesn’t return a falsey :result
Breaking change because the :result used to be much richer than just a Boolean value, and clients might not know to look for :result-data

alexmiller 20:44:14

it just looks to me like it found a failing example in the old one but didn’t in the new one

alexmiller 20:44:37

which might just be due to what the random seed turned up, not a real difference

mfikes 20:44:47

Ahh. Right.

mfikes 20:45:03

I’ll increase the number of tests to see what occurs.

alexmiller 20:46:47

I guess maybe it’s surprising that t.c found 1000 successful cases with a hard-coded result in the new one

mfikes 20:47:32

True. Seems unlikely to have pulled that off :slightly_smiling_face:

alexmiller 20:48:26

so maybe just looking at (gen/sample (s/gen (:args (s/get-spec user/ranged-rand))) 1000) might be interesting as a before / after

mfikes 20:50:17

user=&gt; (gen/sample (s/gen (:args (s/get-spec 'user/ranged-rand))) 10) ((0 1) (-1 0) (-1 2) (0 1) (-23 2) (1 3) (-16 -6) (-92 -10) (-1 2) (2 7))

mfikes 20:50:38

(2 7) should fail, right?

alexmiller 20:51:17

as should (-92 -10)

mfikes 20:51:30

Yep… many don’t include 0

alexmiller 20:51:32

or (1 3)

alexmiller 20:51:44

or (-1 0) - which is the failing example in the old one

alexmiller 20:52:04

so the generator seems fine

alexmiller 20:52:14

and next we should suspect the prop checking

mfikes 20:59:41

I revised my ranged-rand to prn its args, and also revised the :fn part of the fdef to print its argument, and at least those parts look correct
user=&gt; (st/check `ranged-rand {:clojure.spec.test.check/opts {:num-tests 2}}) -1 1 {:args {:start -1, :end 1}, :ret 0} -4 -2 {:args {:start -4, :end -2}, :ret 0} ({:spec #object[clojure.spec.alpha$fspec_impl$reify__1215 0x77b18e9a "clojure.spec.alpha$fspec_impl$reify__1215@77b18e9a"], :clojure.spec.test.check/ret {:result true, :num-tests 2, :seed 1497981519034}, :sym user/ranged-rand})

gfredericks 21:08:14

hi I'm back

mfikes 21:08:37

TL;DR of the above is just reproing it in Clojure

mfikes 21:10:30

I suppose there is the twist that there might be an internal bug in t.c (disregarding any potential interface changes)

gfredericks 21:11:01

@mfikes regarding the first issue, it definitely looks like I didn't retain the old behavior; an extra clause here should fix it: https://github.com/clojure/test.check/blob/23e1fcc1bb7f3f2d0202ed99eee202a0b1f2c652/src/main/clojure/clojure/test/check/results.cljc#L18

gfredericks 21:11:24

I'm assuming cljs has something like Throwable for extending error types, but maybe that's not true?

mfikes 21:11:48

It might be possible to extend js/Error or somesuch

mfikes 21:19:52

@gfredericks Evidently you can. Not sure what the implications might be. ``` cljs.user=> (defprotocol Result
#=> (passing? [result]) #=> (result-data [result] "A map of data about the trial.")) nil
cljs.user=> (extend-protocol Result
#=> js/Error #=> (passing? [this] false) #_=> (result-data [this] (ex-data this)))

object[Function "function (this$){

var this$$1 = this;
return cljs.core.ex_data.call(null,this$
$1);
}"] cljs.user=> (result-data (ex-info "hi" {:a 1}))
{:a 1} ```

mfikes 21:20:10

^ works in JVM and self-hosted ClojureScript

gfredericks 21:20:16

@mfikes does it work for (throw "some string")?

mfikes 21:22:54

@gfredericks If you extract using ex-message: ``` cljs.user=> (extend-protocol Result
#=> js/Error #=> (passing? [this] false) #_=> (result-data [this] (ex-message this)))

object[Function "function (this$){

var this$$1 = this;
return cljs.core.ex_message.call(null,this$
$1);
}"] cljs.user=> (result-data (js/Error. "some string"))
"some string" ```

mfikes 21:24:43

I’ll see what happens if I build a t.c that further extends Result to exceptions

gfredericks 21:30:28

I was just concerned that if you throw a string, the thrown thing is not a js/Error and so wouldn't match that clause

mfikes 21:43:48

At least for the exception thrown for the case above, further extending does the trick

mfikes 21:44:40

cljs.user=&gt; (st/check `ranged-rand) [{:spec #object[cljs.spec.alpha.t_cljs$spec$alpha5600], :clojure.test.check/ret {:result false, :result-data {:a 1}, :seed 1497984200982, :failing-size 0, :num-tests 1, :fail [(-1 0)], :shrunk {:total-nodes-visited 0, :depth 0, :result false, :result-data {:a 1}, :smallest [(-1 0)]}}, :sym cljs.user/ranged-rand, :failure false}]

With this kind of extension:
(passing? [this] false) - (result-data [this] {})) + (result-data [this] {}) + + #?(:clj Throwable :cljs js/Error) + (passing? [this] false) + (result-data [this] {:a 1}))

mfikes 21:48:06

Perhaps :failure false is not what you’d want. Hrm.