fscheck / FsCheck

Random Testing for .NET
https://fscheck.github.io/FsCheck/
BSD 3-Clause "New" or "Revised" License
1.15k stars 154 forks source link

Feature: In-box shortcut to obtain default generator for a type #638

Closed bartelink closed 10 months ago

bartelink commented 11 months ago

While the release notes mention that one can simply use ArbMap.defaults |> ArbMap.generate, I find that in practice a) I use that chain a lot in simple test suites b) I often want to specify the type explicitly, which clutters the chain a lot, but does not make the calling out of the type stick out c) when porting from V2, the migration path from the previous availability of an equivalent helper is non-obvious (IIRC there was an Arb.generate or similar?)

Which means I invariably end up sprinkling around helpers like:

let genDefault<'t> = ArbMap.defaults |> ArbMap.generate<'t>

Examples of people making helpers and/or not making helpers:

https://github.com/search?q=repo%3Ajet%2Fequinox%20ArbMap.defaults&type=code

https://github.com/search?q=repo%3Afscheck%2FFsCheck%20ArbMap.defaults&type=code

I'm about to make myself an ArbMap.defGen helper, but I'd love for an in-box equivalent, so I can delete it and use that

Naming ideas (I assume AbMap.defGen does not align with general naming and will not be acceptable):

ploeh commented 11 months ago

The first time I picked up v3 I also struggled finding the correct API.

Since ArbMap.generate ArbMap.defaults returns a Gen<'a>, I would find it more intuitive that such a function were to be defined in the Gen module; Gen.default or (if you want to explicitly include the type) Gen.defaultFor<'a>?

ploeh commented 11 months ago

...or perhaps Gen.fromDefaults?

bartelink commented 11 months ago

While I like Gen.defaultFor the most from the above, the main negative is probably the fact that it doesn't convey that the root of it all is ArbMap.defaults and/or that it might be appropriate to consider whether you should be doing something at the Arb level.

I guess Kurt will have an opinion; I feel the important bits are:

That last piece is something I have grokked to any major degree, and any nudge the common APIs that people will run into via the FsCheck.Xunit onramp (i.e. feeling your way into using FsCheck by having it feed stuff into a [<Property>], only customising the Generators "to shut FsCheck up" without actually using any Gen or gen { stuff, or properly considering whether your inputs are actually getting appropriate variation induced at all).

kurtschelfthout commented 11 months ago

Don't have a a strong opinion. My theory that "explicit is better than implicit, until people find out that explicit means more typing" still going strong I see :)

The idea of v3 was that there'd be some integrated shrinking, so Gen wouldn't be so important, but obviously I never got there.

In principle putting a method on Gen would make most sense, not sure if that's easy to arrange, because the Arb code occurs in a module after the Gen code.

kurtschelfthout commented 10 months ago

Decided against this.

The explicit arbitrary map is an important piece of the new design for extending and dealing with what was "arbitrary registration" before. In particular, ArbMap.defaults is now immutable - whenever you want to change something in it, ArbMap.defaults |> ArbMap.generate will NOT pick up changes. 2.x had side-effects which was a source of much confusion.

Making it explicit that there is an immutable map from types T to Arbitrary<T> which you can extend or change by making a new one, is important. Making a Gen.default or whatever obscures this (esp. in light of the previous design).

If you prefer, there is a GeneratorFor extension method in FsCheck.Fluent. ArbMap.defaults.GeneratorFor<T>()

bartelink commented 9 months ago

Apologies for not responding earlier; had been letting it percolate in the hope of coming up with a counter proposal. Writing this in case it helps others, rather than as an actual counter-argument to your decision (which I feel is the correct call)

If you prefer, there is a GeneratorFor extension method in FsCheck.Fluent

I don't use anything else in the .Fluent namespace, so for me that's not particularly useful.

In general I agree with your call though; concealing pivotal abstractions in the name of discoverability that will only help people that are porting is definitely not ideal. (Would an Obsolete stub for Arb.generate have helped #644 ?)


I currently have this helper blob in most projects and don't see myself removing it:

open FsCheck.FSharp

module ArbMap =
    let defGen<'t> = ArbMap.defaults |> ArbMap.generate<'t>

Ultimately, I use FsCheck in a pretty declarative manner:

  1. all properties are via inheriting FsCheck.Xunit.Property
  2. most generation is by demanding Choice types, Active Patterns etc and/or a Fixture that does the generation based on constructor arguments
  3. in some cases, I need to disambiguate construction of id types that fail out of the box, which is generally a very boring one-liner
type FsCheckGenerators =
    static member SkuId = ArbMap.defGen |> Gen.map SkuId |> Arb.fromGen
    static member RequestId = ArbMap.defGen<Guid> |> Gen.map (fun x -> RequestId.parse %x) |> Arb.fromGen
type DomainPropertyAttribute() =
    inherit FsCheck.Xunit.PropertyAttribute(QuietOnSuccess = true, Arbitrary=[| typeof<FsCheckGenerators> |])

I suspect this is not a normal usage pattern, and probably reflects a very naive take on how to wield it...