Implement such-that maximum retries option

[jobtravaini]

Context

clojure.test.check has the option to increase the maximum number of retries for generators. When creating complex schemas, you may face flakiness of generators and tests because of conditionals and other types of constraints. If the reader is interested in reading more about the context, the issue opened #28 has more context.

Changes

To enable configuration of max-retries on such-that.

Changes:

• Overload the function schema-generators.generators/generator to allow a new argument named opts to be provided as options (enabling further extension required).
• Change the CompositeGenerator protocol to accept opts as argument
• Implement tests for maximum number of retries and guarantee the existing tests are working
• Bump clojure to 1.11.1

[w01fe]

Thanks for the PR! Left a few questions, please let me know your thoughts.

[w01fe]

This isn't quite right, you want to call options at the place where you have the schema you are trying to sample, which will typically be a subschema of schema.

[jobtravaini]

I've made the change on 2a48605, is it what you have described?

[w01fe]

Err not quite, sorry.

I think you want to store options in params as another key like subschema-options.

Then I'd define a helper function

(def max-retries [params schema] (((safe-get params :subschema-options) s) :max-retries 10))

and wherever you have

(get params :max-retries 10)

currently you'd use (max-retries params s) instead.

Does that make sense?

[jobtravaini]

I have tried what you have described. The (get params :max-retries 10) is part of the CompositeGenerator protocol implementation. Given this context, I have implemented the following code:

(defn max-retries
  [schema params]
  (((macros/safe-get params :subschema-options) schema) :max-retries 10))

(defprotocol CompositeGenerator
  (composite-generator [s params]))

(extend-protocol CompositeGenerator
  schema.spec.variant.VariantSpec
  (composite-generator [s params]
    (generators/such-that
     (fn [x]
       (let [pre (.-pre ^schema.spec.variant.VariantSpec s)
             post (.-post ^schema.spec.variant.VariantSpec s)]
         (not
          (or (pre x)
              (and post (post x))))))
     (generators/one-of
       (for [o (macros/safe-get s :options)]
         (if-let [g (:guard o)]
           (generators/such-that g (sub-generator o params) (max-retries s params))
           (sub-generator o params))))
     (max-retries s params)))

  ;; TODO: this does not currently capture proper semantics of maps with
  ;; both specific keys and key schemas that can override them.
  schema.spec.collection.CollectionSpec
  (composite-generator [s params]
    (generators/such-that
     (complement (.-pre ^schema.spec.collection.CollectionSpec s))
     (generators/fmap (:konstructor s) (elements-generator (:elements s) params))
     (max-retries s params)))

  schema.spec.leaf.LeafSpec
  (composite-generator [s _]
    (macros/assert! false "You must provide a leaf generator for %s" s)))

This implementation does not work because the s parameter received in the composite-generator function is (s/spec s) which provides me no longer a Schema, but a VariantSpec. Below you can check the options content on a runtime example:

{#schema.core.ConditionalSchema
 {:error-symbol nil
  :preds-and-schemas [[#<clojure.core$int_QMARK_@1d8946ca>
                       #<java.lang.Class@18b4aac2 class java.lang.String>]]} {:max-retries 100}}

The s parameter on this same example:

#schema.spec.variant.VariantSpec
{:err-f #<schema.core.ConditionalSchema$fn__6078@3641ebdc>
 :options ({:guard #<clojure.core$int_QMARK_@1d8946ca>
            :schema #<java.lang.Class@18b4aac2 class java.lang.String>})
 :post nil
 :pre #<schema.spec.core$_PLUS_no_precondition_PLUS_@4aafe9d4>}

---

I can change the implementation to perform the max-retries call on generator function level and use safe-get on the composite-generator to fetch the max-retries.

But I think it won't be much different from the current implementation.

[w01fe]

Argh, my apologies, it's been too long and I forgot how this code worked, you had it right in 2a48605. Sorry again!

[w01fe]

If there's a default so this should always exist, can we safe-get please?

[jobtravaini]

Changes on 2a48605

[w01fe]

Looking good, just a couple remaining minor comments. Thanks for the updates !

[w01fe]

Looks great, thanks for the fix!