racket / htdp

Other
91 stars 69 forks source link

Quickcheck imported into htdp/asl could depend on separate quickcheck library #184

Closed dbp closed 1 year ago

dbp commented 1 year ago

ASL has support for property based testing, brought over from deinprogramm. The code that does that has been extracted out to a separate library called "quickcheck". If you want to, for example, define your own generators (say you define a struct, a pretty common thing!), you currently need to require some of the deinprogramm libraries, rather than the standalone library, because check-property/for-all (not sure where exactly) calls into deinprogramm code, and thus needs various structs to be defined there.

It might have been nice if the code had been more structural, so this wasn't an issue, but as is, it seems that the htdp/asl versions of this should just use the versions from the extracted library, so that users can (require quickcheck) when needed.

As a concrete example, here's what I have to do if I want to use my own struct (as far as I understand: if there is a better way to do this, I'm all ears!):

#lang htdp/asl

(require deinprogramm/quickcheck/quickcheck)
(require deinprogramm/signature/signature)

(define-struct person [name age])

(set-signature-arbitrary! Person
                          (arbitrary-record make-person
                                            (list person-name
                                                  person-age)
                                            arbitrary-string
                                            arbitrary-integer))

(check-property (for-all [(p Person)] (string? (person-name p))))

(note that because define-struct in ASL is defining signatures, I have to mutate the signature to add the arbitrary. I don't love this, and would rather that the define-struct form either allowed the arbitrary instances to be included, or didn't generate the signature, but that may be more work).

mfelleisen commented 1 year ago

@mikesperber This is probably something that belongs into the DeinProgram collects. Do you want to address it?

mikesperber commented 1 year ago

Check out this:

https://github.com/racket/htdp/pull/83

Upshot is this: define-struct could define the generator for you if it wasn't mutable. (Using PersonOf in this case.) In ASL, it is mutable though.

If we can make progress on the issues mentioned in that ticket, we can resolve this issue as well in all probability.

(My preference would be a new struct-defining form that allows inline signatures.)

dbp commented 1 year ago

Hmm. So that issue is merged, but obviously the part that I care about doesn't work. You mentioned that it's because the struct is mutable (and thus you can't create the generator)... I don't quite understand that. For others (as I know you know what doesn't work!), here's the example:

#lang htdp/asl

(define-struct person [name age])

(check-property (for-all [(p (PersonOf String Integer))]
                  (string? (person-name p))))

And the error:

check-property encountered the following error
:: Signature does not have a generator

If that worked it would certainly handle one of the uses that I care about (and indeed, the one that shows up right away!).

The other one is custom generators for a type that already exists. For example, I might want to generate sorted lists. I can try to restrict them with ==>, but that'll result in just about no (interesting) data actually getting through, and a pretty useless test. That's a pretty important part of PBT, and something that ideally isn't too painful to support... (the set-signature-arbitrary! isn't that bad, but it's a little bit odd -- it might be nice, for example, if check-property/for-all would also work for just arbitrary instances, rather than needing signatures that had generators inside).

mikesperber commented 1 year ago

Huh, I thought a little bit more about what's problematic about parametric signatures for mutable records, and all I can come up with is that they're slow because they can't be lazy. However, that has nothing to do with generators.

I just pushed a patch that makes parametric signatures from mutable-record definitions have generators, too. Let me know how it goes for you.

mfelleisen commented 1 year ago

Thanks for the quick turn-around (@dbp Here we go!)

dbp commented 1 year ago

This is great! It certainly gives me everything I need for the beginning (which was the goal).

More a question for @mfelleisen (or others) -- what does the release schedule look like? More wondering about when things go into the package database (I'm fine with having them update a package), rather than when a new version of the whole thing comes out.

rfindler commented 1 year ago

@dbp The Racket release process is described here, on the wiki. If you want to plan ahead, the best time to push new stuff is the day after the release branch is created. This means that we'll have a full release cycle to play around with the new stuff before it is released. Once something is released, it is a lot harder to change it.

dbp commented 1 year ago

@rfindler So just to confirm my understanding, if (hypothetically :P) I wanted to be able to use this code in a class starting Jan 9th, I would need to create my own package (with some version of ASL), as the next release will be Jan 30th.

mfelleisen commented 1 year ago

If you run on git head, all this is available because 2htdp/ and DeinProgram/ are in the main distro.

We will release one more time before the semester starts.

mfelleisen commented 1 year ago

OOPS. It’s fall. So nothing new until January so as to not upset things.

dbp commented 1 year ago

I was able to test it myself; just wondering for students.

rfindler commented 1 year ago

Yes, you're reading the release cycle correctly. Sorry :(

mikesperber commented 1 year ago

Closing this PR as  @dbp says issue is resolved.