borkdude / speculative

Unofficial community-driven specs for clojure.core
Eclipse Public License 1.0
187 stars 16 forks source link

create and run tests for clojure.spec.alpha-2 #124

Open slipset opened 5 years ago

slipset commented 5 years ago

clojure.core has started work on https://github.com/clojure/spec-alpha2 It would be nice to run our tests against this version as well.

borkdude commented 5 years ago

Our tests are running in this branch, except those that use test/check, this seems to be broken in spec-alpha2.

Repro:

$ clj -Srepro -Sdeps '{:deps {clojure/spec-alpha2 {:git/url "https://github.com/clojure/spec-alpha2" :sha "639767ebe1491536ec7404d8b478628ecfe9200b"}}}'
Clojure 1.9.0
user=> (require '[clojure.spec-alpha2.test :as stest])
nil
user=>  (require '[clojure.spec-alpha2 :as s])
nil
user=>  (defn foo [x] true)
#'user/foo
user=>  (s/fdef foo :args (s/cat :x int?))
user/foo
user=> (stest/check `foo)
IllegalArgumentException Symbol must be namespace-qualified  clojure.lang.Var.find (Var.java:140)
(user=> (stest/check 'user/foo)
IllegalArgumentException Symbol must be namespace-qualified  clojure.lang.Var.find (Var.java:140)

cc @puredanger

borkdude commented 5 years ago

@puredanger With update to SHA 50ff9ba32cca0ddc87df2d01d4ff424a747c6270:

#?(:clj (s/def ::java-map
          (s/with-gen #(instance? java.util.Map %)
            (fn [] (gen/fmap #(java.util.HashMap. %)
                             (s/gen ::map))))))

no longer works, it now gives:

Caused by: java.lang.IllegalArgumentException: no conversion to symbol
    at clojure.core$symbol.invokeStatic(core.clj:596)
    at clojure.core$symbol.invoke(core.clj:589)
    at clojure.spec_alpha2$explicate_1$fn__904.invoke(spec_alpha2.clj:314)

This spec broke:

(defn seqable-of
  "every is not designed to deal with seqable?, this is a way around it"
  [spec]
  (s/with-gen (s/and seqable?
                     (s/or :empty empty?
                           :seq (s/and (s/conformer seq)
                                       (s/every spec))))
    #(s/gen (s/nilable (s/every spec :kind coll?)))))

(s/def ::seqable-of-map-entry (seqable-of ::map-entry))

with:

Caused by: java.lang.IllegalArgumentException: No method in multimethod 'create-spec' for dispatch value: speculative.specs/seqable-of

I tried something here but didn't get very far: https://github.com/borkdude/speculative/blob/1bf0b3d0ba4faf8d6a741e34e3bb5b3710247341/src/speculative/specs.cljc#L87

borkdude commented 5 years ago

With SHA a5ed7684fd87e75f5c2f99e186c7163b6bf48696 I get:

Caused by: java.lang.IllegalArgumentException: No method in multimethod 'create-spec' for dispatch value: clojure.spec-alpha2/spec*

on

(defn seqable-of
  "every is not designed to deal with seqable?, this is a way around it"
  [spec]
  (s/with-gen (s/and seqable?
                     (s/or :empty empty?
                           :seq (s/and (s/conformer seq)
                                       (s/every spec))))
    #(s/gen (s/nilable (s/every spec :kind coll?)))))

(s/def ::seqable-of-map-entry (seqable-of ::map-entry))

@puredanger Any clues how to port this spec?

borkdude commented 5 years ago

With SHA a5ed7684fd87e75f5c2f99e186c7163b6bf48696 I get:

Caused by: clojure.lang.ArityException: Wrong number of args (3) passed to: clojure.spec-alpha2.impl/or-spec-impl

on

(s/def ::reducible-coll
  (s/with-gen
    (s/or
     :seqable    ::seqable
     :reducible  (s/nilable ::reducible)
     :iterable   (s/nilable ::iterable))
    #(s/gen ::seqable)))

cc @puredanger

puredanger commented 5 years ago

few updates on latest master:

seqable-of is interesting, still looking at where we're going with that

puredanger commented 5 years ago

I think for something like seqable-of, you are making specs from specs and really in the realm of making your own custom spec op now. As in https://github.com/clojure/spec-alpha2/wiki/Differences-from-spec.alpha#implementing-custom-specs - implementing the op macro and the create-spec defmethod that reifies Spec.

borkdude commented 5 years ago

@puredanger

SHA: 0d98d147a12cc63ce0179d3cb525cd287a51a2b3

This also seems to work:

(defn seqable-of
  [spec]
  (s/spec*
   `(s/with-gen (s/and seqable?
                       (s/or :empty empty?
                             :seq (s/and (s/conformer seq)
                                         (s/every ~spec))))
      #(s/gen (s/nilable (s/every ~spec :kind coll?))))))
$ clj -A:test
Clojure 1.10.0
user=> (require '[clojure.spec-alpha2 :as s])
nil
user=> (require '[speculative.specs :as ss])
nil
user=> (s/valid? ::ss/seqable-of-map-entry {:a 1})
true

Some specs that don't work:

(s/def :atom/validator ifn?)
(s/def :atom/meta map?)
(s/fdef clojure.core/atom
  :args (s/cat :x any? :options (s/keys* :opt-un [:atom/validator :atom/meta]))
  :ret #(instance? clojure.lang.IAtom %))

Syntax error (IllegalArgumentException) compiling at (core.cljc:220:1).
No method in multimethod 'create-spec' for dispatch value: clojure.spec-alpha2/spec*
(s/fdef clojure.core/map
  :args (s/alt :transducer (s/cat :f ifn?)
               :seqable (s/cat :f ifn? :colls (s/+ seqable?)))
  :ret (s/or :transducer ifn? :seqable? ifn?)
  :fn (fn [{:keys [args ret]}]
        (= (key args) (key ret))))

Syntax error (IllegalArgumentException) compiling fn* at (core.cljc:271:1).
fn params must be Symbols
puredanger commented 5 years ago

Both fixed in latest sha. Bugs!

borkdude commented 5 years ago

@puredanger Updated to SHA a3474645b88c0467cad90abe8298c6b1e4a2e636

The map spec now seems to work.

For the atom spec I now get:

$ clj -A:test
Clojure 1.10.0
user=> (require '[clojure.spec-alpha2 :as s])
nil
(s/def :atom/validator ifn?)
:atom/validator
user=> (s/def :atom/meta map?)
:atom/meta
(s/fdef clojure.core/atom
  :args (s/cat :x any? :options (s/keys* :opt-un [:atom/validator :atom/meta]))
  :ret #(instance? clojure.lang.IAtom %))
Syntax error compiling at (REPL:1:1).
No such namespace: gen

TODO:

puredanger commented 5 years ago

Sorry! I made another change after I fixed and broke it in a different way. Fixed that in latest sha.

puredanger commented 5 years ago

Well, maybe not.

puredanger commented 5 years ago

ok, try again with latest. I reverted my last changes around keys*.

borkdude commented 5 years ago

@puredanger Seems to work now, except that some error messages have a missing bit that is present in spec1 in this example:

spec1:

user=> (dissoc 1)
Execution error - invalid arguments to clojure.core/dissoc at (REPL:1).
1 - failed: map? at: [:map :clojure.spec.alpha/pred] spec: :speculative.specs/map
1 - failed: nil? at: [:map :clojure.spec.alpha/nil]

spec2:

user=> (dissoc 1)
Execution error - invalid arguments to clojure.core/dissoc at (test.clj:129).
1 - failed: map? at: [:map :clojure.spec-alpha2/pred]
1 - failed: nil? at: [:map :clojure.spec-alpha2/nil]

Not sure if that's important, but there may be tools like expound which rely on this.

borkdude commented 5 years ago

@puredanger Next issue:

user=> (require '[clojure.spec-alpha2 :as s])
nil
user=> (require '[clojure.spec-alpha2.gen :as gen])
nil
user=> (gen/sample (s/gen string?))
Execution error (IllegalArgumentException) at clojure.spec-alpha2.protocols/eval189$fn$G (protocols.clj:11).
No implementation of method: :gen* of protocol: #'clojure.spec-alpha2.protocols/Spec found for class: clojure.core$string_QMARK___5395

This however works:

user=> (gen/sample (gen/gen-for-pred string?))
("" "" "6" "7" "qH" "" "g8F" "7Tz332" "X8RB1c" "oZ7jW")

This also works:

user=> (gen/sample (s/gen (s/spec string?)))
("" "I" "" "4Cv" "" "Cs" "" "864w" "4" "w8TIh7")

Solution: this is expected. Use s/spec around a built-in predicate in s/gen.

borkdude commented 5 years ago

@puredanger New bug:

speculative.specs=> (s/def ::or-spec (s/with-gen (s/or :vector vector?) #(s/gen (s/spec vector?))))
:speculative.specs/or-spec
speculative.specs=> (s/valid? ::or-spec [1 2 3])
false

This seems to be specific for the combination of s/with-gen and s/or.

puredanger commented 5 years ago

I assume that should be vector? in the s/or, not vector?

borkdude commented 5 years ago

Still doesn't work with vector?.

borkdude commented 5 years ago

@puredanger Possibly another bug:

(s/def ::sequential-of-non-sequential
  (s/every (complement sequential?) :kind sequential?))

(s/valid? ::sequential-of-non-sequential [])
Execution error (IllegalArgumentException) at clojure.spec-alpha2/spec* (spec_alpha2.clj:197).
No method in multimethod 'create-spec' for dispatch value: clojure.core/complement
puredanger commented 5 years ago

complement is not a symbolic spec op so you’ll need #(not (sequential? %)) or something like that. Similar thing from Sean re comp. maybe something we’ll revisit later.

borkdude commented 5 years ago

@puredanger

This works in spec1 and 2: (gen/sample (s/gen (s/every number? :kind vector?))) This works in spec1 but doesn’t work in spec2. Should it? (gen/sample (s/gen (s/every number? :kind coll?)))

borkdude commented 5 years ago

Apart from these issues, the speculative tests pass with spec-alpha2!

TODO:

borkdude commented 5 years ago

NOTE: It just occurred to me that libs can maintain compatibility with spec1 and spec2 by doing this: (:require [clojure.spec.alpha :as s1] [clojure.spec-alpha2 :as s2]) and then define specs for each version of spec: (s1/def :foo number?) (s2/def :foo number?) (probably using a helper macro when spec forms are already compatible with both).

puredanger commented 5 years ago
borkdude commented 5 years ago

@puredanger Yeah, the every example was bad, but glad you were able to fix it. As of now there are no open issues. 🎉 Will keep testing future commits.

borkdude commented 5 years ago

@puredanger I started playing with s/defop after seeing Sean's example in your latest blog. This works:

(s/defop spec2-seqable-of
  [spec]
  :gen #(s/gen (s/nilable (s/every spec :kind coll?)))
  (s/and seqable?
         (s/or :empty empty?
               :seq (s/and (s/conformer seq)
                           (s/every spec)))))
#'user/spec2-seqable-of
user=> (s/def ::seqable-of-maps (spec2-seqable-of ::map))
:user/seqable-of-maps
user=> (s/valid? ::seqable-of-maps ["foo"])
false
user=> (s/valid? ::seqable-of-maps [{:a 1}])
true
user=> (require '[clojure.spec-alpha2.gen :as gen])
nil
user=> (gen/sample (s/gen ::seqable-of-maps))
(#{{}} ({} {-1 -1.0} {} {-1 U*/tV} {} {}) #{{1.0 -1/3} {true 1, :h? x/pf} {0 -2} {0 1, -3/2 :*E/v} {} {true \I, ?? :c, :?/-* ??} {#uuid "8d01f9a8-6dab-42e3-b8c4-5a76e14141ba" \-} {1 2, d/H -3/2, \< ""} {\z #uuid "e7b7fa13-10b8-415f-b559-66bec2508763"} {-1 \M, \r _/xh, :hx/t -3/4} {-1 0.5, -3 -3} {1.0 m/c8, "Jz" -2} {:?/R. _6/?v, Y true, :j #uuid "b68f2e13-1756-40db-b101-350d674dd748"} {+ -0.6875, -3 :*E/Ct}} #{{:?/y? -2, :. "~"} {\6 0.5} {#uuid "74a68951-7051-4a3b-b61a-3b12ec3e8cb0" \L, true "M!#", \r \9}} ({} {"AD<" -3/2, 1.875 :fd/MP} {h2 -1.0} {1 0, -5 -1.0} {I/RX "uuSr", true -3} {"b]" ?A/e, true -3.75, -3 #uuid "f5fa7648-cd01-4f93-9463-2864c944f10f", V- "8l'1\\u"} {-5 :+/!V, "_\"" 4/3, \u -2/5, #uuid "b7c783e0-2544-45a0-9de7-051ba4e0de07" "}+", +v/. .} {:/ -2, :m+/U8? 0, 5/3 -2/3, l/n 2.0, "" false} {4 :?_, g.! #uuid "ea557c10-50aa-4294-92c2-f45dcbdbe5b1"} {i1+/O+! true, true false, 1/2 3, false true} {} {:rr 5, *A+/qV! C, 3.5 Mg, -0.71875 1.9375} {true #uuid "90ca2f6c-70b5-4f00-bd9b-b927612928b1", 0 3/4, WB/p._ :R*?, :!R -4} {} {*b./bH mv/_, :Iy? 1, 1 -3.0, "j<R(K" :Y/cV} {\7 0, #uuid "97c715b7-83fa-4e49-9382-e4c9a503beef" -5/4, -2.375 j!./+} {#uuid "b44068c2-d811-4895-92a1-7325655c6adc" 1, 0 -1, "" false} {false :Ok, U2S 4}) #{{:x/- "'!Vs]", \K -4/5, 0 "t|W", \i false} {-1 true, -4 :u3T, T+./y5V \8, -2 \|, \! false} {0 3, Y/?h -5, \& :ru/Z, -5 -1} {true !/+, \_ \H} {} #:Ob{p #uuid "796ea6e7-d58b-483b-8df1-9ce6d9665c4c"} {:s/E?u 1/4, \Z #uuid "63f2b5da-5379-4448-a2a7-92d5a335c55f", s false, -2 G/_, \E #uuid "24b3e398-311f-487d-b7cf-1a4aeb25eba3"} {:. \H, false "", 1/2 :-gO, 3 -3} {3 :h, :u :H/ou, #uuid "83c14998-1d59-4f72-8d97-6cfbafc62f70" "WHAi", J #uuid "1df28979-32c0-411c-8272-c323bb28515c"} {"Pd" false} {#uuid "a3081adf-12df-4366-81ca-c9967f95c2ac" _/q+, #uuid "cac544d2-cab6-4802-b24e-35d5f8b9115e" 4, :c27 V/A3, -2.75 -16}} {} nil [{:z :h, #uuid "cd09387d-91d1-44bc-a79c-75287651f494" 6.5, 3/4 -7.28125, -4/7 :__H/E, #uuid "cfdd2ff9-fe03-4b4f-bf89-f0463dd91f32" *2*9, :pBI2 i:d/yKX, "/U:`TexDt" wOg/?, #uuid "d13a42de-b6c9-4f00-825a-8594486d56e4" :!} {0 -5/7, 0.5 3.30078125, 1/3 true, 31 X, _w3 :Y/_, "To" :_8!/l8, :s #uuid "6f72736b-987b-48b5-b010-21d5b9530752", true 0.625, -9/8 -7} {:xF*/zL+ 5, "y{qLz" false, 7 true, :nd1 :.VVd, :cr7/yr6 -5.0} {:Y \A, -10 -3/2, #uuid "abd0ccfc-bca4-401e-8d4b-c30a07b647a1" *Y-./d, 219 "*"} {?6P/! 9, :e?- kl/!3_, 9/5 tN, :_/XR:. -7, -0.875 :p, ??-/c:7? 4.0} {}] [])

It seems I can only use this "higher order spec" when I use a defined spec, not with an "anonymous" spec:

user=> (s/def ::seqable-of-nilable-maps (spec2-seqable-of (s/nilable ::map)))
Syntax error compiling fn* at (REPL:1:1).
Can't embed object in code, maybe print-dup not defined: clojure.spec_alpha2.impl$nilable_impl$reify__1895@3fb9a67f

Will this be supported? This macro does support it:

(defmacro spec2-seqable-of [spec]
  `(s/with-gen (s/and seqable?
                      (s/or :empty empty?
                            :seq (s/and (s/conformer seq)
                                        (s/every ~spec))))
     #(s/gen (s/nilable (s/every ~spec :kind coll?)))))
puredanger commented 5 years ago

No, this will not be supported by defop.

puredanger commented 5 years ago

To expand, defop is designed for cases where you need to parameterize a fixed spec. It cannot be used to create specs of arbitrary specs. Doing that is really creating a full spec op and requires reifying the Spec protocol. Or, you can macro at the front as you're doing to construct the desired symbolic spec.

A consequence of the macro approach is that the s/form of the generated spec will contain the expanded spec form with replacement, not a new symbolic spec like (spec2-seqable-of int?) or whatever. That's presumably fine here as that's what you were getting before too.

borkdude commented 5 years ago

Thanks! Yeah, I really just wanted to avoid writing some boilerplate for this one and don't see this macro as something other users would call, so I guess that's fine then.

borkdude commented 5 years ago

@puredanger

I may have found another bug in spec (that was already there since the last time I tested spec, but I only discovered it now):

$ clj -A:test
Checking out: https://github.com/clojure/spec-alpha2 at 2877e61f1d027ac0378815c298f8a1851981fa7d
Clojure 1.10.0
user=> (require '[clojure.spec-alpha2 :as s])
nil
user=> (s/def ::foo #{\a \b})
Execution error (AssertionError) at clojure.spec-alpha2/set-impl (spec_alpha2.clj:159).
Assert failed: set specs must contain constant values
(every? constant-val? set-vals)
borkdude commented 5 years ago

@puredanger

I encountered the above bug in this spec which I used for generating random regexes. I replaced the sets of chars with sets of strings to work around the above issue:

(s/def ::regex.gen.sub-pattern
  (s/cat :pattern
         (s/alt :chars (s/+ #{"a" "b"})
                :group (s/cat :open-paren #{"("}
                              :inner-pattern ::regex.gen.sub-pattern
                              :closing-paren #{")"}))
         :maybe (s/? #{"?"})))

Now I'm seeing a new bug:

Caused by: java.lang.Exception: Unable to resolve spec: :speculative.specs/regex.gen.sub-pattern

It seems like spec2 has trouble dealing with recursive specs. Also spec2 doesn't deal with out of order specs (first define ::bar that uses ::foo and then define ::foo).

puredanger commented 5 years ago

The forward references for regex specs is a known issue - Sean ran into that too and I will get it eventually but it's tedious. Until that's fixed, this example won't work.

puredanger commented 5 years ago

And I fixed the char thing.