<< Back to previous view

[TCHECK-126] Refactor the quick-check process as a state machine to allow more extensibility Created: 29/Nov/16  Updated: 29/Nov/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicolás Berger Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File test.check2.patch    
Patch: Code and Test

 Description   

We can think of the quick-check process as a state machine with a state flow like:

       started
          v
  trying, trying, [...]
          v
succeeded | failure
                 v
    shrinking, shrinking, [...]
                 v
               shrunk"

The process starts, runs trials based on the generated values and either it succeeds or it fails. If it succeeds, the process is done. If it fails, then it runs successive shrinks of the failed args until it gets to the terminal shrunk state.

Modelling the process this way, we can call a step-fn at interesting points of the process that serves two purposes:

  • Provide feedback to the user about the process (via side-effects)
  • Augment/modify the state being tracked by the process, making it easy to implement things like gracefully aborting the process before it finishes, calculating statistics on the generated values (TCHECK-87), add timestamps (TCHECK-8, TCHECK-95, TCHECK-96), etc.

The attached patch aims at being 100% backwards compatible. Adds a new clojure.test.check2/quick-check function. The "old" clojure.test.check/quick-check is re-implemented by calling the new one, maintaining all the current behavior by lifting the reporter-fn into a step-fn via a reporter-fn->step-fn adapter fn.






[TCHECK-125] should fail gracefully when clojure.test/report is not a multimethod Created: 28/Nov/16  Updated: 28/Nov/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I don't know if there is a good reason for this to happen, but it has definitely happened a lot.






[TCHECK-124] defspec does not set property :name properly Created: 23/Nov/16  Updated: 23/Nov/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Chas Emerick Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File TCHECK-124.diff    
Patch: Code and Test

 Description   

If one uses trial-report-periodic, output like this is produced:

Passing trial 884 / 1000 for (prop/for-all* [] (fn* [] (do (Thread/sleep 1) true)))

This can get very unwieldy for specs defined within `defspec` forms. The same "representation" of specs is used in reporting shrinking, which isn't very helpful if you're trying to track down a problem. What we want (and what I'm sure I meant to write (the blame is tricky b/c of the renames for cljc'ing, but I think this one's on me originally) is something like:

Passing trial 884 / 1000 for long-running-spec

Patch attached to do this.



 Comments   
Comment by Nicolás Berger [ 23/Nov/16 6:04 PM ]

LGTM





[TCHECK-87] New stats feature: Adds ability to assign labels to test cases to report the test case distribution when running a test Created: 03/Dec/15  Updated: 23/Nov/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicolás Berger Assignee: Nicolás Berger
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File TCHECK87-add-stats-feature-3.patch    
Patch: Code and Test

 Description   

Inspired in the analog feature that exists in Haskell's QuickCheck. Adds a classify fn that is intended to be used as a wrapper of prop/for-all, returning a property (a generator) appropriate for test.check/quick-check that will augment the result-map returned by the underlying property, adding the collected labels under the :labels key. Also triggers a new event :stats in the default-reporter-fn whose default implementation calls test.check.stats/print, printing the classification of trials with the following format:

12.7% :lt-30
14.5% :gte-30
29.1% :lt-30, :lt-20
43.6% :lt-30, :lt-10, :lt-20

(note that multiple labels might be assigned to some test cases)

I think it answers the question "How could we collect stats about the sorts of things generated?" from the test.check design page



 Comments   
Comment by Nicolás Berger [ 28/Feb/16 7:52 PM ]

The patches here no longer apply after the introduction of reporter-fn.

While I'm working on new patches, I'd like to know other people's (and especially gfredericks) opinion on having this new :labels concept in test.check. Each trial's result map can optionally include some :labels that are included in the :complete report so the stats can be computed.

An alternative would be to implement the stats as an external library/reporter, but to make that possible we need the trials loop to keep arbitrary state from each trial's result-map where we could accumulate the labels. Or maybe we could make the (reporter-fn :type :trial ...) to include the entire result-map so the reporter-fn has the chance to "save" this state, but this way doesn't look very elegant...

Comment by Gary Fredericks [ 29/Feb/16 9:40 PM ]

Things to think about:

  • The stats are only a function of the args, so is it necessary to wrap the entire property?
    • haskell does this though
  • On the other hand, I like the idea of stats being a particular instance of a generic feature of the check namespace, but the current implementation doesn't quite achieve that since we have lots of label stuff in the check namespace
  • Buuuut it's hard to move it out because if check just exposed a generic reduce feature, then users going directly to quick-check would have to supply two things – a wrapped property, and the proper reduce function
Comment by Nicolás Berger [ 12/Mar/16 1:07 PM ]

Uploaded a new patch that takes advantage of the new reporter-fn mechanism.

In this new patch some stuff was simplified:

  • Removed the new report-stats var that was going to be used as a way to enable/disable printing of stats for each test. The default-reporter-fn will now print the stats only when there are labels present in the result-map. To not print the stats it's just a matter of using a different :reporter-fn that doesn't print the stats.
  • In the stats report only lines with labels are printed now. In the previous version there was a line with the percentage of trials with "No labels". I removed it now to make it less verbose.

Also docstrings were added, made it sure the stats work in clojurescript and other minor improvements.

Comment by Nicolás Berger [ 19/Mar/16 2:26 PM ]

I'm thinking it would be nice to give more ways for assigning labels to trials. For example:

1. (classify prop): It just uses the vector of args as the label.
2. (classify prop label-fn): Obtains the label by applying a label-fn to the args. Example: (classify prop count) - assigns the count of the arg (a vector, string, etc) as the label
3. (classify prop label-fn pred): Applies label-fn only when pred yields a truthy value. Example: (classify prop count #(> (count %) 1)) - assigns the count of the arg as the label, but only when count is greater than 1.
4. (classify prop pred): Uses the vector of args as the label, but only when pred yields a truthy value. Example: (classify prop #(<= (count %) 1)) - assigns the count of the arg as the label, but only when count is lower or equal to 1.
5. (classify prop pred label): This is the current signature. Assigns label only when pred yields truthy

So I'm thinking about changing the signature to receive a map of (:pred :label-fn :label) possible keys. The three keys are optional. :label and :label-fn can't be both present at the same time.

From what I could understand, Haskell QuickCheck (https://hackage.haskell.org/package/QuickCheck-2.8.2/docs/Test-QuickCheck-Property.html) has different functions to provide similar alternatives:
(classify prop) would be similar to collect in Haskell QC
(classify prop label-fn) would be similar to label in Haskell QC
(classify prop label-fn pred) would be similar to classify in Haskell QC

What do you think @gfredericks?

Comment by Gary Fredericks [ 05/Apr/16 10:02 PM ]

After discussing it, my hunch is that 2 and 5 are the most natural ones, maybe calling 2 collect to mirror the haskell version.

We talked about maybe treating nil as a flag for not labelling in collect, but I don't particularly like that idea I don't think.

Comment by Gary Fredericks [ 05/Apr/16 10:09 PM ]

I should also not forget that this might overlap with changes to address the "Test Failure Feedback" issue on the confluence page: http://dev.clojure.org/display/design/test.check

Comment by Gary Fredericks [ 05/Apr/16 10:09 PM ]

I should also not forget that this might overlap with changes to address the "Test Failure Feedback" issue on the confluence page: http://dev.clojure.org/display/design/test.check

Comment by Nicolás Berger [ 10/Apr/16 10:49 PM ]

We talked about maybe treating nil as a flag for not labelling in collect, but I don't particularly like that idea I don't think.

Perhaps we can use a namespaced keyword to signal that a label should be ignored? Something like clojure.test.check.stats/ignore. This way we can easily implement classify in terms of collect by creating a function that returns stats/ignore when pred doesn't match:

(defn collect
  [prop label-fn]
  (gen/fmap
    (fn [{:keys [args] :as result-map}]
      (let [label (apply label-fn args)]
        (if (= ::ignore label)
          result-map
          (update result-map :labels conj label))))
    prop))

(defn classify
  [prop pred label]
  (collect prop (fn [& args]
                  (if (apply pred args)
                    label
                    ::ignore))))

Another option could be to add an extra arity in collect, to receive a flag about whether nil should be treated as a label or if it should be ignored. I like :stats/ignore more

Comment by Nicolás Berger [ 19/Apr/16 9:19 AM ]

Replaced with new patch that adds both stats/classify & stats/collect as discussed. I think this new patch implements what was discussed, covering cases 2 & 5 with function names analog to the haskell impl, leaving out any special treatment for nil (it is a valid label) and not adding :stats/ignore as I suggested in a previous comment.

Comment by Nicolás Berger [ 17/Nov/16 7:27 AM ]

Added a new patch TCHECK87-add-stats-feature-2.patch that is rebased on current master. I don't think it's the final version (I'd like to find a way to not pollute the main quickcheck loop with the labels stuff for example) but it's hopefully getting closer.

Comment by Nicolás Berger [ 23/Nov/16 5:53 PM ]

Added new patch TCHECK87-add-stats-feature-3.patch rebased on current master with some squashed commits. Also fixes the print-stats test in ClojureScript.





[TCHECK-123] Interactive documentation Created: 27/Oct/16  Updated: 16/Nov/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Gary Fredericks
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File TCHECK-123.patch     Text File TCHECK-123.patch    

 Description   

Transform the code snippets of the test.check documentation into interactive code snippets using the codox klipse theme.



 Comments   
Comment by Yehonathan Sharvit [ 27/Oct/16 2:50 PM ]

This is the patch for interactive documentation using codox klipse theme https://github.com/viebel/codox-klipse-theme
Live demo of the result:

http://viebel.github.io/klipse/examples/test.check/doc/intro.html

http://viebel.github.io/klipse/examples/test.check/doc/generator-examples.html

http://viebel.github.io/klipse/examples/test.check/doc/clojure.test.check.html#var-quick-check

Will add interactive snippets to the other functions later.

Comment by Gary Fredericks [ 07/Nov/16 7:40 PM ]

Would this require having klipse annotations in the docstrings of all the functions we'd want to include?

Comment by Yehonathan Sharvit [ 07/Nov/16 10:28 PM ]

Yes.

Comment by Yehonathan Sharvit [ 16/Nov/16 12:03 AM ]

Updated the patch with changes only in project.clj
Now, we don't have to include the klipse annotations in the docstrings.
The interactive doc is opt-out - there is a link in the top of the page to activate it.





[TCHECK-122] include :last-generated-value in error thrown by such-that-helper Created: 18/Oct/16  Updated: 26/Oct/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Chelimsky Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: enhancement, generator

Attachments: Text File such-that-error-data.patch    

 Description   

The message "Couldn't satisfy such-that predicate after <n> tries" isn't enough to understand which generator failed when generating a structure with such-that generators in multiple locations within the structure. The attached patch adds a :last-generated-value to the ex-data of the error. Ideally we'd also be able to see the predicate that failed, but that would require a much more invasive change and adding the generated value is very low hanging fruit and can be helpful in many cases, so I think this is a good start.



 Comments   
Comment by Gary Fredericks [ 18/Oct/16 12:45 PM ]

When you say "ideally we'd also be able to see the predicate that failed" are you referring to something beyond the fact that the ex-data map you're changing already has the predicate in it?

Comment by David Chelimsky [ 18/Oct/16 3:13 PM ]

Yes. What's on the master branch now (there is no ex-data in the such-that error in the 0.9.0 release) helps when the predicate is the value of a Var, but when it's an anonymous fn you get something like #object[clojure.test.check.test$fn_3195$fn3202$fn3203 0x7da34b26 "clojure.test.check.test$fn3195$fn3202$fn_3203@7da34b26". Ideally, you'd get the actual form that was passed to such-that, but you'd need to something like make such-that a macro and store that information somewhere (the more invasive change I mentioned).

Comment by Gary Fredericks [ 18/Oct/16 4:40 PM ]

Got it. I think this change makes sense, thanks!

Comment by Gary Fredericks [ 26/Oct/16 11:31 AM ]

David Chelimsky are you thinking of use cases where the data is merely printed and the user doesn't have the opportunity to interact with it? Otherwise they could pull out the generator and sample it to get similar feedback, in a way that doesn't have the potential to spam the user with a Very Large piece of data?

Comment by David Chelimsky [ 26/Oct/16 11:45 AM ]

The problem is that you don't know which generator is the one that can't satisfy such-that when there are more than one such-that generator in a nested structure. Does that make sense? I want something to help me figure out which generator is the problem, and printing the last value that failed such-that can help me figure that out. I'm definitely open to other solutions - this was just the solution that I proposed.

Comment by Gary Fredericks [ 26/Oct/16 1:02 PM ]

The exception thrown by such-that (on master, not 0.8.0) contains in the ex-data the generator that failed – so you should be able to pull that generator out of the ex-data and sample it, which gives you more information, theoretically, than just the last failing value.





[TCHECK-111] The latest recursive-gen algorithm seems to exhibit a peculiar lack of variety in depths Created: 22/Jun/16  Updated: 09/Oct/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

In particular the output of (generate (recursive-gen vector nat) 1000) is rather less varied than I would hope.



 Comments   
Comment by Felix Andrews [ 29/Aug/16 10:18 PM ]

Yes, recursive-gen seems not to generate scalars, which means any-printable doesn't generate scalars.

(require '[clojure.test.check.generators :as g])
(->> (g/sample g/any-printable 1000) (remove coll?))
;; ()
Comment by Gary Fredericks [ 29/Aug/16 10:25 PM ]

recursive-gen should generate scalars on master

Comment by Felix Andrews [ 29/Aug/16 10:31 PM ]

D'oh, sorry for the noise.

Comment by Sean Corfield [ 07/Oct/16 11:56 AM ]

Does this also cause http://dev.clojure.org/jira/browse/CLJ-2036 ?

Comment by Gary Fredericks [ 09/Oct/16 2:07 PM ]

I think the "doesn't return scalars" problem is fixed on master; this is a subtler problem.

Comment by Gary Fredericks [ 09/Oct/16 2:08 PM ]

TCHECK-83 in particular, is done.





[TCHECK-121] for-all's signature suggests multiple expressions are allowed, but this doesn't work as you'd expect Created: 04/Sep/16  Updated: 10/Sep/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: lvh Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

The signature of for-all, which includes & body, might suggest you could write something like:

(defspec empty-schema-validation
  (prop/for-all
   [v (s/gen ::js/json-type)]
   (nil? (explain-with-schema {} v))
   (nil? (explain-with-spec {} v))))

That works, but IIUC not as you might expect; in that only the latter expr is checked. I'm not sure if the signature should be amended, or if for-all should just create a fn for each of those exprs. The latter is more work, but seems more useful.



 Comments   
Comment by Gary Fredericks [ 10/Sep/16 5:54 PM ]

I appreciate the confusion, and I think maybe keeping it to one expression would have been a better idea, but at this point it can't be changed in a backwards-compatible way.

I'm planning to overhaul the docstrings before the next release though, so I will make sure to clarify this.





[TCHECK-105] Support for bootstrapped / self-host ClojureScript Created: 01/May/16  Updated: 07/Sep/16  Resolved: 06/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: Gary Fredericks
Resolution: Completed Votes: 1
Labels: bootstrap, release-0.10.0

Attachments: Text File TCHECK-105-2.patch     Text File TCHECK-105.patch    
Patch: Code and Test

 Description   

Revise test.check so that it can be used in bootstrapped environments. Additionally, add an ability to regression-test test.check for bootstrapped use.

Rationale: ClojureScript libraries and/or applications may wish to target bootstrapped ClojureScript while also making use of test.check.

Background: The cljs.test library has recently been ported for use with bootstrapped ClojureScript. (CLJS-1626). It has been shown that the test.check library can be made work with minor modification (removal of the :clj reader conditional guarding defspec). Further work would need to be done to ensure that such a modification is acceptable and that test.check's unit tests can be executed in a bootstrap environment (perhaps via a script running the tests in Node.)



 Comments   
Comment by Mike Fikes [ 04/May/16 10:23 PM ]

Attached TCHECK-105.patch. The commit comment in it, along with comments near the changed code provide details regarding the patch. Happy to elaborate or further revise if needed.

Comment by Mike Fikes [ 04/Jun/16 2:47 PM ]

With the ClojureScript 1.9.36 release, the latest shipping cljs.jar will now encode compiler analysis metadata cache files as Transit by default, and the self-host test framework in TCHECK-105.patch is not set up to read Transit files from within self-hosted ClojureScript. (The self-host tests will operate with the cljs.jar that shipped with the 1.9.14 release.)

While this could probably be addressed by sorting out how to get the test framework to read the Transit files, CLJS-1666 provides another route: A flag will potentially be added that will cause the ClojureScript compiler to emit edn files instead.

Comment by Mike Fikes [ 02/Aug/16 12:12 PM ]

Hey Gary, CLJS-1666 hasn't been addressed (and I think there is a desire to avoid addressing CLJS-1666 unless absolutely necessary). Is resolving the Transit issue a precondition for accepting this patch? (A consequence of not sorting this out is that the self-host portion of the unit tests included with the patch can only be executed using ClojureScript 1.9.14, until a solution exists.)

Comment by Gary Fredericks [ 02/Aug/16 12:43 PM ]

So the worst case is just that we're not catching self-hosting regressions, is that right? That's fine with me, since that situation seems strictly better than not doing anything at all.

I'm still planning to accept this patch, as soon as I can get some time to plan the next release of test.check more generally.

Comment by Mike Fikes [ 02/Aug/16 12:48 PM ]

Gary, yes, that's correct: There could be some change in ClojureScript 1.9.36 or later that causes a regression for test.check when used in self-host mode that won't be possible to easily catch using the regression test capability built into the patch until the Transit issue is addressed.

Comment by Gary Fredericks [ 03/Sep/16 4:41 PM ]

Now that I'm rereading the comments, it sounds like we could have automated tests just by adding transit? If it's not difficult, I'd definitely prefer tests, since the code is uglier this well and I'll be forever tempted to make it nicer .

If so, I'd also prefer that most of the technical explanation live wherever the entrypoint to the tests is, with the inline comments just pointing to that.

Is there a reason why cljs.jar or something equivalent can't be obtained through maven?

Sorry for the slow turnaround.

Comment by Mike Fikes [ 04/Sep/16 2:20 PM ]

Hi Gary, I'll see if I can get it to work with perhaps a special lein profile that specifies the latest ClojureScript along with Transit for the purposes of testing under self-hosted ClojureScript. That way there'd be no need to manually download anything and tests could be run in an automated fashion by just running script/test-self-host.

I'll also need to update things for the latest stuff in master of test.check.

Comment by Mike Fikes [ 04/Sep/16 3:52 PM ]

Hi Gary, TCHECK-105-2.patch attached for your comment.

This revision passes when applied to master (you simply need to run script/test-self-host).

It makes use of lein to obtain the latest ClojureScript JAR (employing a special profile for self host). This differs from the previous patch which required you to manually grab cljs.jar and place it in the lib directory.

Since it depends on the shipping ClojureScript dep (and not the standalone cljs.jar, which bakes in Transit), the entire Transit issue is sidestepped: The ClojureScript compiler only emits Transit if the Transit dep is present; since it is not with this revision, everything works in terms of EDN.

Happy to revise things, and comment where you believe could be helpful, as you see fit.

Comment by Gary Fredericks [ 06/Sep/16 9:34 PM ]

I just applied the patch to master, thanks for putting it together!

Unless self-hosted cljs becomes rather more popular, I'm not planning on putting active effort into supporting it, but I intend to at least keep the tests passing. If there's any particular regression you're worried about, feel free to add more tests.

Comment by Mike Fikes [ 07/Sep/16 6:03 AM ]

Thanks Gary!





[TCHECK-98] gen/let uses gen/bind even when it doesn't have to Created: 17/Mar/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

This was a known drawback to gen/let and it was implemented this way for lack of any other rigorous options.

(gen/let [a gen-a, b gen-b] ...)
;; ↑ expands (more or less) to ↓
(gen/bind gen-a (fn [b] (gen/fmap (fn [a] ...) gen-b)))
;; instead of ↓
(gen/fmap (fn [[a b]] ...) (gen/tuple gen-a gen-b))

The difficulty is that you need to recognize that the expression gen-b does not use the local a, which requires a robust analyzer to do without nasty edge cases.

At first I believed that this was not a problem and that gen/let shouldn't do this kind of magical dependency detection, that users should use gen/tuple themselves or else macros like gen/let should have explicit features for generating things in parallel (like test.chuck/for has), but I've changed my mind after thinking about how natural it is to write independent clauses, especially when the macro is named let (in contrast to for). I also came across evidence that this is how people tend to use it.

According to Nicola tools.analyzer will work just fine for this, but I'm hesitant to bring in such a large dependency just for a single macro. He told me that a smaller subset of the analyzer could accomplish this, so I may wait to see if such a library could be extracted.



 Comments   
Comment by Gary Fredericks [ 02/Aug/16 10:06 AM ]

I created CLJ-1997 to raise this issue in clojure proper.

Comment by Gary Fredericks [ 04/Aug/16 3:34 PM ]

I discussed this with alex miller and we thought that adding the :parallel feature from test.chuck/for might be the most promising route.

Comment by Gary Fredericks [ 03/Sep/16 5:09 PM ]

Fixed on master by adding an alternate map syntax to gen/let.





[TCHECK-107] such-that should provide a way to customize its error message Created: 08/Jun/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

clojure.spec uses such-that for (at least) s/and specs, and for certain specs the default such-that is likely to throw an exception to a user who didn't know the such-that was there in the first place. For usability it would be good for clojure.spec to be able to substitute or add info to the error somehow.



 Comments   
Comment by Gary Fredericks [ 16/Jul/16 1:01 PM ]

I made a branch here.

Comment by Gary Fredericks [ 03/Sep/16 4:16 PM ]

Fixed on master; added ex-fn to such-that and all the distinct collection generators.





[TCHECK-108] Clarify docstrings for dev functions in the generators namespace Created: 09/Jun/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

gen/sample, gen/generate, and anything similar should have notes in the docstring saying that they aren't intended to be used to compose generators.



 Comments   
Comment by Gary Fredericks [ 03/Sep/16 3:28 PM ]

Fixed on master





[TCHECK-114] gen/frequency should shrink to earlier generators Created: 02/Aug/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

This might be as simple as changing the impl from using gen-bind to bind.



 Comments   
Comment by Gary Fredericks [ 03/Sep/16 3:28 PM ]

This is as fixed on master, at least in the sense that frequency behaves like one-of now. See TCHECK-120 for more details.





[TCHECK-115] run tests in parallel on the jvm Created: 15/Aug/16  Updated: 03/Sep/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Goals

  • deterministic – calling quick-check with parallel opts gives the same answer as without
    • we have to give up some determinism though, because I don't think we could reasonably
      say that the reporter-fn will be called the same way every time, or even that
      the :num-tests value returned should always be the same
  • does the best possible thing on the jvm w.r.t. things hanging
  • is it necessary to have some sort of dynamic var with a thread id or something similar,
    so users can use it to coordinate usage of global resources? alternatively they could
    use a ThreadLocal by hand, or a pool

Issues

  • Is this really worth it? The downside is it makes the API and impl of quick-check more complex, and arguably most use cases could be served by calling quick-check multiple times from separate threads; would a generic test-suite parallelizer do the job for most people?





[TCHECK-120] size of args is conflated with depth in shrink tree Created: 03/Sep/16  Updated: 03/Sep/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This came up when working on TCHECK-114.

I had written a generator of the form (gen/frequency [[1 gen-1] [2 gen-2]]) and was trying to figure out why failures that originated in gen-2 didn't always shrink to gen-1.

When I examined the shrink tree, I realized that the shrinking algorithm did try shrinking to gen-1, but even after finding a failure there it would continue looking for alternatives and eventually shrunk to something from gen-2, because it was deeper in the shrink tree and therefore believed to be smaller.

So the shrinking algorithm has no way to compare the "size" of inputs except for their depth in the search tree, which is sometimes misleading. I doubt there's a general solution to this besides maybe adding an option to quick-check for a custom comparator function.






[TCHECK-118] Pass shrunk input on to clojure.test when using defspec Created: 23/Aug/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Sebastian Poeplau Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

test.check provides integration with clojure.test through the defspec macro. It reports failing test inputs to clojure.test by calling clojure.test/report with an event of type :clojure.test.check.clojure-test/shrinking. However, the shrunk input is not reported.

CIDER has recently gained the ability to display generated test input. For tests defined through defspec it currently displays the failing inputs, but it would be very nice to have it receive the shrunk input as well (which currently only works for test.chuck's checking). I would therefore suggest to have clojure.test.check.clojure-test/default-reporter-fn forward the event of type :shrunk that quick-check generates; CIDER is ready to process it, and I assume others will just ignore it.



 Comments   
Comment by Gary Fredericks [ 03/Sep/16 12:07 PM ]

This is fixed on master





[TCHECK-117] Use a default-opts var instead of default-reporter-fn Created: 18/Aug/16  Updated: 03/Sep/16  Resolved: 03/Sep/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Comments   
Comment by Gary Fredericks [ 03/Sep/16 11:17 AM ]

Fixed on master





[TCHECK-119] Checking of interesting edge cases is not very creative Created: 03/Sep/16  Updated: 03/Sep/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

There may not be a single solution to all of these, and some of them may be hard.

Some examples:

  • When specifying bounded integer ranges, we don't intentionally check near all of min,0,max
    • At first glance this sounds easy, but it involves making not-obvious decisions about the meaning of "small" examples and where to shrink to. E.g., should (gen/generate (gen/choose 5 Integer/MAX_VALUE) 3) be highly likely to generate Integer/MAX_VALUE? (Intuitively that may not match what the user means when passing 3 as the size); and if a test fails for 1000 but not for any smaller numbers, should it shrink to 1000 or to Integer/MAX_VALUE since the latter is "simpler" in a certain sense?
  • The size of a collection correlates with the size of its elements
    • i.e., (gen/list gen/nat) is highly unlikely to generate a list of 20 0's.





[TCHECK-116] With clojure.test, debug prn should be optional, so you can run tests with minimal output Created: 18/Aug/16  Updated: 25/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Brent Spell Assignee: Gary Fredericks
Resolution: Unresolved Votes: 3
Labels: release-0.10.0
Environment:

osx el capitan, leiningen 2.6.1, clojure 1.8, test.check 0.9.0



 Description   

There is an unconditional prn in assert-check (https://github.com/clojure/test.check/blob/master/src/main/clojure/clojure/test/check/clojure_test.cljc#L19) that outputs the test configuration (seed, etc.) every time a defspec is used. For projects with many property tests/defspecs, this results in a large test output when tests are successful.

For tests that pass, it seems like this output is unnecessary, since failing tests always report the seed, etc. Currently, the only obvious way to suppress this message is to monkey-patch assert-check and disable prn. It would be nice to have an option to disable these messages through configuration for successful runs.



 Comments   
Comment by Gary Fredericks [ 18/Aug/16 6:54 PM ]

Agreed – is there any cleaner mechanism for this than a dedicated global var?

Perhaps a general callback function would be better than a flag for printing. E.g.,

(def ^:dynamic *output-fn* prn)

On second thought, the master branch has a :reporter-fn option to quick-check, and the clojure-spec namespace has a default reporter fn that could be overrideable. If the printing were folded into that, then it could be disabled that way, though that's rather more roundabout for the user :/.

Comment by Brent Spell [ 25/Aug/16 8:11 AM ]

Both of those solutions sound like a reasonable burden on the user to me, and they would be more robust than having to monkey-patch the assert-check function.





[TCHECK-48] Add sublist generator Created: 28/Oct/14  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Reid Draper Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Comments   
Comment by Gary Fredericks [ 21/May/15 9:40 PM ]

Related (but probably different): https://github.com/gfredericks/test.chuck/blob/20f17550acecdfed4ca3259ebdad34e088fd453a/src/com/gfredericks/test/chuck/generators.clj#L141-150





[TCHECK-70] Redesign gen/choose Created: 06/Jun/15  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

Currently gen/choose is the generator that everything else is based off of, and it's also a user-facing generator. Its current implementation takes a [0,1) double and scales it to the desired range. This approach degrades w.r.t. distribution as you get higher, and presumably has pretty terrible properties once your range is larger than 2^53. It could also be optimized better for when the range is a power of 2 (no need to go through Double). Seeing as how gen/choose is user facing, we should attempt to redesign it so that it doesn't degrade in this fashion, and can handle bigints.

Requirements

gen/choose should have a uniform distribution regardless of bounds

gen/choose should accept arbitrary integer bounds and handle them correctly

Things to investigate

  • Note that the SplittableRandom class generates integer ranges without going through floating point. We should check if this is faster, since it's definitely a more uniform distribution.
  • Is it worth optimizing for the power-of-two case, the same with SplittableRandom does in the link above?
  • When it's possible for the generator to return a long instead of a bigint is somewhat subtle. E.g., if you call (gen/choose Long/MIN_VALUE Long/MAX_VALUE), then when analyzing the size of the range you'll have to use a bigint (because 2^64 is not representable as a long). gen/choose should generate longs when both args are in long range, probably?
  • Is it actually possible to satisfy the requirements above without a performance hit? You'd think it was just a generator-creation-time change, but bind can blur the line between generator creation and generation time.


 Comments   
Comment by Gary Fredericks [ 12/Nov/15 7:32 AM ]

On second thought, I might be more in favor of leaving choose the way it is, discouraging its use by users, and adding a uniform-integer generator that is specifically designed to handle arbitrary ranges. A big question is what it should do in JavaScript, and I have a couple ideas about that.





[TCHECK-4] Handle sigint Created: 04/Mar/14  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

During shrinking, this should simply return the best shrink so-far. During testing, perhaps it should just return the number of tests that've been run. In both cases, some indication should be given in the return value that the user pressed ctrl-c.

Original issue - #41 on GitHub






[TCHECK-21] Rerunning a particular failure is difficult. Created: 11/Apr/14  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Test.check has the concept of a seed that is used when a property is tested. The seed can be supplied or generated by the current time. Either way, the single seed is used to generate all the trials for the test. If a test fails, the only way to try to re-test the property on the same inputs is to run all of the tests again using the same seed. If the tests were running for hours before a failure was found, this is prohibitive.

I would like instead to be able to check the property for exactly the input that failed. One step in that direction is to have an explicit different seed for each trial (perhaps generated from an initial metaseed), and on failure to report a [seed size] pair that can be used to run a test a single time.

The way this interacts with shrinking is interesting though, since it's just as likely that I'll want to re-run on the shrunk value from a failure. Since the shrunk value was never explicitly generated from a known [seed size], just that information is insufficient. We could perhaps report [seed size shrink-path] where the third element is a list of indexes to walk through the shrink tree with. Probably the worst part about that is it might be rather long.

In any case I think something like this would be more useful than the current setup. I'll probably be hacking up something similar for a fork branch I'm maintaining, but if we can settle on a specific design I'd be happy to cook up a patch.



 Comments   
Comment by Reid Draper [ 12/Apr/14 10:28 AM ]

When a test fails, we do return the arguments that originally failed, and the arguments of the shrunk test. Is this insufficient?

For example:

{:result false,
       :failing-size 45,
       :num-tests 46,
       :fail [[10 1 28 40 11 -33 42 -42 39 -13 13 -44 -36 11 27 -42 4 21 -39]],
       :shrunk {:total-nodes-visited 38,
                :depth 18,
                :result false,
                :smallest [[42]]}}

see :fail and [:shrunk :smallest].

Comment by Gary Fredericks [ 12/Apr/14 11:00 AM ]

It's certainly adequate to reproduce in theory, but I don't know of any built in mechanism that makes this easy. Here's what I end up doing:

Given this:

(defspec foo
  (prop/for-all [x gen]
    (f x)))

and after getting a failure with a 50-line shrunk value (e.g. :bar), I manually pretty-print it and paste it back into my file like so:

(def shrank
  ':bar)

(defspec foo
  (prop/for-all [x #_gen (gen/return shrank)]
    (f x)))

And now I can run (foo 1) to re-run with the failing value.

This awkwardness is partly due to three facts:

  1. My generators create large unwieldy values
  2. My property is a large body of code rather than a single function
  3. There's no easy way to test a property on a specific value

I could mitigate some of the pain by fixing #2 – having each one of my tests split into a defspec / prop/for-all and a plain function. But #1 is not easy to fix, and having a small tuple is rather nicer for me.

I've already written a POC for this and it's working rather well. I used the term :key to refer to the tuple, so the term doesn't conflict with :seed. I end up calling the test like (foo 0 :key [329489249329323 19 []]), but I'll probably think up something else that doesn't require passing a dummy first arg.

Comment by Gary Fredericks [ 03/Mar/16 10:09 AM ]

I think this kind of issue becomes more important with TCHECK-96, because if a shrink aborts prematurely it would be nice to be able to run it again without the time limit (without having to run all the prior passing tests like you would if you re-ran quick-check with the same seed).

I just had an intermediate idea for this that doesn't have the downside of the arbitrarily-long "key" that I've used in my fork: we promote the RNG to be a true serializable value, and then in quick-check we keep track of the [rng size] pair used for each trial, and on a failure we report that pair. Then we have a new function similar to quick-check that takes a property and a pair and runs a single trial (with shrinking if it fails).





[TCHECK-58] Alternate clojure.test integration Created: 05/Jan/15  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Colin Williams Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I've got a macro that integrates with clojure.test at a different level than defspec, which I've found useful in migrating tests from non-generative to generative:

(deftest something-important
  (checking "something really important" [x gen/int]
    (is (= (important-calculation x) (inc x)))))

I've found it natural to reach the above from something like this:

(deftest something-important
  (testing "something really important"
    (is (= (important-calculation 1) 2))))

There are still a lot of rough edges, but I wanted to get peoples reactions before I started polishing it for a patch.



 Comments   
Comment by Reid Draper [ 20/Jan/15 10:36 PM ]

Thanks for the comment Colin. At the moment, I'd like to keep the integration simple (meaning there is one way to do things), and keep 'alternative syntax' macros and functions out of test.check itself. Gary's test.chuck might be a good home for this.





[TCHECK-97] Support Unicode chararacters for char based generators Created: 06/Mar/16  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Matthew O. Smith Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently the default char generator is only in the range from 0 to 255. Java chars can range from \0000 to \FFFF. If this is something of interest, I will add a patch as I need to do this anyway.



 Comments   
Comment by Gary Fredericks [ 06/Mar/16 1:53 PM ]

There's definitely a need for this, but I don't think the details of a solution are obvious – in particular what distribution such a generator ought to have.

My hazy understanding of unicode is that a great many (the majority I think) of the code points are not assigned to any particular character, and so if you picked code points at random you would mostly get unprintable stuff.

I spent a while on this issue when I implemented string-from-regex in test.chuck (using a uniform distribution), and you can see the results by doing (gen/sample (com.gfredericks.test.chuck.generators/string-from-regex #".*")):

("" "󲦯" "" "򝡁񌨠񆂧" "" "􇆞򸣒𡖎򫶯" "򅬢򘏁󏉊񪦛" "z𫺑" "񷴡𽟔󙤓" "")

I could see an argument that this is the only reasonable implementation of gen/char, but I don't think that's obviously true since from a user's perspective it would mostly be annoying to see and seem rather silly.

I'm also open to arguments of the form "yes there's no good comprehensive solution to gen/char but implementation X is at least strictly better than the current implementation".

I'd be interested to have somebody research what other quickchecks (in particular haskell & python) do for this as well.

Comment by Matthew O. Smith [ 07/Mar/16 7:38 AM ]

You make some great points. I will also review the Java Character class as it seems to have some Unicode information encoded that could be put to good use.

Comment by Matthew O. Smith [ 02/Apr/16 3:44 PM ]

;;
;; Unicode support for test.check
;;
;; Unicode support is divided into 2 sections: char based and code-point/int based
;;
;; Ranges and choices
;; Ranges are a vector of range defs
;; A range def is either
;; A single character
;; A pair (vector) of the start and end of a range
;;
;; choices is a generator that choose from a vector of ranges. For example,
;; (choices [1 2 [100 200])
;; would return 1 and 2 and the numbers from 100 to 200. The members of the range pair 100 and 200 in this
;; example, can be anything accepted by choose.
;;
;;
;; The char based Unicode support mirrors the normal char and string generators
;;

Standard Generator Unicode Generator Generates
char uchar valid Unicode characters (char) from \u0000 to \uFFFF.
char-asciii uchar-alpha letter Unicode characters.
  uchar-numeric digit Unicode characters
char-alphanumeric uchar-alphanumeric letter and digit Unicode characters
string ustring Unicode strings consisting of only chars
string-alphanumeric ustring-alphanumeric Unicode alphanumeric strings.
  ustring-choices Unicode strings in the given ranges.
namespace unamespace Unicode strings suitable for use as a Clojure namespace
keyword ukeyword Unicode strings suitable for use as a Clojure keyword
keyword-ns ukeyword-ns Unicode strings suitable for use as a Clojure keyword with optional namespace
symbol usymbol Unicode strings suitable for use as a Clojure symbol
symbol-ns usymbol-ns Unicode strings suitable for use as a Clojure symbol with optional namespace

;; Code-point or int based characters

Standard Generator Unicode Generator Unicode Desc
string ustring-from-code-point Generates Unicode strings consisting of any valid code point.
char code-point Generates a valid Unicode code point
Comment by Gary Fredericks [ 10/Apr/16 5:53 PM ]

Are you thinking that these generators will generally have uniform distributions, and that the problem of mostly-unprintable-values is not a big enough problem to do anything special about?

Should the second group of generators include analogs for keyword, symbol, etc. as well?

I think anything that involves dozens of new generators I'll be inclined to put in a separate namespace.

Comment by Matthew O. Smith [ 11/Apr/16 12:00 PM ]

I listed all the new generators I was wanting to build. Basically, I want to map the normal string based generators to have similar behavior to current ones. For example, keywords and symbols have a ukeyword and usymbol for unicode keywords and symbols.

Adding the apply-to from TCHECK-99 will make it easier for people to create a Unicode string generators.

I expect the Unicode versions of the functions to have a very similar distribution to the current versions. The exception is the ones based on "choices" which distributes even across each range, regardless of the size of the range.





[TCHECK-101] Add a prop/for-all alternative to c.t.c.clojure-test that uses clojure.test/is &c Created: 10/Apr/16  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This would be similar to https://github.com/gfredericks/test.chuck/blob/9f424c474b02f6e89b423049b5c5daebf71cec9c/src/com/gfredericks/test/chuck/clojure_test.cljc#L112.

Have to think carefully about when to print clojure.test's standard failure stuff with respect to shrinking.

on the other hand

am I just wanting to do this because of the "test output is bad" problem? Is there a better solution that attacks that directly?






[TCHECK-102] Provide a mechanism for getting output from a test run after interrupting it. Created: 10/Apr/16  Updated: 06/Aug/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Related: TCHECK-4, TCHECK-8.



 Comments   
Comment by Gary Fredericks [ 10/Apr/16 5:59 PM ]

For TCHECK-4 I imagine this could only be done by some seriously sad monkeypatching.





[TCHECK-113] `lein test` crashes when clojure.test.check.clojure-test is required at runtime Created: 20/Jul/16  Updated: 03/Aug/16  Resolved: 20/Jul/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: None


 Description   

This isn't test.check's fault, but it may become common enough that it warrants accounting for somehow.

lein test monkeypatches clojure.test/report, which causes it to no longer be a multimethod.

clojure.test.check.clojure-test extends the multimethod when the namespace is loaded.

It appears the monkeypatching doesn't happen until after all namespaces are required, so this is not normally an issue.

However, clojure.spec loads some of test.check at runtime.

Although it may be that this is only a problem on versions <= 0.9.0, when clojure.test.check require clojure.test.check.clojure-test; now that that dependency has been reversed, this may not happen in practice.

If it's determined that this is common enough to do something about, we could wrap the defmethods in a check to make sure the thing is currently a multimethod, and print a warning if not.



 Comments   
Comment by Gary Fredericks [ 20/Jul/16 9:25 PM ]

Closing this since I don't expect it to be an issue worth working on. Can reopen if something changes.

Comment by Alex Miller [ 03/Aug/16 10:19 AM ]

In case someone does run into this, you can work around it by telling lein not to monkeypatch clojure-test:

:monkeypatch-clojure-test false

You will lose "lein retest" functionality, but I don't think many people use that anyways.





[TCHECK-112] bind doesn't shrink very well Created: 22/Jun/16  Updated: 26/Jun/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is a long-standing Hard Problem.

The Problem

The classic example is shrinking a 2d matrix with a poisoned element in it:

(def gen-matrix
  (gen/let [width gen/nat]
    (gen/vector (gen/vector gen/large-integer width))))

(quick-check 10000
             (prop/for-all [matrix gen-matrix]
               (->> matrix
                    (apply concat)
                    (not-any? #{42}))))
;; =>
{:result false,
 :seed 1466646880334,
 :failing-size 16,
 :num-tests 17,
 :fail [[[290 42 10 3 1 3 196]
         [-1793 3484 -5795 -206 -1 -8 464]
         [2 3 -1951 -761 -28829 5518 1]
         [-32 4477 1 -4086 0 1640 -22185]
         [-485 3156 -625 4082 -2 -845 513]
         [-3 -1 26 323 232 5 -1]
         [32 51 -1 240 -1814 0 -190]
         [2417 -4239 326 -4096 -8 1898 75]
         [-509 1 0 466 199 -1 10]
         [-23 5838 -441 30741 -6724 -1169 -171]
         [-4 3974 -1432 -4 698 -56 1210]
         [-2148 -6526 -1 453 19 -5343 461]]],
 :shrunk {:total-nodes-visited 31,
          :depth 8,
          :result false,
          ;; would be better if shrunk to [[[42]]]
          ;;
          ;; note that it's plausible the smallest value here could have the same width (7)
          ;; as the original failure -- the only reason it was able to shrink the width at
          ;; all was that it got lucky by generating an entirely new matrix with the smaller
          ;; width that happened to have 42 in it
          :smallest [[[0 42 0]]]}}

Ideas






[TCHECK-110] Enable shrinking on recursive generators Created: 22/Jun/16  Updated: 22/Jun/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Timothy Baldridge Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I wrote a AST generator for a search engine using test.check. It works quite well, but for some reason recursive generators don't shrink "vertically", instead they only shrink vertically. For exmaple, let's say this AST failed:

{:op :and
 :args [{:op :and
         :args [{:op :term ...}
                {:op :term ...}
                {:op :term ...}]}]}

After Shrinking

{:op :and
 :args [{:op :and
         :args [{:op :term ...}]}]}

The problem is in `{:op :term}` but for some reason test.check doesn't try to remove the recursive `:and` nodes. Not a big deal, but would be a nice feature to have.



 Comments   
Comment by Gary Fredericks [ 22/Jun/16 7:12 PM ]

Does this use gen/recursive-gen?

Comment by Timothy Baldridge [ 22/Jun/16 7:17 PM ]

Yes, I'm using recursive-gen.

Comment by Gary Fredericks [ 22/Jun/16 7:25 PM ]

So this looks like the same issue?

(quick-check 10000
             (prop/for-all [tree (gen/recursive-gen gen/vector gen/large-integer)]
               (->> tree
                    (tree-seq coll? seq)
                    (not-any? #{4242}))))
;; =>
{:result false,
 :seed 1466641044276,
 :failing-size 151,
 :num-tests 3152,
 :fail [[[-250312923371676 -2634403398808308]
         [-134580]
         1190117809715
         [1736827773692]
         [91379147228 281572852]
         []
         []
         [264322377680727]
         [-2 -2005122340306]
         []
         []
         [2133414023]
         []
         [-7203148411369 2093087]
         [-1 -1]
         [-350804570003194 -24726]
         [-2145238760990835556]
         [-4410884650149229158 27914810]
         []
         [21126727]
         [816412492 102]
         [1]
         [-119 -2132126120873]
         [50]
         [1590594626470485464 -555554916273244]
         [4242 322325]]],
 :shrunk {:total-nodes-visited 57,
          :depth 11,
          :result false,
          ;; should have shrunk to `4242`
          :smallest [[[4242]]]}}
Comment by Gary Fredericks [ 22/Jun/16 7:36 PM ]

I'm not 100% sure there's a clean solution to this, but note to self: shrinking to sub-trees should be the first thing we try

Comment by Gary Fredericks [ 22/Jun/16 8:58 PM ]

If TCHECK-112 ends up being figured out it might apply here too.





[TCHECK-109] clojure.test example in README out of date Created: 16/Jun/16  Updated: 16/Jun/16  Resolved: 16/Jun/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Adam Frey Assignee: Gary Fredericks
Resolution: Declined Votes: 0
Labels: documentation

Attachments: Text File fix-clojure-text-example.patch    
Patch: Code

 Description   

The `defspec` function signature was changed in this commit https://github.com/clojure/test.check/commit/9873cdbcb12ba84ae96b93143a0f5e63f89e3317 leading up to v0.8.0 but the
README was not updated to reflect the change.

I also removed the over-obvious comment and fixed the indentation.



 Comments   
Comment by Adam Frey [ 16/Jun/16 3:18 PM ]

I just realized that I misread the new function signature and the current README example is not invalid, as I previously thought. Feel free to close.





[TCHECK-80] last-trial-report (used by trial-report-periodic) is not being reset at :begin-test-var in cljs Created: 22/Sep/15  Updated: 06/Jun/16  Resolved: 06/Jun/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Nicolás Berger Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0

Attachments: Text File TCHECK-80.patch    
Patch: Code

 Description   

The dispatch value of the cljs version of the defmethod that resets the last-trial-report atom for trial-report-periodic is wrong. It should be [::ct/default :begin-test-var] but it's [::ct/default :begin-test].

This issue was introduced during the conversion of the clojure.test.check.clojure-test ns to .cljc

I haven't tried running a test with trial-report-periodic, but I understand that this issue makes it to show the first report at the first trial (because last-trial-report starts at 0), and then it reports it as expected. And it only affects the clojurescript version, so the impact doesn't seem to be very high.



 Comments   
Comment by Gary Fredericks [ 15/Apr/16 1:28 PM ]

Can we also add a test that catches the original bug?

Comment by Nicolás Berger [ 15/Apr/16 1:47 PM ]

Yes, I'll add a test. By the way, this issue is also fixed by http://dev.clojure.org/jira/browse/TCHECK-103, so if the other one is going to be merged, this one can be declined. But still, the test is a nice addition in both tickets.

Comment by Nicolás Berger [ 16/Apr/16 1:40 PM ]

Replaced the patch with a new one that includes a cljs implementation for the test about report-trial-periodic, and also a modification of the test to verify the issue on :begin-test-var

Comment by Gary Fredericks [ 06/Jun/16 4:42 PM ]

Applied the patch to master; thanks!





[TCHECK-61] defspec throwing errors in cljs Created: 18/Mar/15  Updated: 06/Jun/16  Resolved: 06/Jun/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Viktor Eriksson Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: release-0.10.0
Environment:

OSX



 Description   

Hi,

I have the following issue:

I tried to integrate test.check into my current cljs.test setup. The generators are working like a charm but I have issues with the defspec macro. I get the an error when trying the example from: https://github.com/clojure/test.check#cljstest-integration

(defspec first-element-is-min-after-sorting ;; the name of the test 
         100 ;; the number of iterations for test.check to test 
         (prop/for-all [v (gen/not-empty (gen/vector gen/int))] 
                       (= (apply min v) 
                          (first (sort v)))))

The error is the following:

ERROR in (first-element-is-min-after-sorting) (TypeError:NaN) 
Uncaught exception, not in assertion. 
expected: nil 
  actual: 
#<TypeError: 'undefined' is not an object (evaluating 'f.cljs$lang$maxFixedArity')>

I saw that there is a function called for-all* as well and tried it but it didn't work either, but I got a different error:

ERROR in (first-element-is-min-after-sorting) (Error:NaN) 
Uncaught exception, not in assertion. 
expected: nil 
  actual: 
#<Error: Assert failed: Args to tuple must be generators 
(every? generator? generators)>

Any ideas what the issue might be? Anyone that successfully ran test.check in cljs with defspec?

The tests are run with phantomjs using this example: https://gitlab.com/keeds/cljsinit and my ns requires the following:

[cljs.test.check.generators :as gen] 
[cljs.test.check.cljs-test :as ct :refer-macros [defspec]] 
[cljs.test.check.properties :as prop :include-macros true] 
[cljs.test :as test :refer-macros [deftest testing is use-fixtures]] 

versions: 
[org.clojure/test.check "0.7.0"] 
[org.clojure/clojurescript "0.0-2843"]


 Comments   
Comment by Alex Engelberg [ 30/May/15 4:38 PM ]

defspec's macroexpansion includes (apply cljs.test.check/quick-check ...). However, you are not requiring cljs.test.check in your namespace. And that specific error arises whenever you try to call apply on an undefined function. Therefore, the workaround is to simply add (:require cljs.test.check) at the top of your namespace.

Could we prevent this error from happening in general by simply adding (:require cljs.test.check) to the top of cljs/test/check/cljs_test.cljs?

Comment by Gary Fredericks [ 30/May/15 6:01 PM ]

My guess is that this is basically the same thing as TCHECK-33 (except in clojurescript), so hopefully both can be fixed together.

Comment by Gary Fredericks [ 30/May/15 6:02 PM ]

in the CLJ side simply fixing it with a require as you suggest isn't possible because it would create a circular dependency. So there's some larger refactoring that needs to happen.

Comment by Gary Fredericks [ 30/May/15 6:02 PM ]

adding more details on TCHECK-33

Comment by David Nolen [ 01/Jun/15 8:49 AM ]

Just chiming in to agree that this indeed looks like the ClojureScript version of TCHECK-33.

Comment by Unnikrishnan Geethagovindan [ 13/Apr/16 1:54 AM ]

I tried this out in cljs repl and it seemed to be working fine.

(ns cljs.user
  (:require [clojure.test.check :as tc]
            [clojure.test.check.generators :as gen]
            [clojure.test.check.properties :as prop :include-macros true]
            [clojure.test.check.clojure-test :as ct :include-macros true]))

(ct/defspec first-element-is-min-after-sorting 
	    100 
	    (prop/for-all [v (gen/not-empty (gen/vector gen/int))] 
			  (= (apply min v) 
			     (first (sort v)))))

(first-element-is-min-after-sorting)
{:result true, :num-tests 100, :seed 1460529061530}
Comment by Gary Fredericks [ 13/Apr/16 7:58 AM ]

Unnikrishnan, could you also verify that your code fails on 0.9.0?

Comment by Unnikrishnan Geethagovindan [ 13/Apr/16 11:41 AM ]

I actually ran it on [org.clojure/test.check "0.9.0"].

Comment by Gary Fredericks [ 16/Apr/16 11:25 AM ]

I think it's supposed to fail in 0.9.0, since the fix for this didn't come until 550a459.

Maybe if you remove the [clojure.test.check :as tc] require (which is the workaround suggested in the first comment on this ticket) it will fail?

Comment by Gary Fredericks [ 06/Jun/16 3:23 PM ]

I've confirmed that this is fixed on master.





[TCHECK-106] Variable-sized collection generators have exponential sizing issues when composed Created: 06/Jun/16  Updated: 06/Jun/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

E.g., this sort of code can OOM:

(-> gen/boolean 
    gen/vector
    gen/vector
    gen/vector
    gen/vector
    gen/vector
    gen/vector
    gen/generate)





[TCHECK-73] gen/int can generate doubles Created: 17/Jun/15  Updated: 06/Jun/16  Resolved: 06/Jun/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File 0001-TCHECK-73-Ensure-that-gen-int-returns-a-long.patch    
Patch: Code and Test

 Description   

gen/int will generate doubles if the size parameter is a double, which is possible when using gen/resize in a sloppy manner.

(gen/generate gen/int 20) => -9
(gen/generate gen/int 20.0) => 5.0


 Comments   
Comment by Justin Holguin [ 16/Aug/15 2:21 PM ]

This issue causes gen/int to always return a double within a recursive generator. I'm attaching a patch that fixes the behavior, please let me know what you think. Also, please note that I have signed the Clojure CA.

As always, thanks very much to the authors/maintainers for a fantastic library!

Comment by Gary Fredericks [ 16/Aug/15 9:29 PM ]

Dangit, I reported this but can't remember if it pertained to 0.7.0 or not.

It sounds like this is a regression in the 0.8.0 release (vs 0.7.0), is that right?

Comment by Justin Holguin [ 16/Aug/15 9:35 PM ]

Yes, exactly. I noticed this as soon as I tried to upgrade to 0.8.0, then bisected to narrow it down to this commit that went in between 0.7.0 and 0.8.0.

Comment by Gary Fredericks [ 17/Aug/15 9:44 AM ]

Okay, I suspect this is something I meant to get into 0.8.0 but forgot about. I'll plan on a 0.8.1 once I can review this.

Comment by Gary Fredericks [ 17/Aug/15 9:35 PM ]

Thanks for tracking this down and putting together such a clear patch. This is released with 0.8.1.

Comment by Steve Miner [ 22/Aug/15 4:23 PM ]

Sorry, I'm late to the party on this one, but I have an issue with the change introduced with this patch. You will lose accuracy at extreme values because you're now doing double math where the previous code was expecting long math. In particular, consider a range between (- Long/MAX_VALUE 10) and Long/MAX_VALUE. Intuitively, the difference is 10, but it's not if you convert to doubles (as with multiplication by a double factor).

(- (Long/MAX_VALUE) (- Long/MAX_VALUE 10))
;=> 10
(- (Long/MAX_VALUE) (double (- Long/MAX_VALUE 10)))
;=> 0.0

When I wrote that rand-range (as in 0.8), I assumed that the lower and upper were longs. If you want to allow doubles for these values, you should cast to long up front. Or change the upstream code appropriately. Otherwise, the math doesn't work right. If you want to allow doubles outside the long range, you need to reconsider the whole thing.

Comment by Steve Miner [ 22/Aug/15 4:26 PM ]

This patch introduces double math where it should be long math and will cause a severe lack of randomness with extreme values.

Comment by Steve Miner [ 22/Aug/15 4:27 PM ]

I'll come up with a test to demonstrate the problem tomorrow. Maybe I should open a new issue since this patch has already been released in 0.8.1.

Comment by Gary Fredericks [ 22/Aug/15 7:48 PM ]

Yes, new ticket please.

Thanks for catching that Steve. I'm looking forward to seeing a test that would have failed if we'd had it .

Comment by Steve Miner [ 23/Aug/15 1:34 PM ]

New ticket: TCHECK-77

Comment by Steve Miner [ 23/Aug/15 1:41 PM ]

Just for reference a test like this would have caught the problem, or at least raised suspicion. We don't expect all generated longs between (- Long/MAX_VALUE 100) and Long/MAX_VALUE to be exactly Long/MAX_VALUE. We want some randomness in the choice of those 100 longs. In 0.8.1, the following fails. The inappropriate double math ends up always picking Long/MAX_VALUE or Long/MIN_VALUE even when you would expect a reasonable spread of values.

;; regression introduced by TCHECK-73 patch
(deftest randomness-at-extremes
  (testing
      "Should have some randomness with extreme bounds"
    (let [upper Long/MAX_VALUE
          lower Long/MIN_VALUE]
      (is (not (every? #(= % upper) (gen/sample (gen/choose (- upper 100) upper) 1000)))
          (str "Should not always return " upper))
      (is (not (every? #(= % lower) (gen/sample (gen/choose lower (+ lower 100)) 1000)))
          (str "Should not always return " lower)))))

I will put a new patch in TCHECK-77.

Comment by Gary Fredericks [ 23/Aug/15 1:47 PM ]

We do currently have at least one statistical test (https://github.com/clojure/test.check/blob/78bbef770911d72c83d4d5bc0474f8f773afae88/src/test/clojure/clojure/test/check/test.clj#L512-523), and I'm happy to have more – I think as long as you could make a hand-wavy statistical argument that the test shouldn't fail before the end of the universe, it's a good test. And my guess is that this one is in that category.

Comment by Gary Fredericks [ 06/Jun/16 2:45 PM ]

I just came across this again and as far as I can tell there's no reason for it to still be open.





[TCHECK-99] Create a generator that will limit the size of a generated sequence Created: 30/Mar/16  Updated: 06/May/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Matthew O. Smith Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File TCHECK-99-2.patch     Text File TCHECK-99-3.patch     Text File TCHECK-99-4.patch     Text File TCHECK-99-5.patch     Text File TCHECK-99.patch    

 Description   

There are a lot of generators that return a sequence (list, vector, tuple, map, string, etc). Sometimes it us useful to set size limits, either min or max, on the generated sequence. It would be nice to have a generator that would accept a sequence generator and ensure that the sequences are of a certain length. Three examples would be:
(of-length min max gen)
(of-max-length max gen) => (of-length 0 max gen)
(of-min-length min gen) => (of-length min nil gen)

of-length check the length of the generated sequence. If it is too small, use (take min (cycle s)) to extend the length of the sequence.
If it is too long, use (take max s) to return the max length
It will need to be careful to return the same type it received.
If it does not receive a sequence, treat it as though it was one element sequence.
If min is not 0, use such-that not nil to ensure a proper seq is generated.



 Comments   
Comment by Gary Fredericks [ 30/Mar/16 5:49 PM ]

Most of the collection generators already accept options for controlling their size; is there a use case you have in mind where those controls are not sufficient?

Comment by Matthew O. Smith [ 30/Mar/16 7:22 PM ]

I completely missed those that take the :min and :max options

In particular I was looking at the string based generators as I need to populate database tables and so the strings have to match the column definition like not null and varchar (15). As I was thinking about the problem it seemed like writing a composable generator to enforce length restraints might make more sense than trying to retro fit all the generators that produce sequence-like structures.

A short check of generators.cljc shows list, hash-map and even the array based ones could use min and max options.

Comment by Matthew O. Smith [ 31/Mar/16 9:54 AM ]

If I try to update string from a def to a defn in an attempt to allow multiple arguments for sizes like

(defn string 
  ([]  (fmap clojure.string/join (vector char)))
  ([size]  (fmap clojure.string/join (vector char size)))
  ([min max]  (fmap clojure.string/join (vector char min max))))

Then (sample string) no longer works but (sample (string)) (sample (string 5)) (sample (string 3 7)) all work

Is there a trick to getting a just "string" to work? Or is there a better way to modify string to accept multiple arugments?

Comment by Gary Fredericks [ 31/Mar/16 10:32 AM ]

I don't think there's any clean approach to solving the problem you noted, which is one reason that I don't see an obvious solution here.

I faced this issue when creating the large-integer and double generators, and I decided to create two generators: large-integer is a generator with default behavior, and large-integer* is a function that takes options and returns a generator.

In hindsight I don't know if this was the best choice, since it's confusing. I asked David MacIver about how he handles this in hypothesis (a similar python library) and he said he doesn't have any raw generators, just functions that return generators (sometimes with 0 arguments). I like the uniformity of that approach, but it would obviously be a big breaking change for test.check (though there are some hacky tricks that could preserve backwards compatibility).

That issue is a bit bigger than this ticket. With respect to the code you showed, I think I'd prefer an API closer to what gen/set has, with an options map rather than positional args.

Now that you've got me thinking about this, I'm starting to get fixated on the idea of a big breaking API change that uses hacks to preserve backwards compatibility for a few versions, for the sake of cleaning up a lot of inconsistencies. But again, that's bigger than this ticket.

If we can't come up with a clean short-term approach, my standards for accepting things are much more relaxed at test.chuck ☺.

Comment by Matthew O. Smith [ 31/Mar/16 11:22 AM ]

Which do you prefer? adding something like of-length to limit existing seq generators or create (def string ...) and (defn string* ...)? I am just finishing up the Unicode implementation for TCHECK-97 as well so I'll follow which ever pattern we decide.

Comment by Gary Fredericks [ 31/Mar/16 1:28 PM ]

Don't we have at least three generators for strings? Are you thinking of having alternatives to each one?

Comment by Matthew O. Smith [ 31/Mar/16 3:21 PM ]

My very thought for making this request. Do we really want to double the number of string generators? Not to mention other things like arrays etc.

Another thought would be to add a single new generator, to-string, that can take a different character generator and use it to make strings based on an element generator:

(defn to-string
   ([element-gen] (to-string element-gen {})))
   ([element-gen [{:keys [num-elements min-elements max-elements max-tries ] :or {num-elements nil min-elements 0 max-elements nil max-retries 10}}]
                   (fmap clojure.string/join (vector char-gen ==apply-options==))))

I left out the code to handle the options properly but hopefully you get the idea.

It would be called like (to-string char {:min-elements 5 :max-elements 10}) or (to-string char-ascii {:num-elements 5})

This way it would be possible to add new character generators that could easily be converted to strings.

Comment by Matthew O. Smith [ 31/Mar/16 4:52 PM ]

Looking at it, coll-distinct-by almost does exactly what I need, except the allow-dups? flag does not do what I was expecting.

(sample (coll-distinct-by [] identity true false (elements [\a \b \c \d \e ]) {:num-elements 4})) always return distinct elements even though allow-dups? is true

Comment by Gary Fredericks [ 31/Mar/16 6:05 PM ]

coll-distinct-by is pretty specialized and probably not relevant here.

I think you could translate `[num-elements min-elements max-elements]` to the args taken by `gen/vector` pretty easily. `max-retries` shouldn't apply.

Something like to-string might be good, especially if it fits with whatever you're thinking about for TCHECK-97 (e.g., TCHECK-97 provides args to use with to-string).

Comment by Matthew O. Smith [ 31/Mar/16 7:50 PM ]

A patch generated with git --no-pager diff master feature/TCHECK-99 on https://github.com/m0smith/test.check.git

Comment by Matthew O. Smith [ 31/Mar/16 7:54 PM ]

Ignore that patch. I am way out of date with master. New patch on its way

Comment by Matthew O. Smith [ 31/Mar/16 8:01 PM ]

TCHECK-99-2.patch was generated after syncing test.check with my fork. Should be the latest.

Comment by Matthew O. Smith [ 01/Apr/16 10:34 AM ]

I also had a thought on how to allow a function to be treated like a generator. Add a :generator-factory meta data to a function like

(defn to-string
  "Generates strings of element-gen, default to char"
   {:generator-factory to-string}
   ([] (to-string char {})))
   ([element-gen] (to-string element-gen {})))
   ([element-gen [{:keys [num-elements min-elements max-elements]}]
                   (fmap clojure.string/join (vector char-gen ==apply-options==))))

The generator? could then be updated to accept anything with a :generator-factory meta data
call-gen could be updated to look for the new meta data and if there, call the associated function and use the result as the Generator.

Comment by Gary Fredericks [ 01/Apr/16 10:50 AM ]

yeah, that's basically the idea I had for preserving backward compatibility – the metadata generator would have some sort of deprecation warning attached somehow.

Comment by Matthew O. Smith [ 01/Apr/16 11:00 AM ]

Also, on the to-string, clojure.string/join supports a seperator which we be easily added as an additional option like:

(to-string int {:sep ","})

If you are ok with it, I will update the patch.

Comment by Matthew O. Smith [ 02/Apr/16 3:41 PM ]

TCHECK-99-3.patch is updated to support a :sep option. Also, had it based upon a new function 'apply-to' that will be useful for TCHECK-97 as well. See https://github.com/m0smith/test.check/tree/feature/TCHECK-97 for the proposed Unicode implementation

Comment by Matthew O. Smith [ 03/Apr/16 12:57 PM ]

This one has support for suffix and prefix. Using this one makes thing like HTML much easier

Comment by Matthew O. Smith [ 03/Apr/16 4:25 PM ]

Here is an example of how to-string would be used to simulate the lines in a tab-separated file

(def gen-char-upper (gen/fmap char (gen/choose \A \Z)))
(def gen-char-lower (gen/fmap char (gen/choose \a \z)))
(def gen-char-digit (gen/fmap char (gen/choose \0 \9)))
(def gen-char-postal-format (gen/fmap char (gen/one-of [(gen/return \#) 
                                                        (gen/return \@)
                                                        gen-char-upper])))

(def gen-iso-2 (gen/to-string gen-char-upper {:num-elements 2}))
(def gen-iso-3 (gen/to-string gen-char-upper {:num-elements 3}))
(def gen-iso-numeric (gen/to-string gen-char-digit {:num-elements 3}))
(def gen-fips (gen/to-string gen-char-upper {:num-elements 2}))
(def gen-country gen/string)
(def gen-capital gen/string)
(def gen-area  (gen/large-integer* {:min 100 :max 999999999}))
(def gen-population (gen/large-integer* {:min 100 :max 999999999}))
(def gen-continent (gen/elements ["AS" "NA" "SA" "EU" "OC" "AF"]))
(def gen-tld (gen/to-string gen-char-lower {:num-elements 2 :prefix "."}))
(def gen-currency-code (gen/to-string gen-char-upper {:num-elements 3}))
(def gen-currency-name gen/string)
(def gen-phone  (gen/large-integer* {:min 1 :max 999}))
(def gen-simple-language (gen/to-string gen-char-lower {:num-elements 2}))
(def gen-language (gen/one-of [gen-simple-language 
                               (gen/to-string [gen-simple-language (gen/return "-") gen-iso-2])]))
(def gen-languages (gen/to-string gen-language {:sep ","}))
(def gen-geonameid (gen/large-integer* {:min 1 :max 999999999}))
(def gen-neighbors (gen/to-string gen-iso-2 {:sep ","}))
(def gen-equivilent-fips (gen/to-string gen-char-upper {:num-elements 2}))

(defn to-postal-regex [postal-format]
  (str "^" (clojure.string/join (map #(cond (= \# %) "\\d" (= \@ %) "[A-Z]" :default %) postal-format)) "$"))

(def gen-country-line 
  (gen/let [postal-format (gen/to-string gen-char-postal-format {:min-elements 2 :max-element 9})]
    (gen/to-string [gen-iso-2 gen-iso-3 gen-iso-numeric gen-fips
                    gen-country gen-capital gen-area gen-population gen-continent
                    gen-tld gen-currency-code gen-currency-name gen-phone
                    (gen/return postal-format) (gen/return (to-postal-regex postal-format))
                    gen-languages gen-geonameid gen-neighbors gen-equivilent-fips
                    ] {:sep \tab})))
Comment by Gary Fredericks [ 10/Apr/16 5:08 PM ]

If I had been trying to write a generator like gen-country-line I probably would have combined gen/tuple and string/join, e.g.:

(gen/let [fields (gen/tuple gen-iso-2 gen-iso-3 ... gen-equivilent-fips)]
  (string/join \tab fields))

it looks like you wrote `gen/to-string` to accept either a generator or a vector of generators? I don't like that kind of ambiguity generally, and I think the example used of gen/let just above demonstrates that you can live without the vector feature without much pain.

Is the :sep feature still as compelling without the vector feature? I'm a little unsure about :sep, at least inasmuch as I'm hesitant to add features in general without a strong reason to.

Comment by Matthew O. Smith [ 11/Apr/16 4:42 PM ]

I see what you are saying and that makes a lot of sense. It would be good to include that in the docs as an example.

The :sep is used between the fields of the result regardless if one generator or multiple are provided. It makes it nice to create a comma separated list of integers (to-string int {:sep ","})

Comment by Matthew O. Smith [ 06/May/16 3:09 PM ]

I removed all the extra options to to-string. I also added an example of creating an XML generator.





[TCHECK-95] quick-check should report how long it ran Created: 29/Feb/16  Updated: 22/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0

Attachments: Text File add_time_to_result_map.patch    

 Description   

Just an extra key in the return value, showing milliseconds that it was running.

Could maybe also add a key to the shrunk map saying how long it spent shrinking.



 Comments   
Comment by Unnikrishnan Geethagovindan [ 20/Apr/16 2:55 AM ]

Adding

  • :time-elapsed "100 ms" for success
  • :failed-after "2 ms" for failure
    • :time-spent-shrinking "2 ms" for time spent shrinking

or would a common key :elapsed "10 ms" would be better for all the 3 cases ?

Comment by Nicolás Berger [ 22/Apr/16 8:34 AM ]

I think this feature could be implemented as a reporter-fn. It doesn't even need to be in the c.t.check or c.t.c.clojure-test namespace.





[TCHECK-103] Remove dependency of default-reporter-fn on ct/report, by removing the defmethod's in c.t.c.clojure-test namespace Created: 12/Apr/16  Updated: 18/Apr/16  Resolved: 17/Apr/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicolás Berger Assignee: Gary Fredericks
Resolution: Declined Votes: 0
Labels: None

Attachments: Text File TCHECK-103.patch    
Patch: Code

 Description   

The current implementation of default-reporter-fn is based on reporting via clojure.test/report. That's the reason behind 2 of the 3 defmethod ct/report ... in the c.t.c.clojure-test namespace: the ones for ::trial and ::shrinking type. We could easily remove them by inlining its implementation into default-reporter-fn.

I think this is an acceptable breaking change: the new :reporter-fn mechanism is a much better way of hooking into the test.check reporting, and those defmethod's are basically an implementation detail of how reporting was done before. The breaking change would only affect those using an alternative ct/report with explicit cases for ::trial and :shrinking types. I suspect there would not be many users affected.

We could also get rid of the (defmethod ct/report :begin-test-var in the same namespace by adding a new call to reporter-fn with :type :starting at the beginning of the quick-check loop.

One advantage of getting rid of these defmethods is that they don't play well for vim-fireplace users (like myself): vim-fireplace binds ct/report to a custom function (not a multimethod) when running tests, so the defmethods throw an ugly exception when trying to run tests that use test.check in the REPL with vim-fireplace. The related code is in https://github.com/tpope/vim-fireplace/blob/233f023cdde372e1124b77940a30cb0eb6d06a13/plugin/fireplace.vim#L1655-L1666. I think vim-fireplace is not doing anything wrong here: ct/report has :dynamic true for good reason, and nothing dictates it has to be bound to a multimethod, so assuming that ct/report is a multimethod creates this issue, which I think we can get rid of.



 Comments   
Comment by Nicolás Berger [ 12/Apr/16 10:15 PM ]

Patch attached

Comment by Gary Fredericks [ 17/Apr/16 1:42 PM ]

Discussed this out of band and decided it was vim-fireplace's fault for compiling user code while clojure.test/report is rebound.

Comment by Nicolás Berger [ 18/Apr/16 4:34 PM ]

For the record: the issue in vim-fireplace was fixed in https://github.com/tpope/vim-fireplace/pull/268





[TCHECK-59] s-pos-int and s-pos-int generators have an off-by-1 issue Created: 22/Jan/15  Updated: 17/Apr/16  Resolved: 17/Apr/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jay A. Patel Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0
Environment:

Clojure 1.6.0
test.check 0.6.2


Attachments: Text File final_fix_docstring.patch     Text File fix_docstring.patch    

 Description   

=== Problem ===

gen/s-pos-int and gen/s-neg-int do not strictly respect size parameter. Generated values can be off-by-1 from the size value.

=== Samples ===

test=> (gen/sample (gen/resize 5 gen/s-pos-int))
(6 1 3 2 2 1 3 6 6 3)

test=> (gen/sample (gen/resize 5 gen/s-neg-int))
(-1 -6 -2 -6 -5 -3 -2 -1 -4 -3)

=== Root Cause ===

(def s-pos-int
"Generate strictly positive integers bounded by the generator's `size`
parameter."
(fmap inc nat))

(def s-neg-int
"Generate strictly negative integers bounded by the generator's `size`
parameter."
(fmap dec neg-int))

=== Suggestions ===

Instead of using fmap, use such-that to remove the 0 value?



 Comments   
Comment by Jay A. Patel [ 22/Jan/15 12:24 AM ]

I didn't change the priority value. Not a major issue for me.

Comment by Unnikrishnan Geethagovindan [ 13/Apr/16 8:07 AM ]

I don' think using such-that would be a good replacement, because for each value the pred fails, it will re-try with an incremented size - which kind of defeats the purpose of resize.

If we are to use such-that then we would have to modify the such-that-helper to avoid incrementing the size for the retry here

(recur max-tries pred gen (dec tries-left) r2 (inc size))

removing the inc would work, but I don't know if that would break the functionality of such-that (my understanding is, it shouldn't).

Comment by Gary Fredericks [ 13/Apr/16 9:19 AM ]

Removing the inc in such-that would break it in subtle ways. It increments the size to increase the variety of things generated, hopefully increasing the probability that it finds something that passes the predicate.

I'm thinking maybe a docstring rewrite would be better, especially since the docstring as is is probably nonsensical when size is zero.

The docstrings could be rewritten to include the qualifier "roughly", or perhaps not to reference size at all and refer to the differences from pos-int and neg-int respectively.

Comment by Unnikrishnan Geethagovindan [ 13/Apr/16 11:32 AM ]

Sounds good.

I'm not sure if this would be ideal but alternatively, we could implement resize n gen which would bind the size to n - 1. What do think ?

Comment by Gary Fredericks [ 13/Apr/16 11:40 AM ]

that was my first idea, but that's the one that gets weird when size = 0.

Comment by Unnikrishnan Geethagovindan [ 13/Apr/16 11:50 AM ]

Wouldn't enforcing a validation or throwing an proper exception, that size has to > 0, be a way this could be considered ?

Comment by Gary Fredericks [ 13/Apr/16 6:55 PM ]

No, users don't control size except as an opt-in thing – in a normal test run the size ranges from 0 to 200, so all the normal generators need to expect that.

Comment by Unnikrishnan Geethagovindan [ 14/Apr/16 2:53 AM ]

Yea, makes sense.

I guess we could use the following docstring :

"Generate strictly positive integers i.e excluding zero. Ranging between 1 and `size + 1`"

Comment by Gary Fredericks [ 16/Apr/16 11:38 AM ]

Okay, a patch for a docstring change like that would be good.

How about "Generates strictly positive integers bounded by `size` + 1"

Comment by Unnikrishnan Geethagovindan [ 16/Apr/16 11:18 PM ]

Attaching patch - final_fix_docstring.patch

Comment by Gary Fredericks [ 17/Apr/16 1:11 PM ]

Applied on master, thanks!





[TCHECK-91] On test completion, reporter-fn should be called with :type :complete Created: 27/Feb/16  Updated: 16/Apr/16  Resolved: 16/Apr/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nicolás Berger Assignee: Nicolás Berger
Resolution: Completed Votes: 0
Labels: release-0.10.0

Attachments: Text File add-reporter-fn-events.patch     Text File improve-shrink-step-events.patch    
Patch: Code

 Description   

The :complete call could be used as a hook for things like printing generator statistics (TCHECK-87), or simply notifying the completion of a test execution



 Comments   
Comment by Gary Fredericks [ 28/Feb/16 6:58 PM ]

I was talking to Colin about integration with cursive and we decided that it would be most helpful if the events to reporter-fn were more comprehensive, along these lines. I think it'd also be nice to have an event after each smaller shrunken value is found, and maybe an event when shrinking is done as well.

Comment by Nicolás Berger [ 28/Feb/16 7:22 PM ]

Cool, sounds good to me

Comment by Nicolás Berger [ 05/Mar/16 10:43 AM ]

Replaced the patch with a new one that also adds :shrink-step and :shrunk events, so the new events are:

:complete: called when a successful quick-check is done
:shrink-step: called on each step of the shrink loop
:shrunk: called as soon as the shrink loop is done

Comment by Nicolás Berger [ 11/Mar/16 8:24 AM ]

Updated the patch to sync it with current master after merge of TCHECK-92

Comment by Gary Fredericks [ 12/Mar/16 8:06 AM ]

Did you intentionally write the :shrink-step part so that it would report at each step rather than just the failing ones?

I can imagine a use for having everything reported, but maybe we need more info in the map about e.g. A) whether the test case passed or failed (which isn't exactly the same as the :result key), and B) whether it's a new "smallest" case (which I don't think is true on every failing case).

Comment by Gary Fredericks [ 12/Mar/16 8:10 AM ]

I'm going to apply this once the tests pass and we can adjust the :shrink-step stuff if needed separately.

Comment by Gary Fredericks [ 12/Mar/16 8:16 AM ]

Applied the patch on master to help unblock the stats work; I'll leave this ticket open for :shrink-step.

Comment by Nicolás Berger [ 12/Mar/16 8:44 AM ]

Thanks! I should be able to upload a new patch for stats soon, having this merged helps indeed.

About :shrink-step, it was not intentional to report each and every step "as is", but more like an oversight. I like the idea of adding whether it passed or failed, and whether it's a new smallest case. I'll try to add that as an additional patch here, if you don't do it before

Comment by Nicolás Berger [ 13/Mar/16 9:25 PM ]

Added a patch improving the shrink-step events, by adding two new keys into it: :pass? and :current-smallest. Includes a test to check its behavior.

Comment by Gary Fredericks [ 16/Apr/16 9:00 AM ]

Just applied on master – I loved the test, thanks!





[TCHECK-104] Rose join NPE on nil root Created: 14/Apr/16  Updated: 15/Apr/16  Resolved: 15/Apr/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Darrick Wiebe Assignee: Gary Fredericks
Resolution: Declined Votes: 0
Labels: bug, patch

Attachments: Text File tcheck-104.patch    

 Description   

In system.check, I build a rose tree with a specialized shrink operation[1] in which not every possible shrinking is valid. In those cases, the shrink produces a (rose/pure nil). This patch fixes a crash in rose/join when that scenario is encountered.

[1]: https://github.com/pangloss/system.check/blob/da3fd21ef38e6779abb56ff0a54c42c9bd4d8d7d/src/macroscale/system/check.clj#L57



 Comments   
Comment by Gary Fredericks [ 14/Apr/16 11:20 PM ]

at a glance this doesn't strike me as being consistent with the haskellian-monadic spirit of the rose tree code, so I'll need to understand better what it is about your use case that made the combination of a nil root being passed to rose/join (which intentionally assumes all the elements are rose trees) necessary.

Comment by Darrick Wiebe [ 14/Apr/16 11:36 PM ]

If you can consider nil to be the monadic zero of a rose tree, then I think the spirit holds. I think that's also pretty well in line with the general philosophy behind Clojure's nil punning. Is there another zero value that I've somehow missed?

Comment by Gary Fredericks [ 15/Apr/16 9:24 AM ]

I don't think monads in general have a zero (in contrast with monoids or MonadPlus) – e.g., generators are a monad too, and unless I'm crazy they don't have a zero. So rose trees feel the same to me (e.g., the equivalent haskell code doesn't seem to have a zero either).

Comment by Darrick Wiebe [ 15/Apr/16 9:55 AM ]

Oh yes I suppose I was thinking of MonadPlus. In that case I'll just change my code to return (rose/pure nil) rather than simply nil. That felt to me like a workaround, but if it is the intended behaviour, that's fine.

Comment by Gary Fredericks [ 15/Apr/16 11:06 AM ]

Okay, sounds good.





[TCHECK-94] Collections don't shrink very aggressively Created: 29/Feb/16  Updated: 14/Apr/16  Resolved: 14/Apr/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

Collections / strings / etc. can potentially shrink a lot faster.

I've already written some code for this that I think works: https://github.com/clojure/test.check/compare/master...gfredericks:fast-collection-shrinking



 Comments   
Comment by Gary Fredericks [ 14/Apr/16 3:41 PM ]

Fixed in 1f1e4fc.





[TCHECK-100] --test ticket-- Created: 05/Apr/16  Updated: 14/Apr/16  Resolved: 14/Apr/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: None


 Description   

this is not a real ticket; just testing jira stuff



 Comments   
Comment by Gary Fredericks [ 05/Apr/16 9:54 PM ]

wasn't a real ticket

Comment by Gary Fredericks [ 14/Apr/16 3:37 PM ]

does this work? jira I'm having trouble closing TCHECK-94

Comment by Gary Fredericks [ 14/Apr/16 3:37 PM ]

huh. I guess it works here at least.





[TCHECK-89] the clojure-test namespace has a global atom Created: 23/Feb/16  Updated: 14/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: release-0.10.0

Attachments: Text File remove_global_atom.patch    

 Description   

There's a global atom in the clojure-test namespace, for tracking something or other across calls to the report functions I think.

This might be problematic for an implementation of parallel tests. I'm not sure, I think it would depend on the details of that implementation.



 Comments   
Comment by Nicolás Berger [ 05/Apr/16 10:24 PM ]

The atom is there as part of the trial-report-periodic implementation, it's not used for anything else. I think we could reimplement trial-report-function under the new reporter-fn mechanism, with a function that encloses the atom, thus avoiding having a global atom.

Comment by Unnikrishnan Geethagovindan [ 12/Apr/16 2:16 AM ]

The atom being set when `:being-test-var` is called and used in the `trial-report-periodic` ? Wouldn't the atom have to be global for it to work ?

This might be a silly question (considering I'm fairly new to clojure), How can this be implemented without the atom being global ? If someone can point me in the right direction, I can try to fix this.

Comment by Nicolás Berger [ 12/Apr/16 7:30 AM ]

The atom being set when `:being-test-var` is called and used in the `trial-report-periodic` ? Wouldn't the atom have to be global for it to work ?

You are totally right: to reset the atom in :begin-test-var the atom needs to be global. But if we find how to reset the atom in a way that it doesn't need to be global, we could make it non-global . I'm thinking of something like:

(defn make-periodic-reporter-fn
  ([] (make-periodic-reporter-fn *trial-report-period*))
  ([report-period] (make-periodic-reporter-fn report-period default-reporter-fn))
  ([report-period reporter]
   (let [last-trial-report (atom 0)]
     (fn [m]
       (case (:type m)
         :started
         (reset! last-trial-report (get-current-time-millis))

         :trial
         (let [t (get-current-time-millis)]
           (when (> (- t report-period) @last-trial-report)
             (with-test-out*
               (fn []
                 (println "Passing trial"
                          (-> m ::trial first) "/" (-> m ::trial second)
                          "for" (get-property-name m))))
             (reset! last-trial-report t)))
         ;; else
         (reporter m))))))

We would also need to add the following at the beginning of c.t.c/quick-check:

(reporter-fn {:type :started
              :property property
              :num-tests num-tests})

This would allow us to use it as (defspec some-spec {:reporter-fn (make-periodic-reporter-fn)} some-prop)

To make it usable via the *report-trials* mechanism (so it is backwards-compatible), we could do something like:

(def trial-report-periodic
  ;; I'm using (constantly nil) as the underlying reporter-fn because we only need to support the :type ::trial
  ;; message. The other messages are already handled by default-reporter-fn, which is going to call us on :type ::trial
  (make-periodic-reporter-fn *trial-report-period* (constantly nil)))
Comment by Unnikrishnan Geethagovindan [ 12/Apr/16 1:01 PM ]

Yea, this works, with a minor change in println

(:so-far m) "/" (:num-tests m)

Should I submit a patch with these following changes incorporated along with corresponding change in tests ?

Comment by Nicolás Berger [ 12/Apr/16 1:12 PM ]

Thanks for the fix in the println . Adding a patch sounds good to me, but of course we'll have to hear what @gfredericks thinks about it.

Comment by Unnikrishnan Geethagovindan [ 14/Apr/16 1:29 AM ]

Just realised that we will need to add :type ::started to the default-reporter-fn so that it is backward-compatible !

Comment by Unnikrishnan Geethagovindan [ 14/Apr/16 1:30 AM ]

Attaching patch





[TCHECK-8] Bound tests and shrinks in time Created: 04/Mar/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Unresolved Votes: 1
Labels: release-0.10.0


 Description   

It should be possible to bound tests and shrinks in time.

The original issue also mentions shrink interruption through Ctrl-c, but this is already covered in #TCHECK-4.

Original issue - #9 GitHub



 Comments   
Comment by Gary Fredericks [ 10/Apr/16 5:58 PM ]

Idea: a new function in c.t.check that takes a collection of arglists for c.t.check/quick-check, as well as a max-time, and runs the test in an interleaving fashion.

Not sure how c.t.c.clojure-test integration would look.





[TCHECK-3] Add ability to customize test output when a test fails Created: 04/Mar/14  Updated: 10/Apr/16  Resolved: 10/Apr/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: release-0.10.0


 Description   

From the original description:

I would like the ability to control / enhance / replace the output generated when a test fails. Printing the raw values works well as long as those values have a useful output for the test at hand, but this doesn't always hold true.

To give a bit of background, I was playing with generating sequences of partially-applied functions and then applying them to an initial state. Here's a small excerpt from the full gist:

:smallest [(#<core$partial$fn__4190 clojure.core$partial$fn__4190@2a799171>)]

What would be more useful to me is if the output had something like this, which is more meaningful:

:smallest [(- 45 %)]

In this example, (- 45 %) is all my custom output string. 45 is a value from an upstream generator (and would thus change), and % is just a string modeled after the anonymous function syntax.

I could even see my particular application rolled up as a generator:

(def gen-inc
  (gen/bind gen/pos-int #(gen/partial + %)))

Which could handle some of the heavy lifting for the user.

Original issue



 Comments   
Comment by Gary Fredericks [ 10/Apr/16 5:15 PM ]

I'm going to say this has been completed as much as I'm interested in doing it.

For clojure.test.check/quick-check, I think this is a non-issue, since the user has the interested data returned and can do whatever they want with it, including transforming it for readability.

For clojure.test.check.clojure-test/defspec, there is now (on master) the ability to customize printing via the :reporter-fn option, so this should be sufficient for other transformations.





[TCHECK-34] clojure.test reporting is uninformative Created: 21/Jun/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Philip Potter Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

The clojure.test reporting results in output like:

FAIL in (always-fail) (clojure_test.clj:18)
expected: result
  actual: false

Note that the file and line number are always clojure_test.clj:18 and the expected value is always the literal string "result".

I'm not sure what the right output would be for expected/actual, but the incorrect file and line reporting means that clojure-test-mode consistently highlights the wrong line.



 Comments   
Comment by Philip Potter [ 21/Jun/14 9:10 AM ]

I think this is because the assertion is checked by calling clojure.test/is. clojure.test/is is being called for its return value, but it has the side effect of reporting failures. It's a macro, and it takes the literal symbol "result" as its expected expression, and takes the current file and line number to attach to the failure. It's really designed to be called by user code, not library code.

Comment by John Walker [ 11/Aug/14 3:55 PM ]

Well, one solution would involve taking advantage of the report functionality.

https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L204
https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L371





[TCHECK-96] quick-check should accept an arg for maximum shrinking time Created: 29/Feb/16  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0





[TCHECK-14] re-organize README and doc/ Created: 29/Mar/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Reid Draper Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

I'd like to re-organize the README, and focus on making it easier for users to find the information they need. The README is currently pretty long, and could be information overload. My current thinking is that the new README should be short, and focus on providing links to people based on their experience with test.check. I think something like the following three headings would be useful:

1. First time user. Show me what this is all about.

  • Focus on 'seeing something cool' within five minutes. Maybe include some example tests in examples/. This way new users can simply clone the repository and run a test in under two minutes.

2. Still getting started, show me some tutorials and examples.

  • This will probably build on the existing doc/intro.md file, which currently describes how to write generators.

3. Existing user, show me API documentation, release notes, advanced techniques.

  • API documentation
  • Release-notes, make it clear these are up-to-date, and have clear instructions when there are breaking changes
  • Advanced techniques/tutorials





[TCHECK-26] defspec does not respect (is (thrown? ...)) Created: 20/Apr/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Sung Pae Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

The following test does not pass:

(quick-check 1
  (for-all [x g/any]
    (is (thrown? Throwable (assert false)))))

This can be worked around by wrapping the (is ...) form with a call to boolean:

(quick-check 1
  (for-all [x g/any]
    (boolean (is (thrown? Throwable (assert false))))))

The (is (thrown? ...)) form does not return true|false, but will return the Exception itself when it passes.

Thank you!



 Comments   
Comment by Reid Draper [ 13/May/14 6:14 PM ]

This is correct. And I'm not yet sure what the best way forward is, but my hunch is that we'll need to support non-boolean return values from tests. Maybe something that implements a ->pass? function.





[TCHECK-31] Doc improvements: how to separate "size" from "range" Created: 11/Jun/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Nygard Assignee: Gary Fredericks
Resolution: Unresolved Votes: 1
Labels: release-0.10.0


 Description   

Size is discussed mainly in terms of recursive generators. However, it is crucial to understanding other aspects such as vector length and integer ranges.

Some examples like these would be helpful:

  • How to get a small number of samples with a large range.
  • How to get a large number of samples with a small range.


 Comments   
Comment by Reid Draper [ 12/Jun/14 9:13 AM ]

Agree. I'll try and get a patch up in the next couple of days. Do you just have something in mind going in the doc/ dir?

Comment by Brian Craft [ 12/Nov/14 12:05 AM ]

I have exactly this problem. Are there any examples from projects using test.check?

Comment by Reid Draper [ 12/Nov/14 9:12 AM ]

Brian, can you be more specific about your problem? Some helpful functions regarding sizing and domain-generation are documented here. Take a look specifically at gen/choose, gen/resize, and gen/sized.

Comment by Brian Craft [ 12/Nov/14 10:29 AM ]

I don't need to test, for example, strings that are 500 chars long, but do need to test lists of 500 strings. If I have a generator that builds lists of strings & size it larger, both the lists and strings get larger. Then I start hitting irrelevant resource limits, like db field widths, or jvm memory. It seems like I need a way to specify how different data elements grow. Just setting the range, like (gen/vector ... 0 5000), seems wrong, because then it always picks up to 5000, while I suspect for shrinking it needs to respond to "size". Something like this?

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

I gave this a try, but hit the vector stack overflow problem. I will try the fmap/flatten workaround, but am wondering if this is the right approach. Also wondering about the range of "size". From the code it looks like it goes to 100 due to sample-seq. So functions of size passed to gen/sized should expect range from 0-99, and return the largest meaningful data when given size parameter 99?

Comment by Reid Draper [ 15/Nov/14 12:03 PM ]

I don't need to test, for example, strings that are 500 chars long, but do need to test lists of 500 strings. If I have a generator that builds lists of strings & size it larger, both the lists and strings get larger.

Indeed, by default, all generators will 'grow' together. And as you've correctly noticed, you can use gen/sized and gen/resize to further control this.

Just setting the range, like (gen/vector ... 0 5000), seems wrong, because then it always picks up to 5000, while I suspect for shrinking it needs to respond to "size". Something like this?

Shrinking is completely separate from the size parameter. You can safely use (gen/vector ... 0 n) without negatively affecting shrinking.

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

I gave this a try, but hit the vector stack overflow problem. I will try the fmap/flatten workaround, but am wondering if this is the right approach.

Your example looks fine. And you can avoid the stack overflow problem by simply not allowing size to grow above a certain limit, say 5000: (gen/resize (min 5000 (* size size)) ...).

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

Also wondering about the range of "size". From the code it looks like it goes to 100 due to sample-seq. So functions of size passed to gen/sized should expect range from 0-99, and return the largest meaningful data when given size parameter 99?

When testing, the default max size is 200. However, you can override this simply by passing in a :max-size n argument to tc/quick-check. For example: (tc/quick-check 100 my-prop :max-size 75).

Hope this helps, and don't hesitate to ask anything else.

Comment by Brian Craft [ 18/Nov/14 11:41 AM ]

At the moment the hardest bit for me is understanding the shrinking. Composing generators, I inadvertently build things that won't shrink. E.g. to generate a 2d matrix of numbers with row & column names, I make use of bind so I can size vectors of names to match the matrix size, but then the resulting matrix/vectors will not shrink.

Here's a small example. Generate a vector of short strings, and test that they are all unique.

(tc/quick-check 40 (prop/for-all [f (gen/bind gen/int (fn [i] (gen/vector (gen/resize 5 (gen/such-that not-empty gen/string-ascii)) i)))] (do (println "f" f "count" (count f)) (= (count f) (count (set f))))))

Run this a few times & it will hit a duplicate string, failing the test. It is then unable to shrink the vector.

Here's a case generating two vectors of the same size by generating one vector, then using bind to generate a second of the same size. Testing uniqueness on the second vector, it is able to shrink, however it does so by regenerating the 2nd vector for every test.

(tc/quick-check 40 (prop/for-all [f (gen/bind (gen/vector (gen/resize 5 (gen/such-that not-empty gen/string-ascii))) (fn [v] (gen/hash-map :a (gen/return v) :b (gen/vector gen/int (count v)))))] (do (println "f" f "count" (count (:a f))) (= (count (:b f)) (count (set (:b f)))))))

I expect this will shrink poorly in many cases, because regenerating the second vector destroys the (possibly very rare) failure condition. I suspect this is why my 2d matrices shrink poorly. One really wants to be able to shrink by dropping the same positional element from both vectors. In the case of a fixed dimension (two), you can rewrite it to generate a vector of tuples. For a 2d matrix, though, I'm not sure what to do. Ideally it would shrink by dropping either a column or a row, and the corresponding column or row label. Perhaps I need to write a recursive generator like vector?

Are there any docs/papers describing how the shrinking works?

Comment by Gary Fredericks [ 20/Nov/14 9:45 AM ]

I've included a generator called cap-size in the test.chuck library.

Comment by Reid Draper [ 30/Nov/14 6:11 PM ]

I think both max-size and min-size functions would be a nice addition to test.check proper.

Comment by Jay A. Patel [ 22/Jan/15 7:29 PM ]

Hi Reid –

I am not sure if this is the right place for my comment, but it's closely related to this issue –

As you show, one can use `resize` to reconfigure the default generators. However, here is one issue with default generators for numbers. The default numeric generators (`int`, `nat`, `pos-int`, etc.) all generate numbers < 100 because default `size` is 100. Given JVM interns Integer/Long objects <= 127:

user=> (identical? 127 127)
true
user=> (identical? 128 128)
false

This could be an issue where test.check default number generators miss out on certain class of errors. I opine that a default value of `size` as 100 makes good sense for collections, but not for numbers.

If you agree, a few alternatives could be:

  • change default `size` to > 128, say 256
  • change `int` generator to choose between >1 multiple of size
  • have two different defaults: one for `length` (or `range`) and one for `size`

I don't know though. I think all alternatives have a drawback. I maybe missing something.

Comment by Gary Fredericks [ 22/Jan/15 8:03 PM ]

We've talked a bit about your third option, perhaps having two different classes of numeric generators. Clearly not ever generating big numbers is a pretty bad default for a lot of applications.

Comment by Gary Fredericks [ 26/Feb/16 9:29 AM ]

Jay,

A couple more comments:

  1. It's actually always been the case that the default size is 200 rather than 100, so the edge case you're describing should already be tested for tests that have more than 128 runs; I assume the reason you thought the default size was 100 is because that's what gen/sample uses for no reason in particular, so it might be worth changing that to 200 just to avoid this sort of confusion in the future
  2. As of 0.9.0 there are new integer generators (gen/large-integer and gen/large-integer*) aimed at use cases where small integers don't make sense, so that should help as well




[TCHECK-9] Properly document how :max-size works Created: 04/Mar/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: release-0.10.0


 Description   

I have had several issues where I thought increasing quick-check's num-tests argument would increase the time taken linearly, but surprisingly it grew quadratically. This is related to the auto-increasing of the size to call-gen, which I believe it isn't documented well enough. It also seems like other people have had trouble with this functionality, see the reply to announcement of clojure.test.check.

Preferably, it should be mentioned in the docstring and the tutorial that the size of the structures generated grows linearly with the amount of tests you have, up to 200. As a result, doubling the amount of tests does not necessarily double the amount of time taken to run the tests.

In some of my tests, this behaviour increases the running time exponentially, and I actually used quite some time figuring out why. The documentation on recursive generators decreases the size of the binary tree in half, but it is not explained why it is split in half. While obvious in hindsight, explicitly explaining that recursive structures should have an amount of nodes proportional to the input size argument and how it is done, would be beneficial.



 Comments   
Comment by Reid Draper [ 04/Mar/14 8:50 PM ]

+1. This is not well-documented now. Further, I'm still not 100% sure that halving inside of a recursive generator is the right thing to do. For some structures, log2 may be more appropriate. In the mailing list post you mention, gen/any is clearly not bounded enough at roughly size=90, consider it's using more than 1GB of heap.

Comment by Reid Draper [ 14/Apr/14 8:58 PM ]

I've made things a little better in 57df9b9b8f2fe61a798fbf683046e8d1aa128228.





[TCHECK-44] for-all should support nesting Created: 22/Sep/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File for-alls-nest.diff    
Patch: Code and Test

 Description   

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.



 Comments   
Comment by Reid Draper [ 24/Sep/14 11:32 AM ]

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?

Comment by Michael Sperber [ 25/Sep/14 2:19 AM ]

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?

Comment by Reid Draper [ 25/Sep/14 11:10 PM ]

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

Comment by Reid Draper [ 21/Oct/14 10:23 PM ]

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]
Comment by Michael Sperber [ 22/Oct/14 2:00 AM ]

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

Comment by Reid Draper [ 28/Oct/14 10:55 PM ]

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.

Comment by Lars Andersen [ 04/Oct/15 4:42 AM ]

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 )))
Comment by Gary Fredericks [ 04/Oct/15 9:47 AM ]

We've been thinking about a similar problem in TCHECK-81, and in the meantime there's an even-fancier variant of for-all here.





[TCHECK-57] Performance regression in map generator between 0.5.8 and 0.5.9 Created: 03/Jan/15  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Brian Kim Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.5.1
OSX 10.9.5



 Description   

Tried updating test.check version to latest, but it seems like the map generator has gotten slower from version 0.5.9 up. I tried to make this easy to reproduce in the git repo below, but please let me know if you'd like more information/can't reproduce. Thanks!
https://github.com/brainkim/testcheckperf



 Comments   
Comment by Reid Draper [ 04/Jan/15 2:01 PM ]

Thanks Brian. I'm still digging in to this, but it's appearing like the performance regression is in keyword generation, not map generation. There was also a major change to keywords between 0.5.8 and 0.5.9 in 4528b5ed391d6127b79f4db6bc5e086613da0d81.





[TCHECK-60] string from regular expression generator Created: 23/Jan/15  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Steve Miner Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A regular expression is a succinct and flexible way to describe a set of strings. It would be useful to have generator that takes a regular expression and generates matching strings.

There was some hallway discussion about this feature request at the Clojure Conj in November 2014.



 Comments   
Comment by Steve Miner [ 23/Jan/15 9:12 AM ]

https://github.com/gfredericks/test.chuck recently added string-from-regex. That's a good work-around for most users.

Comment by Steve Miner [ 23/Jan/15 9:16 AM ]

https://github.com/miner/herbert recently added a new implementation of a string from regex generator (in v0.6.7). Herbert supports a more limited regex syntax than the generator from test.chuck, but it has no dependencies on other libraries so it can more easily be adopted in a contrib library.





[TCHECK-2] pos-int is confusingly named Created: 04/Mar/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Unresolved Votes: 2
Labels: None


 Description   

The generators neg-int and pos-int generate 0, which is confusing as (= (pos? 0} (neg? 0) false). The suggestion is to change s-pos-int and s-neg-int to pos-int and neg-int respectively, and replace the current neg-int and pos-int. Suggestions for new names include non-x-int and not-x-int.

Original issue






[TCHECK-19] Permit a data-structure containing generators to be used as a generator Created: 11/Apr/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Darrick Wiebe Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File add_record.diff    

 Description   

I'm working through some of the examples in John Hughes' "QuickCheck Testing for Fun and Profit" paper. I'd like the structures I generate in those tests to look like the this:

[:apply `reg [(gen-name) (gen/elements pids)]]

Rather than the way they must be expressed now:

(gen/tuple (gen/return :apply) (gen/return `reg) (gen/tuple (gen-name) (gen/elements pids)))

I think a simple recursive function could be used to generate these, with the caveat that currently generators create a map {:gen #<fn...>}, which I'd propose to change to a record so that we could distinguish between a generator and a map data-structure.

This seems to be implemented to good effect in Quvig QuickCheck already:

In general, Quviq QuickCheck permits any data-structure containing embedded generators to be used as a generator for data-structures of that shape—something which is very convenient for users, but quite impossible in Haskell, where embedding a generator in a data-structure would normally result in a type error. This technique is used throughout the generators written at Ericsson. – Hughes



 Comments   
Comment by Darrick Wiebe [ 11/Apr/14 5:06 PM ]

Playing around in the REPL today, I came up with this code that seems to work well converting arbitrary literals into generators:

(defprotocol LiteralGenerator
  (-literal->generator [literal]))

(defn literal->generator [literal]
  (cond
    (satisfies? LiteralGenerator literal) (-literal->generator literal)
    (vector? literal) (apply gen/tuple (mapv literal->generator literal))
    (and (map? literal) (= [:gen] (keys literal)) (fn? (:gen literal))) literal
    (map? literal) (gen/fmap (partial into {}) (literal->generator (mapv vec literal)))
    (set? literal) (gen/fmap set (literal->generator (vec literal)))
    (list? literal) (gen/fmap (partial apply list) (literal->generator (vec literal)))
    :else (gen/return literal)))

Generating from a record is probably something that would be generally useful, so I added this as well:

(defn record->generator
  ([record]
   ; Is there a better way to do this?
   (let [ctor (eval (read-string (str "map->" (last (clojure.string/split (str (class record)) #"\.")))))]
     (record->generator ctor record)))
  ([ctor record]
   (gen/fmap ctor (literal->generator (into {} record)))))

Which enables me to extend a record like this:

(defrecord Foo [a b]
  LiteralGenerator
  (-literal->generator [this] (record->generator map->AbcDef this)))

I haven't looked at the possibility of baking this code into test.check at all yet. I'd like feedback on what I've got so far before pursuing this any further.

Comment by Reid Draper [ 11/Apr/14 5:31 PM ]

So, I have kind of mixed feelings about this. I agree it's convenient, but I also worry that being loose like that can allow more accidents to occur, and doesn't force users to make the distinction between values and generators. For example, what if you do something like this: [int int]. Did you mean to type [gen/int gen/int], or do you really intend to have written the equivalent of [(gen/return int) (gen/return int)]? If every function that expects a generator can also take a value, we can no longer start adding error-checking to make sure you're correctly passing generators. On that same token, I do see the benefit of being able to use data-structure literals. What if we wrote a function that you had to use to create a generator with this syntax, something like: {{(gen/literal [:age gen/int])}}. That way you could opt-in to this syntax, but only within the scope of gen/literal?

Comment by Darrick Wiebe [ 11/Apr/14 5:40 PM ]

I agree that is a concern, and since you're not guaranteed to see generator output, it might be better to be explicit about using gen/literal. It's still a nice clean solution. Fortunately, that's exactly what I've written! We'd just need to rename literal->generator to literal and install it into the clojure.test.check.generators namespace.

Comment by Reid Draper [ 11/Apr/14 6:56 PM ]

I think the first step is changing generators to use Records. Interested in making a patch for that?

Comment by Darrick Wiebe [ 11/Apr/14 8:31 PM ]

A very simple patch to use a Generator record rather than a simple {:gen ƒ) map.

Comment by Reid Draper [ 12/Apr/14 6:15 PM ]

I've applied the record-patch in ef132b5f85a07879f01417c9104aa6dea771fdb4. Thanks. I've also added a generator? helper function.

Comment by Philip Potter [ 22/Jun/14 4:39 PM ]

I spotted something which seemed relevant: ztellman/collection-check defines a tuple* fn which is like gen/tuple but automatically wraps literals with gen/return.

Notably, despite not solving the general case, this would have solved the example in the issue description.

Comment by Gary Fredericks [ 20/Aug/15 10:28 PM ]

There is now a library aimed at this: https://github.com/holguinj/jen





[TCHECK-41] Add helpers for deprecating generators Created: 01/Sep/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: John Walker Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File deprecation-helpers.diff    
Patch: Code

 Description   

It should be clearly communicated to the user when generators are deprecated. This patch supplies helpers for easily deprecating generators.

def-throwing-generator receives the metadata that its generator will throw if its called. As an example, suppose deprecated-generator was deprecated in 0.5.9.

(def-throwing-generator deprecated-generator
  "deprecated-generator has been deprecated. Use current-generator instead."
  {:deprecated "0.5.9"})

Then if the user looked up its documentation, they would see from cider:

clojure.test.check.generators/deprecated-generator
Deprecated in 0.5.9
  deprecated-generator has been deprecated. Use current-generator instead.

or if they used the generator and ran cider-test, they would see:

Test Summary
foobar.core-test

Ran 1 tests, in 1 test functions
1 errors


Results

Error in a-test
FIXME, I fail.
expected: (= "a" (sample deprecated-generator 5))
  actual: clojure.lang.ExceptionInfo: deprecated-generator has been deprecated. Use current-generator instead. {:deprecated "0.5.9"}





[TCHECK-38] Generators for functions? Created: 15/Aug/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

test.check has generators for first-order types, but not for functions.

In the original QuickCheck, an additional type class ("Arbitrary") handles
functions.

If I implemented this and submitted a patch, would it be likely to be included?

Background: I wrote my own Clojure QuickCheck clone here:

https://bitbucket.org/sperber/deinprogramm.quickcheck

This has arbitraries and function generators, but not all of test.check's goodies.

I'm trying to decide whether to continue maintaining my QuickCheck clone or
instead to contribute ot test.check, so feedback would be much appreciated.



 Comments   
Comment by Reid Draper [ 21/Aug/14 2:16 PM ]

I'd love a patch for this! Would you like to write up a quick implementation proposal first, so we can iron out any architectural details before looking at the code?

Comment by Michael Sperber [ 25/Aug/14 2:06 AM ]

Would you be willing to look at my QuickCheck clone as a starting point? Its general implementation approach is very similar to yours, so I think you should feel right at home.

The caveat is that it introduces a type separate from generators - the "arbitrary", corresponding to the `Arbitray' type class in the Haskell code. Doing this to `test.check' would change the API, so maybe there, the `Generator' type should be extended by an optional `transformer' field.

Comment by Reid Draper [ 26/Aug/14 2:26 PM ]

Sure, I've already read through it a bit, but need to read in more detail. How does your implementation handle printing (showing) functions? Do functions shrink? Have you seen Showing and Shrinking Functions by Claessen ?

Comment by Michael Sperber [ 27/Aug/14 2:21 AM ]

Yes. Haven't done that yet, but it would be on my list. Given that functions are objects in Clojure, I think printing should be a little easier than in Haskell.





[TCHECK-84] Add gen/uniform-double Created: 12/Nov/15  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Would be somewhat of a utility generator – generates a double from 0.0 (inclusive) to 1.0 (exclusive), and doesn't shrink.

gen/choose could even be rewritten with gen-fmap if we had this.






[TCHECK-85] prop/for-all should let later bindings refer to earlier bindings Created: 18/Nov/15  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Dan Burton Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

For example:

(prop/for-all [x (gen/nat)
y (gen/choose 0 (dec x))] ;; y is a nat less than x
...)



 Comments   
Comment by Gary Fredericks [ 20/Nov/15 10:13 PM ]

I've thought about this before, and I'm pretty hesitant since it would probably mean using bind whenever there are multiple bindings, which means that old-style uses with independent generators would all of a sudden start shrinking poorly (due to the difficulty of shrinking bind in general).

I can see the value in consistency with gen/let though.

Another option would be an alternate for-all with the bind behavior, though I'm not sure would it would be called.





[TCHECK-7] Add nested property support Created: 04/Mar/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gary Fredericks
Resolution: Unresolved Votes: 2
Labels: None


 Description   

Including functionality to add support for nested properties would be beneficial. An example of how this may work follows:

(defn exists?
  [e coll]
  (some #{e} coll))

(defn remove-elem
  [e coll]
  (remove #{e} coll))

;; this test should fail, and illustrates nested properties
(quick-check (for-all [v (gen/vector gen/int)]
               (for-all [e (elements v)]
                 #(exists? e (remove-elem e v)))))

The requirement for this is support for reifying properties, which is not currently implemented.

Original issue - GitHub #10



 Comments   
Comment by Maciej Jaśkowski [ 14/May/14 4:14 PM ]

What's the status of this issue?
It would help us a lot to have this functionality up and running in the following case: I want to verify my path finding algorithm finds optimal solution in the given graph G, so I generate the graph and a big number of random paths and check that all the random paths are "less optimal" then the one found by the algorithm.

Having nested for-all that would be simple. Is there a way around?

Comment by Reid Draper [ 14/May/14 4:24 PM ]

Sorry for the lack of update here. Nested properties are still in a bit of hammock-time on my part. The good news is they are merely convenient, and you can achieve the exact same effect with clojure.test.check.generators/bind. You can see an example of bind being used here.

Comment by Maciej Jaśkowski [ 15/May/14 12:08 AM ]

Apparently I need some more hints because all I can generate (using gen/bing, btw) are pairs '(graph list-of-random-paths)

That is useful as long as the algorithm works as expected. However, once it fails it's pretty hard to figure out which of random-paths failed it.

Comment by Reid Draper [ 15/May/14 1:02 PM ]

Can you show me how you would write the test with nested properties, and then perhaps I can help you rewrite it to use bind instead?

Comment by Maciej Jaśkowski [ 15/May/14 3:58 PM ]

Great! Thank you!

Please, find the most important part below.

(defn gen-matrix-and-paths [size]
    (gen/bind
      (gen-matrix gen/s-pos-int size)
      (fn [matrix]
        (let [ nodes (into [] (range (count matrix)))
               gen-perms (gen-permutations nodes)
               perms (distinct (gen/sample gen-perms 500)) ]
          (gen/tuple 
            (gen/return matrix) 
            (gen/return perms))))))

gen-matrix generates a square matrix of incidence e.g. [[0 1] [4 0]]
gen-permutations generates a permutation of provided vector

Comment by Reid Draper [ 15/May/14 4:41 PM ]

I'm confused. What issue are you running into? The code you've pasted uses bind, and not a nested property.

Comment by Maciej Jaśkowski [ 17/May/14 3:26 AM ]

Ok but now I can only write something like this:

(prop/for-all [data (gen-matrix-and-paths 6)]
   (let [ [matrix random-paths] data ]
     (not-worse-then-others? tsp matrix random-paths))))

which, if my 'tsp' algorithm is not working correctly would point me to a matrix and a vector (length 500) of paths one of which is counterexample. It would be better if instead I got the matrix and a single counterexample.

Comment by Reid Draper [ 17/May/14 12:44 PM ]

I'm still not sure what this would look like using a nested-property. Can you please write the code the way you'd like to see it as a nested property? This will involve at least two for-alls.

Comment by Maciej Jaśkowski [ 18/May/14 4:56 PM ]

Sure!

I would like to write something like that:

(prop/for-all [matrix (gen-matrix 6)]
  (prop/for-all [random-path (gen-random-path matrix)]
    (<= (cost (tsp matrix)) (cost random-path))))
Comment by Maciej Jaśkowski [ 23/May/14 6:22 AM ]

ping!

Comment by Reid Draper [ 26/May/14 11:47 AM ]

Ok, let's translate that to using gen/bind. Just like with nested properties, bind allows you to create generators that depend on the value of another generator. For example, your gen-random-path depends on the previously generated matrix variable. So let's write a generator that returns both a matrix and a random-path.

(def matrix-and-path
  "Return a two-tuple of [matrix random-path]"
  (gen/bind (gen-matrix 6)
            (fn [matrix]
               (gen/tuple (gen/return matrix) (gen-random-path matrix)))))

(prop/for-all [[matrix random-path] matrix-and-path]
  (<= (cost (tsp matrix)) (cost random-path))




[TCHECK-15] gen/for macro for alternate combinator syntax Created: 02/Apr/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File TCHECK-15-p1.patch     Text File TCHECK-15-p2.patch     Text File TCHECK-15-p3.patch     Text File TCHECK-15-p4.patch     Text File TCHECK-15.patch    

 Description   

I think the syntax of clojure.core/for would be a good fit for test.check's combinators. For example:

(defn gen-even-subset
  "Returns a generator that generates an even-cardinality
   subset of the given elements"
  [elements]
  (gen/for [bools (apply gen/tuple (repeat (count elements) gen/boolean))
            :let [true-count (->> bools (filter identity) (count))]
            :when (even? true-count)]
    (->> (map list bools elements)
         (filter first)
         (map second)
         (set))))

This combines the capabilities of fmap, bind, and such-that into a familiar syntax.

One downside here is the temptation to use multiple clauses for independent generators, resulting in a use of gen/bind when gen/tuple would be simpler and presumably shrink easier. An approach to this is an additional supported clause, perhaps called :parallel, that uses the syntax of :let to provide the functionality of gen/tuple:

(gen/for [:parallel [n1 gen/nat
                     n2 gen/nat]
          :let [sum (+ n1 n2)]]
  {:nums [n1 n2] :sum sum})

Compared to gen/tuple, this has the advantage of placing generators syntactically next to names, rather than segregating the generators from the names.

The :parallel feature has not been added to the current patches.



 Comments   
Comment by Gary Fredericks [ 05/Apr/14 3:23 PM ]

I think there might be some design ambiguity around the meaning of :when. In particular, in the following contrived example:

(for [n nat
      v (vec (return n))
      :let [sum (reduce + v)]
      :when (pos? sum)]
  v)

In my default design this can hang, for the same reason that this code can hang:

(bind nat 
      (fn [n]
        (such-that
          (fn [v] (pos? (reduce + v)))
          (vector (return n)))))

But it could just as well have been written:

(such-that 
  (fn [v] (pos? (reduce + v)))
  (bind nat (fn [n] (vector (return n)))))

So the issue is whether a :when filter is applied to just the previous generator or to all of the previous generators. I have some hazy notion that the latter would be less efficient in some cases, but I'm not sure what. So I think our options are:

  1. Decide to always do it one way or the other
  2. Provide a third keyword (:when-all?) with different behavior
  3. Don't write this macro at all because it's too difficult to understand

My gut is to do option 1 and just apply :when to the previous generator.

Comment by Gary Fredericks [ 08/Apr/14 9:24 PM ]

Attached my initial draft. The implementation took a lot more thought than I expected, and is a bit subtle, so I included some inline comments explaining the structure of the macro.

Comment by Gary Fredericks [ 13/Apr/14 8:33 PM ]

Attached TCHECK-15-p1.patch, updated to apply to the current master.

Comment by Gary Fredericks [ 16/Apr/14 9:51 PM ]

Attached TCHECK-15-p2.patch which adds a note to the docstring about independent clauses, shrinking, and tuple.

Comment by Gary Fredericks [ 16/Apr/14 9:58 PM ]

Attached TCHECK-15-p3.patch which fixes one bug and one redundancy in namespace aliases.

Comment by Gary Fredericks [ 13/May/14 10:37 AM ]

Attached TCHECK-15-p4.patch which fixes a bug with destructuring (and adds a regression test).

Comment by Gary Fredericks [ 13/May/14 10:38 AM ]

Also might be helpful to note that I've put this in my test.check utility library for now: https://github.com/fredericksgary/test.chuck#for.

Comment by Michael Blume [ 08/Jan/15 12:21 AM ]

I wonder if it'd be possible to avoid :parallel by analyzing the code and checking whether the bindings can be run in parallel?

Comment by Gary Fredericks [ 08/Jan/15 9:16 AM ]

I think it's possible in theory, but we'd need access to a non-buggy code walker.

Additionally you might argue that it makes the meaning of the code a lot more subtle.





[TCHECK-53] property creating function similar to map as alternative to for-all* Created: 17/Nov/14  Updated: 10/Apr/16

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None


 Description   

As an alternative to the `for-all*` syntax, I suggest using a function signature that looks more like `map` with the predicate as the first arg and the generators as the rest of the args.

For example, these forms would all be equivalent:

(property (comp integer? *) gen/int gen/int gen/int) ;; proposed style

(for-all* [gen/int gen/int gen/int] (comp integer? *)) ;; current style



 Comments   
Comment by Steve Miner [ 17/Nov/14 8:20 AM ]

It's easy enough to write for myself. I'll make a patch if you want it.

(defn property 
    ([pred gen] (prop/for-all* [gen] pred))
    ([pred gen1 gen2] (prop/for-all* [gen1 gen2] pred))
    ([pred gen1 gen2 & more] 
        (prop/for-all* (list* gen1 gen2 more) pred)))




[TCHECK-92] Discrepancy between trial-number and num-tests in case of failure Created: 28/Feb/16  Updated: 11/Mar/16  Resolved: 11/Mar/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicolás Berger Assignee: Nicolás Berger
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix-num-tests-discrepancy.patch    
Patch: Code

 Description   

In clojure.test.check/failure there's a difference of 1 between the trial-number passed to reporter-fn and the num-tests returned by the function, which in turn ends being the return value of clojure.test.check/quick-check. It's not clear what's the reason behind this difference. Is it because trial-number means "the count of successful trials" while num-tests is "the count of trials"? Is it for historical reasons? Is it just a bug?

The default-reporter-fn is not using the trial-number, this could explain why this inconsistency was not noted before. I stumbled upon this issue while working on TCHECK-87, where the total count of trials (not only the successful ones) is needed to compute the statistics.

I'm not sure what's the best way to fix this discrepancy (explain it in the docstring? make them equal? include both values in both places?), so I'm opening this issue for discussion.



 Comments   
Comment by Gary Fredericks [ 29/Feb/16 8:58 PM ]

What about changing the key to be :trials-run, and if you have a test where there were three successful trials and then the fourth one failed you would see:

:trials-run 4
Comment by Gary Fredericks [ 29/Feb/16 9:01 PM ]

I think we need to re-think the different names we use across the different messages to make sure they're all intuitive and consistent with each other.

Comment by Nicolás Berger [ 06/Mar/16 4:02 PM ]

I agree, the naming needs some improvement, to get more clarity and consistency.

But I also think that's a separate issue from this off-by-1 bug, which can be fixed by the current patch in this issue. I think the naming change deserves a separate discussion/issue, at least to me it's very clear that the naming needs to be fixed but I'm not sure what would be the new names, and mixing in this small fix doesn't help.

Comment by Gary Fredericks [ 11/Mar/16 7:45 AM ]

Applied on master, thanks!





[TCHECK-93] gen/any can be disproportionately slow in clojurescript Created: 29/Feb/16  Updated: 06/Mar/16  Resolved: 06/Mar/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

https://github.com/clojure/test.check/blob/bb41a667f2574c4baa37514dfb9f3f8490ea727d/src/test/clojure/clojure/test/check/test.cljc#L963

⇑ This might be specific to node.js, I haven't tried it in a browser yet.



 Comments   
Comment by Gary Fredericks [ 06/Mar/16 9:23 PM ]

After a decent amount of investigation I'm going to chalk this up to inadvertently pushing the limits of my node.js default memory settings. See this commit for more details.





[TCHECK-90] Recursive generators seem to be highly stackful in cljs Created: 26/Feb/16  Updated: 27/Feb/16  Resolved: 27/Feb/16

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Declined Votes: 0
Labels: None


 Description   

The test introduced in this commit gives a stack exception pretty reliably in CLJS, even though I don't think it should be particularly stacky.



 Comments   
Comment by Gary Fredericks [ 27/Feb/16 8:04 PM ]

I think this is caused by http://dev.clojure.org/jira/browse/CLJS-1594.





[TCHECK-32] Default sizing on gen/any needs re-evaluation Created: 13/Jun/14  Updated: 26/Feb/16  Resolved: 26/Feb/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

The following innocuous-looking test blows the heap and then crashes a 4GB JVM with an out-of-memory error:

(tc/defspec merge-is-idempotent
  100
  (prop/for-all [m (gen/map gen/any gen/any)]
    (= m (merge m m))))

I understand how this happens, and how to fix it by adjusting the size parameters.

However, it would be great if using the defaults did not have the potential for such a nasty failure mode (particularly as the unwary user will have trouble determining that the fault is not in their code).



 Comments   
Comment by Philip Potter [ 21/Jun/14 8:29 AM ]

I was going to raise a separate ticket for this, but it seems like it might be related: gen/any (and any-printable) seem to take exponential time in the size of the input. See for example this session:

user> (time (dorun (gen/sample (gen/resize 50 gen/any-printable))))
"Elapsed time: 2204.284643 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 55 gen/any-printable))))
"Elapsed time: 2620.717337 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 60 gen/any-printable))))
"Elapsed time: 5923.636336 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 65 gen/any-printable))))
"Elapsed time: 9035.762191 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 70 gen/any-printable))))
"Elapsed time: 15393.687184 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 75 gen/any-printable))))
"Elapsed time: 9510.571668 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 80 gen/any-printable))))
"Elapsed time: 39591.543565 msecs"
nil

Apart from the anomaly at 75, adding 10 to the size seems to increase runtime by almost 3x.

Comment by Reid Draper [ 23/Jun/14 5:07 PM ]

I believe I should have a fix for this today, as well as an easier way to write custom, recursive generators.

Comment by Reid Draper [ 23/Jun/14 5:21 PM ]

This isn't fixing the OOM yet, but is making a big difference in making the size have a linear relationship with the number of elements generated, in a recursive generator. Here's branch: https://github.com/clojure/test.check/compare/feature;recursive-generator-helpers.

Comment by Reid Draper [ 23/Jun/14 7:16 PM ]

Fixed in https://github.com/clojure/test.check/commit/19ca756c95141af3fb9caa5e053b9d01120e5d7e. I'll try and get a snapshot build up tonight. Will be 0.5.9-SNAPSHOT. Check out the commit-message for a full explanation of how this now works.

Comment by Philip Potter [ 28/Jun/14 3:26 PM ]

awesome work! The new recursive-gen fn is great, too: I can immediately see a use case for generating arbitrary JSON-compatible data (ie vectors, maps, strings, numbers, bools, but no rationals, characters, symbols...).

Comment by Philip Potter [ 29/Jun/14 6:48 AM ]

Just re-tested on my machine; performance is vastly improved though still superlinear:

clojure.test.mode> (time (dorun (gen/sample (gen/resize 100 gen/any-printable))))
"Elapsed time: 101.907628 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 200 gen/any-printable))))
"Elapsed time: 302.341697 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 400 gen/any-printable))))
"Elapsed time: 1154.098163 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 800 gen/any-printable))))
"Elapsed time: 2954.889396 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 1600 gen/any-printable))))
"Elapsed time: 22335.200578 msecs"
nil

although since the default max-size is 200, this is very much no big deal.

Comment by Reid Draper [ 03/Jul/14 10:52 AM ]

I'm not too surprised that performance is still superlinear. What should be linear is the number of leaf nodes in the generated tree. We may eventually be able to do something a little more complex and have the total number of nodes be linear (including internal nodes). For now, however, it should be considered a bug if the relationship between leaf nodes doesn't grow linearly with the size parameter.

Comment by Reid Draper [ 04/Aug/14 8:35 PM ]

I'm still seeing that the original example is OOMing. I'm not sure the best way to solve that. For a single layer of nested generators, the above patch seems to be making a big difference. But when you make a map of gen/any, map doesn't know that its arguments are themselves recursive generators. This means you might create 100 keys and values, each themselves, large recursive generators. There's a balance that has to be had between making the default recursive generators be 'large enough' by themselves, but not too large when used like this. Hmm.. I think I'll go ahead and release 0.5.9, and keep on thinking of ways to make this even better.

Comment by Gary Fredericks [ 26/Feb/16 5:52 PM ]

I just did a serious overhaul of recursive-gen (which underlies gen/any), and I think I can consider this issue resolved. gen/any now performs reasonably for sizes up to 200, and even the (gen/map gen/any gen/any) from the ticket description won't OOM under reasonable JVM memory conditions.

I added the test in the description as a regression, and it runs fine on the JVM (even up to size 200), but gives a stack error in clojurescript. I decided to treat this as a separate issue and created TCHECK-90.

The more general issue that this ticket relates to is the fact that composing collection generators (non-recursively) naively will very quickly result in generators of massive sizes. E.g., (gen/vector (gen/vector (gen/vector (gen/vector gen/boolean)))) will likely OOM at the slightest provocation. This is why (gen/map gen/any gen/any) blew up – when you use it with size 200, it will likely decide to generate a map of roughly 200 keys and so will be generating from gen/any 400 times, so as gen/any was already rather large this pushed it over the edge. I'm not sure whether there's any good solution to this, but the fact that gen/recursive-gen wasn't sizing very well was a real (different) problem and should be solved now.





[TCHECK-83] gen/any only generates collections Created: 03/Nov/15  Updated: 26/Feb/16  Resolved: 26/Feb/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

I would think based on the name and the docstring that it ought to generate non-collection things as well.



 Comments   
Comment by Gary Fredericks [ 26/Feb/16 1:23 PM ]

Fixed in https://github.com/clojure/test.check/commit/838d8ac676da6f68bb581e135107256b9c59ba48.





[TCHECK-82] Shrinking is not entirely lazy Created: 03/Nov/15  Updated: 25/Feb/16  Resolved: 25/Feb/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

I think this has to do with vectors (aka chunked seqs), and the following change seems to fix it but I haven't really reasoned through it or verified that this doesn't break anything:

diff --git a/src/main/clojure/clojure/test/check/rose_tree.cljc b/src/main/clojure/clojure/test/check/rose_tree.cljc
index 9f8a98b..3b063bc 100644
--- a/src/main/clojure/clojure/test/check/rose_tree.cljc
+++ b/src/main/clojure/clojure/test/check/rose_tree.cljc
@@ -125,7 +125,8 @@
   [f roses]
   (if (core/seq roses)
     (make-rose (apply f (map root roses))
-               (map #(shrink f %) (remove roses)))
+               ;; THIS IS THE NON-LAZY PART; PROBABLY
+               (map #(shrink f %) (remove (take Double/POSITIVE_INFINITY roses))))
     (make-rose (f) [])))

 (defn collapse


 Comments   
Comment by Gary Fredericks [ 25/Feb/16 6:18 PM ]

Fixed in https://github.com/clojure/test.check/commit/4f0c95ee43e02b9f558994ec94ba810819e1077e.





[TCHECK-88] defspec is needlessly anaphoric Created: 23/Feb/16  Updated: 25/Feb/16  Resolved: 25/Feb/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-88-clean-up-defspec.patch    
Patch: Code and Test

 Description   

The defspec macro expands to a defn that uses non-gensymed names (in particular times, seed, max-size, and quick-check-opts).

This should (I haven't checked) mean that code such as this is buggy (and will fail):

(def seed 42)

(defspec seed-is-42 100
  (prop/for-all [x gen/int]
    (= seed 42)))

I assume it was written this way so that the documentation arglist on the defined function would have readable names, but this can be accomplished by setting the :arglists metadata directly.



 Comments   
Comment by Steve Miner [ 24/Feb/16 3:23 PM ]

defspec patch to avoid anaphora

Comment by Steve Miner [ 24/Feb/16 3:27 PM ]

Patch makes some serious changes to defspec and clarifies the documentation. Added a test that fails in the current release due to anaphora (shadowing 'seed'). Added some tests to show that the defspec test function actually works with quick-check keyword args.

Comment by Steve Miner [ 24/Feb/16 3:46 PM ]

updated patch that does a better job with the arglists of the generated test function

Comment by Gary Fredericks [ 24/Feb/16 5:58 PM ]

Thanks, this looks pretty solid. There's a ~' inside the arglists that smelled redundant to me, but that's the only thing I noticed on a quick scan.

Comment by Steve Miner [ 25/Feb/16 2:00 PM ]

The ~' prevents the symbols in the arglist (such as `num-times`) from being auto-namespaced.

Comment by Steve Miner [ 25/Feb/16 2:00 PM ]

I'm wondering if a default doc-string would be appropriate. Maybe something like "Test defined with defspec". It would be easy to add a common generic doc-string. A bit more work to try to derive something useful from the given property.

There's no option for the user to supply a doc-string. I guess that would be a new feature deserving its own ticket.

Comment by Gary Fredericks [ 25/Feb/16 2:08 PM ]

ah yeah I see what the ~' is doing – it threw me off because I expected it higher up, around the whole list of arglists, but it works either way.

Comment by Gary Fredericks [ 25/Feb/16 2:11 PM ]

I applied the patch on master – thanks again!





[TCHECK-33] test.check is entangled with clojure.test Created: 21/Jun/14  Updated: 23/Feb/16  Resolved: 23/Feb/16

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Philip Potter Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: None


 Description   

test.check is tightly coupled to clojure.test; this means that it's difficult to use it with other test frameworks.

There is a dependency cycle between clojure.test.check and clojure.test.check.clojure-test.

quick-check uses clojure.test/report to print dots to the screen, meaning that quick-check necessarily depends on clojure.test



 Comments   
Comment by Philip Potter [ 21/Jun/14 8:54 AM ]

I wonder if the solution is to allow you to provide your own reporting functions to quick-check to report successes, failures, and trials? That way different frameworks can inject different behaviour as required.

Comment by Gary Fredericks [ 23/Mar/15 2:07 PM ]

This problem, according to anecdote, has the more severe effect of requiring a user of clojure.test.check.clojure-test to require clojure.test.check even if it's not otherwise being used (the defspec macro attempts to fix this but I believe that trick doesn't actually work, though the problem doesn't usually surface).

Comment by Gary Fredericks [ 30/May/15 6:03 PM ]

I was working on this a while ago but somehow got side-tracked.

I think my main idea was to add another option arg to quick-check which would be a reporting callback function. Then defspec would simply supply a function that does whatever it currently does, maybe integrating with clojure.test somehow.

Comment by Gary Fredericks [ 23/Feb/16 4:51 PM ]

This has been fixed on master and should be included in whichever release comes after 0.9.0.





[TCHECK-75] gen-seq->seq-gen is named out of sync with its description/comment (and presumably impl). Created: 08/Jul/15  Updated: 29/Nov/15  Resolved: 29/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jonathan Leonard Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None
Environment:

N/A



 Description   

Title pretty much says it all.



 Comments   
Comment by Gary Fredericks [ 09/Jul/15 8:44 AM ]

You're looking at this code?

The story reminds me of the previously bad docstring on the lazy-random-states function defined just before gen-seq->seq-gen, but I fixed that one too. I think it should be fixed on 0.8.0-alpha3.

Comment by Gary Fredericks [ 09/Jul/15 8:45 AM ]

In case my previous comment wasn't clear, I don't see anything wrong with the current gen-seq->seq-gen docstring, so if you were looking at the latest code then you'll have to be more specific about what's wrong with it.

Comment by Jonathan Leonard [ 09/Jul/15 2:44 PM ]

Here's the confusion:

gen-seq = generator of sequences.
or
gen-seq = generator sequence.

seq-gen = sequence of generators.
or
seq-gen = sequence generator.

I didn't see the second interpretations at first but I see them now.

Comment by Gary Fredericks [ 09/Jul/15 9:03 PM ]

Ha me neither. In any case the function is almost internal, so I'm curious if you actually had a use for it or if you were just trying to clean up the code. I think tuple is the public version of that sort of thing?

Comment by Gary Fredericks [ 29/Nov/15 8:41 AM ]

I renamed it to gen-tuple on master.





[TCHECK-76] `sample` should pass num-samples to `sample-seq` as max-size to avoid unnecessary generation of cases. Created: 09/Jul/15  Updated: 29/Nov/15  Resolved: 29/Nov/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jonathan Leonard Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None
Environment:

N/A



 Description   

Title says it all.



 Comments   
Comment by Gary Fredericks [ 09/Jul/15 9:05 PM ]

Maybe this is confusion about what max-size does?

The function is lazy so it shouldn't generate any unnecessary cases. The max-size parameter is about the values of size passed to the generator, not the number of generated values.

Comment by Gary Fredericks [ 29/Nov/15 7:39 AM ]

Closing this since there hasn't been a reply to my comment.





[TCHECK-43] Test.check lacks a generator for all floats and floats in a range Created: 17/Sep/14  Updated: 29/Nov/15  Resolved: 29/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Adam Clements Assignee: Reid Draper
Resolution: Completed Votes: 1
Labels: enhancement

Attachments: File float-generators.diff    
Patch: Code and Test

 Description   

Test.check lacks generators for floats and float ranges. I have implemented a float generator which shrinks to zero based on gen/ratio and also a float-in-range which is bounded by size.



 Comments   
Comment by Reid McKenzie [ 14/Dec/14 12:22 PM ]

Duplicate of http://dev.clojure.org/jira/browse/TCHECK-5

Comment by Gary Fredericks [ 29/Nov/15 7:36 AM ]

gen/double was added in 0.9.0, which has growing/shrinking characteristics based on the semantics of doubles instead of ratios.





[TCHECK-86] Eastwood gives bad-arglists warning for defspec Created: 20/Nov/15  Updated: 29/Nov/15  Resolved: 29/Nov/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Atamert Ölçgen Assignee: Reid Draper
Resolution: Declined Votes: 0
Labels: None
Environment:

org.clojure/clojure "1.7.0"
jonase/eastwood "0.2.1"
org.clojure/test.check "0.9.0"



 Description   

Any spec gives the warning, just to illustrate:

(defspec bad-arglists
  20
  (prop/for-all [i clojure.test.check.generators/int]
                (= (+ i i)
                   (* i 2))))

This is the actual warning message:

bad-arglists: Function on var bad-arglists defined taking # args [0 1 :or-more] but :arglists metadata has # args [0]


 Comments   
Comment by Gary Fredericks [ 20/Nov/15 10:10 PM ]

That's peculiar. The defspec macro doesn't do anything weird w.r.t. arglists, and I bet if you checked (-> #'bad-arglists meta :arglists) you wouldn't see anything strange.

At a glance I can't tell if this is test.check's fault or eastwood's.

Comment by Atamert Ölçgen [ 21/Nov/15 12:34 AM ]

Actually [] is there:

(-> #'bad-arglists meta :arglists)
;; => ([] [times & {:as quick-check-opts, :keys [seed max-size]}])

But I have just checked the source and arglists looks fine, the macro defines multiple arities.

I think this is not a test.check issue. It is already being discussed in eastwood#51. So I suggest closing this.





[TCHECK-81] Create gen/let for syntax sugar and to address fmap/bind confusion Created: 25/Sep/15  Updated: 07/Nov/15  Resolved: 07/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Gary Fredericks
Resolution: Completed Votes: 1
Labels: None


 Description   

Problem Statement

fmap/bind confusion

Users who are not big fans of monads may find the names of and distinction between fmap and bind to be confusing. A combination of the two can exist, where users can return either a value or a generator and the system chooses to use fmap or bind automatically. The downside of this approach is it is ambiguous (i.e., a user cannot use it to generate a generator, though this is admittedly uncommon), and it offends people who like thinking about the precise types of functions (see this criticism in particular regarding a similar behavior in jQuery). However, neither of these are very serious restrictions, since falling back to fmap and bind is always an option.

syntax sugar

Writing complex generators can be very syntactically convoluted, with heavy nesting and confusing structure especially caused by the opposite argument order of fmap and bind. In the same way that clojure.core/let provides a flatter (especially when multiple clauses are present) syntax for something that can technically be accomplished using fn, a macro could flatten the syntax of generators that make a lot of use of bind and fmap while providing the same functionality.

Prior Art

This feature supposedly exists in the erlang quickcheck.

Another attempt at solving the same problem is test.chuck's for macro.

Proposal

A macro in the generators namespace called let, which uses bind for multiple clauses, and whose body is processed with bind or fmap depending on whether it is a generator.

Examples:

(gen/let [x gen/nat
          x-or-inc-x (gen/elements [x (inc x)])]
  (gen/list x-or-inc-x))

;; => generates something like (3 4 4 3 4 4)

;; equivalent to
(gen/let [x gen/nat
          x-or-inc-x (gen/elements [x (inc x)])
          list-of-x-or-inc-x (gen/list x-or-inc-x)]
  list-of-x-or-inc-x)

Feature Comparison

gen/let and test.chuck's for differ in a couple independent ways, so it might be helpful to be explicit about what the different options are here. I also included Erlang's let, based on how I imagine it works.

Feature Erlang's let gen/let test.chuck/for
Multiple clauses  
Subsequent clauses handled with bind N/A
Body uses bind if it's a generator  
Supports filtering    
Supports binding intermediate values    
Supports parallel generators (via tuple)    


 Comments   
Comment by Gary Fredericks [ 07/Nov/15 7:49 PM ]

Added gen/let on master





[TCHECK-74] gen-int generates very small ints Created: 02/Jul/15  Updated: 05/Nov/15  Resolved: 05/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jonathan Leonard Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

Sorry for creating an issue for this but I couldn't find any way to communicate with the developers of this project otherwise. Can someone explain why taking 100,000 ints never yields an int larger than 100? Is it due to the 'size' parameter? Does the runtime just adjust 'size' on the fly to reach 2.7 billion or what not?

If manually sampling a generator, how can one adjust the size?



 Comments   
Comment by Gary Fredericks [ 02/Jul/15 3:22 PM ]

This is a pretty major gotcha currently, and one that definitely would have been fixed by now if there was an obvious best way to fix it.

See the "Numerics" section of this page if you're interested in my ideas for fixing it.

Fortunately there's nothing blocking you from generating what you want once you understand how the pieces work together.

By default the size parameter ranges from 0-200 in normal testing (even smaller if you run your tests less than 200 times at once), which means that gen/int and similar generators stay quite small.

There are at least a couple options for opting-in to larger numbers currently:

  • use the new gen/scale function, e.g. (gen/scale #(* % 10000) gen/int)
  • use the gen/choose function, with the main downside being it ignores the size parameter and generates uniformly within the given range (but it does shrink in the normal way)

As for setting the size when sampling, I recommend the new gen/generate function, which takes an optional size argument.

Comment by Gary Fredericks [ 02/Jul/15 3:23 PM ]

I was looking for another ticket on this topic since I figured one must exist, but I didn't find it. So I'll probably leave this ticket open as a representative of the broader numerics issue.

Comment by Jonathan Leonard [ 02/Jul/15 4:24 PM ]

Thanks for the reply/explanation.

However, doesn't the `gen/scale` solution still have the drawback that it will be biased towards larger numbers-- i.e., it will be impossible to return integers between 1 and 9999 in your example code.

Comment by Gary Fredericks [ 02/Jul/15 8:23 PM ]

The distribution should be a uniform range of size roughly (* 10000 size), so small numbers are definitely possible:

user> (def g (gen/scale #(* % 10000) gen/int))
#'user/g
user> (gen/sample g)
(0 -3423 -14429 788 -16910 49010 4363 56297 -1776 58167)
Comment by Jonathan Leonard [ 07/Jul/15 12:34 PM ]

But is 10,000 a large enough factor to make Long/MAX_VALUE possible? When I tried scaling by (/ MAX_VALUE 100), I didn't see any small numbers.

Comment by Gary Fredericks [ 07/Jul/15 3:01 PM ]

They'll be there, they're just less likely. This is part of what makes in not obvious what the best built-in generators should be, different people have different expectations.

It sounds like what you want is probably something that generates longs with a uniform distribution over the log of the number? So small numbers are just as likely as large numbers?

Comment by Jonathan Leonard [ 07/Jul/15 3:19 PM ]

Yes, I think uniform distribution over the width of the number (whether 8-bit, 16-bit, 32-bit, 64-bit or what have you) makes the most sensible default. Can you show an example of how that would be achieved for one of the above widths?

Thanks!

Comment by Gary Fredericks [ 07/Jul/15 4:22 PM ]

The thing that makes it slightly trickier is how it interacts with size. It's not too hard to write a generator that ignores size and gives you the uniform-width distribution.

Maybe another option is something like:

(gen/bind (gen/sized (fn [size] (let [max-bits (min 60 size)] (gen/choose 0 max-bits)))) 
          #(gen/resize (bit-shift-left 1 %) gen/int))

I used 60 instead of 63 or 64 because I got errors with 63 that I expect are related to how gen/int works with a very high size. I'm not sure if that should be considered a bug or not – depends on if generators should be able to work with arbitrarily large sizes.

Comment by Jonathan Leonard [ 08/Jul/15 8:27 PM ]

Thanks for the sample code! That may work nicely for me (will test it out).

Comment by Gary Fredericks [ 05/Nov/15 10:32 PM ]

Just added gen/large-integer and gen/large-integer* on master.





[TCHECK-5] Include a float generator Created: 04/Mar/14  Updated: 05/Nov/15  Resolved: 05/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Reid Draper
Resolution: Completed Votes: 4
Labels: None


 Description   

There seems to be a demand for a general float generator.

One of the bigger issues with a float generator is shrinking, which is not straightforward. Reid Draper intially considered including a small implementation, using fmap to coerce a ratio into a float:

(def float (gen/fmap float gen/ratio))
(gen/sample float 20)
;; => (0.0 0.0 -1.0 0.0 -1.0 0.75 0.5 0.8 -2.0 -0.5714286 2.6666667 -2.0
;;     -0.46153846 -0.125 -12.0 -1.75 -2.4 0.41666666 0.64285713 -1.125)

The Haskell implementation is performing float shrinking through fractions.

Chas Emerick implemented a bigdec generator using ideas from data.generators, but mentioned that manipulating the long and int bits would be more performant. However, such shrinking would require good knowledge on how IEEE 754 works, in order to avoid bugs whenever shrinking is performed.

A slower, but more understandable and portable double generator was also suggested, again by Chas Emerick. The implementation of said generator could be found at https://gist.github.com/cemerick/7599452

Original issue - GitHub #36



 Comments   
Comment by Gary Fredericks [ 10/Oct/14 1:41 PM ]

I've written one and added it to test.chuck here.

It scales exponentially with the size so it very quickly fills the whole double range (I think once size > 53 or so).

It will generate +/-Infinity but not NaN.

It shrinks in an okayish way.

I'm not sure what the design goals are here, so don't know if it's worth submitting yet.

Comment by Gary Fredericks [ 05/Nov/15 10:32 PM ]

Added gen/double and gen/double* on master.





[TCHECK-65] add a float generator Created: 06/Apr/15  Updated: 04/Nov/15  Resolved: 04/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tom Crayford Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File add_float_generator.diff    
Patch: Code

 Description   

Adds a `gen/float` generator. Internally it uses `gen/ratio` and `gen/int`



 Comments   
Comment by Gary Fredericks [ 15/Apr/15 9:55 AM ]

Looks like you added a stray (:import java.util.Random)?

Comment by Gary Fredericks [ 15/Apr/15 9:58 AM ]

I expect the reason we haven't added something like this before is that it wasn't clear what domain we should be targeting; in particular whether all possible floats are likely to be generated. This fits into the broader category of numeric generators that have been in need of a redesign.

For another approach this generator in test.chuck attempts to generate all possible Doubles.

Comment by Gary Fredericks [ 04/Nov/15 10:53 PM ]

I just added a double generator on master and it took me like twelve hours to write it. My hands hurt.





[TCHECK-69] Create a UUID Generator Created: 29/May/15  Updated: 04/Nov/15  Resolved: 04/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

UUIDs have become first-class objects in clojure with the built-in reader tag #uuid. I can't think of any thorny issues surrounding generating uuids, so it seems like something test.check should have.

Probably it should always generate type-4 UUIDs, i.e. it should set the version field to 4.

I think the only question is whether it should shrink by default. I'm inclined to say no, as most code handling UUIDs is probably treating it opaquely, and so not prone to bugs caused by specific values, and therefore shrinking the UUID would be a waste of time.

Shrinking also gets messy if users are relying on different generated values being unique (which is mildly questionable but useful as well; on the other hand we should have better support for unique values in the near future).

On the other hand calling gen/no-shrink yourself is pretty easy, and having the generator not shrink would be an exception among existing test.check generators.



 Comments   
Comment by Gary Fredericks [ 04/Nov/15 10:03 AM ]

Added a uuid generator on master that creates uniformly random UUIDs and doesn't shrink.





[TCHECK-51] Add set generator Created: 12/Nov/14  Updated: 04/Nov/15  Resolved: 04/Nov/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Daniel Compton Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: None


 Description   

test.check has generators for maps, vectors, and lists. Can we also add sets as a generator with the same arguments as vector?



 Comments   
Comment by Reid Draper [ 15/Nov/14 1:53 PM ]

There are a few subtleties to adding a set generator with the same arguments as vector, particularly, asking for a particular size set. For example, what should the set generator do with the following arguments: gen/set gen/boolean 5? The domain of the boolean generator is only two elements, yet we've asked for a set with 5 elements. A vector can have duplicates, so this is no problem.

In the meantime, a set can be created:

(defn set-gen
  [vector-gen]
  (gen/fmap set vector-gen))
Comment by Gary Fredericks [ 21/May/15 9:47 PM ]

I have some code for this on a branch, and I think it will probably be included in the next release.

The trick was just to bite the bullet and write this is as a low-level primitive instead of trying to use combinators. You also have to include an abort mechanism similar to such-that – if the generator passed in can't generate unique elements fast enough, you give up.

Comment by Gary Fredericks [ 04/Nov/15 9:35 AM ]

I included a set (and a sorted-set) generator with the new distinct-collections code on master, to be released soon.





[TCHECK-63] add sizing arguments to map generator Created: 06/Apr/15  Updated: 04/Nov/15  Resolved: 04/Nov/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tom Crayford Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File better_map_generation.diff    

 Description   

Adds sizing arguments to `gen/map`, just like `gen/vector`.



 Comments   
Comment by Gary Fredericks [ 15/Apr/15 10:04 AM ]

This doesn't seem like a bad idea, but glancing at the code I suspect that it might inadvertently return smaller maps when there are duplicate keys; does that sound right?

Might be tricky to get this to perform well especially under shrinking.

Comment by Tom Crayford [ 15/Apr/15 6:59 PM ]

Yeah, that's right. I don't think that's a huge issue though - there's no guarantee that it will hit the max limit.

By "perform well" - are you thinking about the results, or how slow shrinking will be? I assume the results will be ok.

Comment by Gary Fredericks [ 15/Apr/15 8:21 PM ]

Well by "smaller" I meant "smaller than the minimum" in particular for the arity where you set a minimum size.

Comment by Gary Fredericks [ 28/Apr/15 12:16 PM ]

I just hacked up some generators for distinct collections that should make it easier to implement this in a solid way.

Not sure how soon it'll make it into master/release, just wanted to make a note here.

Comment by Gary Fredericks [ 04/Nov/15 9:34 AM ]

The distinct collections code is on master now, and should be released soon.





[TCHECK-79] Convert to .cljc Created: 10/Sep/15  Updated: 24/Oct/15  Resolved: 24/Oct/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicolás Berger Assignee: Nicolás Berger
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 10-generators-ns.patch     Text File 1-move-cljs-tests-to-clojure.patch     Text File 2-upgrade-cljsbuild.patch     Text File 3-clojure-test-ns.patch     Text File 4-clojure-test-check-ns.patch     Text File 4-clojure-test-check-ns-with-header.patch     Text File 5-clojure-test-check-test-ns.patch     Text File 6-rose-tree-ns.patch     Text File 7-properties-ns.patch     Text File 8-not-import-pseudorandom.patch     Text File 9-remove-random-states-fn.patch    

 Description   

There are separate patches for each namespace, numbered in the order that they should be applied.



 Comments   
Comment by Nicolás Berger [ 11/Sep/15 9:56 AM ]

Except for the namespaces related to RNG, all the namespaces have been converted.

Tests are green both in clj and cljs.

Namespaces related to RNG are so different from clj to cljs that I don't think they should be converted to cljc.

Comment by Gary Fredericks [ 19/Sep/15 1:51 PM ]

I've applied the first three patches, thanks.

The fourth one introduces a new namespace – could you paste in the header from one of the other files?

Comment by Nicolás Berger [ 19/Sep/15 2:47 PM ]

Sorry, I don't get what's the issue with the new namespace in the patch. Is it something about the following header?

diff --git a/src/main/clojure/clojure/test/check/impl.cljc b/src/main/clojure/clojure/test/check/impl.cljc
new file mode 100644
index 0000000..9bdcad4
— /dev/null
+++ b/src/main/clojure/clojure/test/check/impl.cljc

I tried with git am --keep-cr -s --ignore-whitespace < 4-clojure-test-check-ns.patch and it works fine. Is that what you are running?

Comment by Gary Fredericks [ 19/Sep/15 4:05 PM ]

I'm talking about the comment at the top of each test.check source file:

;   Copyright (c) Rich Hickey, Reid Draper, and contributors.
;   All rights reserved.
;   The use and distribution terms for this software are covered by the
;   Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
;   which can be found in the file epl-v10.html at the root of this distribution.
;   By using this software in any fashion, you are agreeing to be bound by
;   the terms of this license.
;   You must not remove this notice, or any other, from this software.

Comment by Nicolás Berger [ 19/Sep/15 4:23 PM ]

Oh, that header. Sorry, didn't see it. Will upload a new patch in a minute

Comment by Nicolás Berger [ 19/Sep/15 4:25 PM ]

Added new version of patch 4 as 4-clojure-test-check-ns-with-header.patch, now including the license header

Comment by Gary Fredericks [ 24/Oct/15 2:32 PM ]

These have all been applied on master; thanks!





[TCHECK-77] randomness lost Created: 23/Aug/15  Updated: 12/Sep/15  Resolved: 10/Sep/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: regression

Attachments: Text File TCHECK-77-randomness-preservation-act.patch    
Patch: Code and Test

 Description   

The patch for TCHECK-73, shipped in 0.8.1, loses precision by doing inappropriate double math. This leads to a loss of randomness in gen/choose when given extreme values (such as Long/MAX_VALUE).



 Comments   
Comment by Steve Miner [ 23/Aug/15 1:44 PM ]

Here's another test that demonstrates the problem in 0.8.1 around the extreme bounds in gen/choose.

(deftest minimal-randomness
  (testing
      "Should not get the same random value more than 90% of the time"
    (are [low high] (< (apply max (vals (frequencies (gen/sample (gen/choose low high) 1000)))) 900)
      ;; low and high should be too close, 100 is a reasonable spread
      (- Long/MAX_VALUE 100) Long/MAX_VALUE
      Long/MIN_VALUE (+ Long/MIN_VALUE 100)
      Long/MIN_VALUE 0
      0 100
      -100 0
      0 Long/MAX_VALUE
      Long/MIN_VALUE Long/MAX_VALUE)))

Although there's a very small chance you might generate all the same values at random, you wouldn't expect it to happen every time. The problem is the change introduced in TCHECK-73.

Comment by Steve Miner [ 23/Aug/15 1:56 PM ]

The original issue in TCHECK-73 was that the user wanted to use doubles (not longs) as the arguments for 'size', etc. I admit that I had assumed that 'size' parameters were always long integers. I want to restore the old code for rand-range that does the right thing with extreme values, and still satisfy the complaint in TCHECK-73 by fixing it in gen/choose to cast the parameters to longs. I'm sure that reverting to my fix for rand-range is an improvement. I'm less confident about just fixing gen/choose. However, that is sufficient to satisfy the tests introduced by TCHECK-73 so that's what I'm going to do in my patch. Reviewers should also consider if gen/resize and gen/sized should tolerate doubles as arguments in which case we might need to fix those as well.

Comment by Steve Miner [ 23/Aug/15 1:57 PM ]

Fix rand-range to preserve randomness with extreme bounds

Comment by Steve Miner [ 23/Aug/15 2:00 PM ]

The patch refactors the old code for rand-range to make it more testable and adds some tests to show that it works well with extreme bounds. The patch also fixes TCHECK-73 by casting to longs in gen/choose – I did this so that I could keep the tests introduced in TCHECK-73. Maybe it might be better to handle that separately, but we'd need to revert the old TCHECK-73 patch first. That seemed like more work.

Comment by Gary Fredericks [ 29/Aug/15 7:43 AM ]

I think it'd be more idiomatic to put (set! *assert* false) at the top of the file than to comment out the pre/post conditions, but I'm not sure of that.

Comment by Gary Fredericks [ 29/Aug/15 8:11 AM ]

Though the problem with both approaches to assertions is that the user has no way to turn them on without editing the code. This seems to be an issue for libraries in general.

I'm curious if there's an intended way to use it. My guess is that something like the following would allow user control (but still default to off, which is important):

(when (not= *assert* :force)
  (set! *assert* false))
Comment by Gary Fredericks [ 29/Aug/15 10:42 AM ]

Now that I look more closely, I don't think that *assert* trick would be a good idea (it leaks into requiring namespaces); disregard everything I've said so far, I'll be looking at the whole ticket more closely shortly.

Comment by Gary Fredericks [ 29/Aug/15 4:22 PM ]

I think the test you left here in the comments should actually work fine even for high and low being 1 apart. If I call (gen/choose (dec Long/MAX_VALUE) Long/MAX_VALUE), the chance of getting either of those values more than 900 times in 1000 ought to be negligible (some hasty arithmetic suggests it's around 10^-163), so I wouldn't at all mind tests of that sort.

Comment by Gary Fredericks [ 29/Aug/15 4:24 PM ]

Oh hey Wolfram Alpha knows how to answer the question: http://www.wolframalpha.com/input/?i=probability+of+%3E900+heads+in+1000+coin+tosses

Comment by Gary Fredericks [ 29/Aug/15 4:39 PM ]

I'm confused about why it wouldn't work to revert the TCHECK-73 change and coerce the args (instead of the return value) to gen/choose; also not sure why that would break the TCHECK-73 tests.

Comment by Gary Fredericks [ 29/Aug/15 4:39 PM ]

Sorry, I mean coerce the args to rand-range.

Comment by Gary Fredericks [ 29/Aug/15 4:40 PM ]

Coerce the args to rand-range inside of rand-range. Having a difficult time thinking apparently.

Comment by Gary Fredericks [ 08/Sep/15 4:27 PM ]

Implemented my idea, and added the test from your comment, in this commit. The tests are passing so as far as I can tell this should work. Let me know if anything looks weird.

Comment by Steve Miner [ 09/Sep/15 11:52 AM ]

Sorry, been away from JIRA for a while. Catching up with this bug now.

Just to explain my approach a little more: rand-range gets called a lot so I didn't want to do the defensive casting there. It seemed to me that the caller should do the cast, and rand-range should be explicit about expecting longs. I have a bias against supporting doubles in rand-range so I hate to pay extra to do it.

As the math was a bit tricky in rand-range, I thought it should be factored out to allow more deterministic testing. As a matter of style, I prefer to treat random calls like IO. In particular, I would prefer to write pure functions that take a parameter that captures the random value. That allows for better testing of the pure functions. Again, just a matter of style.

The important thing for me is that your fix also preserves the randomness with extreme values, and the regression test would likely catch any future mistakes along that path.

Comment by Gary Fredericks [ 09/Sep/15 8:22 PM ]

Thanks for the explanation, I like your approach better.

I think the only thing I would change about your patch is that your long calls inside of choose can be hoisted outside the function definition, so they only get run once.

If you do that and maybe add the regression test from my commit as well, I'll accept it.

Thanks!

Comment by Steve Miner [ 10/Sep/15 2:09 PM ]

new patch with fix and tests

Comment by Steve Miner [ 10/Sep/15 2:12 PM ]

Yes, good point about hoisting the long casts in gen/choose. I updated the patch with that fix and added your test.

Comment by Gary Fredericks [ 10/Sep/15 9:36 PM ]

Applied on master (a0e950f), thanks.

Comment by Gary Fredericks [ 12/Sep/15 2:49 PM ]

Released in 0.8.2.





[TCHECK-78] Minor refactoring: Replace (let [_ _] (if empty? a b) with (if-let [_ (seq _)] b a) Created: 23/Aug/15  Updated: 29/Aug/15  Resolved: 29/Aug/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nicolás Berger Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: refactoring

Attachments: Text File if-let-patch.patch     Text File if-let-patch.patch    
Patch: Code

 Description   

I think this way is more idiomatic and a bit more compact and easier to understand (perhaps just more simple?).



 Comments   
Comment by Gary Fredericks [ 29/Aug/15 7:33 AM ]

I want to accept this, but cannot apply the patch for some reason. Can you verify that you can do git am if-let-patch.patch from the project directory with master checked out?

Comment by Nicolás Berger [ 29/Aug/15 7:40 AM ]

Correct version of patch, previous one was generated with diff.noprefix=true

Comment by Nicolás Berger [ 29/Aug/15 7:44 AM ]

For some reason I had `diff.noprefix=true` in my gitconfig, that's why the patch could not be applied. I just uploaded the correct one, git am if-let-patch.patch worked in my machine now. Not sure if I should delete the old patch.

Comment by Gary Fredericks [ 29/Aug/15 11:14 AM ]

naming the patches differently can make things slightly easier, but this is fine.

Comment by Gary Fredericks [ 29/Aug/15 11:18 AM ]

Applied on master, thanks.





[TCHECK-62] ClojureScript example in README doesn't work Created: 23/Mar/15  Updated: 30/Jun/15  Resolved: 30/Jun/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Branislav Hašto Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None
Environment:

[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-3126"]
[org.clojure/test.check "0.7.0"]


Attachments: Text File TCHECK-62-000.patch    

 Description   

ClojureScript example in README doesn't seem to work:

(ns hello.core
    (:require [cljs.test.check :as tc]
              [cljs.test.check.generators :as gen]
              [cljs.test.check.properties :as prop]))

(def sort-idempotent-prop
  (prop/for-all [v (gen/vector gen/int)]
    (= (sort v) (sort (sort v)))))

(tc/quick-check 100 sort-idempotent-prop)

Compilation gives these warnings:
WARNING: Use of undeclared Var cljs.test.check.properties/for-all at line 7 src/hello/core.cljs
WARNING: Use of undeclared Var hello.core/v at line 7 src/hello/core.cljs
WARNING: Use of undeclared Var hello.core/v at line 8 src/hello/core.cljs
WARNING: Use of undeclared Var hello.core/v at line 8 src/hello/core.cljs

When I change the namespace declaration to include [cljs.test.check.properties :as prop :include-macros true] it seems to work fine.

I am beginner in ClojureScript so if this is some special example that works in cljs.user but doesn't work in custom namespace, then I'm sorry (I tried to search first).



 Comments   
Comment by Neil Kirsopp [ 30/Jun/15 9:21 AM ]

Patch for README to add :include-macros

Comment by Gary Fredericks [ 30/Jun/15 3:29 PM ]

I saw this pull request from Sean Grove before I saw the ticket, so ended up fixing it myself. Thanks for the patch!





[TCHECK-71] Exponential generator Created: 07/Jun/15  Updated: 07/Jun/15

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jason Felice Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File exponential.diff     File exponential.diff    
Patch: Code

 Description   

A generator for generating exponential values. I'm using this to generate queue latency values, to simulate out-of-order processing of events in some test.check specifications.



 Comments   
Comment by Jason Felice [ 07/Jun/15 1:31 PM ]

Corrected exponential.diff (previous one missing parenthesis)

Comment by Jason Felice [ 07/Jun/15 1:33 PM ]
user=> (require '[clojure.test.check.generators :as gen])
nil
user=> (gen/sample (gen/exponential 1/500))
(2377.5877207475132 320.68681344358293 144.18262694905863 1422.342802626106 300.8014248144241 79.8053977813952 36.18465309993865 93.65252446233575 806.4125368253829 37.88916712893676)
user=> (gen/sample (gen/exponential 1/500))
(61.431732383617884 93.51860239112023 557.6183626329674 57.553819177701584 60.4171805107351 7.152938152340775 31.84874217796871 215.71248467032976 470.69904975577214 222.89410284392252)
user=> (gen/sample (gen/exponential 1/500))
(231.68194071961852 74.5153426725424 135.57104449489148 48.2988483633354 566.3041916582175 925.9673009296939 169.65855270732143 914.1156710720605 1445.3554035600935 674.1395344157389)
user=> (gen/sample (gen/exponential 1/500))
(1467.6817487657213 1084.3796561112276 444.15866332358667 491.2566970512585 737.2979791960513 92.05590733338545 341.58867531092477 254.66969942333122 1150.8085459403007 111.21710097838688)




[TCHECK-72] Poisson generator Created: 07/Jun/15  Updated: 07/Jun/15

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jason Felice Assignee: Gary Fredericks
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File poisson.diff    
Patch: Code

 Description   

Adds `gen/poisson`, which samples numbers from a Poisson distribution.



 Comments   
Comment by Jason Felice [ 07/Jun/15 1:40 PM ]
user=> (gen/sample (gen/poisson 1/500) 500)
(0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0)
user=> (gen/sample (gen/poisson 1/5) 500)
(1 0 0 0 0 0 0 0 0 0 0 0 0 1 0 1 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 1 0 0 1 0 0 0 0 0 0 0 0 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 1 0 0 0 1 1 0 1 0 0 0 0 0 0 0 0 0 1 0 0 0 0 1 0 0 0 0 0 0 0 1 0 0 0 0 0 0 1 0 0 1 1 0 0 0 0 0 1 0 0 0 1 0 0 0 0 0 0 0 1 0 0 1 0 0 0 0 0 0 0 0 0 0 0 1 0 0 1 0 0 0 0 0 0 1 0 1 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 1 0 0 0 0 0 0 0 0 1 0 1 0 1 0 0 0 0 0 0 0 1 1 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 1 1 0 0 0 0 0 1 0 0 0 1 0 0 0 0 0 1 1 0 1 0 0 0 1 0 0 0 0 0 0 0 0 1 0 0 1 1 0 0 0 0 0 0 0 0 1 1 1 0 1 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 0 1 0 0 1 1 0 1 1 0 0 0 0 0 1 0 0 0 0 0 0 0 0 1 0 0 1 1 0 0 1 1 0 0 0 1 0 0 0 0 0 0 0 0 0 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 1 1 0 0 0 0 0 0 0 0 0 0 1 1 1 0 0 1 1 0 1 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 1 1 0 0 0 0 0 0 1 0 0 0 0 0 0)
user=> (gen/sample (gen/poisson 1) 500)
(2 2 1 1 1 0 2 2 0 0 0 1 0 1 2 1 2 2 2 2 1 1 2 2 0 1 2 0 0 1 1 2 1 1 0 1 2 0 1 1 0 1 1 1 0 2 2 0 2 1 1 1 0 1 1 0 0 1 2 0 1 1 1 2 0 1 1 0 0 0 0 2 2 0 1 1 2 2 0 0 1 0 2 0 0 1 2 1 1 1 1 1 2 2 0 1 0 1 1 1 0 1 0 2 1 0 1 0 1 0 0 0 2 1 1 0 0 2 0 1 2 1 0 2 1 0 1 0 0 1 1 2 1 2 0 1 1 2 0 1 1 2 1 1 0 0 2 1 2 1 0 0 2 1 1 2 0 1 2 1 1 0 2 2 1 1 2 1 1 2 0 1 0 1 0 0 0 1 2 0 2 1 1 2 1 0 1 0 0 1 1 0 1 0 1 1 2 0 0 2 1 0 2 0 1 0 2 1 1 0 2 1 0 1 1 0 1 0 2 0 0 0 0 2 1 0 0 1 1 2 1 0 2 2 2 2 0 1 2 1 0 1 1 1 2 1 0 0 1 0 0 1 2 1 0 0 0 2 2 0 1 1 1 0 2 0 0 0 1 1 1 0 0 0 2 1 2 1 0 0 2 1 1 1 2 1 1 2 2 0 1 2 1 2 2 0 0 1 2 2 0 1 1 2 0 1 1 2 1 1 1 1 0 2 2 2 1 0 1 1 0 1 2 0 1 1 1 1 2 1 0 1 2 0 0 2 1 0 2 0 0 1 0 1 1 1 2 1 1 0 0 2 1 1 1 2 2 0 0 2 1 1 1 0 0 2 2 0 1 0 1 2 1 1 0 0 0 2 0 1 1 1 2 2 1 1 0 2 1 0 1 2 1 0 0 0 1 0 0 2 1 1 0 1 2 1 1 0 2 1 0 0 0 1 2 1 1 2 1 1 0 2 0 0 2 2 1 0 2 0 2 2 2 1 1 2 0 0 0 1 0 1 2 0 2 1 2 0 1 0 1 2 1 0 1 0 1 0 0 0 1 1 2 2 0 0 2 1 1 1 2 1 1 0 2 1 2 0 0 1 0 0 1 0 1 0 0 0 1 1 1 0 1 0 1 0 0 0 0 2)




[TCHECK-50] Sampling one in the obvious way could return a more random value Created: 06/Nov/14  Updated: 07/Jun/15  Resolved: 07/Jun/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Jessica Kerr Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: generator


 Description   

To get values from a generator I need to call gen/sample. This returns a sequence of values of increasing size, starting from 0.

From the outside, the obvious way to get one value from a generator is (first (gen/sample g)). This always produces a value of generator's size 0. If it's a vector or string, it's empty. If it's an integer, it's 0.

This is not documented or obvious.

There are many uses for generators outside of testing. At work we use them to populate test databases. And for example tests that are in the process of becoming property tests but not there yet. Let's make that harder to screw up.

suggestions: 1) specify this in docs around sample
2) either reverse sample OR
3) create a sample-one function that provides one value with a size like the default max size in checked properties (currently 200).



 Comments   
Comment by Jessica Kerr [ 06/Nov/14 5:09 PM ]

didn't mean to make it Major. Three attempts to resubmit later...

Comment by Andy Fingerhut [ 06/Nov/14 11:59 PM ]

Jessica, you only get one chance to pick the ticket attributes when you create one, unless you have been granted permission to edit them. This permission is given to people who have signed a Clojure Contributor Agreement: http://clojure.org/contributing

No worries, though. Those responding to tickets can easily modify them if they wish.

Comment by Gary Fredericks [ 21/May/15 9:57 PM ]

I like options 1 and 3.

I'm also pondering different approaches to option 3. Size 200 might be reasonable for some generators but enormous (at least for human consumption) for others. Maybe a function with a required size arg so that people have to be thinking about it when they call it?

Comment by Gary Fredericks [ 06/Jun/15 9:25 PM ]

I was also just poking through QuickCheck and noticed that they have (in addition to a similar sample function) an extra function called generate which just returns a random value of size 30. I think such a thing would probably be cool to have, especially with a second optional size argument.

Comment by Gary Fredericks [ 07/Jun/15 9:04 AM ]

Just added gen/generate on master, so it should go out in the next release.





[TCHECK-64] better ratio generator Created: 06/Apr/15  Updated: 06/Jun/15  Resolved: 06/Jun/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tom Crayford Assignee: Reid Draper
Resolution: Declined Votes: 0
Labels: None

Attachments: File better_ratio_generator.diff    

 Description   

Fixes the caveat in the `ratio` generator - always generates valid ratios rather than numbers.



 Comments   
Comment by Gary Fredericks [ 15/Apr/15 10:01 AM ]

Am I right that this intentionally only generates ratios <1? What was the reasoning behind that?

Comment by Tom Crayford [ 15/Apr/15 6:57 PM ]

Ack, you're right. Gonna update it so it generates other ratios as well. I was trying hard to make sure it always returned ratios, rather than occasionally returning longs (which I think happens when the division is perfect?).

Comment by Gary Fredericks [ 15/Apr/15 8:23 PM ]

Can you think of any use cases for code that expects a ratio and can't handle an integer? I feel like the behavior of /, returning integers when possible, suggests that code handling ratios ought to be able to take integers as well.

Did you run into something yourself that needed this change?

Comment by Gary Fredericks [ 06/Jun/15 2:52 PM ]

Declining since this doesn't seem like a good idea to me. Can re-open if anybody provides a counter argument.





[TCHECK-66] Make rose trees a deftype Created: 14/Apr/15  Updated: 06/Jun/15  Resolved: 06/Jun/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tom Crayford Assignee: Gary Fredericks
Resolution: Completed Votes: 0
Labels: performance

Attachments: Text File deftype_rose_tree.patch     Text File deftype_rose_tree.patch    
Patch: Code

 Description   

This turns rose trees into a custom `deftype` that implements `clojure.lang.Indexed` (for backwards compatability - it means you can still use nth on them without any changes, and means we can interchange vectors with them).

It's just a performance patch - I noticed when profiling the test.check suite I have at Yeller that a lot of time was spent creating clojure vectors (it was the highest sample in the suite).

This produced a noticeable difference (at 95% confidence rating as measured across test runs using ministat) on my test suite, but didn't make such a difference on `test.check`'s (no noticeable difference at 95% confidence). I assume that's because of generator complexity.

I've also minorly changed `root` and `children` in rose-tree.clj - they no longer call `nth` twice, just once. This produced again, a minorly noticeable perf difference. I tried as well overriding them to an interface and having `RoseTree` implement it, but apparently the JIT's inlining is smart enough (even in C1) that there's no noticeable difference between that and children/root being implemented with nth.



 Comments   
Comment by Gary Fredericks [ 15/Apr/15 8:07 AM ]

I think one difference this patch makes is that (nth rt 2) no longer throws an IndexOutOfBoundsException, which probably doesn't make a difference currently but could make future bugs harder to find. Was this an intentional change?

Otherwise this looks pretty straightforward.

Comment by Tom Crayford [ 15/Apr/15 7:02 PM ]

Sent a second patch (it's the one under 15th April in Jira for me).

Comment by Gary Fredericks [ 06/Jun/15 2:47 PM ]

Applied on master, thanks!





[TCHECK-67] The new rand-range has a nasty edge case Created: 28/Apr/15  Updated: 06/Jun/15  Resolved: 06/Jun/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-67-fix-rand-range-with-new-rand-double.patch    
Patch: Code and Test

 Description   

Turns out (Math/abs Long/MIN_VALUE) returns Long/MIN_VALUE which is negative.

This means that gen/rand-range and basically every other generator can return invalid output if the rng ever generates Long/MIN_VALUE.

Generating Long/MIN_VALUE is quite improbable, but this is definitely worth fixing before the next release.



 Comments   
Comment by Steve Miner [ 28/Apr/15 11:09 AM ]

Patch checks for negative result of Math/abs (for Long/MIN_VALUE)

Comment by Gary Fredericks [ 28/Apr/15 12:05 PM ]

Thanks for the quick patch! I'll have to stare at this closer but my gut says that we don't actually want to generate 1.0 from that function (i.e., we want inclusive on the left and exclusive on the right). It might be that this aspect was broken in the old code as well.

Comment by Gary Fredericks [ 28/Apr/15 12:07 PM ]

Ideally we could generate an unsigned long and return the closest double to x / (2^64), which would have the correct inclusive/exclusive properties. Not sure if there's an easy way to accomplish that with the signed longs though.

Comment by Gary Fredericks [ 28/Apr/15 12:09 PM ]

Now that I think about it maybe the easiest way would be to ignore signs altogether by truncating the long to something like 60 bits. We lose information when converting to double anyhow. If you bit-and with the appropriate mask to get a long in the range [0,2^60) (exclusive at the top again), then you could just divide by the Double equivalent of 2^60.

Comment by Steve Miner [ 28/Apr/15 2:08 PM ]

My first patch kept the original logic in rand-range the same but avoided the (Math/abs Long/MIN_VALUE) issue. On further consideration, I think the original calculation wasn't exactly right. I revised the patch with another change to rand-range that seems to work for me. I'll leave it here for others to consider. If you have a new approach, that's fine with me.

The generator API such as gen/choose is typically inclusive so I think you'll want that for rand-range as well.

Comment by Gary Fredericks [ 28/Apr/15 4:21 PM ]

Apologies, my discussion about inclusiveness was imprecise – there's actually two inclusiveness issues to consider here.

rand-range itself is inclusive on both ends, which is fine and I wasn't suggesting changing that. The thing I was talking about was the long->double mapping. Normally when you want to map some kind of floating point interval to some kind of integer interval, it works best to have the floating point interval be inclusive on the left and exclusive on the right. E.g., if I want to generate numbers from 0-4, inclusive (so five possible values), I'll use (Math/floor (* 5 x)), which will give me a roughly even distribution from 0-4, assuming x never equals 1.0. If x=1.0, we get 5, which is out of our target integer range.

As a final example, note that Math.random() has the inclusive/exclusive property I'm talking about and that clojure.core/rand-int takes advantage of this.

I regret that my assessment of "low-hanging fruit" might have been a bit premature :/.

Comment by Steve Miner [ 29/Apr/15 10:46 AM ]

Agreed. I removed my patches as they did not address the larger issue of delivering a roughly even distribution.

Comment by Steve Miner [ 29/Apr/15 10:51 AM ]

Oops, I made another mistake. Patch withdrawn.

Comment by Gary Fredericks [ 29/Apr/15 10:55 AM ]

I don't think mod should be necessary; you could do a bit-and to set the sign bit to 0 (which should give you an even distribution between 0 inclusive and 2^63 exclusive), then divide by the Double version of 2^63.

Comment by Steve Miner [ 03/May/15 4:25 PM ]

New patch that masks the sign bit (instead of using Math/abs) and uses Double/nextUp to be sure that the factor is always less than 1.0.

Comment by Gary Fredericks [ 03/May/15 8:04 PM ]

Oh Double/nextUp is an interesting idea there – I hadn't thought about the fact that (= (double Long/MAX_VALUE) (apply * (repeat 63 2.0))), which I think is true because of the low precision of doubles in that range. It feels a little funny since now we're essentially dividing by (2^63 + 2048), but I suspect it's probably the right thing to do.

My gut says that since (Math/nextUp (double Long/MAX_VALUE)) is a constant, we might be better off perf-wise if we move that into a (def ^:const ...), rather than computing it every time.

Comment by Steve Miner [ 03/May/15 8:37 PM ]

Not sure, but I expect the compiler (or JIT) to figure out the constant expression. I'll have to check that.

Comment by Steve Miner [ 04/May/15 3:06 PM ]

Ran some tests with criterium and couldn't find any meaningful performance difference between the const approach and the let calculation (as in the patch).

Comment by Steve Miner [ 04/May/15 3:07 PM ]

I'm running a few more tests and I think I found a problem with the patch. There seems to be an edge case with a random value of exactly Long/MAX_VALUE. Thought I tested for that previously, but must have missed something in refactoring. Will update when I track this down.

Comment by Steve Miner [ 07/May/15 2:34 PM ]

I decided I was approaching this the wrong way, trying to patch up the existing code with a simple refactoring. I had to get back to basics and re-acquaint myself with floating-point math. Some of my mysterious problems were caused by ignoring the fundamental fact that large longs are not accurately represented by doubles. (I knew that but I didn't look at the code critically with that in mind.) Basically, any value outside +/- (bit-shift-right Long/MAX_VALUE 10) might not preserve integer precision. (See also Math/ulp) For example, adding 1.0 to a large long doesn't work the way you might expect. Also, some of my attempts to calculate a factor using double division were subject to tricky rounding errors. Of course, the errors depend on the exact values you're testing with.

After doing a little research, I discovered that the java.util.SplittableRandom had a clever way to generate a double in the desired half-open range [0.0,1.0). So why not port that? It's probably been tested by smarter people than me. Not much to it, and it actually worked nicely.

The other issue for me was losing a nice distribution at extreme values. Again, this was due to double math. After some more experimentation, I think I figured out the appropriate way to handle this. If the width of the bounds is less Long/MAX_VALUE, then we can be (long) accurate. Outside that, we have to double check the calculation for rounding errors at the high end – using (min upper calculated-value) cleans up the problem.

New patch coming. I've made enough mistakes with this previously that it merits a critical review. Anyway, I think it's right now.

As a bonus, my testing revealed a bug in Clojure 1.7-beta2, so I'm not the only one with floating-point problems. CLJ-1727

Comment by Steve Miner [ 07/May/15 2:41 PM ]

Patch with new rand-double to fix rand-range. Added a test to confirm rand-double works like j.u.SplittableRandom. Tried to be careful with the width calculation to preserve accuracy. Consider extreme bounds with relatively small widths such as (- Long/MAX_VALUE 1000) to Long/MAX_VALUE. Old approach would tend to bunch up at one of the bounds. The new patch provides a nice distribution for that case.

Comment by Gary Fredericks [ 09/May/15 4:00 PM ]

Oh man oh man.

Nice find with the SplittableRandom algorithm – that's definitely an appropriate thing to steal.

The patch looks good at a glance. Sometime soon I'll give it a more thorough stare.

Thanks for working through this.

Comment by Gary Fredericks [ 31/May/15 8:36 AM ]

Would this get simpler/easier if we changed rand-range to use an implicit lower-bound of 0 and take just the upper-bound as an arg? And then choose would do the appropriate subtraction to get the right result.

This might be for another ticket, but I also think it would be reasonable to prohibit a large range in choose, at least so they are not larger than 2^53. Either that or choose would detect overlarge ranges and use multiple calls to rand-range somehow or something.

I haven't analyzed the patch thoroughly yet, but I'm suspicious of the use of -', which I guess is what motivated the above thoughts.

Comment by Gary Fredericks [ 06/Jun/15 9:02 AM ]

I can work on the other stuff separately – thanks for figuring this out!

Applied the patch on master.





[TCHECK-27] stateful PRNG + bind => nondeterministic shrink tree Created: 10/May/14  Updated: 21/May/15  Resolved: 21/May/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-27-p1.patch    

 Description   

bind inherently needs to consume randomness while shrinking, since the inner generator has to be run multiple times. Since the shrink tree is lazy and there's only one Random object providing the randomness, the resulting shrank values are sensitive to what order the tree is walked in.

I don't believe this affects normal usage, but there are various modifications I've made that I think are useful (e.g., resuming slow shrinks) that are thwarted by this nondeterminism.

Ideally we could stop using a stateful PRNG, but I think this would be a rather extreme change. A much smaller fix is to change bind to create a new Random object each time the inner function is called, based on a seed from the outer Random. I will have a patch that does this shortly, and it seems to fix the issues I've had.

Code for reproducing the issue:

(ns user
  (:require [clojure.test.check.generators :as gen]
            [clojure.test.check.rose-tree :as rose]))

(def the-gen
  (gen/bind gen/nat
            (fn [n]
              (gen/fmap (fn [m] [n m]) gen/nat))))

(defn reproduced?
  [seed]
  (let [make-rose-tree #(gen/call-gen the-gen (java.util.Random. seed) 100)
        rose-1 (make-rose-tree)
        rose-2 (doto (make-rose-tree)
                 ((fn [rt1]
                    ;; force the tree to be realized in a different way
                    (dorun (for [rt2 (rose/children rt1)
                                 rt3 (rose/children rt2)]
                             42)))))
        last-child #(-> % rose/children last rose/root)]
    ;; when these two are not equal, we've reproduced the problem
    (not= (last-child rose-1)
          (last-child rose-2))))

(frequencies (map reproduced? (range 10000)))
;; => {false 9952, true 48}


 Comments   
Comment by Gary Fredericks [ 10/May/14 2:16 PM ]

Attached TCHECK-27-p1, which makes the change I described in the description to the bind-helper function.

Comment by Gary Fredericks [ 11/May/14 8:27 AM ]

The only function that uses bind-helper is bind; however, bind is also implemented with gen-bind, and several other functions use gen-bind. So we'll have to check if gen-bind is sensitive to this as well (e.g., perhaps frequencies has this issue?). If so, I'm not sure what that means about the best way to fix it. Maybe there's an alternative fix that could happen in gen-bind, or maybe the other functions could be rewritten with bind?

Comment by Gary Fredericks [ 21/May/15 9:38 PM ]

This ought to be all fixed with the new immutable RNG.





[TCHECK-37] Make random-number generation in test.check thread-safe Created: 15/Aug/14  Updated: 21/May/15  Resolved: 21/May/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: enhancement

Attachments: File your-patch-file.diff    
Patch: Code and Test

 Description   

test.check uses the Java random-number generator, which is imperative.

This makes the testing non-deterministic in many settings.

It would be better if test.check used a purely functional rng, just
like the original Haskell version of QuickCheck.

I've attached a patch that does this. All but one of the original test
pass; I've modified the only failing test to not rely on an imperative
rng.

Note that it still has quick-check/make-rng supply a random seed.
IMHO, it should supply a fixed seed.



 Comments   
Comment by Reid Draper [ 15/Aug/14 5:45 PM ]

Thanks for the patch! I haven't had a chance to look it over in detail, but I do have a few introductory questions. You're correct that test.check currently uses a mutable RNG. While it may not be ideal, I haven't run into any user-facing cases where the use is non-deterministic. Do you have some particular example in mind where test.check is not deterministic now, given the same RNG seed? One place this could be an issue in the future is if we changed the way we walk the shrink tree, which would cause different values to be generated. I'd be keen on fixing this, but it's not an issue at the moment.

tl;dr: Under what circumstances, given the same seed, is test.check non-deterministic?

Comment by Michael Sperber [ 20/Aug/14 6:50 AM ]

Thanks for the prompt answer, and sorry for taking so long to reply.

The answer to your question is: None I'm currently encountering.

My motivation for the patch was a bit different, though:

1. I don't see an easy way to make things deterministic with defspec, running tests from, say, Leiningen.

2. While I can get reproducibility specifying the seed explicitly, this complicates the testing process, which should be as smooth as possible.

3. With the purely-functional rng, the generator monad is splittable: This enables me to write deterministic tests for concurrent programs.

#1 and #2 could also be addressed with the imperative RNG, not #3, though.

Comment by Reid Draper [ 21/Aug/14 2:15 PM ]

I don't see an easy way to make things deterministic with defspec, running tests from, say, Leiningen.

Indeed, this is simply an omission in the defspec macro. Calling the quick-check function directly allows you to pass in a seed. You can see the tests for this.

While I can get reproducibility specifying the seed explicitly, this complicates the testing process, which should be as smooth as possible.

This is how Haskell QuickCheck works too. If you don't specify a seed, tests are random. Would you prefer to get the same result, even without explicitly supplying a seed?

With the purely-functional rng, the generator monad is splittable: This enables me to write deterministic tests for concurrent programs.

You can write tests for concurrent programs now. The generated values are completely constructed before your code that may use threads is called. What am I missing?

Comment by Michael Sperber [ 25/Aug/14 2:02 AM ]

Ah, I didn't realize the shipped QuickCheck does this - sorry. But yes, I would greatly prefer to get the same result, unless explicitly instructed otherwise: This would solve the `defspec' problem.

Also, some test suites will pass or fail depending on the seed - particularly those involving floating-point may need a lot of test cases before they fail, and non-reproducibility might convince you that they pass - for a while.

You can write tests for concurrent programs now.

I was being less than clear: I meant concurrent tests for concurrent programs, where the concurrency is initiated by the test itself. In that case, you may want to call `quickcheck' from different threads, and would thus like to give each its own random-number generator.

Comment by Reid Draper [ 04/Sep/14 9:00 PM ]

Just noticed that newer Haskell QuickCheck uses this random number generator now .

Comment by Reid Draper [ 05/Sep/14 1:45 PM ]

Some more discussion of the problems with the proposed implementation:

Comment by Gary Fredericks [ 21/May/15 9:34 PM ]

Reid and I ended up putting together an immutable generator based on the java.util.SplittableRandom algorithm. It's on master here and you can try it out with the 0.8.0-ALPHA release.

I think the point about having the tests use a fixed seed by default is an orthogonal issue, and could have a separate ticket if you're still interested. I don't think changing the current behavior would be good, but maybe an opt-in feature for defspec would be possible.

Comment by Gary Fredericks [ 21/May/15 9:37 PM ]

I'm going to close this as completed, since the main thrust of the ticket seemed to be immutability.





[TCHECK-6] Rose-filter may assume too much Created: 04/Mar/14  Updated: 20/May/15  Resolved: 20/May/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Reid Draper
Resolution: Declined Votes: 0
Labels: None


 Description   

Gary Fredericks commented that the rose-filter will discard a node and all of its children if the node doesn't pass the predicate. On the other hand, some of its children may pass the predicate.

Reid Draper said that he has attempted to solve this previously, but it was not evident how one should exclude a node from the tree. One solution considered was to replace the node with all of its children, then repeat the procedure recursively.

Gary Fredericks came with another solution, where you promote the first child to the root and concatenate the remaining children to the first child's children.

Original issue - GitHub #33



 Comments   
Comment by Scott Feeney [ 06/Aug/14 1:21 AM ]

Am I correct in thinking the motivation here is to make such-that applied to generators still find the minimal failing example in all cases?

If so, a complete fix seems impractical. Say we have a tree that lets us shrink from C to B to A, but B fails the predicate while C and A pass. Then it's fairly straightforward to keep A in:

     C
    / \
   B  ...
  / \
 A  ...

becomes

     C
    / \
   A  ...

But what if multiple levels are taken out? Say the shrink is like Y→X1→X2→X3→...→Xn→W where Y and W pass the predicate but all the Xs don't? If we accommodate that, then a single call to (first (children Y)) could, in the worst case, require eager exploration of the entire tree under Y, which could easily be exponential.

Comment by Reid Draper [ 07/Aug/14 3:21 PM ]

Indeed. I believe your analysis is correct. And so far, not having this optimization has not resulted in sub-optimal shrinks, as far as I know. Perhaps we should close this issue, and re-open it if we're able to find a specific test whose shrinking is poor because?

Comment by Scott Feeney [ 07/Aug/14 4:28 PM ]

That seems like a reasonable course of action. But I have a question, and feel free to stop me if this issue isn't the correct place to discuss this, but what do people actually use such-that for? Would it not be preferable to use fmap?

For example: generating even integers. (gen/such-that even? gen/int) has a potential problem finding the optimal shrink, but (gen/fmap #(* 2 %) gen/int) does not. Also, once every thousand runs, the former will error out without finding an appropriate value; the latter is reliable. It does produce larger values, but if it's important, you could cancel that out with:

(defn half-size [f]
  (gen/sized
    (fn [size]
      (gen/resize (/ size 2) f))))

(gen/fmap #(* 2 %) (half-size gen/int))

Isn't it better to find a mapping from all values to the subset of values you want? In what case would that not be possible (yet such-that is sufficiently reliable)?

Comment by Gary Fredericks [ 20/May/15 9:14 PM ]

Scott I agree that fmap is better when possible, but I don't think it's always possible. How would you implement not-empty otherwise?

Comment by Gary Fredericks [ 20/May/15 9:15 PM ]

Closing this since (apparently) I started it in the first place and I agree that it doesn't seem to be a problem.





[TCHECK-25] shuffle generator Created: 17/Apr/14  Updated: 20/May/15  Resolved: 20/May/15

Status: Closed
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gunnar Völkel Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

Adding a generator that shuffles the output of a given collection generator would be really useful.
Short example:

(gen/sample (gen/shuffle (gen/vector get/nat))) ;=> ([5 2 4] [9 1 7 8] ...)

This could be used to add a generator for permutations of integers 1,2,...,n which I also missed several times already.



 Comments   
Comment by Reid Draper [ 13/May/14 6:17 PM ]

Yeah this would be cool. Should it shrink toward the original collection-ordering?

Comment by Gunnar Völkel [ 26/May/14 10:55 AM ]

Yes, that was the idea when we talked about that topic on #clojure. Otherwise, you can just use gen/int to create a seed and use java.util.Random together with java.util.Collections/shuffle.

Comment by Steve Miner [ 23/Jan/15 9:32 AM ]

v0.6.2 (or perhaps earlier) has the shuffle generator so this issue can be closed.

Comment by Gary Fredericks [ 20/May/15 9:09 PM ]

Closing since this was added a while back.





[TCHECK-68] Support scaling of generators Created: 15/May/15  Updated: 20/May/15  Resolved: 20/May/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tcheck-68.patch    

 Description   

When working with test.check and core.matrix, I have found myself often needing to control the rate of scaling of test.check generators (because of potential exponential explosion in generated array sizes).

My solution was to write a rescaling function that applies a function to the size parameter of a generator:

(defn gen-scale 
  "Creates a generator that pre-modifies the 'size' pramater with the function f. Use if you want to 
   have the size grow at a different rate from the normal linear scaling."
  ([f gen]
    (let [gf (or (:gen gen) "gen paramter must be a test.check generator")
          new-gf (fn [rnd size]
                   (gf rnd (f size)))]
      (clojure.test.check.generators.Generator. new-gf))))

This means you can have e.g. generated numbers growing at O(sqrt(N)):

(gen/sample (gen-scale sqrt gen/pos-int) 100)
=> (0 1 2 2 2 2 2 1 0 2 2 3 3 1 0 0 1 4 1 4 2 2 4 5 3 4 2 2 1 2 1 3 1 2 1 1 2 4 1 1 5 5 5 1 1 7 5 6 6 4 1 7 2 2 5 4 7 5 5 5 6 7 1 2 8 5 6 4 7 6 4 3 4 4 3 6 1 4 2 2 6 2 6 3 5 1 1 2 2 6 3 8 5 6 4 2 7 6 3 7)

I think this or something like this should be a core function: if this is acceptable then let me know and I will create a patch.



 Comments   
Comment by Gary Fredericks [ 16/May/15 11:50 AM ]

I don't think this is a bad idea; though currently I think it's a little easier than you make it out to be, as you can just combine sized and resize.

So one might argue that it's not needed since it's easy enough to do ad-hoc, but I don't know if I'd make that argument.

Assuming it's worth being in core, we'd have to think of a better name for it. It's not inherently about scaling, it's about applying an arbitrary transformation to the size. It makes me wish resize wasn't already taken, as this is a more general and probably more useful version of resize.

Comment by Mike Anderson [ 16/May/15 8:42 PM ]

I used scaling as a general term for "changing the size". It is certainly intended to be used with monotonic scaling functions - I guess arbitrary transformations could be used, but that might interact with shrinking in strange ways?

Alternatively the term "growth" might make sense, because you are controlling the rate of growth of the size. I'm thinking of usage like "(gen/growth sqrt gen/pos-int)"

I certainly see how sized and resize could be combined to do this. However that is a bit non-trivial for newcomers so I think this has potential value as a core function. Either that or a clear documented example of how to do this kind of size scaling would be good.

Anyway, let me know if you want a patch and if so what is the preferred name.

Comment by Gary Fredericks [ 17/May/15 8:29 AM ]

When I hear "scaling" I think of linear algebra and the concept of multiplying by a constant (so that even sqrt doesn't qualify as scaling).

Also I don't think that there's any danger of shrinking happening weirdly for weird size functions, because shrinking actually doesn't interact with size at all.

I do appreciate that the sized/resize combo is subtle, so I'm happy to support this with a good name. I'll have to defer to Reid Draper for the decision on this though.

I'll keep thinking about names. transform-size is the least-worst thing I've thought of so far.

Comment by Gary Fredericks [ 20/May/15 3:19 PM ]

I poked my head into the haskell QC docs and noticed they have this function and it's called scale (here). So seeing as how that name is currently available I will set aside my reservations and prefer consistency with the haskell names.

Reid deferred back to me, so if you put together a patch with the name scale and using the sized/resize impl, I'll accept it.

Thanks!

Comment by Mike Anderson [ 20/May/15 6:53 PM ]

Patch attached.

Comment by Gary Fredericks [ 20/May/15 9:03 PM ]

Applied on master, thanks again





[TCHECK-56] defspec silently ignoring additional properties Created: 22/Dec/14  Updated: 04/Jan/15  Resolved: 04/Jan/15

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Colin Williams Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File defspec-arity.patch    
Patch: Code

 Description   

I spent a good deal of time trying to figure out why my tests in the form

(defspec foo 100
    (prop/for-all [] true)
    (prop/for-all [] false))

were passing. I finally realized that defspec was only testing the first property, and silently ignoring any further arguments.

If I had got an arity complaint it would have been obvious, so I refactored defspec to do that.



 Comments   
Comment by Reid Draper [ 22/Dec/14 8:04 PM ]

Hey Colin, thanks for the patch. I looked for your name on http://clojure.org/contributing, but didn't see it. Have you signed a Contributor Agreement? You can do it all online, instructions at http://clojure.org/contributing.

Comment by Colin Williams [ 22/Dec/14 8:45 PM ]

I just did it tonight, maybe there's a delay in the list updating?

Comment by Reid Draper [ 22/Dec/14 9:10 PM ]

Ah yes, there probably is. If it doesn't show up shortly, I'll ping Alex Miller. Thanks again for the patch. Taking a look now so we can merge it as soon as you show up.

Comment by John Walker [ 22/Dec/14 10:34 PM ]

Reid and Colin (and others), so sorry for causing this. I had no idea that defspec could do all these things when I submitted my patch.

Comment by Reid Draper [ 22/Dec/14 11:23 PM ]

No problem at all John, I didn't know either

Comment by Colin Williams [ 23/Dec/14 8:09 AM ]

Don't worry about it, I actually enjoyed the chance to dive into test.check code.

Comment by Colin Williams [ 24/Dec/14 11:38 AM ]

Still not showing up on the list. Reid, I'd ping Alex myself, but I don't know how to reach him.

Comment by Reid Draper [ 24/Dec/14 12:38 PM ]

I'll ping Alex after the holidays.

Comment by Reid Draper [ 04/Jan/15 1:49 PM ]

Fixed in b0da2025a9e22874ff15b8dabf4654ce8c9b34d6.
Cljs port in a1b6d35d6a9941f3d768d8159a5d24842d1f586c.





[TCHECK-49] StackOveflowError when generating large vectors Created: 03/Nov/14  Updated: 17/Dec/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: James Aley Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None
Environment:

java version "1.7.0_71"
OpenJDK Runtime Environment (IcedTea 2.5.3) (Arch Linux build 7.u71_2.5.3-1-x86_64)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)

Clojure 1.6.0
test.check 0.5.9


Attachments: Text File TCHECK-49-p1.patch    

 Description   

If you attempt to generate a vector with tens of thousands of elements, you'll get a StackOverflowError:

(gen/sample (gen/vector gen/int 1 20000))
;; StackOverflowError   clojure.test.check.generators/gen-bind/fn--10435 (generators.clj:77)

Because it's fun to be meta, here's a test.check test to reproduce it

(defspec does-not-overflow 1000
  (prop/for-all [max (gen/fmap (partial apply *)
                               (gen/tuple gen/s-pos-int gen/s-pos-int))]
                (< 0 (count (gen/sample (gen/vector gen/int 1 max))))))

I'm generating the the max parameter with that product, because it seems gen/s-pos-int doesn't actually generate large enough values to produce the error by itself.

Further, I'm actually just looking for a way to generate two things:

  • Byte arrays under 250kb
  • Bute arrays over 250kb

My app has different expected behaviour for each case. I was going about this by generating vectors and fmapping byte-array over it, but hit this overflow issue. I couldn't see a nice way to just generate data with the given size constraints directly. Perhaps I missed something? This is actually an issue I have quite often with test.check – integers in a particular range, strings of some minimum or maximum length, etc. Am I doing it wrong?



 Comments   
Comment by James Aley [ 03/Nov/14 12:06 PM ]

Oh - ignore my question about integers - I just found gen/choose

Comment by Reid Draper [ 08/Nov/14 2:04 PM ]

Thanks for filing this issue. First, the StackOverflow is indeed a problem. I believe I've figured out the root cause, but figuring out an acceptable fix may take some time. However, I believe I have some workarounds for you.

It sounds like your function/system has a magic number at 250Kb. I've run into some tests like this before, and what I've been able to do is to actually make this magic number parameterizable, at least for tests. During testing, could you just set the magic number to say, 250 bytes? This will work around the stack overflow issue, and make your tests both faster and less resource-intensive.

If you do end up needing to generate large values, you can get around this by simply combining several smaller generators into a large one. The stack overflow only happens when one collection generator is large. So instead of:

(def my-gen (gen/vector gen/int 10000))

try:

(def my-gen (gen/fmap flatten (gen/vector (gen/vector gen/int 100) 100)))

You're just flattening together 100 one-hundred element vectors, effectively doing the same thing.

Does this help?

Comment by Gary Fredericks [ 16/Nov/14 1:26 PM ]

I'm thinking the cleanest thing to do is to abandon the generic impl of sequence and replace it with smarter seq-of-gens->gen-of-seqs code (since that's all sequence is used for anyhow). Any objections to this?

Comment by Reid Draper [ 16/Nov/14 1:29 PM ]

I have no objection to that.

Comment by Gary Fredericks [ 16/Nov/14 1:33 PM ]

Patch is forthcoming.

Comment by James Aley [ 16/Nov/14 1:38 PM ]

Reid - sorry, the activity on this thread just reminded me that I still owe you a reply!

Yes, you're absolutely right about the magic number. I did end up refactoring the code to make it a parameter, and so the work-arounds weren't necessary.

Thanks for the great work on test.check, by the way. I'm really enjoying breaking my code with it!

Comment by Gary Fredericks [ 16/Nov/14 1:58 PM ]

Attached TCHECK-49-p1.patch.

Comment by Reid Draper [ 17/Nov/14 10:23 PM ]

Resolved in 40ccf07085bf987dd977306a3d9b69d8ce4a18ca.

Comment by Gary Fredericks [ 17/Dec/14 11:15 PM ]

FYI: the fix for this was included in today's release, 0.6.2.





[TCHECK-17] shrinking/test running treat ctrl-c (InterruptedException) as Created: 04/Apr/14  Updated: 16/Dec/14  Resolved: 16/Dec/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Tom Crayford Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

If you're testing a long running or blocking test (for example, I've been
working on finding race conditions with state machines ala John Hughes' keynote
at clojure west), it's very frustrating to not be able to ctrl-c tests when
they're blocking/stalled/slow. This is because the shrinking/running processes
are naive, and only catch Exception, without special casing
InterruptedException.

I'd very much like to see this fixed - it's a blocker on me actually using
test.check on my project, and very definitely a blocker on the race
condition/state machine work I've been doing. I've had a couple of runs at it,
but got pretty confused at aborting the shrink correctly (likely that's me
being dumb though).

To reproduce:

(tc/quick-check (prop/for-all [a (gen/any)] (Thread/sleep 10000)))

I'd expect to see:

the test run finishes

What happens instead:

test.check treats the InterruptedException as a failure and continues shrinking
as though that run had failed.



 Comments   
Comment by Reid Draper [ 06/Apr/14 11:44 AM ]

Agree. I have some ideas about what the best longterm solution is here, but am still a bit fuzzy. That being said, I think there are some simple incremental solutions that can help people now. I'll take a stab at something this week and gather some feedback.

Comment by Tom Crayford [ 16/Dec/14 2:06 PM ]

This is also closed in 6d81ef59e2e690644849cbc112e281997e1e345b

Comment by Reid Draper [ 16/Dec/14 8:52 PM ]

Fixed in 6d81ef59e2e690644849cbc112e281997e1e345b.





[TCHECK-42] Interruptibility Created: 03/Sep/14  Updated: 15/Dec/14  Resolved: 15/Dec/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File TCHECK-42.patch    

 Description   

Both the main test loop and the shrink loop should have checks for thread interrupts

Patch coming imminently.



 Comments   
Comment by Gary Fredericks [ 03/Sep/14 3:55 PM ]

Attached TCHECK-42.patch.

Comment by Reid Draper [ 21/Oct/14 9:54 PM ]

Thanks, sorry for the delay. Maybe this is a lein repl thing, but when I ctrl-c a test, while using this patch, I get a java.lang.ThreadDeath exception as the :result. Maybe the repl spawns your code in another thread, unlike running lein test?

Comment by Tom Crayford [ 15/Dec/14 2:54 PM ]

this is fixed in master now, right?

Comment by Reid Draper [ 15/Dec/14 3:01 PM ]

Fixed in 6d81ef59e2e690644849cbc112e281997e1e345b





[TCHECK-55] Minor refactor in generators/choose: call int-rose-tree Created: 01/Dec/14  Updated: 08/Dec/14  Resolved: 08/Dec/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nicolás Berger Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File deinline-int-rose-tree.diff    
Patch: Code

 Description   

The last line of gen/choose does exactly the same as calling gen/int-rose-tree with the value.

This patch "de-inlines" that line by calling int-rose-tree. The code is more simple and easier to understand this way.

New tests are not needed because choose behavior is already stressed by the current tests.



 Comments   
Comment by Gary Fredericks [ 03/Dec/14 1:23 PM ]

Looks good to me.

Comment by Reid Draper [ 08/Dec/14 4:29 PM ]

Thanks! Pushed in ff5809c14ff1d333f8a0c3066e05c1a8e7dd6b47.

Comment by Reid Draper [ 08/Dec/14 4:30 PM ]

Fixed in ff5809c14ff1d333f8a0c3066e05c1a8e7dd6b47.





[TCHECK-54] shuffle assumes vector arg Created: 18/Nov/14  Updated: 30/Nov/14  Resolved: 30/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

(gen/sample (gen/shuffle (range 3)))

ClassCastException clojure.lang.LazySeq cannot be cast to clojure.lang.IFn clojure.test.check.generators/swap

(gen/sample (gen/shuffle (vec (range 3)))) works



 Comments   
Comment by Steve Miner [ 18/Nov/14 9:20 AM ]

The easy fix is to change gen/swap...

(defn- swap
  [coll [i1 i2]]
  (let [v (vec coll)]
    (assoc v i2 (v i1) i1 (v i2))))

I can provide a patch if desired.

Comment by Steve Miner [ 18/Nov/14 9:25 AM ]

Or change the documentation to say that only a vector is allowed as opposed to a collection.

Comment by Reid Draper [ 30/Nov/14 6:17 PM ]

Fixed in 32dc2f8d6cb568f5814dc9103c013fa83e02f3a3.

Comment by Reid Draper [ 30/Nov/14 6:17 PM ]

32dc2f8d6cb568f5814dc9103c013fa83e02f3a3





[TCHECK-1] Namespace reorganization Created: 13/Feb/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Reid Draper Assignee: Reid Draper
Resolution: Declined Votes: 0
Labels: None


 Description   

Currently test.check (and simple-check) use three separate namespaces: core, generators and properties. Bronsa has suggested renaming test.check.core to simply test.check. This has me wondering if the entire public API should be exported from just the 'test.check' namespace. This would mean users only need to import one namespace. Thoughts?

[1] https://github.com/clojure/test.check/commit/a2f270ae8e6fa99f337eaff6fbb2051a66dac392#commitcomment-5303415



 Comments   
Comment by Peter Taoussanis [ 28/Feb/14 11:01 AM ]

+1 I'd definitely welcome merging the namespaces. Even merged, the lot would be < 1000 loc - and the core namespace isn't much use w/o the other two.

Comment by Reid Draper [ 28/Feb/14 2:42 PM ]

I'd still like to keep them as separate files, if we do this. Any easy way to do that? Just declare the same namespace in each file?

Comment by Nicola Mometto [ 28/Feb/14 5:22 PM ]

Put the ns decl in the main file and then simply clojure.core/load the different files.

Comment by Bruce Adams [ 01/Mar/14 9:01 AM ]

Merging core and properties is fine, but I like having the generators in a separate namespace.

The complex part of my uses of test.check have been building generators. Having a clear name space for generators helps make my code easier to read. Here are some trivially small examples from my code:

(ns whatever.generators
  (:require (clojure.test.check [generators :as gen])))

(def gen-non-blank-string
  (gen/such-that #(not-empty (.trim %)) gen/string))

(defn gen-nil-or
  [generator]
  (gen/one-of [(gen/return nil) generator]))

(defn gen-set
  [generator]
  (gen/fmap set (gen/vector generator)))
Comment by Chas Emerick [ 03/Apr/14 7:52 AM ]

+1 to making the properties bits available in clojure.test.check. I've never been a fan of using load to separate namespaces into different files; var immigration is another option that could work.

I agree with Bruce that keeping the generators namespace separate is desirable.

Comment by Reid Draper [ 08/Apr/14 9:49 PM ]

Currently I'm thinking this is pretty low priority, given the other issues on the repo. Any disagreement?

Comment by Bruce Adams [ 09/Apr/14 7:19 AM ]

I agree, this is lower priority than its current "Major".





[TCHECK-18] Output of failed defspec should result in copy and paste-able data. Created: 08/Apr/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jason Gilman Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

During a failure of a defspec the generated example data that caused the failure is not output in a way that makes it easy to copy and paste for testing. For example if the failed data was {:a "foo" :b "bar"} it will be displayed in the REPL as {:a foo, :b bar}. This is happening because the result of testing is printed by clojure.test.check.clojure-test/assert-check with println:

(defn- assert-check
[{:keys [result] :as m}]
(println m)
(if (instance? Throwable result)
(throw result)
(ct/is result)))

If it was changed to use pr-str then the output would be copy and paste-able:

(defn- assert-check
[{:keys [result] :as m}]
(println (pr-str m))
(if (instance? Throwable result)
(throw result)
(ct/is result)))

The following example spec output is better when I manipulate a local copy of test.check.

(defspec test-output 1
(for-all [n (gen/return {:a "foo"})]
(= n {:b "foo"})))

Output without change:
{:test-var test-output, :result false, :failing-size 0, :num-tests 1, :fail [{:a foo}], :shrunk {:total-nodes-visited 0, :depth 0, :result false, :smallest [{:a foo}]}}

Output with change:
{:test-var "test-output", :result false, :failing-size 0, :num-tests 1, :fail [{:a "foo"}], :shrunk {:total-nodes-visited 0, :depth 0, :result false, :smallest [{:a "foo"}]}}



 Comments   
Comment by Reid Draper [ 08/Apr/14 9:20 PM ]

Agree. This should get fixed.

Comment by Reid Draper [ 17/Nov/14 10:29 PM ]

Resolved in e857a084725889d0be7bee2a4e53ad70cd9b5f12.





[TCHECK-29] Docstring typo Created: 31/May/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File docstring-typo.patch    

 Description   

Patch attached.



 Comments   
Comment by Reid Draper [ 17/Nov/14 10:28 PM ]

Fixed in 6c553aaa891e0b3c3bc1c6fba3791d75664c530e.





[TCHECK-30] Make Namespacing Consistent in generators.clj Docstrings Created: 31/May/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Michael S. Daines Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File prefix.patch    
Patch: Code

 Description   

Three of the docs with examples in generators.clj aren't namespaced with "gen/", whereas everything else is. Add these in to make the documentation consistent and easy for people learning the library to cut and paste to test.






[TCHECK-36] doc typo: 'excepts' should be 'accepts' Created: 05/Aug/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File typo-excepts-should-be-accepts.patch    
Patch: Code

 Description   

typo in doc/intro.md



 Comments   
Comment by Reid Draper [ 07/Aug/14 11:51 AM ]

Thanks! Pushed in d2f46ad2fba3e7363cc0f22f40272088f2f01eb5.





[TCHECK-40] Typo: alpha-numeric should be alphanumeric Created: 25/Aug/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: John Walker Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File alphanumeric2.diff    
Patch: Code and Test

 Description   

This replaces all instances of alpha-numeric with alphanumeric. This includes renaming the following functions:

char-alpha-numeric -> char-alphanumeric
string-alpha-numeric -> string-alphanumeric



 Comments   
Comment by Reid Draper [ 26/Aug/14 2:22 PM ]

test.check hasn't reached 1.0, so we can break backward compatibility. However, I want to still communicate clearly to users. Maybe we can leave a definition of the hyphenated versions that raises a 'deprecated/removed' exception?

Comment by John Walker [ 27/Aug/14 10:44 AM ]

This seems reasonable. Are generators of this form alright?

(defn make-deprecated-generator
  ([msg map]
     (make-gen
      (fn [_ _]
        (throw (ex-info msg map)))))
  ([msg map cause]
     (make-gen
      (fn [_ _]
        (throw (ex-info msg map cause))))))

(def
  ^{:deprecated "TODO"}
  char-alpha-numeric
  "DEPRECATED - use char-alphanumeric instead."
  (make-deprecated-generator "DEPRECATED - use char-alphanumeric instead." {:deprecated "TODO"}))

If so, I can supply a macro to create these things. It would attach the metadata like docstrings and the deprecation version to the var, and usage might look like

(def-deprecated-generator char-alpha-numeric 
  "DEPRECATED - use char-alphanumeric instead."
  {:deprecated "TODO"})
Comment by Reid Draper [ 01/Sep/14 10:31 PM ]

I think I may be changing my mind here. I'm thinking we should have one release where the two names are both present. Make some noise about it in the release notes, and attach the :deprecated tag on the metadata. Would you mind making a patch to just add the new names, and add a ^{:deprecated ...} tag on the existing names? Sorry for the back and forth.

Comment by John Walker [ 02/Sep/14 2:11 PM ]

Thats fair. How should we handle it after that release?

Comment by John Walker [ 02/Sep/14 4:26 PM ]

This patch replaces alpha-numeric with alphanumeric, and attaches :deprecated "0.6.0" to char-alpha-numeric and string-alpha-numeric in favor of char-alphanumeric and string-alphanumeric.

Comment by Reid Draper [ 02/Sep/14 7:59 PM ]

Patch applied in 6def75ebc80cc5c1ab66ae956c76a7b402198852.





[TCHECK-52] defspec in 0.6.0 doesn't allow symbolic options Created: 15/Nov/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-52-fix-defspec-options.patch    
Patch: Code and Test

 Description   

The new 0.6.0 release has a `defspec` that no longer allows symbolic or calculated options. Basically, the macro was improperly defined to require literals as options. In previous releases, the number of trials for a `defspec` could be a regular Clojure expression that was evaluated at runtime. My project broke on the new test.check because I was using my own var to define the number of trials for all my defspecs. Easy enough to work around once I understood the issue, but it's still needlessly backward incompatible.