for-all should support nesting

Question

Haskell QuickCheck allows for-all expressions to nest. This is useful when there are dependencies between generated values. test.check should allow this, too.

Currently, nested for-alls always succeed, which is somewhat pernicious.

I've added a patch that implements this.

Answer 1

Comment made by: reiddraper

Thanks Michael. I appreciate the patch, but there's a few design details that could be discussed before we get to code-level detail. As a separate, but related issue, I've been wanting to implement something like your {{CheckResult}} type, but as a record with several more fields. These fields would hold things like a plain-text description of any errors found, statistics (ala QuickCheck's {{collect}}, {{classify}}, etc.). I'd also like to write a protocol that allows basic types like booleans to be turned into this record. This would be analogous to Haskell QuickCheck's {{Testable}} Type Class. While this is technically a separate issue, I think it would behoove us to solve it in conjunction with nestable for-alls, particularly since nested for-alls can be simulated by just using bind at the generator level. Does this make sense?

Answer 2

Comment made by: sperber

Absolutely.

I personally would start this patch, and work from there, unless you want to do things fundamentally differently rather than add more stuff.

Either way, how can I help make it happen?

Answer 3

Comment made by: reiddraper

Great question. Let me think on that and get back to you ASAP. I'd also love to make this happen soon.

Answer 4

_Comment made by: reiddraper_

Sorry for the delay, here's my sketch I've been working with:


diff --git a/src/main/clojure/clojure/test/check/properties.clj b/src/main/clojure/clojure/test/check/properties.clj
index 99b5222..139ae9a 100644
--- a/src/main/clojure/clojure/test/check/properties.clj
+++ b/src/main/clojure/clojure/test/check/properties.clj
@@ -8,13 +8,47 @@
 ;   You must not remove this notice, or any other, from this software.
 
 (ns clojure.test.check.properties
+  (:import clojure.test.check.generators.Generator)
   (:require [clojure.test.check.generators :as gen]))
 
+(defrecord Result [result pass? message stamps])
+
+(defprotocol ToResult
+  (to-result [a]))
+
+(extend java.lang.Object
+  ToResult
+  {:to-result (fn [b]
+               ;; not checking for caught exceptions here
+               (->Result b (not (false? b)) nil nil))})
+
+(extend nil
+  ToResult
+  {:to-result (fn [b]
+               (->Result b false nil nil))})
+
+(extend java.lang.Boolean
+  ToResult
+  {:to-result (fn [b]
+               (->Result b b nil nil))})
+
+(extend Generator
+  ToResult
+  {:to-result identity})
+
+(extend Result
+  ToResult
+  {:to-result identity})
+
+(defn message
+  [m property]
+  (assoc property :message m))
+
 (defn- apply-gen
   [function]
   (fn [args]
-    (let [result (try (apply function args) (catch Throwable t t))]
-      {:result result
+    (let [result (to-result (try (apply function args) (catch Throwable t t)))]
+      {:result (:result result)
        :function function
        :args args})))
 
@@ -29,9 +63,18 @@
   (for-all* [gen/int gen/int] (fn [a b] (>= (+ a b) a)))
   "
   [args function]
-  (gen/fmap
-    (apply-gen function)
-    (apply gen/tuple args)))
+  (gen/bind
+    (apply gen/tuple args)
+    (fn [a]
+      (let [result ((apply-gen function) a)]
+        (cond (gen/generator? result) (gen/fmap (fn [r] (println "foo") (update-in r :args #(conj % a))) result)
+              ;; NOTE: quick note to myself before I leave this code for the night,
+              ;; this :else is getting hit because we're wrapping the result
+              ;; with a {:result ...} map. Should probably do that conditionally.
+              ;; We also need two result types I think, a result to return from
+              ;; a property itself, and a reuslt that tacks the 'args' on top of this.
+              :else (do (println "bar") (gen/return result)))))
+    ))
 
 (defn binding-vars
   [bindings]

Answer 5

Comment made by: sperber

Looks OK. However, it's difficult to see why that would get you more quickly where you said you want to go than my patch ...

Answer 6

Comment made by: reiddraper

bq. Looks OK. However, it's difficult to see why that would get you more quickly where you said you want to go than my patch ...

Fair enough. Part of this it that it was easier for me to write up a sketch. The main things I'm trying to cover when supporting nested generators are making sure:

1. We also support the upcoming ability to collect and return statistics about a test, ala {{collect}} from Haskell QuickCheck
2. We have a sane way of returning failing tests to a user. Right now, in the {{:fail}} and {{:smallest}} keys of the returned map, we tell the user the failing arguments. They're always wrapped in at least one vector, since you may use more than one generator using {{prop/for-all}}. What do we do with nested properties? How do we distinguish between multiple generators at the 'same level', vs nested properties? Or do we not need to distinguish? Can whatever we decide to do be backward compatible?

Point being, I want to make sure we're not committing ourselves to nested-properties until we have some of those answers, and for me personally, it's easier to try and play with these things together, and see how they will fit together.

Answer 7

_Comment made by: expez_

I don't really want to have to nest for-all, I'd much rather prefer it work like let where we can refer to previous values.  That said, no matter which solution, this is my biggest gripe with test.check at the moment, so any solution would be preferable to another year of hammock time.

Here's one solution, taken from the wild. I'm sure there are many others, people want this badly, but this one was taken from Nathan Marz's specter library:


(defmacro for-all+ [bindings & body]
  (let [parts (partition 2 bindings)
        vars (vec (map first parts))
        genned (reduce
                (fn [curr [v code]]
                  `(gen/bind ~code (fn [~v] ~curr)))
                `(gen/return ~vars)
                (reverse parts))]
    `(prop/for-all [~vars ~genned]
                   ~@body )))

Answer 8

Comment made by: gfredericks

We've been thinking about a similar problem in (link: http://dev.clojure.org/jira/browse/TCHECK-81 text: TCHECK-81), and in the meantime there's an even-fancier variant of {{for-all}} (link: https://github.com/gfredericks/test.chuck#properties text: here).

Answer 9

Reference: https://clojure.atlassian.net/browse/TCHECK-44 (reported by alex+import)