Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
slipset opened this issue Nov 14, 2018 · 32 comments
Open

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

slipset opened this issue Nov 14, 2018 · 32 comments

Comments

@slipset
Copy link
Sponsor Collaborator

slipset commented Nov 14, 2018

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
Copy link
Owner

borkdude commented Nov 15, 2018

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
Copy link
Owner

borkdude commented Jan 20, 2019

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

(defmethod s/create-spec 'seqable-of

@borkdude borkdude added this to Waiting in Kanban board Jan 23, 2019
@borkdude
Copy link
Owner

borkdude commented Jan 25, 2019

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
Copy link
Owner

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
Copy link

puredanger commented Feb 5, 2019

few updates on latest master:

  • test/check fixed
  • "no conversion to symbol" from class symbols fixed
  • or-spec-impl fixed

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

@puredanger
Copy link

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
Copy link
Owner

borkdude commented Feb 6, 2019

@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
Copy link

Both fixed in latest sha. Bugs!

@borkdude
Copy link
Owner

borkdude commented Feb 8, 2019

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

  • fix atom spec
  • fix fn spec of map, keep, etc.

@puredanger
Copy link

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

@puredanger
Copy link

Well, maybe not.

@puredanger
Copy link

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

@borkdude
Copy link
Owner

borkdude commented Feb 8, 2019

@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
Copy link
Owner

borkdude commented Feb 8, 2019

@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
Copy link
Owner

borkdude commented Feb 9, 2019

@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
Copy link

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

@borkdude
Copy link
Owner

borkdude commented Feb 9, 2019

Still doesn't work with vector?.

@borkdude
Copy link
Owner

borkdude commented Feb 9, 2019

@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
Copy link

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
Copy link
Owner

@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
Copy link
Owner

borkdude commented Feb 10, 2019

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

TODO:

  • fix or spec when used in combination with s/with-gen
  • :kind coll? in s/every doesn't work

@borkdude
Copy link
Owner

borkdude commented Feb 10, 2019

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
Copy link

  • "fix or spec when used in combination with s/with-gen" - fixed
  • ":kind coll? in s/every doesn't work" - I did fix a bug with gen from kind, but coll? is not valid to use with this spec as it will generate maps that fail the predicate (same in spec 1)

@borkdude
Copy link
Owner

borkdude commented Feb 11, 2019

@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
Copy link
Owner

borkdude commented Feb 11, 2019

@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
Copy link

No, this will not be supported by defop.

@puredanger
Copy link

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
Copy link
Owner

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
Copy link
Owner

borkdude commented Mar 16, 2019

@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
Copy link
Owner

borkdude commented Mar 16, 2019

@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
Copy link

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
Copy link

And I fixed the char thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Kanban board
  
Paused
Development

No branches or pull requests

3 participants