Clojurians Log v2

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

@mfikes the issue is spec returning an exception object?

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

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

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.

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

(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.)

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

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

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.

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

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

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

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

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

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 - 
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$reify__1215 0x2af92bb9 "clojure.spec.alpha$fspec_impl$reify__1215@2af92bb9"], :clojure.spec.test.check/ret {:result true, :num-tests 1000, :seed 1497980057359}, :sym user/ranged-rand})

what’s the change?

at a glance I don’t see anything amiss there

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

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

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

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

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

Ahh. Right.

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

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

True. Seems unlikely to have pulled that off 🙂

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

user=> (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))

(2 7) should fail, right?

yeah

as should (-92 -10)

Yep… many don’t include 0

or (1 3)

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

so the generator seems fine

and next we should suspect the prop checking

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=> (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})

hi I'm back

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

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

@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

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

It might be possible to extend js/Error or somesuch

@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}

^ works in JVM and self-hosted ClojureScript

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

@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"

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

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

Ahh

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

cljs.user=> (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}]

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

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

Captured the above with https://dev.clojure.org/jira/browse/TCHECK-131

@mfikes great, thanks

I'll make sure to get another alpha out shortly

Thanks @gfredericks !